-
Notifications
You must be signed in to change notification settings - Fork 24
[auth] Implement CIMD conformance test #37
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
a1d6d5d
add cimd test and negative test
pcarleton 1048f6a
Improve auth-test-no-cimd.ts documentation for clarity
pcarleton c0ac704
Remove .js extensions from relative imports
pcarleton cb4cd61
Skip auth/basic-cimd test until SDK support lands
pcarleton 17cde17
Fix trailing whitespace in auth-test-no-cimd.ts
pcarleton a02d1c8
Merge main into pcarleton/cimd, resolve conflicts
pcarleton File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| #!/usr/bin/env node | ||
|
|
||
| import { Client } from '@modelcontextprotocol/sdk/client/index.js'; | ||
| import { StreamableHTTPClientTransport } from '@modelcontextprotocol/sdk/client/streamableHttp.js'; | ||
| import { withOAuthRetry } from './helpers/withOAuthRetry'; | ||
| import { runAsCli } from './helpers/cliRunner'; | ||
| import { logger } from './helpers/logger'; | ||
|
|
||
| /** | ||
| * Non-compliant client that doesn't use CIMD (Client ID Metadata Document). | ||
| * | ||
| * This client intentionally omits the clientMetadataUrl parameter when the server | ||
| * advertises client_id_metadata_document_supported=true. A compliant client should | ||
| * use CIMD when the server supports it, but this client falls back to DCR (Dynamic | ||
| * Client Registration) instead. | ||
| * | ||
| * Used to test that conformance checks detect clients that don't properly | ||
| * implement CIMD support. | ||
| */ | ||
| export async function runClient(serverUrl: string): Promise<void> { | ||
| const client = new Client( | ||
| { name: 'test-auth-client-no-cimd', version: '1.0.0' }, | ||
| { capabilities: {} } | ||
| ); | ||
|
|
||
| // Non-compliant: omitting clientMetadataUrl causes fallback to DCR | ||
| // A compliant client would pass a clientMetadataUrl here when the server | ||
| // advertises client_id_metadata_document_supported=true | ||
| const oauthFetch = withOAuthRetry( | ||
| 'test-auth-client-no-cimd', | ||
| new URL(serverUrl) | ||
| )(fetch); | ||
|
|
||
| const transport = new StreamableHTTPClientTransport(new URL(serverUrl), { | ||
| fetch: oauthFetch | ||
| }); | ||
|
|
||
| await client.connect(transport); | ||
| logger.debug('Connected to MCP server (without CIMD)'); | ||
|
|
||
| await client.listTools(); | ||
| logger.debug('Successfully listed tools'); | ||
|
|
||
| await client.callTool({ name: 'test-tool', arguments: {} }); | ||
| logger.debug('Successfully called tool'); | ||
|
|
||
| await transport.close(); | ||
| logger.debug('Connection closed successfully'); | ||
| } | ||
|
|
||
| runAsCli(runClient, import.meta.url, 'auth-test-no-cimd <server-url>'); | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| // Namespaced exports for client checks | ||
| import * as client from './client.js'; | ||
| import * as client from './client'; | ||
|
|
||
| export const clientChecks = client; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,99 @@ | ||
| import type { Scenario, ConformanceCheck } from '../../../types'; | ||
| import { ScenarioUrls } from '../../../types'; | ||
| import { createAuthServer } from './helpers/createAuthServer'; | ||
| import { createServer } from './helpers/createServer'; | ||
| import { ServerLifecycle } from './helpers/serverLifecycle'; | ||
| import { SpecReferences } from './spec-references'; | ||
|
|
||
| /** | ||
| * Fixed client metadata URL that clients should use for CIMD tests. | ||
| * This URL doesn't need to resolve - the server will accept it as-is | ||
| * and use hardcoded metadata. | ||
| */ | ||
| export const CIMD_CLIENT_METADATA_URL = | ||
| 'https://conformance-test.local/client-metadata.json'; | ||
|
|
||
| /** | ||
| * Scenario: Client ID Metadata Documents (SEP-991/URL-based client IDs) | ||
| * | ||
| * Tests that when a server advertises client_id_metadata_document_supported=true, | ||
| * clients SHOULD use a URL as their client_id instead of using dynamic client | ||
| * registration. | ||
| */ | ||
| export class AuthBasicCIMDScenario implements Scenario { | ||
| name = 'auth/basic-cimd'; | ||
| description = | ||
| 'Tests OAuth flow with Client ID Metadata Documents (SEP-991/URL-based client IDs). Server advertises client_id_metadata_document_supported=true and client should use URL as client_id instead of DCR.'; | ||
| private authServer = new ServerLifecycle(); | ||
| private server = new ServerLifecycle(); | ||
| private checks: ConformanceCheck[] = []; | ||
|
|
||
| async start(): Promise<ScenarioUrls> { | ||
| this.checks = []; | ||
|
|
||
| const authApp = createAuthServer(this.checks, this.authServer.getUrl, { | ||
| clientIdMetadataDocumentSupported: true, | ||
| onAuthorizationRequest: (data) => { | ||
| // Check if client used URL-based client ID | ||
| const usedUrlClientId = data.clientId === CIMD_CLIENT_METADATA_URL; | ||
| this.checks.push({ | ||
| id: 'cimd-client-id-used', | ||
| name: 'Client ID Metadata Document Usage', | ||
| description: usedUrlClientId | ||
| ? 'Client correctly used URL-based client ID when server supports client_id_metadata_document_supported' | ||
| : 'Client SHOULD use URL-based client ID when server advertises client_id_metadata_document_supported=true', | ||
| status: usedUrlClientId ? 'SUCCESS' : 'WARNING', | ||
| timestamp: data.timestamp, | ||
| specReferences: [ | ||
| SpecReferences.MCP_CLIENT_ID_METADATA_DOCUMENTS, | ||
| SpecReferences.IETF_CIMD | ||
| ], | ||
| details: { | ||
| expectedClientId: CIMD_CLIENT_METADATA_URL, | ||
| actualClientId: data.clientId || 'none' | ||
| } | ||
| }); | ||
| } | ||
| }); | ||
|
|
||
| await this.authServer.start(authApp); | ||
|
|
||
| const app = createServer( | ||
| this.checks, | ||
| this.server.getUrl, | ||
| this.authServer.getUrl | ||
| ); | ||
|
|
||
| await this.server.start(app); | ||
|
|
||
| return { serverUrl: `${this.server.getUrl()}/mcp` }; | ||
| } | ||
|
|
||
| async stop() { | ||
| await this.authServer.stop(); | ||
| await this.server.stop(); | ||
| } | ||
|
|
||
| getChecks(): ConformanceCheck[] { | ||
| // Ensure we have the CIMD check - if not, the client didn't make an auth request | ||
| const hasCimdCheck = this.checks.some( | ||
| (c) => c.id === 'cimd-client-id-used' | ||
| ); | ||
| if (!hasCimdCheck) { | ||
| this.checks.push({ | ||
| id: 'cimd-client-id-used', | ||
| name: 'Client ID Metadata Document Usage', | ||
| description: | ||
| 'Client did not make an authorization request to test CIMD support', | ||
| status: 'FAILURE', | ||
| timestamp: new Date().toISOString(), | ||
| specReferences: [ | ||
| SpecReferences.MCP_CLIENT_ID_METADATA_DOCUMENTS, | ||
| SpecReferences.IETF_CIMD | ||
| ] | ||
| }); | ||
| } | ||
|
|
||
| return this.checks; | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as a user am I expected to understand what this does? Like is this a learning moment for me. If so I think this method could be clearer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yea the readability is a bit busted here for now. we need to upstream some things to the
withOauthmiddleware in typescript and then refactor. for now i think this will be a little gross.