From da27836ee7f005266c2f50f5849698202aa60b6e Mon Sep 17 00:00:00 2001 From: Ryan Slama Date: Mon, 14 Jul 2025 13:24:31 -0400 Subject: [PATCH 1/2] Add nonce support for OpenID Connect flows - Add optional nonce parameter to client startAuthorization() - Auto-generate nonce when scope includes 'openid' - Pass nonce through server authorization handler - Update AuthorizationParams type to include nonce - Add comprehensive tests for nonce handling This enables proper OpenID Connect security by preventing replay attacks on ID tokens. --- package-lock.json | 4 +- src/client/auth.test.ts | 59 ++++++++++++++++++++++ src/client/auth.ts | 13 ++++- src/server/auth/handlers/authorize.test.ts | 57 +++++++++++++++++++++ src/server/auth/handlers/authorize.ts | 4 +- src/server/auth/provider.ts | 1 + 6 files changed, 133 insertions(+), 5 deletions(-) diff --git a/package-lock.json b/package-lock.json index 01bc0953..fa1bde0e 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@modelcontextprotocol/sdk", - "version": "1.15.0", + "version": "1.15.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@modelcontextprotocol/sdk", - "version": "1.15.0", + "version": "1.15.1", "license": "MIT", "dependencies": { "ajv": "^6.12.6", diff --git a/src/client/auth.test.ts b/src/client/auth.test.ts index 3d2f18d7..8d173868 100644 --- a/src/client/auth.test.ts +++ b/src/client/auth.test.ts @@ -728,6 +728,65 @@ describe("OAuth Authorization", () => { expect(authorizationUrl.searchParams.get("prompt")).toBe("consent"); }); + it("generates nonce automatically for OpenID Connect flows", async () => { + const { authorizationUrl, nonce } = await startAuthorization( + "https://auth.example.com", + { + clientInformation: validClientInfo, + redirectUrl: "http://localhost:3000/callback", + scope: "openid profile email", + } + ); + + expect(nonce).toBeDefined(); + expect(authorizationUrl.searchParams.get("nonce")).toBe(nonce); + expect(nonce).toMatch(/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/); + }); + + it("uses provided nonce for OpenID Connect flows", async () => { + const providedNonce = "test-nonce-123"; + const { authorizationUrl, nonce } = await startAuthorization( + "https://auth.example.com", + { + clientInformation: validClientInfo, + redirectUrl: "http://localhost:3000/callback", + scope: "openid profile", + nonce: providedNonce, + } + ); + + expect(nonce).toBe(providedNonce); + expect(authorizationUrl.searchParams.get("nonce")).toBe(providedNonce); + }); + + it("does not include nonce for non-OpenID Connect flows", async () => { + const { authorizationUrl, nonce } = await startAuthorization( + "https://auth.example.com", + { + clientInformation: validClientInfo, + redirectUrl: "http://localhost:3000/callback", + scope: "read write", + } + ); + + expect(nonce).toBeUndefined(); + expect(authorizationUrl.searchParams.has("nonce")).toBe(false); + }); + + it("generates nonce when openid scope is included with other scopes", async () => { + const { authorizationUrl, nonce } = await startAuthorization( + "https://auth.example.com", + { + clientInformation: validClientInfo, + redirectUrl: "http://localhost:3000/callback", + scope: "read openid write profile", + } + ); + + expect(nonce).toBeDefined(); + expect(authorizationUrl.searchParams.get("nonce")).toBe(nonce); + }); + it("uses metadata authorization_endpoint when provided", async () => { const { authorizationUrl } = await startAuthorization( "https://auth.example.com", diff --git a/src/client/auth.ts b/src/client/auth.ts index 2bac386f..6fd2f789 100644 --- a/src/client/auth.ts +++ b/src/client/auth.ts @@ -548,6 +548,7 @@ export async function discoverOAuthMetadata( /** * Begins the authorization flow with the given server, by generating a PKCE challenge and constructing the authorization URL. + * For OpenID Connect flows (when scope includes 'openid'), automatically generates a nonce if not provided. */ export async function startAuthorization( authorizationServerUrl: string | URL, @@ -557,6 +558,7 @@ export async function startAuthorization( redirectUrl, scope, state, + nonce, resource, }: { metadata?: OAuthMetadata; @@ -564,9 +566,10 @@ export async function startAuthorization( redirectUrl: string | URL; scope?: string; state?: string; + nonce?: string; resource?: URL; }, -): Promise<{ authorizationUrl: URL; codeVerifier: string }> { +): Promise<{ authorizationUrl: URL; codeVerifier: string; nonce?: string }> { const responseType = "code"; const codeChallengeMethod = "S256"; @@ -625,7 +628,13 @@ export async function startAuthorization( authorizationUrl.searchParams.set("resource", resource.href); } - return { authorizationUrl, codeVerifier }; + let generatedNonce: string | undefined; + if (scope?.includes("openid")) { + generatedNonce = nonce ?? crypto.randomUUID(); + authorizationUrl.searchParams.set("nonce", generatedNonce); + } + + return { authorizationUrl, codeVerifier, nonce: generatedNonce }; } /** diff --git a/src/server/auth/handlers/authorize.test.ts b/src/server/auth/handlers/authorize.test.ts index 438db6a6..50127afc 100644 --- a/src/server/auth/handlers/authorize.test.ts +++ b/src/server/auth/handlers/authorize.test.ts @@ -302,6 +302,63 @@ describe('Authorization Handler', () => { expect.any(Object) ); }); + + it('propagates nonce parameter for OpenID Connect flows', async () => { + const mockProviderWithNonce = jest.spyOn(mockProvider, 'authorize'); + mockProviderWithNonce.mockClear(); + + const response = await supertest(app) + .get('/authorize') + .query({ + client_id: 'valid-client', + redirect_uri: 'https://example.com/callback', + response_type: 'code', + code_challenge: 'challenge123', + code_challenge_method: 'S256', + scope: 'profile email', + nonce: 'test-nonce-123' + }); + + expect(response.status).toBe(302); + expect(mockProviderWithNonce).toHaveBeenCalledWith( + validClient, + expect.objectContaining({ + nonce: 'test-nonce-123', + redirectUri: 'https://example.com/callback', + codeChallenge: 'challenge123', + scopes: ['profile', 'email'] + }), + expect.any(Object) + ); + }); + + it('handles authorization without nonce parameter', async () => { + const mockProviderWithoutNonce = jest.spyOn(mockProvider, 'authorize'); + mockProviderWithoutNonce.mockClear(); + + const response = await supertest(app) + .get('/authorize') + .query({ + client_id: 'valid-client', + redirect_uri: 'https://example.com/callback', + response_type: 'code', + code_challenge: 'challenge123', + code_challenge_method: 'S256', + scope: 'profile email' + }); + + expect(response.status).toBe(302); + expect(mockProviderWithoutNonce).toHaveBeenCalledWith( + validClient, + expect.objectContaining({ + nonce: undefined, + redirectUri: 'https://example.com/callback', + codeChallenge: 'challenge123', + scopes: ['profile', 'email'] + }), + expect.any(Object) + ); + }); }); describe('Successful authorization', () => { diff --git a/src/server/auth/handlers/authorize.ts b/src/server/auth/handlers/authorize.ts index 126ce006..397f95e5 100644 --- a/src/server/auth/handlers/authorize.ts +++ b/src/server/auth/handlers/authorize.ts @@ -35,6 +35,7 @@ const RequestAuthorizationParamsSchema = z.object({ code_challenge_method: z.literal("S256"), scope: z.string().optional(), state: z.string().optional(), + nonce: z.string().optional(), resource: z.string().url().optional(), }); @@ -115,7 +116,7 @@ export function authorizationHandler({ provider, rateLimit: rateLimitConfig }: A throw new InvalidRequestError(parseResult.error.message); } - const { scope, code_challenge, resource } = parseResult.data; + const { scope, code_challenge, nonce, resource } = parseResult.data; state = parseResult.data.state; // Validate scopes @@ -138,6 +139,7 @@ export function authorizationHandler({ provider, rateLimit: rateLimitConfig }: A scopes: requestedScopes, redirectUri: redirect_uri, codeChallenge: code_challenge, + nonce, resource: resource ? new URL(resource) : undefined, }, res); } catch (error) { diff --git a/src/server/auth/provider.ts b/src/server/auth/provider.ts index 18beb216..9617479a 100644 --- a/src/server/auth/provider.ts +++ b/src/server/auth/provider.ts @@ -8,6 +8,7 @@ export type AuthorizationParams = { scopes?: string[]; codeChallenge: string; redirectUri: string; + nonce?: string; resource?: URL; }; From fd335d2de2cc70bba99be57ac8fa0103a74ebd90 Mon Sep 17 00:00:00 2001 From: Ryan Slama Date: Mon, 14 Jul 2025 17:58:55 -0400 Subject: [PATCH 2/2] Improve test isolation - Add afterEach to restore all mocks - Remove unnecessary mockClear() calls - Ensures tests are properly isolated --- src/server/auth/handlers/authorize.test.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/server/auth/handlers/authorize.test.ts b/src/server/auth/handlers/authorize.test.ts index 50127afc..92773791 100644 --- a/src/server/auth/handlers/authorize.test.ts +++ b/src/server/auth/handlers/authorize.test.ts @@ -102,6 +102,10 @@ describe('Authorization Handler', () => { app.use('/authorize', handler); }); + afterEach(() => { + jest.restoreAllMocks(); + }); + describe('HTTP method validation', () => { it('rejects non-GET/POST methods', async () => { const response = await supertest(app) @@ -305,7 +309,6 @@ describe('Authorization Handler', () => { it('propagates nonce parameter for OpenID Connect flows', async () => { const mockProviderWithNonce = jest.spyOn(mockProvider, 'authorize'); - mockProviderWithNonce.mockClear(); const response = await supertest(app) .get('/authorize') @@ -334,7 +337,6 @@ describe('Authorization Handler', () => { it('handles authorization without nonce parameter', async () => { const mockProviderWithoutNonce = jest.spyOn(mockProvider, 'authorize'); - mockProviderWithoutNonce.mockClear(); const response = await supertest(app) .get('/authorize')