From 7f03f13072446ff90703761945c504a9174d2132 Mon Sep 17 00:00:00 2001 From: Allen Zhou <46854522+allenzhou101@users.noreply.github.com> Date: Mon, 24 Feb 2025 16:07:54 -0800 Subject: [PATCH 01/15] Add ProxyOAuthServerProvider --- src/server/auth/handlers/token.ts | 14 +- src/server/auth/provider.ts | 2 +- src/server/auth/proxyProvider.ts | 224 ++++++++++++++++++++++++++++++ 3 files changed, 234 insertions(+), 6 deletions(-) create mode 100644 src/server/auth/proxyProvider.ts diff --git a/src/server/auth/handlers/token.ts b/src/server/auth/handlers/token.ts index 79312068a..3a29bb55b 100644 --- a/src/server/auth/handlers/token.ts +++ b/src/server/auth/handlers/token.ts @@ -14,6 +14,7 @@ import { TooManyRequestsError, OAuthError } from "../errors.js"; +import { ProxyOAuthServerProvider } from "../proxyProvider.js"; export type TokenHandlerOptions = { provider: OAuthServerProvider; @@ -90,13 +91,16 @@ export function tokenHandler({ provider, rateLimit: rateLimitConfig }: TokenHand const { code, code_verifier } = parseResult.data; - // Verify PKCE challenge - const codeChallenge = await provider.challengeForAuthorizationCode(client, code); - if (!(await verifyChallenge(code_verifier, codeChallenge))) { - throw new InvalidGrantError("code_verifier does not match the challenge"); + // Skip local PKCE verification for proxy providers since the code_challenge is stored on the upstream server. + // The code_verifier will be passed to the upstream server during token exchange. + if (!(provider instanceof ProxyOAuthServerProvider)) { + const codeChallenge = await provider.challengeForAuthorizationCode(client, code); + if (!(await verifyChallenge(code_verifier, codeChallenge))) { + throw new InvalidGrantError("code_verifier does not match the challenge"); + } } - const tokens = await provider.exchangeAuthorizationCode(client, code); + const tokens = await provider.exchangeAuthorizationCode(client, code, code_verifier); res.status(200).json(tokens); break; } diff --git a/src/server/auth/provider.ts b/src/server/auth/provider.ts index 7416c5544..aacd308b8 100644 --- a/src/server/auth/provider.ts +++ b/src/server/auth/provider.ts @@ -36,7 +36,7 @@ export interface OAuthServerProvider { /** * Exchanges an authorization code for an access token. */ - exchangeAuthorizationCode(client: OAuthClientInformationFull, authorizationCode: string): Promise; + exchangeAuthorizationCode(client: OAuthClientInformationFull, authorizationCode: string, codeVerifier?: string): Promise; /** * Exchanges a refresh token for an access token. diff --git a/src/server/auth/proxyProvider.ts b/src/server/auth/proxyProvider.ts new file mode 100644 index 000000000..3771fb2ff --- /dev/null +++ b/src/server/auth/proxyProvider.ts @@ -0,0 +1,224 @@ +import { Response } from "express"; +import { OAuthRegisteredClientsStore } from "./clients.js"; +import { + OAuthClientInformationFull, + OAuthTokenRevocationRequest, + OAuthTokens, + OAuthTokensSchema, +} from "./../../shared/auth.js"; +import { AuthInfo } from "./types.js"; +import { AuthorizationParams, OAuthServerProvider } from "./provider.js"; +import { ServerError } from "./errors.js"; + +export type ProxyEndpoints = { + authorizationUrl?: string; + tokenUrl?: string; + revocationUrl?: string; + registrationUrl?: string; +}; + +export type ProxyOptions = { + /** + * Individual endpoint URLs for proxying specific OAuth operations + */ + endpoints: ProxyEndpoints; + + /** + * Function to verify access tokens and return auth info + */ + verifyToken: (token: string) => Promise; + + /** + * Function to fetch client information from the upstream server + */ + getClient: (clientId: string) => Promise; + +}; + +/** + * Implements an OAuth server that proxies requests to another OAuth server. + */ +export class ProxyOAuthServerProvider implements OAuthServerProvider { + private readonly _endpoints: ProxyEndpoints; + private readonly _verifyToken: (token: string) => Promise; + private readonly _getClient: (clientId: string) => Promise; + + public revokeToken?: ( + client: OAuthClientInformationFull, + request: OAuthTokenRevocationRequest + ) => Promise; + + constructor(options: ProxyOptions) { + this._endpoints = options.endpoints; + this._verifyToken = options.verifyToken; + this._getClient = options.getClient; + if (options.endpoints?.revocationUrl) { + this.revokeToken = async ( + client: OAuthClientInformationFull, + request: OAuthTokenRevocationRequest + ) => { + const revocationUrl = this._endpoints.revocationUrl; + + if (!revocationUrl) { + throw new Error("No revocation endpoint configured"); + } + + const params = new URLSearchParams(); + params.set("token", request.token); + params.set("client_id", client.client_id); + params.set("client_secret", client.client_secret || ""); + if (request.token_type_hint) { + params.set("token_type_hint", request.token_type_hint); + } + + const response = await fetch(revocationUrl, { + method: "POST", + headers: { + "Content-Type": "application/x-www-form-urlencoded", + }, + body: params, + }); + + if (!response.ok) { + throw new ServerError(`Token revocation failed: ${response.status}`); + } + } + } + } + + get clientsStore(): OAuthRegisteredClientsStore { + const registrationUrl = this._endpoints.registrationUrl; + return { + getClient: this._getClient, + ...(registrationUrl && { + registerClient: async (client: OAuthClientInformationFull) => { + const response = await fetch(registrationUrl, { + method: "POST", + headers: { + "Content-Type": "application/json", + }, + body: JSON.stringify(client), + }); + + if (!response.ok) { + throw new ServerError(`Client registration failed: ${response.status}`); + } + + return response.json(); + } + }) + } + } + + async authorize( + client: OAuthClientInformationFull, + params: AuthorizationParams, + res: Response + ): Promise { + const authorizationUrl = this._endpoints.authorizationUrl; + + if (!authorizationUrl) { + throw new Error("No authorization endpoint configured"); + } + + // Start with required OAuth parameters + const targetUrl = new URL(authorizationUrl); + const searchParams = new URLSearchParams({ + client_id: client.client_id, + response_type: "code", + redirect_uri: params.redirectUri, + code_challenge: params.codeChallenge, + code_challenge_method: "S256" + }); + + // Add optional standard OAuth parameters + if (params.state) searchParams.set("state", params.state); + if (params.scopes?.length) searchParams.set("scope", params.scopes.join(" ")); + + targetUrl.search = searchParams.toString(); + res.redirect(targetUrl.toString()); + } + + async challengeForAuthorizationCode( + _client: OAuthClientInformationFull, + _authorizationCode: string + ): Promise { + // In a proxy setup, we don't store the code challenge ourselves + // Instead, we proxy the token request and let the upstream server validate it + return ""; + } + + async exchangeAuthorizationCode( + client: OAuthClientInformationFull, + authorizationCode: string, + codeVerifier?: string + ): Promise { + const tokenUrl = this._endpoints.tokenUrl; + + if (!tokenUrl) { + throw new Error("No token endpoint configured"); + } + const response = await fetch(tokenUrl, { + method: "POST", + headers: { + "Content-Type": "application/x-www-form-urlencoded", + }, + body: new URLSearchParams({ + grant_type: "authorization_code", + client_id: client.client_id, + client_secret: client.client_secret || "", + code: authorizationCode, + code_verifier: codeVerifier || "", + }), + }); + + if (!response.ok) { + throw new ServerError(`Token exchange failed: ${response.status}`); + } + + const data = await response.json(); + return OAuthTokensSchema.parse(data); + } + + async exchangeRefreshToken( + client: OAuthClientInformationFull, + refreshToken: string, + scopes?: string[] + ): Promise { + const tokenUrl = this._endpoints.tokenUrl; + + if (!tokenUrl) { + throw new Error("No token endpoint configured"); + } + + const params = new URLSearchParams({ + grant_type: "refresh_token", + client_id: client.client_id, + client_secret: client.client_secret || "", + refresh_token: refreshToken, + }); + + if (scopes?.length) { + params.set("scope", scopes.join(" ")); + } + + const response = await fetch(tokenUrl, { + method: "POST", + headers: { + "Content-Type": "application/x-www-form-urlencoded", + }, + body: params, + }); + + if (!response.ok) { + throw new ServerError(`Token refresh failed: ${response.status}`); + } + + const data = await response.json(); + return OAuthTokensSchema.parse(data); + } + + async verifyAccessToken(token: string): Promise { + return this._verifyToken(token); + } +} \ No newline at end of file From 9e32fd5464ae9f5f29a15c405d61275e667d2ea1 Mon Sep 17 00:00:00 2001 From: Allen Zhou <46854522+allenzhou101@users.noreply.github.com> Date: Mon, 24 Feb 2025 16:55:36 -0800 Subject: [PATCH 02/15] Only send client secret and code verifier if defined --- src/server/auth/proxyProvider.ts | 33 +++++++++++++++++++++++--------- 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/src/server/auth/proxyProvider.ts b/src/server/auth/proxyProvider.ts index 3771fb2ff..f85ef8535 100644 --- a/src/server/auth/proxyProvider.ts +++ b/src/server/auth/proxyProvider.ts @@ -66,7 +66,9 @@ export class ProxyOAuthServerProvider implements OAuthServerProvider { const params = new URLSearchParams(); params.set("token", request.token); params.set("client_id", client.client_id); - params.set("client_secret", client.client_secret || ""); + if (client.client_secret) { + params.set("client_secret", client.client_secret); + } if (request.token_type_hint) { params.set("token_type_hint", request.token_type_hint); } @@ -158,20 +160,30 @@ export class ProxyOAuthServerProvider implements OAuthServerProvider { if (!tokenUrl) { throw new Error("No token endpoint configured"); } + + const params = new URLSearchParams({ + grant_type: "authorization_code", + client_id: client.client_id, + code: authorizationCode, + }); + + if (client.client_secret) { + params.append("client_secret", client.client_secret); + } + + if (codeVerifier) { + params.append("code_verifier", codeVerifier); + } + const response = await fetch(tokenUrl, { method: "POST", headers: { "Content-Type": "application/x-www-form-urlencoded", }, - body: new URLSearchParams({ - grant_type: "authorization_code", - client_id: client.client_id, - client_secret: client.client_secret || "", - code: authorizationCode, - code_verifier: codeVerifier || "", - }), + body: params.toString(), }); + if (!response.ok) { throw new ServerError(`Token exchange failed: ${response.status}`); } @@ -194,10 +206,13 @@ export class ProxyOAuthServerProvider implements OAuthServerProvider { const params = new URLSearchParams({ grant_type: "refresh_token", client_id: client.client_id, - client_secret: client.client_secret || "", refresh_token: refreshToken, }); + if (client.client_secret) { + params.set("client_secret", client.client_secret); + } + if (scopes?.length) { params.set("scope", scopes.join(" ")); } From 996822f18d38ada17f5775bdde60d2b2dd28b693 Mon Sep 17 00:00:00 2001 From: Allen Zhou <46854522+allenzhou101@users.noreply.github.com> Date: Mon, 24 Feb 2025 16:56:13 -0800 Subject: [PATCH 03/15] Test if code verifier is passed through to upstream proxy --- src/server/auth/handlers/token.test.ts | 61 ++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/src/server/auth/handlers/token.test.ts b/src/server/auth/handlers/token.test.ts index 7d15e44a2..1b44721f0 100644 --- a/src/server/auth/handlers/token.test.ts +++ b/src/server/auth/handlers/token.test.ts @@ -7,6 +7,7 @@ import supertest from 'supertest'; import * as pkceChallenge from 'pkce-challenge'; import { InvalidGrantError, InvalidTokenError } from '../errors.js'; import { AuthInfo } from '../types.js'; +import { ProxyOAuthServerProvider } from '../proxyProvider.js'; // Mock pkce-challenge jest.mock('pkce-challenge', () => ({ @@ -280,6 +281,66 @@ describe('Token Handler', () => { expect(response.body.expires_in).toBe(3600); expect(response.body.refresh_token).toBe('mock_refresh_token'); }); + + it('passes through PKCE verification for proxy providers', async () => { + const originalFetch = global.fetch; + + try { + global.fetch = jest.fn().mockResolvedValue({ + ok: true, + json: () => Promise.resolve({ + access_token: 'mock_access_token', + token_type: 'bearer', + expires_in: 3600, + refresh_token: 'mock_refresh_token' + }) + }); + + const proxyProvider = new ProxyOAuthServerProvider({ + endpoints: { + tokenUrl: 'https://example.com/token' + }, + verifyToken: async (token) => ({ + token, + clientId: 'valid-client', + scopes: ['read', 'write'], + expiresAt: Date.now() / 1000 + 3600 + }), + getClient: async (clientId) => clientId === 'valid-client' ? validClient : undefined + }); + + const proxyApp = express(); + const options: TokenHandlerOptions = { provider: proxyProvider }; + proxyApp.use('/token', tokenHandler(options)); + + const response = await supertest(proxyApp) + .post('/token') + .type('form') + .send({ + client_id: 'valid-client', + client_secret: 'valid-secret', + grant_type: 'authorization_code', + code: 'valid_code', + code_verifier: 'any_verifier' + }); + + expect(response.status).toBe(200); + expect(response.body.access_token).toBe('mock_access_token'); + + expect(global.fetch).toHaveBeenCalledWith( + 'https://example.com/token', + expect.objectContaining({ + method: 'POST', + headers: { + 'Content-Type': 'application/x-www-form-urlencoded' + }, + body: expect.stringContaining('code_verifier=any_verifier') + }) + ); + } finally { + global.fetch = originalFetch; + } + }); }); describe('Refresh token grant', () => { From 476c11149b4cc67b773bfe480d279c407217bf81 Mon Sep 17 00:00:00 2001 From: Allen Zhou <46854522+allenzhou101@users.noreply.github.com> Date: Mon, 24 Feb 2025 17:10:37 -0800 Subject: [PATCH 04/15] Add Proxy Provider test --- src/server/auth/proxyProvider.test.ts | 285 ++++++++++++++++++++++++++ src/server/auth/proxyProvider.ts | 4 +- 2 files changed, 287 insertions(+), 2 deletions(-) create mode 100644 src/server/auth/proxyProvider.test.ts diff --git a/src/server/auth/proxyProvider.test.ts b/src/server/auth/proxyProvider.test.ts new file mode 100644 index 000000000..9a0ae3daa --- /dev/null +++ b/src/server/auth/proxyProvider.test.ts @@ -0,0 +1,285 @@ +import { Response } from "express"; +import { ProxyOAuthServerProvider, ProxyOptions } from "./proxyProvider.js"; +import { AuthInfo } from "./types.js"; +import { OAuthClientInformationFull, OAuthTokens } from "../../shared/auth.js"; +import { ServerError } from "./errors.js"; + +describe("Proxy OAuth Server Provider", () => { + // Mock client data + const validClient: OAuthClientInformationFull = { + client_id: "test-client", + client_secret: "test-secret", + redirect_uris: ["https://example.com/callback"], + }; + + // Mock response object + const mockResponse = { + redirect: jest.fn(), + } as unknown as Response; + + // Base provider options + const baseOptions: ProxyOptions = { + endpoints: { + authorizationUrl: "https://auth.example.com/authorize", + tokenUrl: "https://auth.example.com/token", + revocationUrl: "https://auth.example.com/revoke", + registrationUrl: "https://auth.example.com/register", + }, + verifyToken: jest.fn().mockImplementation(async (token: string) => { + if (token === "valid-token") { + return { + token, + clientId: "test-client", + scopes: ["read", "write"], + expiresAt: Date.now() / 1000 + 3600, + } as AuthInfo; + } + throw new Error("Invalid token"); + }), + getClient: jest.fn().mockImplementation(async (clientId: string) => { + if (clientId === "test-client") { + return validClient; + } + return undefined; + }), + }; + + let provider: ProxyOAuthServerProvider; + let originalFetch: typeof global.fetch; + + beforeEach(() => { + provider = new ProxyOAuthServerProvider(baseOptions); + originalFetch = global.fetch; + global.fetch = jest.fn(); + }); + + afterEach(() => { + global.fetch = originalFetch; + jest.clearAllMocks(); + }); + + describe("authorization", () => { + it("redirects to authorization endpoint with correct parameters", async () => { + await provider.authorize( + validClient, + { + redirectUri: "https://example.com/callback", + codeChallenge: "test-challenge", + state: "test-state", + scopes: ["read", "write"], + }, + mockResponse + ); + + const expectedUrl = new URL("https://auth.example.com/authorize"); + expectedUrl.searchParams.set("client_id", "test-client"); + expectedUrl.searchParams.set("response_type", "code"); + expectedUrl.searchParams.set("redirect_uri", "https://example.com/callback"); + expectedUrl.searchParams.set("code_challenge", "test-challenge"); + expectedUrl.searchParams.set("code_challenge_method", "S256"); + expectedUrl.searchParams.set("state", "test-state"); + expectedUrl.searchParams.set("scope", "read write"); + + expect(mockResponse.redirect).toHaveBeenCalledWith(expectedUrl.toString()); + }); + + it("throws error when authorization endpoint is not configured", async () => { + const providerWithoutAuth = new ProxyOAuthServerProvider({ + ...baseOptions, + endpoints: { ...baseOptions.endpoints, authorizationUrl: undefined }, + }); + + await expect( + providerWithoutAuth.authorize(validClient, { + redirectUri: "https://example.com/callback", + codeChallenge: "test-challenge", + }, mockResponse) + ).rejects.toThrow("No authorization endpoint configured"); + }); + }); + + describe("token exchange", () => { + const mockTokenResponse: OAuthTokens = { + access_token: "new-access-token", + token_type: "Bearer", + expires_in: 3600, + refresh_token: "new-refresh-token", + }; + + beforeEach(() => { + (global.fetch as jest.Mock).mockImplementation(() => + Promise.resolve({ + ok: true, + json: () => Promise.resolve(mockTokenResponse), + }) + ); + }); + + it("exchanges authorization code for tokens", async () => { + const tokens = await provider.exchangeAuthorizationCode( + validClient, + "test-code", + "test-verifier" + ); + + expect(global.fetch).toHaveBeenCalledWith( + "https://auth.example.com/token", + expect.objectContaining({ + method: "POST", + headers: { + "Content-Type": "application/x-www-form-urlencoded", + }, + body: expect.stringContaining("grant_type=authorization_code") + }) + ); + expect(tokens).toEqual(mockTokenResponse); + }); + + it("exchanges refresh token for new tokens", async () => { + const tokens = await provider.exchangeRefreshToken( + validClient, + "test-refresh-token", + ["read", "write"] + ); + + expect(global.fetch).toHaveBeenCalledWith( + "https://auth.example.com/token", + expect.objectContaining({ + method: "POST", + headers: { + "Content-Type": "application/x-www-form-urlencoded", + }, + body: expect.stringContaining("grant_type=refresh_token") + }) + ); + expect(tokens).toEqual(mockTokenResponse); + }); + + it("throws error when token endpoint is not configured", async () => { + const providerWithoutToken = new ProxyOAuthServerProvider({ + ...baseOptions, + endpoints: { ...baseOptions.endpoints, tokenUrl: undefined }, + }); + + await expect( + providerWithoutToken.exchangeAuthorizationCode(validClient, "test-code") + ).rejects.toThrow("No token endpoint configured"); + }); + + it("handles token exchange failure", async () => { + (global.fetch as jest.Mock).mockImplementation(() => + Promise.resolve({ + ok: false, + status: 400, + }) + ); + + await expect( + provider.exchangeAuthorizationCode(validClient, "invalid-code") + ).rejects.toThrow(ServerError); + }); + }); + + describe("client registration", () => { + it("registers new client", async () => { + const newClient: OAuthClientInformationFull = { + client_id: "new-client", + redirect_uris: ["https://new-client.com/callback"], + }; + + (global.fetch as jest.Mock).mockImplementation(() => + Promise.resolve({ + ok: true, + json: () => Promise.resolve(newClient), + }) + ); + + const result = await provider.clientsStore.registerClient!(newClient); + + expect(global.fetch).toHaveBeenCalledWith( + "https://auth.example.com/register", + expect.objectContaining({ + method: "POST", + headers: { + "Content-Type": "application/json", + }, + body: JSON.stringify(newClient), + }) + ); + expect(result).toEqual(newClient); + }); + + it("handles registration failure", async () => { + (global.fetch as jest.Mock).mockImplementation(() => + Promise.resolve({ + ok: false, + status: 400, + }) + ); + + const newClient: OAuthClientInformationFull = { + client_id: "new-client", + redirect_uris: ["https://new-client.com/callback"], + }; + + await expect( + provider.clientsStore.registerClient!(newClient) + ).rejects.toThrow(ServerError); + }); + }); + + describe("token revocation", () => { + it("revokes token", async () => { + (global.fetch as jest.Mock).mockImplementation(() => + Promise.resolve({ + ok: true, + }) + ); + + await provider.revokeToken!(validClient, { + token: "token-to-revoke", + token_type_hint: "access_token", + }); + + expect(global.fetch).toHaveBeenCalledWith( + "https://auth.example.com/revoke", + expect.objectContaining({ + method: "POST", + headers: { + "Content-Type": "application/x-www-form-urlencoded", + }, + body: expect.stringContaining("token=token-to-revoke"), + }) + ); + }); + + it("handles revocation failure", async () => { + (global.fetch as jest.Mock).mockImplementation(() => + Promise.resolve({ + ok: false, + status: 400, + }) + ); + + await expect( + provider.revokeToken!(validClient, { + token: "invalid-token", + }) + ).rejects.toThrow(ServerError); + }); + }); + + describe("token verification", () => { + it("verifies valid token", async () => { + const authInfo = await provider.verifyAccessToken("valid-token"); + expect(authInfo.token).toBe("valid-token"); + expect(baseOptions.verifyToken).toHaveBeenCalledWith("valid-token"); + }); + + it("rejects invalid token", async () => { + await expect( + provider.verifyAccessToken("invalid-token") + ).rejects.toThrow("Invalid token"); + }); + }); +}); \ No newline at end of file diff --git a/src/server/auth/proxyProvider.ts b/src/server/auth/proxyProvider.ts index f85ef8535..d164b6468 100644 --- a/src/server/auth/proxyProvider.ts +++ b/src/server/auth/proxyProvider.ts @@ -78,7 +78,7 @@ export class ProxyOAuthServerProvider implements OAuthServerProvider { headers: { "Content-Type": "application/x-www-form-urlencoded", }, - body: params, + body: params.toString(), }); if (!response.ok) { @@ -222,7 +222,7 @@ export class ProxyOAuthServerProvider implements OAuthServerProvider { headers: { "Content-Type": "application/x-www-form-urlencoded", }, - body: params, + body: params.toString(), }); if (!response.ok) { From cb8135840c22e2c3d21e91f491639d4869164328 Mon Sep 17 00:00:00 2001 From: Allen Zhou <46854522+allenzhou101@users.noreply.github.com> Date: Mon, 24 Feb 2025 17:39:50 -0800 Subject: [PATCH 05/15] Change verifyToken to match parent interface name --- src/server/auth/handlers/token.test.ts | 2 +- src/server/auth/proxyProvider.test.ts | 80 +++++++++++++++++++------- src/server/auth/proxyProvider.ts | 8 +-- 3 files changed, 65 insertions(+), 25 deletions(-) diff --git a/src/server/auth/handlers/token.test.ts b/src/server/auth/handlers/token.test.ts index 1b44721f0..000b29084 100644 --- a/src/server/auth/handlers/token.test.ts +++ b/src/server/auth/handlers/token.test.ts @@ -300,7 +300,7 @@ describe('Token Handler', () => { endpoints: { tokenUrl: 'https://example.com/token' }, - verifyToken: async (token) => ({ + verifyAccessToken: async (token) => ({ token, clientId: 'valid-client', scopes: ['read', 'write'], diff --git a/src/server/auth/proxyProvider.test.ts b/src/server/auth/proxyProvider.test.ts index 9a0ae3daa..8c6106ff2 100644 --- a/src/server/auth/proxyProvider.test.ts +++ b/src/server/auth/proxyProvider.test.ts @@ -3,6 +3,8 @@ import { ProxyOAuthServerProvider, ProxyOptions } from "./proxyProvider.js"; import { AuthInfo } from "./types.js"; import { OAuthClientInformationFull, OAuthTokens } from "../../shared/auth.js"; import { ServerError } from "./errors.js"; +import { InvalidTokenError } from "./errors.js"; +import { InsufficientScopeError } from "./errors.js"; describe("Proxy OAuth Server Provider", () => { // Mock client data @@ -17,6 +19,10 @@ describe("Proxy OAuth Server Provider", () => { redirect: jest.fn(), } as unknown as Response; + // Mock provider functions + const mockVerifyToken = jest.fn(); + const mockGetClient = jest.fn(); + // Base provider options const baseOptions: ProxyOptions = { endpoints: { @@ -25,7 +31,20 @@ describe("Proxy OAuth Server Provider", () => { revocationUrl: "https://auth.example.com/revoke", registrationUrl: "https://auth.example.com/register", }, - verifyToken: jest.fn().mockImplementation(async (token: string) => { + verifyAccessToken: mockVerifyToken, + getClient: mockGetClient, + }; + + let provider: ProxyOAuthServerProvider; + let originalFetch: typeof global.fetch; + + beforeEach(() => { + provider = new ProxyOAuthServerProvider(baseOptions); + originalFetch = global.fetch; + global.fetch = jest.fn(); + + // Setup mock implementations + mockVerifyToken.mockImplementation(async (token: string) => { if (token === "valid-token") { return { token, @@ -34,23 +53,15 @@ describe("Proxy OAuth Server Provider", () => { expiresAt: Date.now() / 1000 + 3600, } as AuthInfo; } - throw new Error("Invalid token"); - }), - getClient: jest.fn().mockImplementation(async (clientId: string) => { + throw new InvalidTokenError("Invalid token"); + }); + + mockGetClient.mockImplementation(async (clientId: string) => { if (clientId === "test-client") { return validClient; } return undefined; - }), - }; - - let provider: ProxyOAuthServerProvider; - let originalFetch: typeof global.fetch; - - beforeEach(() => { - provider = new ProxyOAuthServerProvider(baseOptions); - originalFetch = global.fetch; - global.fetch = jest.fn(); + }); }); afterEach(() => { @@ -271,15 +282,44 @@ describe("Proxy OAuth Server Provider", () => { describe("token verification", () => { it("verifies valid token", async () => { + const validAuthInfo: AuthInfo = { + token: "valid-token", + clientId: "test-client", + scopes: ["read", "write"], + expiresAt: Date.now() / 1000 + 3600, + }; + mockVerifyToken.mockResolvedValue(validAuthInfo); + const authInfo = await provider.verifyAccessToken("valid-token"); - expect(authInfo.token).toBe("valid-token"); - expect(baseOptions.verifyToken).toHaveBeenCalledWith("valid-token"); + expect(authInfo).toEqual(validAuthInfo); + expect(mockVerifyToken).toHaveBeenCalledWith("valid-token"); }); - it("rejects invalid token", async () => { - await expect( - provider.verifyAccessToken("invalid-token") - ).rejects.toThrow("Invalid token"); + it("passes through InvalidTokenError", async () => { + const error = new InvalidTokenError("Token expired"); + mockVerifyToken.mockRejectedValue(error); + + await expect(provider.verifyAccessToken("invalid-token")) + .rejects.toBe(error); + expect(mockVerifyToken).toHaveBeenCalledWith("invalid-token"); + }); + + it("passes through InsufficientScopeError", async () => { + const error = new InsufficientScopeError("Required scopes: read, write"); + mockVerifyToken.mockRejectedValue(error); + + await expect(provider.verifyAccessToken("token-with-insufficient-scope")) + .rejects.toBe(error); + expect(mockVerifyToken).toHaveBeenCalledWith("token-with-insufficient-scope"); + }); + + it("passes through unexpected errors", async () => { + const error = new Error("Unexpected error"); + mockVerifyToken.mockRejectedValue(error); + + await expect(provider.verifyAccessToken("valid-token")) + .rejects.toBe(error); + expect(mockVerifyToken).toHaveBeenCalledWith("valid-token"); }); }); }); \ No newline at end of file diff --git a/src/server/auth/proxyProvider.ts b/src/server/auth/proxyProvider.ts index d164b6468..81727548c 100644 --- a/src/server/auth/proxyProvider.ts +++ b/src/server/auth/proxyProvider.ts @@ -26,7 +26,7 @@ export type ProxyOptions = { /** * Function to verify access tokens and return auth info */ - verifyToken: (token: string) => Promise; + verifyAccessToken: (token: string) => Promise; /** * Function to fetch client information from the upstream server @@ -40,7 +40,7 @@ export type ProxyOptions = { */ export class ProxyOAuthServerProvider implements OAuthServerProvider { private readonly _endpoints: ProxyEndpoints; - private readonly _verifyToken: (token: string) => Promise; + private readonly _verifyAccessToken: (token: string) => Promise; private readonly _getClient: (clientId: string) => Promise; public revokeToken?: ( @@ -50,7 +50,7 @@ export class ProxyOAuthServerProvider implements OAuthServerProvider { constructor(options: ProxyOptions) { this._endpoints = options.endpoints; - this._verifyToken = options.verifyToken; + this._verifyAccessToken = options.verifyAccessToken; this._getClient = options.getClient; if (options.endpoints?.revocationUrl) { this.revokeToken = async ( @@ -234,6 +234,6 @@ export class ProxyOAuthServerProvider implements OAuthServerProvider { } async verifyAccessToken(token: string): Promise { - return this._verifyToken(token); + return this._verifyAccessToken(token); } } \ No newline at end of file From c7ce1a29c53b73a40c40ba5f4c5add68542e2633 Mon Sep 17 00:00:00 2001 From: Allen Zhou <46854522+allenzhou101@users.noreply.github.com> Date: Mon, 24 Feb 2025 17:43:16 -0800 Subject: [PATCH 06/15] improve proxy code verifier test naming --- src/server/auth/handlers/token.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/server/auth/handlers/token.test.ts b/src/server/auth/handlers/token.test.ts index 000b29084..0c4edbffc 100644 --- a/src/server/auth/handlers/token.test.ts +++ b/src/server/auth/handlers/token.test.ts @@ -282,7 +282,7 @@ describe('Token Handler', () => { expect(response.body.refresh_token).toBe('mock_refresh_token'); }); - it('passes through PKCE verification for proxy providers', async () => { + it('passes through code verifier when using proxy provider', async () => { const originalFetch = global.fetch; try { From aa5690b182eda522b72e5dc61f0c7b3380708426 Mon Sep 17 00:00:00 2001 From: Allen Zhou <46854522+allenzhou101@users.noreply.github.com> Date: Mon, 24 Feb 2025 20:17:08 -0800 Subject: [PATCH 07/15] Zod parsing for registration response --- src/server/auth/proxyProvider.test.ts | 34 ++++++++++----------------- src/server/auth/proxyProvider.ts | 4 +++- 2 files changed, 16 insertions(+), 22 deletions(-) diff --git a/src/server/auth/proxyProvider.test.ts b/src/server/auth/proxyProvider.test.ts index 8c6106ff2..ab73d7b41 100644 --- a/src/server/auth/proxyProvider.test.ts +++ b/src/server/auth/proxyProvider.test.ts @@ -64,6 +64,16 @@ describe("Proxy OAuth Server Provider", () => { }); }); + // Add helper function for failed responses + const mockFailedResponse = () => { + (global.fetch as jest.Mock).mockImplementation(() => + Promise.resolve({ + ok: false, + status: 400, + }) + ); + }; + afterEach(() => { global.fetch = originalFetch; jest.clearAllMocks(); @@ -178,13 +188,7 @@ describe("Proxy OAuth Server Provider", () => { }); it("handles token exchange failure", async () => { - (global.fetch as jest.Mock).mockImplementation(() => - Promise.resolve({ - ok: false, - status: 400, - }) - ); - + mockFailedResponse(); await expect( provider.exchangeAuthorizationCode(validClient, "invalid-code") ).rejects.toThrow(ServerError); @@ -221,13 +225,7 @@ describe("Proxy OAuth Server Provider", () => { }); it("handles registration failure", async () => { - (global.fetch as jest.Mock).mockImplementation(() => - Promise.resolve({ - ok: false, - status: 400, - }) - ); - + mockFailedResponse(); const newClient: OAuthClientInformationFull = { client_id: "new-client", redirect_uris: ["https://new-client.com/callback"], @@ -265,13 +263,7 @@ describe("Proxy OAuth Server Provider", () => { }); it("handles revocation failure", async () => { - (global.fetch as jest.Mock).mockImplementation(() => - Promise.resolve({ - ok: false, - status: 400, - }) - ); - + mockFailedResponse(); await expect( provider.revokeToken!(validClient, { token: "invalid-token", diff --git a/src/server/auth/proxyProvider.ts b/src/server/auth/proxyProvider.ts index 81727548c..2b5ad3f2e 100644 --- a/src/server/auth/proxyProvider.ts +++ b/src/server/auth/proxyProvider.ts @@ -2,6 +2,7 @@ import { Response } from "express"; import { OAuthRegisteredClientsStore } from "./clients.js"; import { OAuthClientInformationFull, + OAuthClientInformationFullSchema, OAuthTokenRevocationRequest, OAuthTokens, OAuthTokensSchema, @@ -106,7 +107,8 @@ export class ProxyOAuthServerProvider implements OAuthServerProvider { throw new ServerError(`Client registration failed: ${response.status}`); } - return response.json(); + const data = await response.json(); + return OAuthClientInformationFullSchema.parse(data); } }) } From 96f3d81aa3ebb2b77c3522f130b3642f36d2e586 Mon Sep 17 00:00:00 2001 From: Allen Zhou <46854522+allenzhou101@users.noreply.github.com> Date: Tue, 25 Feb 2025 15:29:20 -0800 Subject: [PATCH 08/15] Adjust function types to be accessible to subclasses --- src/server/auth/proxyProvider.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/server/auth/proxyProvider.ts b/src/server/auth/proxyProvider.ts index 2b5ad3f2e..805b3d669 100644 --- a/src/server/auth/proxyProvider.ts +++ b/src/server/auth/proxyProvider.ts @@ -40,11 +40,11 @@ export type ProxyOptions = { * Implements an OAuth server that proxies requests to another OAuth server. */ export class ProxyOAuthServerProvider implements OAuthServerProvider { - private readonly _endpoints: ProxyEndpoints; - private readonly _verifyAccessToken: (token: string) => Promise; - private readonly _getClient: (clientId: string) => Promise; + protected readonly _endpoints: ProxyEndpoints; + protected readonly _verifyAccessToken: (token: string) => Promise; + protected readonly _getClient: (clientId: string) => Promise; - public revokeToken?: ( + revokeToken?: ( client: OAuthClientInformationFull, request: OAuthTokenRevocationRequest ) => Promise; From 57f59c69a1acbe916d2206bd40fad943309ff1b8 Mon Sep 17 00:00:00 2001 From: Allen Zhou <46854522+allenzhou101@users.noreply.github.com> Date: Wed, 26 Feb 2025 14:00:34 -0800 Subject: [PATCH 09/15] Make authorizationUrl and tokenUrl mandatory --- package-lock.json | 4 +- src/server/auth/handlers/token.test.ts | 1 + src/server/auth/proxyProvider.test.ts | 41 ++------------- src/server/auth/proxyProvider.ts | 73 ++++++++++---------------- 4 files changed, 36 insertions(+), 83 deletions(-) diff --git a/package-lock.json b/package-lock.json index cfd8a6225..bc13aa0b5 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@modelcontextprotocol/sdk", - "version": "1.5.0", + "version": "1.6.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@modelcontextprotocol/sdk", - "version": "1.5.0", + "version": "1.6.0", "license": "MIT", "dependencies": { "content-type": "^1.0.5", diff --git a/src/server/auth/handlers/token.test.ts b/src/server/auth/handlers/token.test.ts index 0c4edbffc..809978578 100644 --- a/src/server/auth/handlers/token.test.ts +++ b/src/server/auth/handlers/token.test.ts @@ -298,6 +298,7 @@ describe('Token Handler', () => { const proxyProvider = new ProxyOAuthServerProvider({ endpoints: { + authorizationUrl: 'https://example.com/authorize', tokenUrl: 'https://example.com/token' }, verifyAccessToken: async (token) => ({ diff --git a/src/server/auth/proxyProvider.test.ts b/src/server/auth/proxyProvider.test.ts index ab73d7b41..5f38d6e53 100644 --- a/src/server/auth/proxyProvider.test.ts +++ b/src/server/auth/proxyProvider.test.ts @@ -22,7 +22,7 @@ describe("Proxy OAuth Server Provider", () => { // Mock provider functions const mockVerifyToken = jest.fn(); const mockGetClient = jest.fn(); - + // Base provider options const baseOptions: ProxyOptions = { endpoints: { @@ -66,7 +66,7 @@ describe("Proxy OAuth Server Provider", () => { // Add helper function for failed responses const mockFailedResponse = () => { - (global.fetch as jest.Mock).mockImplementation(() => + (global.fetch as jest.Mock).mockImplementation(() => Promise.resolve({ ok: false, status: 400, @@ -103,20 +103,6 @@ describe("Proxy OAuth Server Provider", () => { expect(mockResponse.redirect).toHaveBeenCalledWith(expectedUrl.toString()); }); - - it("throws error when authorization endpoint is not configured", async () => { - const providerWithoutAuth = new ProxyOAuthServerProvider({ - ...baseOptions, - endpoints: { ...baseOptions.endpoints, authorizationUrl: undefined }, - }); - - await expect( - providerWithoutAuth.authorize(validClient, { - redirectUri: "https://example.com/callback", - codeChallenge: "test-challenge", - }, mockResponse) - ).rejects.toThrow("No authorization endpoint configured"); - }); }); describe("token exchange", () => { @@ -128,7 +114,7 @@ describe("Proxy OAuth Server Provider", () => { }; beforeEach(() => { - (global.fetch as jest.Mock).mockImplementation(() => + (global.fetch as jest.Mock).mockImplementation(() => Promise.resolve({ ok: true, json: () => Promise.resolve(mockTokenResponse), @@ -176,23 +162,6 @@ describe("Proxy OAuth Server Provider", () => { expect(tokens).toEqual(mockTokenResponse); }); - it("throws error when token endpoint is not configured", async () => { - const providerWithoutToken = new ProxyOAuthServerProvider({ - ...baseOptions, - endpoints: { ...baseOptions.endpoints, tokenUrl: undefined }, - }); - - await expect( - providerWithoutToken.exchangeAuthorizationCode(validClient, "test-code") - ).rejects.toThrow("No token endpoint configured"); - }); - - it("handles token exchange failure", async () => { - mockFailedResponse(); - await expect( - provider.exchangeAuthorizationCode(validClient, "invalid-code") - ).rejects.toThrow(ServerError); - }); }); describe("client registration", () => { @@ -202,7 +171,7 @@ describe("Proxy OAuth Server Provider", () => { redirect_uris: ["https://new-client.com/callback"], }; - (global.fetch as jest.Mock).mockImplementation(() => + (global.fetch as jest.Mock).mockImplementation(() => Promise.resolve({ ok: true, json: () => Promise.resolve(newClient), @@ -239,7 +208,7 @@ describe("Proxy OAuth Server Provider", () => { describe("token revocation", () => { it("revokes token", async () => { - (global.fetch as jest.Mock).mockImplementation(() => + (global.fetch as jest.Mock).mockImplementation(() => Promise.resolve({ ok: true, }) diff --git a/src/server/auth/proxyProvider.ts b/src/server/auth/proxyProvider.ts index 805b3d669..d9dd1b37d 100644 --- a/src/server/auth/proxyProvider.ts +++ b/src/server/auth/proxyProvider.ts @@ -1,9 +1,9 @@ import { Response } from "express"; import { OAuthRegisteredClientsStore } from "./clients.js"; -import { - OAuthClientInformationFull, - OAuthClientInformationFullSchema, - OAuthTokenRevocationRequest, +import { + OAuthClientInformationFull, + OAuthClientInformationFullSchema, + OAuthTokenRevocationRequest, OAuthTokens, OAuthTokensSchema, } from "./../../shared/auth.js"; @@ -12,8 +12,8 @@ import { AuthorizationParams, OAuthServerProvider } from "./provider.js"; import { ServerError } from "./errors.js"; export type ProxyEndpoints = { - authorizationUrl?: string; - tokenUrl?: string; + authorizationUrl: string; + tokenUrl: string; revocationUrl?: string; registrationUrl?: string; }; @@ -24,14 +24,14 @@ export type ProxyOptions = { */ endpoints: ProxyEndpoints; - /** - * Function to verify access tokens and return auth info - */ - verifyAccessToken: (token: string) => Promise; + /** + * Function to verify access tokens and return auth info + */ + verifyAccessToken: (token: string) => Promise; - /** - * Function to fetch client information from the upstream server - */ + /** + * Function to fetch client information from the upstream server + */ getClient: (clientId: string) => Promise; }; @@ -45,7 +45,7 @@ export class ProxyOAuthServerProvider implements OAuthServerProvider { protected readonly _getClient: (clientId: string) => Promise; revokeToken?: ( - client: OAuthClientInformationFull, + client: OAuthClientInformationFull, request: OAuthTokenRevocationRequest ) => Promise; @@ -55,15 +55,15 @@ export class ProxyOAuthServerProvider implements OAuthServerProvider { this._getClient = options.getClient; if (options.endpoints?.revocationUrl) { this.revokeToken = async ( - client: OAuthClientInformationFull, + client: OAuthClientInformationFull, request: OAuthTokenRevocationRequest ) => { const revocationUrl = this._endpoints.revocationUrl; - + if (!revocationUrl) { throw new Error("No revocation endpoint configured"); } - + const params = new URLSearchParams(); params.set("token", request.token); params.set("client_id", client.client_id); @@ -73,7 +73,7 @@ export class ProxyOAuthServerProvider implements OAuthServerProvider { if (request.token_type_hint) { params.set("token_type_hint", request.token_type_hint); } - + const response = await fetch(revocationUrl, { method: "POST", headers: { @@ -81,7 +81,7 @@ export class ProxyOAuthServerProvider implements OAuthServerProvider { }, body: params.toString(), }); - + if (!response.ok) { throw new ServerError(`Token revocation failed: ${response.status}`); } @@ -115,18 +115,12 @@ export class ProxyOAuthServerProvider implements OAuthServerProvider { } async authorize( - client: OAuthClientInformationFull, - params: AuthorizationParams, + client: OAuthClientInformationFull, + params: AuthorizationParams, res: Response ): Promise { - const authorizationUrl = this._endpoints.authorizationUrl; - - if (!authorizationUrl) { - throw new Error("No authorization endpoint configured"); - } - // Start with required OAuth parameters - const targetUrl = new URL(authorizationUrl); + const targetUrl = new URL(this._endpoints.authorizationUrl); const searchParams = new URLSearchParams({ client_id: client.client_id, response_type: "code", @@ -144,7 +138,7 @@ export class ProxyOAuthServerProvider implements OAuthServerProvider { } async challengeForAuthorizationCode( - _client: OAuthClientInformationFull, + _client: OAuthClientInformationFull, _authorizationCode: string ): Promise { // In a proxy setup, we don't store the code challenge ourselves @@ -153,16 +147,10 @@ export class ProxyOAuthServerProvider implements OAuthServerProvider { } async exchangeAuthorizationCode( - client: OAuthClientInformationFull, + client: OAuthClientInformationFull, authorizationCode: string, codeVerifier?: string ): Promise { - const tokenUrl = this._endpoints.tokenUrl; - - if (!tokenUrl) { - throw new Error("No token endpoint configured"); - } - const params = new URLSearchParams({ grant_type: "authorization_code", client_id: client.client_id, @@ -177,7 +165,7 @@ export class ProxyOAuthServerProvider implements OAuthServerProvider { params.append("code_verifier", codeVerifier); } - const response = await fetch(tokenUrl, { + const response = await fetch(this._endpoints.tokenUrl, { method: "POST", headers: { "Content-Type": "application/x-www-form-urlencoded", @@ -195,15 +183,10 @@ export class ProxyOAuthServerProvider implements OAuthServerProvider { } async exchangeRefreshToken( - client: OAuthClientInformationFull, + client: OAuthClientInformationFull, refreshToken: string, scopes?: string[] ): Promise { - const tokenUrl = this._endpoints.tokenUrl; - - if (!tokenUrl) { - throw new Error("No token endpoint configured"); - } const params = new URLSearchParams({ grant_type: "refresh_token", @@ -219,7 +202,7 @@ export class ProxyOAuthServerProvider implements OAuthServerProvider { params.set("scope", scopes.join(" ")); } - const response = await fetch(tokenUrl, { + const response = await fetch(this._endpoints.tokenUrl, { method: "POST", headers: { "Content-Type": "application/x-www-form-urlencoded", @@ -237,5 +220,5 @@ export class ProxyOAuthServerProvider implements OAuthServerProvider { async verifyAccessToken(token: string): Promise { return this._verifyAccessToken(token); - } + } } \ No newline at end of file From 89c4ec327267c9b51d7f281c46b2a92aedda4073 Mon Sep 17 00:00:00 2001 From: Allen Zhou <46854522+allenzhou101@users.noreply.github.com> Date: Thu, 27 Feb 2025 10:45:17 -0800 Subject: [PATCH 10/15] Add baseUrl param to override issuerUrl for fallback construction --- src/server/auth/router.ts | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/server/auth/router.ts b/src/server/auth/router.ts index 30e22c417..49d451c29 100644 --- a/src/server/auth/router.ts +++ b/src/server/auth/router.ts @@ -17,6 +17,13 @@ export type AuthRouterOptions = { */ issuerUrl: URL; + /** + * The base URL of the authorization server to use for the metadata endpoints. + * + * If not provided, the issuer URL will be used as the base URL. + */ + baseUrl?: URL; + /** * An optional URL of a page containing human-readable information that developers might want or need to know when using the authorization server. */ @@ -41,6 +48,7 @@ export type AuthRouterOptions = { */ export function mcpAuthRouter(options: AuthRouterOptions): RequestHandler { const issuer = options.issuerUrl; + const baseUrl = options.baseUrl; // Technically RFC 8414 does not permit a localhost HTTPS exemption, but this will be necessary for ease of testing if (issuer.protocol !== "https:" && issuer.hostname !== "localhost" && issuer.hostname !== "127.0.0.1") { @@ -62,18 +70,18 @@ export function mcpAuthRouter(options: AuthRouterOptions): RequestHandler { issuer: issuer.href, service_documentation: options.serviceDocumentationUrl?.href, - authorization_endpoint: new URL(authorization_endpoint, issuer).href, + authorization_endpoint: new URL(authorization_endpoint, baseUrl || issuer).href, response_types_supported: ["code"], code_challenge_methods_supported: ["S256"], - token_endpoint: new URL(token_endpoint, issuer).href, + token_endpoint: new URL(token_endpoint, baseUrl || issuer).href, token_endpoint_auth_methods_supported: ["client_secret_post"], grant_types_supported: ["authorization_code", "refresh_token"], - revocation_endpoint: revocation_endpoint ? new URL(revocation_endpoint, issuer).href : undefined, + revocation_endpoint: revocation_endpoint ? new URL(revocation_endpoint, baseUrl || issuer).href : undefined, revocation_endpoint_auth_methods_supported: revocation_endpoint ? ["client_secret_post"] : undefined, - registration_endpoint: registration_endpoint ? new URL(registration_endpoint, issuer).href : undefined, + registration_endpoint: registration_endpoint ? new URL(registration_endpoint, baseUrl || issuer).href : undefined, }; const router = express.Router(); From 8d7b3875455d00152bcdca5ab45cffd15404c572 Mon Sep 17 00:00:00 2001 From: Allen Zhou <46854522+allenzhou101@users.noreply.github.com> Date: Thu, 27 Feb 2025 12:41:14 -0800 Subject: [PATCH 11/15] Clean up pkce skip check for proxy providers --- src/server/auth/handlers/token.ts | 10 ++++++---- src/server/auth/proxyProvider.ts | 10 ++++++++++ 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/src/server/auth/handlers/token.ts b/src/server/auth/handlers/token.ts index 3a29bb55b..acf2c0ab2 100644 --- a/src/server/auth/handlers/token.ts +++ b/src/server/auth/handlers/token.ts @@ -91,16 +91,18 @@ export function tokenHandler({ provider, rateLimit: rateLimitConfig }: TokenHand const { code, code_verifier } = parseResult.data; - // Skip local PKCE verification for proxy providers since the code_challenge is stored on the upstream server. - // The code_verifier will be passed to the upstream server during token exchange. - if (!(provider instanceof ProxyOAuthServerProvider)) { + const skipLocalPkceValidation = provider instanceof ProxyOAuthServerProvider ? provider.skipLocalPkceValidation : false; + + // Perform local PKCE validation unless explicitly delegated (e.g. by proxy provider) + if (!skipLocalPkceValidation) { const codeChallenge = await provider.challengeForAuthorizationCode(client, code); if (!(await verifyChallenge(code_verifier, codeChallenge))) { throw new InvalidGrantError("code_verifier does not match the challenge"); } } - const tokens = await provider.exchangeAuthorizationCode(client, code, code_verifier); + // Passes the code_verifier to the provider if PKCE validation didn't occur locally + const tokens = await provider.exchangeAuthorizationCode(client, code, skipLocalPkceValidation ? code_verifier : undefined); res.status(200).json(tokens); break; } diff --git a/src/server/auth/proxyProvider.ts b/src/server/auth/proxyProvider.ts index d9dd1b37d..3fbff54b5 100644 --- a/src/server/auth/proxyProvider.ts +++ b/src/server/auth/proxyProvider.ts @@ -43,6 +43,16 @@ export class ProxyOAuthServerProvider implements OAuthServerProvider { protected readonly _endpoints: ProxyEndpoints; protected readonly _verifyAccessToken: (token: string) => Promise; protected readonly _getClient: (clientId: string) => Promise; + + /** + * Always true for proxy providers since PKCE validation happens at the upstream server. + * Can consider adding to the base OAuthServerProvider interface if it becomes useful elsewhere. + * This ensures that: + * 1. We skip local PKCE validation (which would fail since we don't store challenges) + * 2. The code_verifier is still passed through to the upstream server + * 3. The upstream server performs the actual PKCE validation + */ + readonly skipLocalPkceValidation = true; revokeToken?: ( client: OAuthClientInformationFull, From b90192a020375f4c5b09ea6114bebfed9b2c3dba Mon Sep 17 00:00:00 2001 From: Allen Zhou <46854522+allenzhou101@users.noreply.github.com> Date: Fri, 28 Feb 2025 13:59:23 -0800 Subject: [PATCH 12/15] Add skipLocalPkceValidation param and create providers folder --- src/server/auth/handlers/token.test.ts | 2 +- src/server/auth/handlers/token.ts | 6 +++--- src/server/auth/provider.ts | 9 +++++++++ .../{ => providers}/proxyProvider.test.ts | 10 +++++----- .../auth/{ => providers}/proxyProvider.ts | 20 ++++++------------- 5 files changed, 24 insertions(+), 23 deletions(-) rename src/server/auth/{ => providers}/proxyProvider.test.ts (96%) rename src/server/auth/{ => providers}/proxyProvider.ts (89%) diff --git a/src/server/auth/handlers/token.test.ts b/src/server/auth/handlers/token.test.ts index 809978578..bf41b5ebd 100644 --- a/src/server/auth/handlers/token.test.ts +++ b/src/server/auth/handlers/token.test.ts @@ -7,7 +7,7 @@ import supertest from 'supertest'; import * as pkceChallenge from 'pkce-challenge'; import { InvalidGrantError, InvalidTokenError } from '../errors.js'; import { AuthInfo } from '../types.js'; -import { ProxyOAuthServerProvider } from '../proxyProvider.js'; +import { ProxyOAuthServerProvider } from '../providers/proxyProvider.js'; // Mock pkce-challenge jest.mock('pkce-challenge', () => ({ diff --git a/src/server/auth/handlers/token.ts b/src/server/auth/handlers/token.ts index acf2c0ab2..28412a014 100644 --- a/src/server/auth/handlers/token.ts +++ b/src/server/auth/handlers/token.ts @@ -14,7 +14,6 @@ import { TooManyRequestsError, OAuthError } from "../errors.js"; -import { ProxyOAuthServerProvider } from "../proxyProvider.js"; export type TokenHandlerOptions = { provider: OAuthServerProvider; @@ -91,9 +90,10 @@ export function tokenHandler({ provider, rateLimit: rateLimitConfig }: TokenHand const { code, code_verifier } = parseResult.data; - const skipLocalPkceValidation = provider instanceof ProxyOAuthServerProvider ? provider.skipLocalPkceValidation : false; + const skipLocalPkceValidation = provider.skipLocalPkceValidation; - // Perform local PKCE validation unless explicitly delegated (e.g. by proxy provider) + // Perform local PKCE validation unless explicitly skipped + // (e.g. to validate code_verifier in upstream server) if (!skipLocalPkceValidation) { const codeChallenge = await provider.challengeForAuthorizationCode(client, code); if (!(await verifyChallenge(code_verifier, codeChallenge))) { diff --git a/src/server/auth/provider.ts b/src/server/auth/provider.ts index aacd308b8..dc186bcaf 100644 --- a/src/server/auth/provider.ts +++ b/src/server/auth/provider.ts @@ -54,4 +54,13 @@ export interface OAuthServerProvider { * If the given token is invalid or already revoked, this method should do nothing. */ revokeToken?(client: OAuthClientInformationFull, request: OAuthTokenRevocationRequest): Promise; + + /** + * Whether to skip local PKCE validation. + * + * If true, the server will not perform PKCE validation locally and will pass the code_verifier to the upstream server. + * + * NOTE: This should only be true if the upstream server is performing the actual PKCE validation. + */ + skipLocalPkceValidation?: boolean; } \ No newline at end of file diff --git a/src/server/auth/proxyProvider.test.ts b/src/server/auth/providers/proxyProvider.test.ts similarity index 96% rename from src/server/auth/proxyProvider.test.ts rename to src/server/auth/providers/proxyProvider.test.ts index 5f38d6e53..6e842ea33 100644 --- a/src/server/auth/proxyProvider.test.ts +++ b/src/server/auth/providers/proxyProvider.test.ts @@ -1,10 +1,10 @@ import { Response } from "express"; import { ProxyOAuthServerProvider, ProxyOptions } from "./proxyProvider.js"; -import { AuthInfo } from "./types.js"; -import { OAuthClientInformationFull, OAuthTokens } from "../../shared/auth.js"; -import { ServerError } from "./errors.js"; -import { InvalidTokenError } from "./errors.js"; -import { InsufficientScopeError } from "./errors.js"; +import { AuthInfo } from "../types.js"; +import { OAuthClientInformationFull, OAuthTokens } from "../../../shared/auth.js"; +import { ServerError } from "../errors.js"; +import { InvalidTokenError } from "../errors.js"; +import { InsufficientScopeError } from "../errors.js"; describe("Proxy OAuth Server Provider", () => { // Mock client data diff --git a/src/server/auth/proxyProvider.ts b/src/server/auth/providers/proxyProvider.ts similarity index 89% rename from src/server/auth/proxyProvider.ts rename to src/server/auth/providers/proxyProvider.ts index 3fbff54b5..be4503050 100644 --- a/src/server/auth/proxyProvider.ts +++ b/src/server/auth/providers/proxyProvider.ts @@ -1,15 +1,15 @@ import { Response } from "express"; -import { OAuthRegisteredClientsStore } from "./clients.js"; +import { OAuthRegisteredClientsStore } from "../clients.js"; import { OAuthClientInformationFull, OAuthClientInformationFullSchema, OAuthTokenRevocationRequest, OAuthTokens, OAuthTokensSchema, -} from "./../../shared/auth.js"; -import { AuthInfo } from "./types.js"; -import { AuthorizationParams, OAuthServerProvider } from "./provider.js"; -import { ServerError } from "./errors.js"; +} from "../../../shared/auth.js"; +import { AuthInfo } from "../types.js"; +import { AuthorizationParams, OAuthServerProvider } from "../provider.js"; +import { ServerError } from "../errors.js"; export type ProxyEndpoints = { authorizationUrl: string; @@ -44,15 +44,7 @@ export class ProxyOAuthServerProvider implements OAuthServerProvider { protected readonly _verifyAccessToken: (token: string) => Promise; protected readonly _getClient: (clientId: string) => Promise; - /** - * Always true for proxy providers since PKCE validation happens at the upstream server. - * Can consider adding to the base OAuthServerProvider interface if it becomes useful elsewhere. - * This ensures that: - * 1. We skip local PKCE validation (which would fail since we don't store challenges) - * 2. The code_verifier is still passed through to the upstream server - * 3. The upstream server performs the actual PKCE validation - */ - readonly skipLocalPkceValidation = true; + skipLocalPkceValidation = true; revokeToken?: ( client: OAuthClientInformationFull, From 3573afda31c351b36f5a370a447c246a32f54254 Mon Sep 17 00:00:00 2001 From: Allen Zhou <46854522+allenzhou101@users.noreply.github.com> Date: Tue, 18 Mar 2025 11:13:29 -0700 Subject: [PATCH 13/15] Update package-lock.json --- package-lock.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/package-lock.json b/package-lock.json index bc13aa0b5..33d551aa8 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@modelcontextprotocol/sdk", - "version": "1.6.0", + "version": "1.7.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@modelcontextprotocol/sdk", - "version": "1.6.0", + "version": "1.7.0", "license": "MIT", "dependencies": { "content-type": "^1.0.5", From 5edf379f8069a62674b559b2a4ebc96e723d927f Mon Sep 17 00:00:00 2001 From: Allen Zhou <46854522+allenzhou101@users.noreply.github.com> Date: Wed, 2 Apr 2025 20:28:10 -0700 Subject: [PATCH 14/15] Add documentation on ProxyOAuthServerProvider --- README.md | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/README.md b/README.md index a959ae3dd..ee7c5fca1 100644 --- a/README.md +++ b/README.md @@ -21,6 +21,7 @@ - [Low-Level Server](#low-level-server) - [Writing MCP Clients](#writing-mcp-clients) - [Server Capabilities](#server-capabilities) + - [Proxy OAuth Server](#proxy-authorization-requests-upstream) ## Overview @@ -489,6 +490,52 @@ const result = await client.callTool({ }); ``` +### Proxy Authorization Requests Upstream + +You can proxy OAuth requests to an external OAuth provider while adding custom validation and client management: + +```typescript +import express from 'express'; +import { ProxyOAuthServerProvider, mcpAuthRouter } from '@modelcontextprotocol/sdk'; + +const app = express(); + +const proxyProvider = new ProxyOAuthServerProvider({ + endpoints: { + authorizationUrl: "https://auth.external.com/oauth2/v1/authorize", + tokenUrl: "https://auth.external.com/oauth2/v1/token", + revocationUrl: "https://auth.external.com/oauth2/v1/revoke", + }, + verifyAccessToken: async (token) => { + return { + token, + clientId: "123", + scopes: ["openid", "email", "profile"], + } + }, + getClient: async (client_id) => { + return { + client_id, + redirect_uris: ["http://localhost:3000/callback"], + } + } +}) + +app.use(mcpAuthRouter({ + provider: proxyProvider, + issuerUrl: new URL("http://auth.external.com"), + baseUrl: new URL("http://mcp.example.com"), + serviceDocumentationUrl: new URL("https://docs.example.com/"), +})) +``` + +This setup allows you to: +- Forward OAuth requests to an external provider +- Add custom token validation logic +- Manage client registrations +- Provide custom documentation URLs +- Maintain control over the OAuth flow while delegating to an external provider + ## Documentation - [Model Context Protocol documentation](https://modelcontextprotocol.io) From 8e23c7c73001d285869c0edce3637eada1dae147 Mon Sep 17 00:00:00 2001 From: Allen Zhou <46854522+allenzhou101@users.noreply.github.com> Date: Wed, 2 Apr 2025 20:29:42 -0700 Subject: [PATCH 15/15] clean up proxy oauth provider doc --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index ee7c5fca1..080532255 100644 --- a/README.md +++ b/README.md @@ -492,7 +492,7 @@ const result = await client.callTool({ ### Proxy Authorization Requests Upstream -You can proxy OAuth requests to an external OAuth provider while adding custom validation and client management: +You can proxy OAuth requests to an external authorization provider: ```typescript import express from 'express';