feat: add positive tests for the Authorization Code Grant by Michito-Okai · Pull Request #267 · modelcontextprotocol/conformance

Michito-Okai · 2026-05-12T05:09:11Z

Motivation and Context

Described in #208

How Has This Been Tested?

npm test (Test Files 10 passed, Tests 126 passed)
npm run start -- authorization --url http://localhost:8080/realms/mcp --client-id testClient --secret qeVFKk0g4qVTyeiX93ZW9ffdOA19L0QO --port 3000

Breaking Changes

add file options for authorization server mode (--client-id --secret --port )

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Documentation update

Checklist

I have read the MCP Documentation
My code follows the repository's style guidelines
New and existing tests pass locally
I have added appropriate error handling
I have added or updated documentation as needed

pkg-pr-new · 2026-05-12T05:09:45Z

Open in StackBlitz

npx https://pkg.pr.new/@modelcontextprotocol/conformance@267

commit: 67acf87

tnorimat · 2026-05-14T01:36:16Z

 export const AuthorizationServerOptionsSchema = z.object({
-  url: z.string().url('Invalid authorization server URL')
+  url: z.string().url('Invalid authorization server URL'),
+  clientId: z.string().min(1, 'Client id cannot be empty'),


The schema REQUIRES clientId and secret while CLI requires them as optional parameter, which causes contradiction.
One way for resolving is that the schema requires the parameters as optional.

Assuming this PR will be merged, the clientId and secret were optional CLI parameters. I have made clientId and secret mandatory CLI parameters.

Thank you for the clarification.

tnorimat · 2026-05-14T01:36:30Z

+- Token response MUST return a JSON response including access_token and token_type`;
+
+  async run(
+    option: any,


I think it might be better to be type-safe as much as we can.
How about using AuthorizationServerOptions defined by schemas.ts ?

I changed it from any to AuthorizationServerOptions.

tnorimat · 2026-05-14T01:44:19Z

+import { request } from 'undici';
+import { randomBytes } from 'crypto';
+
+const STATE = randomBytes(32).toString('base64url');


When only the module is loaded, a pseudo-random number is generated and assigned to STATE, which means that multiple runs share that. For now, it might not cause any issue, but in the future, it might cause the issue.

If possible, I think it might be better to generate the value and assign it to STATE per run.

I modified it so that the value of state changes each time run is called.

tnorimat · 2026-05-14T01:46:52Z

+const STATE = randomBytes(32).toString('base64url');
+const REDIRECT_URI_ORIGIN = 'http://localhost';
+const REDIRECT_URI_PATH = '/callback';
+const CODE_VERIFIER = 'dBjftJeZ4CVP-mB92K27uhbUJU1p1r_wW1gFWFOEjXk';


It seems that CODE_VERIFIER and CODE_CHALLENGE values are from https://datatracker.ietf.org/doc/html/rfc7636#appendix-B as a valid combination of code_challenge and code_verifier.
How about mentioning that here?

I added a comment about CODE_VERIFIER and CODE_CHALLENGE values.

tnorimat · 2026-05-14T01:55:12Z

+    resolveFn = resolve;
+  });
+
+  const server = app.listen(port, () => {


app.listen(port) binds to 0.0.0.0 by default, which means that it listens all IP addresses of the machine running the test. I am afraid that it may cause some security issue.

How about binding explicitly to the loopback address?

const server = app.listen(port, '127.0.0.1', () => { ... });

tnorimat · 2026-05-14T01:58:23Z

+      }
+      const body = resultMetadata.body;
+
+      const callback = startCallbackServer(option.port);


If any request hits the server on a path other than /callback, express returns a default 404 but the server stays open indefinitely (until timeout).

If possible, it might be good if the server receives a request other than /callback, then it server closes and the test finishes as failure.

The callback accepts any path, and the path check is performed during the redirect_uri validation to cause failure if it does not match.

Does it means that the callback receives a request with any path, for example, /anothercallback ?

That is correct. After receiving the callback, the redirect_uri and the callback path are compared, so any path other than /callback will result in failure.

Thank you for the clarification.

tnorimat · 2026-05-14T02:18:25Z

+      grant_type: 'authorization_code',
+      code,
+      redirect_uri: REDIRECT_URI_ORIGIN + ':' + option.port + REDIRECT_URI_PATH,
+      client_id: option.clientId,


It includes client_id and client_secret are included into HTTP Form body and assumes that a target authorization server supports client_secret_post as client authentication method.

I am afraid that if an authorization server does not support client_secret_post but support client_secret_basic, then this token request fails.

Therefore, it might be better to firstly look at token_endpoint_auth_methods_supported of server metadata and determine which authentication method is used.

I fixed.
If the authorization server does not support client_secret_post and client_secret_basic, it is set to SKIPPED. The implementations of client_secret_jwt, private_key_jwt, and tls_client_auth are out of scope and have been added to the To do list of this ISSUE.

It might be better to comment on the source file to notify the reader of the codes of why the codes does not accept an authorization server supporting client_secret_jwt, private_key_jwt, and tls_client_auth.

An authorization server supporting client_secret_jwt, private_key_jwt, and tls_client_auth is valid one. Therefore the tests should have accept such the authorization server but the PR would not accept that because supporting each client_secret_jwt, private_key_jwt, and tls_client_auth client authentication method by the tests requires us to implement a lot of codes so it is better to be handled by other follow-up PRs.

I added a comment.

tnorimat · 2026-05-14T02:23:31Z

+      specReferences: [
+        {
+          id: 'Authorization-Code-Grant',
+          url: 'https://datatracker.ietf.org/doc/html/draft-ietf-oauth-v2-1-15#section-4.1'


I think It might be better to point to MCP spec's reference that mandate https://datatracker.ietf.org/doc/html/draft-ietf-oauth-v2-1-15#section-4.1 .

I fixed the version of the OAuth 2.1 URL.
Also, I added OAUTH_2_1_AUTHORIZATION_CODE_GRANT to spec-refercenses.ts.

tnorimat · 2026-05-14T02:23:40Z

+      specReferences: [
+        {
+          id: 'Authorization-Code-Grant',
+          url: 'https://datatracker.ietf.org/doc/html/draft-ietf-oauth-v2-1-15#section-4.1'


I think It might be better to point to MCP spec's reference that mandate https://datatracker.ietf.org/doc/html/draft-ietf-oauth-v2-1-15#section-4.1 .

Also, it might be good to refer OAuth 2.1 v13 instead of v15 because:

the latest MCP revision, namely 2025-11-25, refers v13.

other MCP client and server tests refers v13.

Also, how about following the MCP client and server test convention for the way of referring the spec?
E.g, spec-refercenses.ts:

export const SpecReferences: { [key: string]: SpecReference } = { RFC_PRM_DISCOVERY: { id: 'RFC-9728', url: '[https://www.rfc-editor.org/rfc/rfc9728.html#section-3.1'](https://www.rfc-editor.org/rfc/rfc9728.html#section-3.1%27) },

I fixed the version of the OAuth 2.1 URL.
Also, I added OAUTH_2_1_AUTHORIZATION_CODE_GRANT to spec-refercenses.ts.

tnorimat · 2026-05-15T02:10:21Z

It seems that the files under client directory should be for MCP client tests. Therefore, it might be good that the file be moved to authorization-server/auth/helpers directory.
What do you think about that?

I added authorization-server/auth/helpers/createCallbackServer.ts and removed client/auth/helpers/createCallbackServer.ts.

tnorimat · 2026-05-15T02:11:09Z

    id: 'RFC-7523-JWT-Client-Auth',
    url: 'https://datatracker.ietf.org/doc/html/rfc7523#section-2.2'
  },
+  OAUTH_2_1_AUTHORIZATION_CODE_GRANT: {


It seems that the files under client directory should be for MCP client tests. Therefore, it might be good that the file be moved to authorization-server/auth/spec-references.ts file.
What do you think about that?

I added authorization-server/auth/spec-references.ts and removed OAUTH_2_1_AUTHORIZATION_CODE_GRANT in client/auth/spec-references.ts.

tnorimat · 2026-05-15T02:11:45Z

+      specReferences: [
+        {
+          id: 'Authorization-Code-Grant',
+          url: 'https://datatracker.ietf.org/doc/html/draft-ietf-oauth-v2-1-15#section-4.1'


tnorimat · 2026-05-15T02:11:57Z

+      specReferences: [
+        {
+          id: 'Authorization-Code-Grant',
+          url: 'https://datatracker.ietf.org/doc/html/draft-ietf-oauth-v2-1-15#section-4.1'


tnorimat · 2026-05-15T02:19:43Z

+      grant_type: 'authorization_code',
+      code,
+      redirect_uri: REDIRECT_URI_ORIGIN + ':' + option.port + REDIRECT_URI_PATH,
+      client_id: option.clientId,


It might be better to comment on the source file to notify the reader of the codes of why the codes does not accept an authorization server supporting client_secret_jwt, private_key_jwt, and tls_client_auth.

An authorization server supporting client_secret_jwt, private_key_jwt, and tls_client_auth is valid one. Therefore the tests should have accept such the authorization server but the PR would not accept that because supporting each client_secret_jwt, private_key_jwt, and tls_client_auth client authentication method by the tests requires us to implement a lot of codes so it is better to be handled by other follow-up PRs.

tnorimat

LGTM

Michito-Okai mentioned this pull request May 12, 2026

Add positive tests for the Authorization Code Grant #208

Open

tnorimat suggested changes May 14, 2026

View reviewed changes

Michito-Okai force-pushed the athorization-code-grant branch from 2f8846b to eeed17f Compare May 14, 2026 06:59

tnorimat suggested changes May 15, 2026

View reviewed changes

Michito-Okai force-pushed the athorization-code-grant branch from eeed17f to 90e0d8c Compare May 15, 2026 04:54

feat: add positive tests for the Authorization Code Grant

67acf87

Michito-Okai force-pushed the athorization-code-grant branch from 90e0d8c to 67acf87 Compare May 15, 2026 07:09

tnorimat approved these changes May 15, 2026

View reviewed changes

Conversation

Michito-Okai commented May 12, 2026

Motivation and Context

How Has This Been Tested?

Breaking Changes

Types of changes

Checklist

Uh oh!

pkg-pr-new Bot commented May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Michito-Okai May 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Michito-Okai May 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

pkg-pr-new Bot commented May 12, 2026 •

edited

Loading

Michito-Okai May 14, 2026 •

edited

Loading

Michito-Okai May 14, 2026 •

edited

Loading