From 9b2915e96d4c382f0487d1376d8ab0a3686fcfe8 Mon Sep 17 00:00:00 2001 From: Jared Hanson Date: Wed, 21 May 2025 19:14:22 -0700 Subject: [PATCH 01/19] Allow OAuthClientProvider to control authentication to token endpoint during authorization code exchange. --- src/client/auth.ts | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/client/auth.ts b/src/client/auth.ts index 16f0a550..bcc7719b 100644 --- a/src/client/auth.ts +++ b/src/client/auth.ts @@ -66,6 +66,8 @@ export interface OAuthClientProvider { * the authorization result. */ codeVerifier(): string | Promise; + + authToTokenEndpoint?(headers: Headers, params: URLSearchParams): void | Promise; } export type AuthResult = "AUTHORIZED" | "REDIRECT"; @@ -131,7 +133,7 @@ export async function auth( // Exchange authorization code for tokens if (authorizationCode !== undefined) { const codeVerifier = await provider.codeVerifier(); - const tokens = await exchangeAuthorization(authorizationServerUrl, { + const tokens = await exchangeAuthorization(authorizationServerUrl, provider, { metadata, clientInformation, authorizationCode, @@ -359,6 +361,7 @@ export async function startAuthorization( */ export async function exchangeAuthorization( authorizationServerUrl: string | URL, + provider: OAuthClientProvider, { metadata, clientInformation, @@ -391,6 +394,9 @@ export async function exchangeAuthorization( tokenUrl = new URL("/token", authorizationServerUrl); } + const headers = new Headers({ + "Content-Type": "application/x-www-form-urlencoded", + }); // Exchange code for tokens const params = new URLSearchParams({ grant_type: grantType, @@ -400,15 +406,15 @@ export async function exchangeAuthorization( redirect_uri: String(redirectUri), }); - if (clientInformation.client_secret) { + if (provider.authToTokenEndpoint) { + provider.authToTokenEndpoint(headers, params); + } else if (clientInformation.client_secret) { params.set("client_secret", clientInformation.client_secret); } const response = await fetch(tokenUrl, { method: "POST", - headers: { - "Content-Type": "application/x-www-form-urlencoded", - }, + headers: headers, body: params, }); From c7737a8d85013c7159b2cbf2c9ef853e3eb013cf Mon Sep 17 00:00:00 2001 From: Jared Hanson Date: Wed, 21 May 2025 19:35:45 -0700 Subject: [PATCH 02/19] Include mockProvider in exchangeAuthorization tests. --- src/client/auth.test.ts | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/src/client/auth.test.ts b/src/client/auth.test.ts index 48be870b..9fc3c828 100644 --- a/src/client/auth.test.ts +++ b/src/client/auth.test.ts @@ -414,6 +414,22 @@ describe("OAuth Authorization", () => { }); describe("exchangeAuthorization", () => { + const mockProvider: OAuthClientProvider = { + get redirectUrl() { return "http://localhost:3000/callback"; }, + get clientMetadata() { + return { + redirect_uris: ["http://localhost:3000/callback"], + client_name: "Test Client", + }; + }, + clientInformation: jest.fn(), + tokens: jest.fn(), + saveTokens: jest.fn(), + redirectToAuthorization: jest.fn(), + saveCodeVerifier: jest.fn(), + codeVerifier: jest.fn(), + }; + const validTokens = { access_token: "access123", token_type: "Bearer", @@ -435,7 +451,7 @@ describe("OAuth Authorization", () => { json: async () => validTokens, }); - const tokens = await exchangeAuthorization("https://auth.example.com", { + const tokens = await exchangeAuthorization("https://auth.example.com", mockProvider, { clientInformation: validClientInfo, authorizationCode: "code123", codeVerifier: "verifier123", @@ -449,9 +465,9 @@ describe("OAuth Authorization", () => { }), expect.objectContaining({ method: "POST", - headers: { + headers: new Headers({ "Content-Type": "application/x-www-form-urlencoded", - }, + }), }) ); @@ -475,7 +491,7 @@ describe("OAuth Authorization", () => { }); await expect( - exchangeAuthorization("https://auth.example.com", { + exchangeAuthorization("https://auth.example.com", mockProvider, { clientInformation: validClientInfo, authorizationCode: "code123", codeVerifier: "verifier123", @@ -491,7 +507,7 @@ describe("OAuth Authorization", () => { }); await expect( - exchangeAuthorization("https://auth.example.com", { + exchangeAuthorization("https://auth.example.com", mockProvider, { clientInformation: validClientInfo, authorizationCode: "code123", codeVerifier: "verifier123", From 439a8d3d6430f4364153748141c3633ed1b21337 Mon Sep 17 00:00:00 2001 From: Jared Hanson Date: Wed, 21 May 2025 19:58:02 -0700 Subject: [PATCH 03/19] Add test case for authToTokenEndpoint during exchangeAuthorization. --- src/client/auth.test.ts | 47 ++++++++++++++++++++++++++++++++++++++--- 1 file changed, 44 insertions(+), 3 deletions(-) diff --git a/src/client/auth.test.ts b/src/client/auth.test.ts index 9fc3c828..7a3bb11b 100644 --- a/src/client/auth.test.ts +++ b/src/client/auth.test.ts @@ -465,12 +465,11 @@ describe("OAuth Authorization", () => { }), expect.objectContaining({ method: "POST", - headers: new Headers({ - "Content-Type": "application/x-www-form-urlencoded", - }), }) ); + const headers = mockFetch.mock.calls[0][1].headers as Headers; + expect(headers.get("Content-Type")).toBe("application/x-www-form-urlencoded"); const body = mockFetch.mock.calls[0][1].body as URLSearchParams; expect(body.get("grant_type")).toBe("authorization_code"); expect(body.get("code")).toBe("code123"); @@ -480,6 +479,48 @@ describe("OAuth Authorization", () => { expect(body.get("redirect_uri")).toBe("http://localhost:3000/callback"); }); + it("exchanges code for tokens with auth", async () => { + mockProvider.authToTokenEndpoint = function(headers: Headers, params: URLSearchParams) { + headers.set("Authorization", "Basic " + btoa(validClientInfo.client_id + ":" + validClientInfo.client_secret)); + params.set("example_param", "example_value") + }; + + mockFetch.mockResolvedValueOnce({ + ok: true, + status: 200, + json: async () => validTokens, + }); + + const tokens = await exchangeAuthorization("https://auth.example.com", mockProvider, { + clientInformation: validClientInfo, + authorizationCode: "code123", + codeVerifier: "verifier123", + redirectUri: "http://localhost:3000/callback", + }); + + expect(tokens).toEqual(validTokens); + expect(mockFetch).toHaveBeenCalledWith( + expect.objectContaining({ + href: "https://auth.example.com/token", + }), + expect.objectContaining({ + method: "POST", + }) + ); + + const headers = mockFetch.mock.calls[0][1].headers as Headers; + expect(headers.get("Content-Type")).toBe("application/x-www-form-urlencoded"); + expect(headers.get("Authorization")).toBe("Basic Y2xpZW50MTIzOnNlY3JldDEyMw=="); + const body = mockFetch.mock.calls[0][1].body as URLSearchParams; + expect(body.get("grant_type")).toBe("authorization_code"); + expect(body.get("code")).toBe("code123"); + expect(body.get("code_verifier")).toBe("verifier123"); + expect(body.get("client_id")).toBe("client123"); + expect(body.get("redirect_uri")).toBe("http://localhost:3000/callback"); + expect(body.get("example_param")).toBe("example_value"); + expect(body.get("client_secret")).toBeUndefined; + }); + it("validates token response schema", async () => { mockFetch.mockResolvedValueOnce({ ok: true, From 92f90300b4b2eb9df80c9c0c8e8f46fb42188068 Mon Sep 17 00:00:00 2001 From: Jared Hanson Date: Wed, 21 May 2025 20:07:23 -0700 Subject: [PATCH 04/19] Allow OAuthClientProvider to control authentication to token endpoint during refresh token exchange. --- src/client/auth.test.ts | 29 ++++++++++++++++++++++------- src/client/auth.ts | 16 ++++++++++------ 2 files changed, 32 insertions(+), 13 deletions(-) diff --git a/src/client/auth.test.ts b/src/client/auth.test.ts index 7a3bb11b..7135aebd 100644 --- a/src/client/auth.test.ts +++ b/src/client/auth.test.ts @@ -559,6 +559,22 @@ describe("OAuth Authorization", () => { }); describe("refreshAuthorization", () => { + const mockProvider: OAuthClientProvider = { + get redirectUrl() { return "http://localhost:3000/callback"; }, + get clientMetadata() { + return { + redirect_uris: ["http://localhost:3000/callback"], + client_name: "Test Client", + }; + }, + clientInformation: jest.fn(), + tokens: jest.fn(), + saveTokens: jest.fn(), + redirectToAuthorization: jest.fn(), + saveCodeVerifier: jest.fn(), + codeVerifier: jest.fn(), + }; + const validTokens = { access_token: "newaccess123", token_type: "Bearer", @@ -583,7 +599,7 @@ describe("OAuth Authorization", () => { json: async () => validTokensWithNewRefreshToken, }); - const tokens = await refreshAuthorization("https://auth.example.com", { + const tokens = await refreshAuthorization("https://auth.example.com", mockProvider, { clientInformation: validClientInfo, refreshToken: "refresh123", }); @@ -595,12 +611,11 @@ describe("OAuth Authorization", () => { }), expect.objectContaining({ method: "POST", - headers: { - "Content-Type": "application/x-www-form-urlencoded", - }, }) ); + const headers = mockFetch.mock.calls[0][1].headers as Headers; + expect(headers.get("Content-Type")).toBe("application/x-www-form-urlencoded"); const body = mockFetch.mock.calls[0][1].body as URLSearchParams; expect(body.get("grant_type")).toBe("refresh_token"); expect(body.get("refresh_token")).toBe("refresh123"); @@ -616,7 +631,7 @@ describe("OAuth Authorization", () => { }); const refreshToken = "refresh123"; - const tokens = await refreshAuthorization("https://auth.example.com", { + const tokens = await refreshAuthorization("https://auth.example.com", mockProvider, { clientInformation: validClientInfo, refreshToken, }); @@ -635,7 +650,7 @@ describe("OAuth Authorization", () => { }); await expect( - refreshAuthorization("https://auth.example.com", { + refreshAuthorization("https://auth.example.com", mockProvider, { clientInformation: validClientInfo, refreshToken: "refresh123", }) @@ -649,7 +664,7 @@ describe("OAuth Authorization", () => { }); await expect( - refreshAuthorization("https://auth.example.com", { + refreshAuthorization("https://auth.example.com", mockProvider, { clientInformation: validClientInfo, refreshToken: "refresh123", }) diff --git a/src/client/auth.ts b/src/client/auth.ts index bcc7719b..29f61547 100644 --- a/src/client/auth.ts +++ b/src/client/auth.ts @@ -151,7 +151,7 @@ export async function auth( if (tokens?.refresh_token) { try { // Attempt to refresh the token - const newTokens = await refreshAuthorization(authorizationServerUrl, { + const newTokens = await refreshAuthorization(authorizationServerUrl, provider, { metadata, clientInformation, refreshToken: tokens.refresh_token, @@ -394,10 +394,10 @@ export async function exchangeAuthorization( tokenUrl = new URL("/token", authorizationServerUrl); } + // Exchange code for tokens const headers = new Headers({ "Content-Type": "application/x-www-form-urlencoded", }); - // Exchange code for tokens const params = new URLSearchParams({ grant_type: grantType, client_id: clientInformation.client_id, @@ -430,6 +430,7 @@ export async function exchangeAuthorization( */ export async function refreshAuthorization( authorizationServerUrl: string | URL, + provider: OAuthClientProvider, { metadata, clientInformation, @@ -459,21 +460,24 @@ export async function refreshAuthorization( } // Exchange refresh token + const headers = new Headers({ + "Content-Type": "application/x-www-form-urlencoded", + }); const params = new URLSearchParams({ grant_type: grantType, client_id: clientInformation.client_id, refresh_token: refreshToken, }); - if (clientInformation.client_secret) { + if (provider.authToTokenEndpoint) { + provider.authToTokenEndpoint(headers, params); + } else if (clientInformation.client_secret) { params.set("client_secret", clientInformation.client_secret); } const response = await fetch(tokenUrl, { method: "POST", - headers: { - "Content-Type": "application/x-www-form-urlencoded", - }, + headers: headers, body: params, }); if (!response.ok) { From 58c2332ffee5e55300cb8081ceffc0062d7fa10d Mon Sep 17 00:00:00 2001 From: Jared Hanson Date: Wed, 21 May 2025 20:10:47 -0700 Subject: [PATCH 05/19] Add test case for authToTokenEndpoint during refreshAuthorization. --- src/client/auth.test.ts | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/src/client/auth.test.ts b/src/client/auth.test.ts index 7135aebd..a8a9448a 100644 --- a/src/client/auth.test.ts +++ b/src/client/auth.test.ts @@ -623,6 +623,44 @@ describe("OAuth Authorization", () => { expect(body.get("client_secret")).toBe("secret123"); }); + it("exchanges refresh token for new tokens with auth", async () => { + mockProvider.authToTokenEndpoint = function(headers: Headers, params: URLSearchParams) { + headers.set("Authorization", "Basic " + btoa(validClientInfo.client_id + ":" + validClientInfo.client_secret)); + params.set("example_param", "example_value") + }; + + mockFetch.mockResolvedValueOnce({ + ok: true, + status: 200, + json: async () => validTokensWithNewRefreshToken, + }); + + const tokens = await refreshAuthorization("https://auth.example.com", mockProvider, { + clientInformation: validClientInfo, + refreshToken: "refresh123", + }); + + expect(tokens).toEqual(validTokensWithNewRefreshToken); + expect(mockFetch).toHaveBeenCalledWith( + expect.objectContaining({ + href: "https://auth.example.com/token", + }), + expect.objectContaining({ + method: "POST", + }) + ); + + const headers = mockFetch.mock.calls[0][1].headers as Headers; + expect(headers.get("Content-Type")).toBe("application/x-www-form-urlencoded"); + expect(headers.get("Authorization")).toBe("Basic Y2xpZW50MTIzOnNlY3JldDEyMw=="); + const body = mockFetch.mock.calls[0][1].body as URLSearchParams; + expect(body.get("grant_type")).toBe("refresh_token"); + expect(body.get("refresh_token")).toBe("refresh123"); + expect(body.get("client_id")).toBe("client123"); + expect(body.get("example_param")).toBe("example_value"); + expect(body.get("client_secret")).toBeUndefined; + }); + it("exchanges refresh token for new tokens and keep existing refresh token if none is returned", async () => { mockFetch.mockResolvedValueOnce({ ok: true, From 9ef072fceb610a202343a4fd3c15aaf8f0855969 Mon Sep 17 00:00:00 2001 From: Jared Hanson Date: Thu, 22 May 2025 15:22:04 -0700 Subject: [PATCH 06/19] Make provider an optional final argument to exchangeAuthorization and refreshAuthorization to maintain compatibility. --- src/client/auth.test.ts | 22 +++++++++++----------- src/client/auth.ts | 16 ++++++++-------- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/src/client/auth.test.ts b/src/client/auth.test.ts index a8a9448a..82d55909 100644 --- a/src/client/auth.test.ts +++ b/src/client/auth.test.ts @@ -451,7 +451,7 @@ describe("OAuth Authorization", () => { json: async () => validTokens, }); - const tokens = await exchangeAuthorization("https://auth.example.com", mockProvider, { + const tokens = await exchangeAuthorization("https://auth.example.com", { clientInformation: validClientInfo, authorizationCode: "code123", codeVerifier: "verifier123", @@ -491,12 +491,12 @@ describe("OAuth Authorization", () => { json: async () => validTokens, }); - const tokens = await exchangeAuthorization("https://auth.example.com", mockProvider, { + const tokens = await exchangeAuthorization("https://auth.example.com", { clientInformation: validClientInfo, authorizationCode: "code123", codeVerifier: "verifier123", redirectUri: "http://localhost:3000/callback", - }); + }, mockProvider); expect(tokens).toEqual(validTokens); expect(mockFetch).toHaveBeenCalledWith( @@ -532,7 +532,7 @@ describe("OAuth Authorization", () => { }); await expect( - exchangeAuthorization("https://auth.example.com", mockProvider, { + exchangeAuthorization("https://auth.example.com", { clientInformation: validClientInfo, authorizationCode: "code123", codeVerifier: "verifier123", @@ -548,7 +548,7 @@ describe("OAuth Authorization", () => { }); await expect( - exchangeAuthorization("https://auth.example.com", mockProvider, { + exchangeAuthorization("https://auth.example.com", { clientInformation: validClientInfo, authorizationCode: "code123", codeVerifier: "verifier123", @@ -599,7 +599,7 @@ describe("OAuth Authorization", () => { json: async () => validTokensWithNewRefreshToken, }); - const tokens = await refreshAuthorization("https://auth.example.com", mockProvider, { + const tokens = await refreshAuthorization("https://auth.example.com", { clientInformation: validClientInfo, refreshToken: "refresh123", }); @@ -635,10 +635,10 @@ describe("OAuth Authorization", () => { json: async () => validTokensWithNewRefreshToken, }); - const tokens = await refreshAuthorization("https://auth.example.com", mockProvider, { + const tokens = await refreshAuthorization("https://auth.example.com", { clientInformation: validClientInfo, refreshToken: "refresh123", - }); + }, mockProvider); expect(tokens).toEqual(validTokensWithNewRefreshToken); expect(mockFetch).toHaveBeenCalledWith( @@ -669,7 +669,7 @@ describe("OAuth Authorization", () => { }); const refreshToken = "refresh123"; - const tokens = await refreshAuthorization("https://auth.example.com", mockProvider, { + const tokens = await refreshAuthorization("https://auth.example.com", { clientInformation: validClientInfo, refreshToken, }); @@ -688,7 +688,7 @@ describe("OAuth Authorization", () => { }); await expect( - refreshAuthorization("https://auth.example.com", mockProvider, { + refreshAuthorization("https://auth.example.com", { clientInformation: validClientInfo, refreshToken: "refresh123", }) @@ -702,7 +702,7 @@ describe("OAuth Authorization", () => { }); await expect( - refreshAuthorization("https://auth.example.com", mockProvider, { + refreshAuthorization("https://auth.example.com", { clientInformation: validClientInfo, refreshToken: "refresh123", }) diff --git a/src/client/auth.ts b/src/client/auth.ts index 29f61547..827f23f7 100644 --- a/src/client/auth.ts +++ b/src/client/auth.ts @@ -133,13 +133,13 @@ export async function auth( // Exchange authorization code for tokens if (authorizationCode !== undefined) { const codeVerifier = await provider.codeVerifier(); - const tokens = await exchangeAuthorization(authorizationServerUrl, provider, { + const tokens = await exchangeAuthorization(authorizationServerUrl, { metadata, clientInformation, authorizationCode, codeVerifier, redirectUri: provider.redirectUrl, - }); + }, provider); await provider.saveTokens(tokens); return "AUTHORIZED"; @@ -151,11 +151,11 @@ export async function auth( if (tokens?.refresh_token) { try { // Attempt to refresh the token - const newTokens = await refreshAuthorization(authorizationServerUrl, provider, { + const newTokens = await refreshAuthorization(authorizationServerUrl, { metadata, clientInformation, refreshToken: tokens.refresh_token, - }); + }, provider); await provider.saveTokens(newTokens); return "AUTHORIZED"; @@ -361,7 +361,6 @@ export async function startAuthorization( */ export async function exchangeAuthorization( authorizationServerUrl: string | URL, - provider: OAuthClientProvider, { metadata, clientInformation, @@ -375,6 +374,7 @@ export async function exchangeAuthorization( codeVerifier: string; redirectUri: string | URL; }, + provider?: OAuthClientProvider ): Promise { const grantType = "authorization_code"; @@ -406,7 +406,7 @@ export async function exchangeAuthorization( redirect_uri: String(redirectUri), }); - if (provider.authToTokenEndpoint) { + if (provider?.authToTokenEndpoint) { provider.authToTokenEndpoint(headers, params); } else if (clientInformation.client_secret) { params.set("client_secret", clientInformation.client_secret); @@ -430,7 +430,6 @@ export async function exchangeAuthorization( */ export async function refreshAuthorization( authorizationServerUrl: string | URL, - provider: OAuthClientProvider, { metadata, clientInformation, @@ -440,6 +439,7 @@ export async function refreshAuthorization( clientInformation: OAuthClientInformation; refreshToken: string; }, + provider?: OAuthClientProvider, ): Promise { const grantType = "refresh_token"; @@ -469,7 +469,7 @@ export async function refreshAuthorization( refresh_token: refreshToken, }); - if (provider.authToTokenEndpoint) { + if (provider?.authToTokenEndpoint) { provider.authToTokenEndpoint(headers, params); } else if (clientInformation.client_secret) { params.set("client_secret", clientInformation.client_secret); From c0e9ef653425937103e52879cda64da2a49079c1 Mon Sep 17 00:00:00 2001 From: Jared Hanson Date: Sun, 25 May 2025 20:48:23 -0700 Subject: [PATCH 07/19] Add url argument to authToTokenEndpoint. --- src/client/auth.test.ts | 12 ++++++++---- src/client/auth.ts | 6 +++--- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/client/auth.test.ts b/src/client/auth.test.ts index 82d55909..0f8b9abf 100644 --- a/src/client/auth.test.ts +++ b/src/client/auth.test.ts @@ -480,9 +480,10 @@ describe("OAuth Authorization", () => { }); it("exchanges code for tokens with auth", async () => { - mockProvider.authToTokenEndpoint = function(headers: Headers, params: URLSearchParams) { + mockProvider.authToTokenEndpoint = function(url: URL, headers: Headers, params: URLSearchParams) { headers.set("Authorization", "Basic " + btoa(validClientInfo.client_id + ":" + validClientInfo.client_secret)); - params.set("example_param", "example_value") + params.set("example_url", url.toString()); + params.set("example_param", "example_value"); }; mockFetch.mockResolvedValueOnce({ @@ -517,6 +518,7 @@ describe("OAuth Authorization", () => { expect(body.get("code_verifier")).toBe("verifier123"); expect(body.get("client_id")).toBe("client123"); expect(body.get("redirect_uri")).toBe("http://localhost:3000/callback"); + expect(body.get("example_url")).toBe("https://auth.example.com/token"); expect(body.get("example_param")).toBe("example_value"); expect(body.get("client_secret")).toBeUndefined; }); @@ -624,9 +626,10 @@ describe("OAuth Authorization", () => { }); it("exchanges refresh token for new tokens with auth", async () => { - mockProvider.authToTokenEndpoint = function(headers: Headers, params: URLSearchParams) { + mockProvider.authToTokenEndpoint = function(url: URL, headers: Headers, params: URLSearchParams) { headers.set("Authorization", "Basic " + btoa(validClientInfo.client_id + ":" + validClientInfo.client_secret)); - params.set("example_param", "example_value") + params.set("example_url", url.toString()); + params.set("example_param", "example_value"); }; mockFetch.mockResolvedValueOnce({ @@ -657,6 +660,7 @@ describe("OAuth Authorization", () => { expect(body.get("grant_type")).toBe("refresh_token"); expect(body.get("refresh_token")).toBe("refresh123"); expect(body.get("client_id")).toBe("client123"); + expect(body.get("example_url")).toBe("https://auth.example.com/token"); expect(body.get("example_param")).toBe("example_value"); expect(body.get("client_secret")).toBeUndefined; }); diff --git a/src/client/auth.ts b/src/client/auth.ts index 827f23f7..80a086e3 100644 --- a/src/client/auth.ts +++ b/src/client/auth.ts @@ -67,7 +67,7 @@ export interface OAuthClientProvider { */ codeVerifier(): string | Promise; - authToTokenEndpoint?(headers: Headers, params: URLSearchParams): void | Promise; + authToTokenEndpoint?(url: URL, headers: Headers, params: URLSearchParams): void | Promise; } export type AuthResult = "AUTHORIZED" | "REDIRECT"; @@ -407,7 +407,7 @@ export async function exchangeAuthorization( }); if (provider?.authToTokenEndpoint) { - provider.authToTokenEndpoint(headers, params); + provider.authToTokenEndpoint(tokenUrl, headers, params); } else if (clientInformation.client_secret) { params.set("client_secret", clientInformation.client_secret); } @@ -470,7 +470,7 @@ export async function refreshAuthorization( }); if (provider?.authToTokenEndpoint) { - provider.authToTokenEndpoint(headers, params); + provider.authToTokenEndpoint(tokenUrl, headers, params); } else if (clientInformation.client_secret) { params.set("client_secret", clientInformation.client_secret); } From a055fb373885fe0522335d13d5e026f745f014cd Mon Sep 17 00:00:00 2001 From: SightStudio Date: Mon, 26 May 2025 16:17:57 +0900 Subject: [PATCH 08/19] feature. Add client_secret_basic and none authentication methods --- src/client/auth.test.ts | 228 ++++++++++++++++++++++++++++++++++++++++ src/client/auth.ts | 180 ++++++++++++++++++++++++++----- 2 files changed, 383 insertions(+), 25 deletions(-) diff --git a/src/client/auth.test.ts b/src/client/auth.test.ts index 1b9fb071..e4f9d208 100644 --- a/src/client/auth.test.ts +++ b/src/client/auth.test.ts @@ -808,4 +808,232 @@ describe("OAuth Authorization", () => { ); }); }); + + describe("exchangeAuthorization with multiple client authentication methods", () => { + const validTokens = { + access_token: "access123", + token_type: "Bearer", + expires_in: 3600, + refresh_token: "refresh123", + }; + + const validClientInfo = { + client_id: "client123", + client_secret: "secret123", + redirect_uris: ["http://localhost:3000/callback"], + client_name: "Test Client", + }; + + const metadataWithBasicOnly = { + issuer: "https://auth.example.com", + authorization_endpoint: "https://auth.example.com/auth", + token_endpoint: "https://auth.example.com/token", + response_types_supported: ["code"], + code_challenge_methods_supported: ["S256"], + token_endpoint_auth_methods_supported: ["client_secret_basic"], + }; + + const metadataWithPostOnly = { + ...metadataWithBasicOnly, + token_endpoint_auth_methods_supported: ["client_secret_post"], + }; + + const metadataWithNoneOnly = { + ...metadataWithBasicOnly, + token_endpoint_auth_methods_supported: ["none"], + }; + + it("uses HTTP Basic authentication when client_secret_basic is supported", async () => { + mockFetch.mockResolvedValueOnce({ + ok: true, + status: 200, + json: async () => validTokens, + }); + + const tokens = await exchangeAuthorization("https://auth.example.com", { + metadata: metadataWithBasicOnly, + clientInformation: validClientInfo, + authorizationCode: "code123", + codeVerifier: "verifier123", + redirectUri: "http://localhost:3000/callback", + }); + + expect(tokens).toEqual(validTokens); + const request = mockFetch.mock.calls[0][1]; + + // Check Authorization header + const authHeader = request.headers["Authorization"]; + const expected = "Basic " + btoa("client123:secret123"); + expect(authHeader).toBe(expected); + + const body = request.body as URLSearchParams; + expect(body.get("client_id")).toBeNull(); // should not be in body + expect(body.get("client_secret")).toBeNull(); // should not be in body + }); + + it("includes credentials in request body when client_secret_post is supported", async () => { + mockFetch.mockResolvedValueOnce({ + ok: true, + status: 200, + json: async () => validTokens, + }); + + const tokens = await exchangeAuthorization("https://auth.example.com", { + metadata: metadataWithPostOnly, + clientInformation: validClientInfo, + authorizationCode: "code123", + codeVerifier: "verifier123", + redirectUri: "http://localhost:3000/callback", + }); + + expect(tokens).toEqual(validTokens); + const request = mockFetch.mock.calls[0][1]; + + // Check no Authorization header + expect(request.headers["Authorization"]).toBeUndefined(); + + const body = request.body as URLSearchParams; + expect(body.get("client_id")).toBe("client123"); + expect(body.get("client_secret")).toBe("secret123"); + }); + + it("uses public client authentication when none method is specified", async () => { + mockFetch.mockResolvedValueOnce({ + ok: true, + status: 200, + json: async () => validTokens, + }); + + const clientInfoWithoutSecret = { + client_id: "client123", + redirect_uris: ["http://localhost:3000/callback"], + client_name: "Test Client", + }; + + const tokens = await exchangeAuthorization("https://auth.example.com", { + metadata: metadataWithNoneOnly, + clientInformation: clientInfoWithoutSecret, + authorizationCode: "code123", + codeVerifier: "verifier123", + redirectUri: "http://localhost:3000/callback", + }); + + expect(tokens).toEqual(validTokens); + const request = mockFetch.mock.calls[0][1]; + + // Check no Authorization header + expect(request.headers["Authorization"]).toBeUndefined(); + + const body = request.body as URLSearchParams; + expect(body.get("client_id")).toBe("client123"); + expect(body.get("client_secret")).toBeNull(); + }); + + it("defaults to client_secret_post when no auth methods specified", async () => { + mockFetch.mockResolvedValueOnce({ + ok: true, + status: 200, + json: async () => validTokens, + }); + + const tokens = await exchangeAuthorization("https://auth.example.com", { + clientInformation: validClientInfo, + authorizationCode: "code123", + codeVerifier: "verifier123", + redirectUri: "http://localhost:3000/callback", + }); + + expect(tokens).toEqual(validTokens); + const request = mockFetch.mock.calls[0][1]; + + // Check no Authorization header + expect(request.headers["Authorization"]).toBeUndefined(); + + const body = request.body as URLSearchParams; + expect(body.get("client_id")).toBe("client123"); + expect(body.get("client_secret")).toBe("secret123"); + }); + }); + + describe("refreshAuthorization with multiple client authentication methods", () => { + const validTokens = { + access_token: "newaccess123", + token_type: "Bearer", + expires_in: 3600, + refresh_token: "newrefresh123", + }; + + const validClientInfo = { + client_id: "client123", + client_secret: "secret123", + redirect_uris: ["http://localhost:3000/callback"], + client_name: "Test Client", + }; + + const metadataWithBasicOnly = { + issuer: "https://auth.example.com", + authorization_endpoint: "https://auth.example.com/auth", + token_endpoint: "https://auth.example.com/token", + response_types_supported: ["code"], + token_endpoint_auth_methods_supported: ["client_secret_basic"], + }; + + const metadataWithPostOnly = { + ...metadataWithBasicOnly, + token_endpoint_auth_methods_supported: ["client_secret_post"], + }; + + it("uses client_secret_basic for refresh token", async () => { + mockFetch.mockResolvedValueOnce({ + ok: true, + status: 200, + json: async () => validTokens, + }); + + const tokens = await refreshAuthorization("https://auth.example.com", { + metadata: metadataWithBasicOnly, + clientInformation: validClientInfo, + refreshToken: "refresh123", + }); + + expect(tokens).toEqual(validTokens); + const request = mockFetch.mock.calls[0][1]; + + // Check Authorization header + const authHeader = request.headers["Authorization"]; + const expected = "Basic " + btoa("client123:secret123"); + expect(authHeader).toBe(expected); + + const body = request.body as URLSearchParams; + expect(body.get("client_id")).toBeNull(); // should not be in body + expect(body.get("client_secret")).toBeNull(); // should not be in body + expect(body.get("refresh_token")).toBe("refresh123"); + }); + + it("uses client_secret_post for refresh token", async () => { + mockFetch.mockResolvedValueOnce({ + ok: true, + status: 200, + json: async () => validTokens, + }); + + const tokens = await refreshAuthorization("https://auth.example.com", { + metadata: metadataWithPostOnly, + clientInformation: validClientInfo, + refreshToken: "refresh123", + }); + + expect(tokens).toEqual(validTokens); + const request = mockFetch.mock.calls[0][1]; + + // Check no Authorization header + expect(request.headers["Authorization"]).toBeUndefined(); + + const body = request.body as URLSearchParams; + expect(body.get("client_id")).toBe("client123"); + expect(body.get("client_secret")).toBe("secret123"); + expect(body.get("refresh_token")).toBe("refresh123"); + }); + }); + }); diff --git a/src/client/auth.ts b/src/client/auth.ts index 7a91eb25..324c713f 100644 --- a/src/client/auth.ts +++ b/src/client/auth.ts @@ -81,6 +81,115 @@ export class UnauthorizedError extends Error { } } +/** + * Determines the best client authentication method to use based on server support and client configuration. + * + * Priority order (highest to lowest): + * 1. client_secret_basic (if client secret is available) + * 2. client_secret_post (if client secret is available) + * 3. none (for public clients) + * + * @param clientInformation - OAuth client information containing credentials + * @param supportedMethods - Authentication methods supported by the authorization server + * @returns The selected authentication method + */ +function selectClientAuthMethod( + clientInformation: OAuthClientInformation, + supportedMethods: string[] +): string { + const hasClientSecret = !!clientInformation.client_secret; + + // If server doesn't specify supported methods, use RFC 6749 defaults + if (supportedMethods.length === 0) { + return hasClientSecret ? "client_secret_post" : "none"; + } + + // Try methods in priority order (most secure first) + if (hasClientSecret && supportedMethods.includes("client_secret_basic")) { + return "client_secret_basic"; + } + + if (hasClientSecret && supportedMethods.includes("client_secret_post")) { + return "client_secret_post"; + } + + if (supportedMethods.includes("none")) { + return "none"; + } + + // Fallback: use what we have + return hasClientSecret ? "client_secret_post" : "none"; +} + +/** + * Applies client authentication to the request based on the specified method. + * + * Implements OAuth 2.1 client authentication methods: + * - client_secret_basic: HTTP Basic authentication (RFC 6749 Section 2.3.1) + * - client_secret_post: Credentials in request body (RFC 6749 Section 2.3.1) + * - none: Public client authentication (RFC 6749 Section 2.1) + * + * @param method - The authentication method to use + * @param clientInformation - OAuth client information containing credentials + * @param headers - HTTP headers object to modify + * @param params - URL search parameters to modify + * @throws {Error} When required credentials are missing + */ +function applyClientAuthentication( + method: string, + clientInformation: OAuthClientInformation, + headers: HeadersInit, + params: URLSearchParams +): void { + const { client_id, client_secret } = clientInformation; + + if (method === "client_secret_basic") { + applyBasicAuth(client_id, client_secret, headers); + return; + } + + if (method === "client_secret_post") { + applyPostAuth(client_id, client_secret, params); + return; + } + + if (method === "none") { + applyPublicAuth(client_id, params); + return; + } + + throw new Error(`Unsupported client authentication method: ${method}`); +} + +/** + * Applies HTTP Basic authentication (RFC 6749 Section 2.3.1) + */ +function applyBasicAuth(clientId: string, clientSecret: string | undefined, headers: HeadersInit): void { + if (!clientSecret) { + throw new Error("client_secret_basic authentication requires a client_secret"); + } + + const credentials = btoa(`${clientId}:${clientSecret}`); + (headers as any)["Authorization"] = `Basic ${credentials}`; +} + +/** + * Applies POST body authentication (RFC 6749 Section 2.3.1) + */ +function applyPostAuth(clientId: string, clientSecret: string | undefined, params: URLSearchParams): void { + params.set("client_id", clientId); + if (clientSecret) { + params.set("client_secret", clientSecret); + } +} + +/** + * Applies public client authentication (RFC 6749 Section 2.1) + */ +function applyPublicAuth(clientId: string, params: URLSearchParams): void { + params.set("client_id", clientId); +} + /** * Orchestrates the full auth flow with a server. * @@ -370,6 +479,15 @@ export async function startAuthorization( /** * Exchanges an authorization code for an access token with the given server. + * + * Supports multiple client authentication methods as specified in OAuth 2.1: + * - Automatically selects the best authentication method based on server support + * - Falls back to appropriate defaults when server metadata is unavailable + * + * @param authorizationServerUrl - The authorization server's base URL + * @param options - Configuration object containing client info, auth code, etc. + * @returns Promise resolving to OAuth tokens + * @throws {Error} When token exchange fails or authentication is invalid */ export async function exchangeAuthorization( authorizationServerUrl: string | URL, @@ -389,40 +507,40 @@ export async function exchangeAuthorization( ): Promise { const grantType = "authorization_code"; - let tokenUrl: URL; - if (metadata) { - tokenUrl = new URL(metadata.token_endpoint); + const tokenUrl = metadata?.token_endpoint + ? new URL(metadata.token_endpoint) + : new URL("/token", authorizationServerUrl); - if ( - metadata.grant_types_supported && + if ( + metadata?.grant_types_supported && !metadata.grant_types_supported.includes(grantType) - ) { - throw new Error( + ) { + throw new Error( `Incompatible auth server: does not support grant type ${grantType}`, - ); - } - } else { - tokenUrl = new URL("/token", authorizationServerUrl); + ); } + const headers: HeadersInit = { + "Content-Type": "application/x-www-form-urlencoded", + }; + // Exchange code for tokens const params = new URLSearchParams({ grant_type: grantType, - client_id: clientInformation.client_id, code: authorizationCode, code_verifier: codeVerifier, redirect_uri: String(redirectUri), }); - if (clientInformation.client_secret) { - params.set("client_secret", clientInformation.client_secret); - } + // Determine and apply client authentication method + const supportedMethods = metadata?.token_endpoint_auth_methods_supported ?? []; + const authMethod = selectClientAuthMethod(clientInformation, supportedMethods); + + applyClientAuthentication(authMethod, clientInformation, headers, params); const response = await fetch(tokenUrl, { method: "POST", - headers: { - "Content-Type": "application/x-www-form-urlencoded", - }, + headers, body: params, }); @@ -435,6 +553,15 @@ export async function exchangeAuthorization( /** * Exchange a refresh token for an updated access token. + * + * Supports multiple client authentication methods as specified in OAuth 2.1: + * - Automatically selects the best authentication method based on server support + * - Preserves the original refresh token if a new one is not returned + * + * @param authorizationServerUrl - The authorization server's base URL + * @param options - Configuration object containing client info, refresh token, etc. + * @returns Promise resolving to OAuth tokens (preserves original refresh_token if not replaced) + * @throws {Error} When token refresh fails or authentication is invalid */ export async function refreshAuthorization( authorizationServerUrl: string | URL, @@ -466,22 +593,25 @@ export async function refreshAuthorization( tokenUrl = new URL("/token", authorizationServerUrl); } + const headers: HeadersInit = { + "Content-Type": "application/x-www-form-urlencoded", + }; + // Exchange refresh token const params = new URLSearchParams({ grant_type: grantType, - client_id: clientInformation.client_id, refresh_token: refreshToken, }); - if (clientInformation.client_secret) { - params.set("client_secret", clientInformation.client_secret); - } + // Determine and apply client authentication method + const supportedMethods = metadata?.token_endpoint_auth_methods_supported ?? []; + const authMethod = selectClientAuthMethod(clientInformation, supportedMethods); + + applyClientAuthentication(authMethod, clientInformation, headers, params); const response = await fetch(tokenUrl, { method: "POST", - headers: { - "Content-Type": "application/x-www-form-urlencoded", - }, + headers, body: params, }); if (!response.ok) { From afe12791a691a65f419dc0ba9d53827a616c0c20 Mon Sep 17 00:00:00 2001 From: Olivier Chafik Date: Fri, 13 Jun 2025 15:40:34 +0100 Subject: [PATCH 09/19] rename authToTokenEndpoint -> addClientAuthentication --- src/client/auth.test.ts | 4 ++-- src/client/auth.ts | 27 ++++++++++++++++++++++----- 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/src/client/auth.test.ts b/src/client/auth.test.ts index 0f0361a1..44605a06 100644 --- a/src/client/auth.test.ts +++ b/src/client/auth.test.ts @@ -505,7 +505,7 @@ describe("OAuth Authorization", () => { }); it("exchanges code for tokens with auth", async () => { - mockProvider.authToTokenEndpoint = function(url: URL, headers: Headers, params: URLSearchParams) { + mockProvider.addClientAuthentication = function(url: URL, headers: Headers, params: URLSearchParams) { headers.set("Authorization", "Basic " + btoa(validClientInfo.client_id + ":" + validClientInfo.client_secret)); params.set("example_url", url.toString()); params.set("example_param", "example_value"); @@ -651,7 +651,7 @@ describe("OAuth Authorization", () => { }); it("exchanges refresh token for new tokens with auth", async () => { - mockProvider.authToTokenEndpoint = function(url: URL, headers: Headers, params: URLSearchParams) { + mockProvider.addClientAuthentication = function(url: URL, headers: Headers, params: URLSearchParams) { headers.set("Authorization", "Basic " + btoa(validClientInfo.client_id + ":" + validClientInfo.client_secret)); params.set("example_url", url.toString()); params.set("example_param", "example_value"); diff --git a/src/client/auth.ts b/src/client/auth.ts index ba62f481..0af172aa 100644 --- a/src/client/auth.ts +++ b/src/client/auth.ts @@ -72,7 +72,24 @@ export interface OAuthClientProvider { */ codeVerifier(): string | Promise; - authToTokenEndpoint?(url: URL, headers: Headers, params: URLSearchParams): void | Promise; + /** + * Adds custom client authentication to OAuth token requests. + * + * This optional method allows implementations to customize how client credentials + * are included in token exchange and refresh requests. When provided, this method + * is called instead of the default authentication logic, giving full control over + * the authentication mechanism. + * + * Common use cases include: + * - Supporting authentication methods beyond the standard OAuth 2.0 methods + * - Adding custom headers for proprietary authentication schemes + * - Implementing client assertion-based authentication (e.g., JWT bearer tokens) + * + * @param url - The token endpoint URL being called + * @param headers - The request headers (can be modified to add authentication) + * @param params - The request body parameters (can be modified to add credentials) + */ + addClientAuthentication?(url: URL, headers: Headers, params: URLSearchParams): void | Promise; } export type AuthResult = "AUTHORIZED" | "REDIRECT"; @@ -538,8 +555,8 @@ export async function exchangeAuthorization( redirect_uri: String(redirectUri), }); - if (provider?.authToTokenEndpoint) { - provider.authToTokenEndpoint(tokenUrl, headers, params); + if (provider?.addClientAuthentication) { + provider.addClientAuthentication(tokenUrl, headers, params); } else { // Determine and apply client authentication method const supportedMethods = metadata?.token_endpoint_auth_methods_supported ?? []; @@ -617,8 +634,8 @@ export async function refreshAuthorization( refresh_token: refreshToken, }); - if (provider?.authToTokenEndpoint) { - provider.authToTokenEndpoint(tokenUrl, headers, params); + if (provider?.addClientAuthentication) { + provider.addClientAuthentication(tokenUrl, headers, params); } else { // Determine and apply client authentication method const supportedMethods = metadata?.token_endpoint_auth_methods_supported ?? []; From 91955879cdc2f855232557d2a085d80b3eae9a55 Mon Sep 17 00:00:00 2001 From: Olivier Chafik Date: Mon, 30 Jun 2025 14:48:22 +0100 Subject: [PATCH 10/19] Fix HTTP Basic authentication in OAuth client MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The applyBasicAuth function was incorrectly trying to set headers using array notation on a Headers object. Fixed by using the proper Headers.set() method instead of treating it as a plain object. This ensures that HTTP Basic authentication works correctly when client_secret_basic is the selected authentication method. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- src/client/auth.test.ts | 20 ++++++++++---------- src/client/auth.ts | 10 +++------- 2 files changed, 13 insertions(+), 17 deletions(-) diff --git a/src/client/auth.test.ts b/src/client/auth.test.ts index 5b56ad2d..0d6abdee 100644 --- a/src/client/auth.test.ts +++ b/src/client/auth.test.ts @@ -684,11 +684,11 @@ describe("OAuth Authorization", () => { expect(body.get("grant_type")).toBe("authorization_code"); expect(body.get("code")).toBe("code123"); expect(body.get("code_verifier")).toBe("verifier123"); - expect(body.get("client_id")).toBe("client123"); + expect(body.get("client_id")).toBeNull(); expect(body.get("redirect_uri")).toBe("http://localhost:3000/callback"); expect(body.get("example_url")).toBe("https://auth.example.com/token"); expect(body.get("example_param")).toBe("example_value"); - expect(body.get("client_secret")).toBeUndefined; + expect(body.get("client_secret")).toBeNull(); }); it("validates token response schema", async () => { @@ -829,10 +829,10 @@ describe("OAuth Authorization", () => { const body = mockFetch.mock.calls[0][1].body as URLSearchParams; expect(body.get("grant_type")).toBe("refresh_token"); expect(body.get("refresh_token")).toBe("refresh123"); - expect(body.get("client_id")).toBe("client123"); + expect(body.get("client_id")).toBeNull(); expect(body.get("example_url")).toBe("https://auth.example.com/token"); expect(body.get("example_param")).toBe("example_value"); - expect(body.get("client_secret")).toBeUndefined; + expect(body.get("client_secret")).toBeNull(); }); it("exchanges refresh token for new tokens and keep existing refresh token if none is returned", async () => { @@ -1632,7 +1632,7 @@ describe("OAuth Authorization", () => { const request = mockFetch.mock.calls[0][1]; // Check Authorization header - const authHeader = request.headers["Authorization"]; + const authHeader = request.headers.get("Authorization"); const expected = "Basic " + btoa("client123:secret123"); expect(authHeader).toBe(expected); @@ -1660,7 +1660,7 @@ describe("OAuth Authorization", () => { const request = mockFetch.mock.calls[0][1]; // Check no Authorization header - expect(request.headers["Authorization"]).toBeUndefined(); + expect(request.headers.get("Authorization")).toBeNull(); const body = request.body as URLSearchParams; expect(body.get("client_id")).toBe("client123"); @@ -1692,7 +1692,7 @@ describe("OAuth Authorization", () => { const request = mockFetch.mock.calls[0][1]; // Check no Authorization header - expect(request.headers["Authorization"]).toBeUndefined(); + expect(request.headers.get("Authorization")).toBeNull(); const body = request.body as URLSearchParams; expect(body.get("client_id")).toBe("client123"); @@ -1717,7 +1717,7 @@ describe("OAuth Authorization", () => { const request = mockFetch.mock.calls[0][1]; // Check no Authorization header - expect(request.headers["Authorization"]).toBeUndefined(); + expect(request.headers.get("Authorization")).toBeNull(); const body = request.body as URLSearchParams; expect(body.get("client_id")).toBe("client123"); @@ -1770,7 +1770,7 @@ describe("OAuth Authorization", () => { const request = mockFetch.mock.calls[0][1]; // Check Authorization header - const authHeader = request.headers["Authorization"]; + const authHeader = request.headers.get("Authorization"); const expected = "Basic " + btoa("client123:secret123"); expect(authHeader).toBe(expected); @@ -1797,7 +1797,7 @@ describe("OAuth Authorization", () => { const request = mockFetch.mock.calls[0][1]; // Check no Authorization header - expect(request.headers["Authorization"]).toBeUndefined(); + expect(request.headers.get("Authorization")).toBeNull(); const body = request.body as URLSearchParams; expect(body.get("client_id")).toBe("client123"); diff --git a/src/client/auth.ts b/src/client/auth.ts index 7e0fd166..8cc3f680 100644 --- a/src/client/auth.ts +++ b/src/client/auth.ts @@ -167,7 +167,7 @@ function selectClientAuthMethod( function applyClientAuthentication( method: string, clientInformation: OAuthClientInformation, - headers: HeadersInit, + headers: Headers, params: URLSearchParams ): void { const { client_id, client_secret } = clientInformation; @@ -193,13 +193,13 @@ function applyClientAuthentication( /** * Applies HTTP Basic authentication (RFC 6749 Section 2.3.1) */ -function applyBasicAuth(clientId: string, clientSecret: string | undefined, headers: HeadersInit): void { +function applyBasicAuth(clientId: string, clientSecret: string | undefined, headers: Headers): void { if (!clientSecret) { throw new Error("client_secret_basic authentication requires a client_secret"); } const credentials = btoa(`${clientId}:${clientSecret}`); - (headers as any)["Authorization"] = `Basic ${credentials}`; + headers.set("Authorization", `Basic ${credentials}`); } /** @@ -635,10 +635,6 @@ export async function exchangeAuthorization( ); } - // const headers: HeadersInit = { - // "Content-Type": "application/x-www-form-urlencoded", - // }; - // // Exchange code for tokens const headers = new Headers({ "Content-Type": "application/x-www-form-urlencoded", From c7f9e36b8ab5939360f1d190353a7d037e7e7baf Mon Sep 17 00:00:00 2001 From: Olivier Chafik Date: Tue, 1 Jul 2025 16:02:55 +0100 Subject: [PATCH 11/19] update tests --- src/client/auth.test.ts | 132 +++++++++++++++------------------------- src/client/auth.ts | 28 +++------ 2 files changed, 58 insertions(+), 102 deletions(-) diff --git a/src/client/auth.test.ts b/src/client/auth.test.ts index 0d6abdee..4122be48 100644 --- a/src/client/auth.test.ts +++ b/src/client/auth.test.ts @@ -1,3 +1,4 @@ +import { mock, Mock } from 'node:test'; import { LATEST_PROTOCOL_VERSION } from '../types.js'; import { discoverOAuthMetadata, @@ -16,8 +17,25 @@ const mockFetch = jest.fn(); global.fetch = mockFetch; describe("OAuth Authorization", () => { + let mockProvider: OAuthClientProvider; + beforeEach(() => { mockFetch.mockReset(); + mockProvider = { + get redirectUrl() { return "http://localhost:3000/callback"; }, + get clientMetadata() { + return { + redirect_uris: ["http://localhost:3000/callback"], + client_name: "Test Client", + }; + }, + clientInformation: jest.fn(), + tokens: jest.fn(), + saveTokens: jest.fn(), + redirectToAuthorization: jest.fn(), + saveCodeVerifier: jest.fn(), + codeVerifier() { return "verifier123"; }, + }; }); describe("extractResourceMetadataUrl", () => { @@ -462,9 +480,9 @@ describe("OAuth Authorization", () => { { metadata: undefined, clientInformation: validClientInfo, - redirectUrl: "http://localhost:3000/callback", resource: new URL("https://api.example.com/mcp-server"), - } + }, + mockProvider ); expect(authorizationUrl.toString()).toMatch( @@ -487,9 +505,9 @@ describe("OAuth Authorization", () => { "https://auth.example.com", { clientInformation: validClientInfo, - redirectUrl: "http://localhost:3000/callback", scope: "read write profile", - } + }, + mockProvider ); expect(authorizationUrl.searchParams.get("scope")).toBe("read write profile"); @@ -500,8 +518,8 @@ describe("OAuth Authorization", () => { "https://auth.example.com", { clientInformation: validClientInfo, - redirectUrl: "http://localhost:3000/callback", - } + }, + mockProvider ); expect(authorizationUrl.searchParams.has("scope")).toBe(false); @@ -512,9 +530,9 @@ describe("OAuth Authorization", () => { "https://auth.example.com", { clientInformation: validClientInfo, - redirectUrl: "http://localhost:3000/callback", state: "foobar", - } + }, + mockProvider ); expect(authorizationUrl.searchParams.get("state")).toBe("foobar"); @@ -525,8 +543,8 @@ describe("OAuth Authorization", () => { "https://auth.example.com", { clientInformation: validClientInfo, - redirectUrl: "http://localhost:3000/callback", - } + }, + mockProvider ); expect(authorizationUrl.searchParams.has("state")).toBe(false); @@ -538,8 +556,8 @@ describe("OAuth Authorization", () => { { metadata: validMetadata, clientInformation: validClientInfo, - redirectUrl: "http://localhost:3000/callback", - } + }, + mockProvider ); expect(authorizationUrl.toString()).toMatch( @@ -557,8 +575,7 @@ describe("OAuth Authorization", () => { startAuthorization("https://auth.example.com", { metadata, clientInformation: validClientInfo, - redirectUrl: "http://localhost:3000/callback", - }) + }, mockProvider) ).rejects.toThrow(/does not support response type/); }); @@ -573,29 +590,12 @@ describe("OAuth Authorization", () => { startAuthorization("https://auth.example.com", { metadata, clientInformation: validClientInfo, - redirectUrl: "http://localhost:3000/callback", - }) + }, mockProvider) ).rejects.toThrow(/does not support code challenge method/); }); }); describe("exchangeAuthorization", () => { - const mockProvider: OAuthClientProvider = { - get redirectUrl() { return "http://localhost:3000/callback"; }, - get clientMetadata() { - return { - redirect_uris: ["http://localhost:3000/callback"], - client_name: "Test Client", - }; - }, - clientInformation: jest.fn(), - tokens: jest.fn(), - saveTokens: jest.fn(), - redirectToAuthorization: jest.fn(), - saveCodeVerifier: jest.fn(), - codeVerifier: jest.fn(), - }; - const validTokens = { access_token: "access123", token_type: "Bearer", @@ -620,10 +620,8 @@ describe("OAuth Authorization", () => { const tokens = await exchangeAuthorization("https://auth.example.com", { clientInformation: validClientInfo, authorizationCode: "code123", - codeVerifier: "verifier123", - redirectUri: "http://localhost:3000/callback", resource: new URL("https://api.example.com/mcp-server"), - }); + }, mockProvider); expect(tokens).toEqual(validTokens); expect(mockFetch).toHaveBeenCalledWith( @@ -632,11 +630,12 @@ describe("OAuth Authorization", () => { }), expect.objectContaining({ method: "POST", + headers: new Headers({ + "Content-Type": "application/x-www-form-urlencoded", + }), }) ); - const headers = mockFetch.mock.calls[0][1].headers as Headers; - expect(headers.get("Content-Type")).toBe("application/x-www-form-urlencoded"); const body = mockFetch.mock.calls[0][1].body as URLSearchParams; expect(body.get("grant_type")).toBe("authorization_code"); expect(body.get("code")).toBe("code123"); @@ -663,8 +662,6 @@ describe("OAuth Authorization", () => { const tokens = await exchangeAuthorization("https://auth.example.com", { clientInformation: validClientInfo, authorizationCode: "code123", - codeVerifier: "verifier123", - redirectUri: "http://localhost:3000/callback", }, mockProvider); expect(tokens).toEqual(validTokens); @@ -705,9 +702,7 @@ describe("OAuth Authorization", () => { exchangeAuthorization("https://auth.example.com", { clientInformation: validClientInfo, authorizationCode: "code123", - codeVerifier: "verifier123", - redirectUri: "http://localhost:3000/callback", - }) + }, mockProvider) ).rejects.toThrow(); }); @@ -721,30 +716,12 @@ describe("OAuth Authorization", () => { exchangeAuthorization("https://auth.example.com", { clientInformation: validClientInfo, authorizationCode: "code123", - codeVerifier: "verifier123", - redirectUri: "http://localhost:3000/callback", - }) + }, mockProvider) ).rejects.toThrow("Token exchange failed"); }); }); describe("refreshAuthorization", () => { - const mockProvider: OAuthClientProvider = { - get redirectUrl() { return "http://localhost:3000/callback"; }, - get clientMetadata() { - return { - redirect_uris: ["http://localhost:3000/callback"], - client_name: "Test Client", - }; - }, - clientInformation: jest.fn(), - tokens: jest.fn(), - saveTokens: jest.fn(), - redirectToAuthorization: jest.fn(), - saveCodeVerifier: jest.fn(), - codeVerifier: jest.fn(), - }; - const validTokens = { access_token: "newaccess123", token_type: "Bearer", @@ -773,7 +750,7 @@ describe("OAuth Authorization", () => { clientInformation: validClientInfo, refreshToken: "refresh123", resource: new URL("https://api.example.com/mcp-server"), - }); + }, mockProvider); expect(tokens).toEqual(validTokensWithNewRefreshToken); expect(mockFetch).toHaveBeenCalledWith( @@ -785,8 +762,6 @@ describe("OAuth Authorization", () => { }) ); - const headers = mockFetch.mock.calls[0][1].headers as Headers; - expect(headers.get("Content-Type")).toBe("application/x-www-form-urlencoded"); const body = mockFetch.mock.calls[0][1].body as URLSearchParams; expect(body.get("grant_type")).toBe("refresh_token"); expect(body.get("refresh_token")).toBe("refresh123"); @@ -846,7 +821,7 @@ describe("OAuth Authorization", () => { const tokens = await refreshAuthorization("https://auth.example.com", { clientInformation: validClientInfo, refreshToken, - }); + }, mockProvider); expect(tokens).toEqual({ refresh_token: refreshToken, ...validTokens }); }); @@ -865,7 +840,7 @@ describe("OAuth Authorization", () => { refreshAuthorization("https://auth.example.com", { clientInformation: validClientInfo, refreshToken: "refresh123", - }) + }, mockProvider) ).rejects.toThrow(); }); @@ -879,7 +854,7 @@ describe("OAuth Authorization", () => { refreshAuthorization("https://auth.example.com", { clientInformation: validClientInfo, refreshToken: "refresh123", - }) + }, mockProvider) ).rejects.toThrow("Token refresh failed"); }); }); @@ -1624,9 +1599,7 @@ describe("OAuth Authorization", () => { metadata: metadataWithBasicOnly, clientInformation: validClientInfo, authorizationCode: "code123", - codeVerifier: "verifier123", - redirectUri: "http://localhost:3000/callback", - }); + }, mockProvider); expect(tokens).toEqual(validTokens); const request = mockFetch.mock.calls[0][1]; @@ -1652,9 +1625,7 @@ describe("OAuth Authorization", () => { metadata: metadataWithPostOnly, clientInformation: validClientInfo, authorizationCode: "code123", - codeVerifier: "verifier123", - redirectUri: "http://localhost:3000/callback", - }); + }, mockProvider); expect(tokens).toEqual(validTokens); const request = mockFetch.mock.calls[0][1]; @@ -1684,9 +1655,7 @@ describe("OAuth Authorization", () => { metadata: metadataWithNoneOnly, clientInformation: clientInfoWithoutSecret, authorizationCode: "code123", - codeVerifier: "verifier123", - redirectUri: "http://localhost:3000/callback", - }); + }, mockProvider); expect(tokens).toEqual(validTokens); const request = mockFetch.mock.calls[0][1]; @@ -1709,14 +1678,13 @@ describe("OAuth Authorization", () => { const tokens = await exchangeAuthorization("https://auth.example.com", { clientInformation: validClientInfo, authorizationCode: "code123", - codeVerifier: "verifier123", - redirectUri: "http://localhost:3000/callback", - }); + }, mockProvider); expect(tokens).toEqual(validTokens); const request = mockFetch.mock.calls[0][1]; - // Check no Authorization header + // Check headers + expect(request.headers.get("Content-Type")).toBe("application/x-www-form-urlencoded"); expect(request.headers.get("Authorization")).toBeNull(); const body = request.body as URLSearchParams; @@ -1764,7 +1732,7 @@ describe("OAuth Authorization", () => { metadata: metadataWithBasicOnly, clientInformation: validClientInfo, refreshToken: "refresh123", - }); + }, mockProvider); expect(tokens).toEqual(validTokens); const request = mockFetch.mock.calls[0][1]; @@ -1791,7 +1759,7 @@ describe("OAuth Authorization", () => { metadata: metadataWithPostOnly, clientInformation: validClientInfo, refreshToken: "refresh123", - }); + }, mockProvider); expect(tokens).toEqual(validTokens); const request = mockFetch.mock.calls[0][1]; diff --git a/src/client/auth.ts b/src/client/auth.ts index 8cc3f680..fb1561d3 100644 --- a/src/client/auth.ts +++ b/src/client/auth.ts @@ -274,13 +274,10 @@ export async function auth( // Exchange authorization code for tokens if (authorizationCode !== undefined) { - const codeVerifier = await provider.codeVerifier(); const tokens = await exchangeAuthorization(authorizationServerUrl, { metadata, clientInformation, authorizationCode, - codeVerifier, - redirectUri: provider.redirectUrl, resource, }, provider); @@ -315,10 +312,9 @@ export async function auth( metadata, clientInformation, state, - redirectUrl: provider.redirectUrl, scope: scope || provider.clientMetadata.scope, resource, - }); + }, provider); await provider.saveCodeVerifier(codeVerifier); await provider.redirectToAuthorization(authorizationUrl); @@ -522,18 +518,17 @@ export async function startAuthorization( { metadata, clientInformation, - redirectUrl, scope, state, resource, }: { metadata?: OAuthMetadata; clientInformation: OAuthClientInformation; - redirectUrl: string | URL; scope?: string; state?: string; resource?: URL; }, + provider: OAuthClientProvider, ): Promise<{ authorizationUrl: URL; codeVerifier: string }> { const responseType = "code"; const codeChallengeMethod = "S256"; @@ -572,7 +567,7 @@ export async function startAuthorization( "code_challenge_method", codeChallengeMethod, ); - authorizationUrl.searchParams.set("redirect_uri", String(redirectUrl)); + authorizationUrl.searchParams.set("redirect_uri", String(provider.redirectUrl)); if (state) { authorizationUrl.searchParams.set("state", state); @@ -607,18 +602,14 @@ export async function exchangeAuthorization( metadata, clientInformation, authorizationCode, - codeVerifier, - redirectUri, resource, }: { metadata?: OAuthMetadata; clientInformation: OAuthClientInformation; authorizationCode: string; - codeVerifier: string; - redirectUri: string | URL; resource?: URL; }, - provider?: OAuthClientProvider + provider: OAuthClientProvider ): Promise { const grantType = "authorization_code"; @@ -635,15 +626,16 @@ export async function exchangeAuthorization( ); } - // // Exchange code for tokens + // Exchange code for tokens const headers = new Headers({ "Content-Type": "application/x-www-form-urlencoded", }); + const codeVerifier = await provider.codeVerifier(); const params = new URLSearchParams({ grant_type: grantType, code: authorizationCode, code_verifier: codeVerifier, - redirect_uri: String(redirectUri), + redirect_uri: String(provider.redirectUrl), }); if (provider?.addClientAuthentication) { @@ -698,7 +690,7 @@ export async function refreshAuthorization( refreshToken: string; resource?: URL; }, - provider?: OAuthClientProvider, + provider: OAuthClientProvider, ): Promise { const grantType = "refresh_token"; @@ -718,10 +710,6 @@ export async function refreshAuthorization( tokenUrl = new URL("/token", authorizationServerUrl); } - // const headers: HeadersInit = { - // "Content-Type": "application/x-www-form-urlencoded", - // }; - // Exchange refresh token const headers = new Headers({ "Content-Type": "application/x-www-form-urlencoded", From 3444b676a8e8fd4e9028754e5a6893d8029409c3 Mon Sep 17 00:00:00 2001 From: Olivier Chafik Date: Tue, 1 Jul 2025 17:14:51 +0100 Subject: [PATCH 12/19] revert / minimize diffs --- src/client/auth.test.ts | 115 ++++++++++++++++++++-------------------- src/client/auth.ts | 42 ++++++++++----- 2 files changed, 85 insertions(+), 72 deletions(-) diff --git a/src/client/auth.test.ts b/src/client/auth.test.ts index 4122be48..36420dfc 100644 --- a/src/client/auth.test.ts +++ b/src/client/auth.test.ts @@ -17,25 +17,8 @@ const mockFetch = jest.fn(); global.fetch = mockFetch; describe("OAuth Authorization", () => { - let mockProvider: OAuthClientProvider; - beforeEach(() => { mockFetch.mockReset(); - mockProvider = { - get redirectUrl() { return "http://localhost:3000/callback"; }, - get clientMetadata() { - return { - redirect_uris: ["http://localhost:3000/callback"], - client_name: "Test Client", - }; - }, - clientInformation: jest.fn(), - tokens: jest.fn(), - saveTokens: jest.fn(), - redirectToAuthorization: jest.fn(), - saveCodeVerifier: jest.fn(), - codeVerifier() { return "verifier123"; }, - }; }); describe("extractResourceMetadataUrl", () => { @@ -480,9 +463,9 @@ describe("OAuth Authorization", () => { { metadata: undefined, clientInformation: validClientInfo, + redirectUrl: "http://localhost:3000/callback", resource: new URL("https://api.example.com/mcp-server"), - }, - mockProvider + } ); expect(authorizationUrl.toString()).toMatch( @@ -505,9 +488,9 @@ describe("OAuth Authorization", () => { "https://auth.example.com", { clientInformation: validClientInfo, + redirectUrl: "http://localhost:3000/callback", scope: "read write profile", - }, - mockProvider + } ); expect(authorizationUrl.searchParams.get("scope")).toBe("read write profile"); @@ -518,8 +501,8 @@ describe("OAuth Authorization", () => { "https://auth.example.com", { clientInformation: validClientInfo, - }, - mockProvider + redirectUrl: "http://localhost:3000/callback", + } ); expect(authorizationUrl.searchParams.has("scope")).toBe(false); @@ -530,9 +513,9 @@ describe("OAuth Authorization", () => { "https://auth.example.com", { clientInformation: validClientInfo, + redirectUrl: "http://localhost:3000/callback", state: "foobar", - }, - mockProvider + } ); expect(authorizationUrl.searchParams.get("state")).toBe("foobar"); @@ -543,8 +526,8 @@ describe("OAuth Authorization", () => { "https://auth.example.com", { clientInformation: validClientInfo, - }, - mockProvider + redirectUrl: "http://localhost:3000/callback", + } ); expect(authorizationUrl.searchParams.has("state")).toBe(false); @@ -556,8 +539,8 @@ describe("OAuth Authorization", () => { { metadata: validMetadata, clientInformation: validClientInfo, - }, - mockProvider + redirectUrl: "http://localhost:3000/callback", + } ); expect(authorizationUrl.toString()).toMatch( @@ -575,7 +558,8 @@ describe("OAuth Authorization", () => { startAuthorization("https://auth.example.com", { metadata, clientInformation: validClientInfo, - }, mockProvider) + redirectUrl: "http://localhost:3000/callback", + }) ).rejects.toThrow(/does not support response type/); }); @@ -590,7 +574,8 @@ describe("OAuth Authorization", () => { startAuthorization("https://auth.example.com", { metadata, clientInformation: validClientInfo, - }, mockProvider) + redirectUrl: "http://localhost:3000/callback", + }) ).rejects.toThrow(/does not support code challenge method/); }); }); @@ -620,8 +605,10 @@ describe("OAuth Authorization", () => { const tokens = await exchangeAuthorization("https://auth.example.com", { clientInformation: validClientInfo, authorizationCode: "code123", + codeVerifier: "verifier123", + redirectUri: "http://localhost:3000/callback", resource: new URL("https://api.example.com/mcp-server"), - }, mockProvider); + }); expect(tokens).toEqual(validTokens); expect(mockFetch).toHaveBeenCalledWith( @@ -647,12 +634,6 @@ describe("OAuth Authorization", () => { }); it("exchanges code for tokens with auth", async () => { - mockProvider.addClientAuthentication = function(url: URL, headers: Headers, params: URLSearchParams) { - headers.set("Authorization", "Basic " + btoa(validClientInfo.client_id + ":" + validClientInfo.client_secret)); - params.set("example_url", url.toString()); - params.set("example_param", "example_value"); - }; - mockFetch.mockResolvedValueOnce({ ok: true, status: 200, @@ -662,7 +643,14 @@ describe("OAuth Authorization", () => { const tokens = await exchangeAuthorization("https://auth.example.com", { clientInformation: validClientInfo, authorizationCode: "code123", - }, mockProvider); + codeVerifier: "verifier123", + redirectUri: "http://localhost:3000/callback", + addClientAuthentication: (url: URL, headers: Headers, params: URLSearchParams) => { + headers.set("Authorization", "Basic " + btoa(validClientInfo.client_id + ":" + validClientInfo.client_secret)); + params.set("example_url", url.toString()); + params.set("example_param", "example_value"); + }, + }); expect(tokens).toEqual(validTokens); expect(mockFetch).toHaveBeenCalledWith( @@ -702,7 +690,9 @@ describe("OAuth Authorization", () => { exchangeAuthorization("https://auth.example.com", { clientInformation: validClientInfo, authorizationCode: "code123", - }, mockProvider) + redirectUri: "http://localhost:3000/callback", + codeVerifier: "verifier123", + }) ).rejects.toThrow(); }); @@ -715,8 +705,10 @@ describe("OAuth Authorization", () => { await expect( exchangeAuthorization("https://auth.example.com", { clientInformation: validClientInfo, + redirectUri: "http://localhost:3000/callback", authorizationCode: "code123", - }, mockProvider) + codeVerifier: "verifier123", + }) ).rejects.toThrow("Token exchange failed"); }); }); @@ -750,7 +742,7 @@ describe("OAuth Authorization", () => { clientInformation: validClientInfo, refreshToken: "refresh123", resource: new URL("https://api.example.com/mcp-server"), - }, mockProvider); + }); expect(tokens).toEqual(validTokensWithNewRefreshToken); expect(mockFetch).toHaveBeenCalledWith( @@ -771,12 +763,6 @@ describe("OAuth Authorization", () => { }); it("exchanges refresh token for new tokens with auth", async () => { - mockProvider.addClientAuthentication = function(url: URL, headers: Headers, params: URLSearchParams) { - headers.set("Authorization", "Basic " + btoa(validClientInfo.client_id + ":" + validClientInfo.client_secret)); - params.set("example_url", url.toString()); - params.set("example_param", "example_value"); - }; - mockFetch.mockResolvedValueOnce({ ok: true, status: 200, @@ -786,7 +772,12 @@ describe("OAuth Authorization", () => { const tokens = await refreshAuthorization("https://auth.example.com", { clientInformation: validClientInfo, refreshToken: "refresh123", - }, mockProvider); + addClientAuthentication: (url: URL, headers: Headers, params: URLSearchParams) => { + headers.set("Authorization", "Basic " + btoa(validClientInfo.client_id + ":" + validClientInfo.client_secret)); + params.set("example_url", url.toString()); + params.set("example_param", "example_value"); + }, + }); expect(tokens).toEqual(validTokensWithNewRefreshToken); expect(mockFetch).toHaveBeenCalledWith( @@ -821,7 +812,7 @@ describe("OAuth Authorization", () => { const tokens = await refreshAuthorization("https://auth.example.com", { clientInformation: validClientInfo, refreshToken, - }, mockProvider); + }); expect(tokens).toEqual({ refresh_token: refreshToken, ...validTokens }); }); @@ -840,7 +831,7 @@ describe("OAuth Authorization", () => { refreshAuthorization("https://auth.example.com", { clientInformation: validClientInfo, refreshToken: "refresh123", - }, mockProvider) + }) ).rejects.toThrow(); }); @@ -854,7 +845,7 @@ describe("OAuth Authorization", () => { refreshAuthorization("https://auth.example.com", { clientInformation: validClientInfo, refreshToken: "refresh123", - }, mockProvider) + }) ).rejects.toThrow("Token refresh failed"); }); }); @@ -1599,7 +1590,9 @@ describe("OAuth Authorization", () => { metadata: metadataWithBasicOnly, clientInformation: validClientInfo, authorizationCode: "code123", - }, mockProvider); + redirectUri: "http://localhost:3000/callback", + codeVerifier: "verifier123", + }); expect(tokens).toEqual(validTokens); const request = mockFetch.mock.calls[0][1]; @@ -1625,7 +1618,9 @@ describe("OAuth Authorization", () => { metadata: metadataWithPostOnly, clientInformation: validClientInfo, authorizationCode: "code123", - }, mockProvider); + redirectUri: "http://localhost:3000/callback", + codeVerifier: "verifier123", + }); expect(tokens).toEqual(validTokens); const request = mockFetch.mock.calls[0][1]; @@ -1655,7 +1650,9 @@ describe("OAuth Authorization", () => { metadata: metadataWithNoneOnly, clientInformation: clientInfoWithoutSecret, authorizationCode: "code123", - }, mockProvider); + redirectUri: "http://localhost:3000/callback", + codeVerifier: "verifier123", + }); expect(tokens).toEqual(validTokens); const request = mockFetch.mock.calls[0][1]; @@ -1678,7 +1675,9 @@ describe("OAuth Authorization", () => { const tokens = await exchangeAuthorization("https://auth.example.com", { clientInformation: validClientInfo, authorizationCode: "code123", - }, mockProvider); + redirectUri: "http://localhost:3000/callback", + codeVerifier: "verifier123", + }); expect(tokens).toEqual(validTokens); const request = mockFetch.mock.calls[0][1]; @@ -1732,7 +1731,7 @@ describe("OAuth Authorization", () => { metadata: metadataWithBasicOnly, clientInformation: validClientInfo, refreshToken: "refresh123", - }, mockProvider); + }); expect(tokens).toEqual(validTokens); const request = mockFetch.mock.calls[0][1]; @@ -1759,7 +1758,7 @@ describe("OAuth Authorization", () => { metadata: metadataWithPostOnly, clientInformation: validClientInfo, refreshToken: "refresh123", - }, mockProvider); + }); expect(tokens).toEqual(validTokens); const request = mockFetch.mock.calls[0][1]; diff --git a/src/client/auth.ts b/src/client/auth.ts index fb1561d3..bb5d8b0d 100644 --- a/src/client/auth.ts +++ b/src/client/auth.ts @@ -274,12 +274,16 @@ export async function auth( // Exchange authorization code for tokens if (authorizationCode !== undefined) { + const codeVerifier = await provider.codeVerifier(); const tokens = await exchangeAuthorization(authorizationServerUrl, { metadata, clientInformation, authorizationCode, + codeVerifier, + redirectUri: provider.redirectUrl, resource, - }, provider); + addClientAuthentication: provider.addClientAuthentication, + }); await provider.saveTokens(tokens); return "AUTHORIZED"; @@ -296,7 +300,8 @@ export async function auth( clientInformation, refreshToken: tokens.refresh_token, resource, - }, provider); + addClientAuthentication: provider.addClientAuthentication, + }); await provider.saveTokens(newTokens); return "AUTHORIZED"; @@ -312,9 +317,10 @@ export async function auth( metadata, clientInformation, state, + redirectUrl: provider.redirectUrl, scope: scope || provider.clientMetadata.scope, resource, - }, provider); + }); await provider.saveCodeVerifier(codeVerifier); await provider.redirectToAuthorization(authorizationUrl); @@ -518,17 +524,20 @@ export async function startAuthorization( { metadata, clientInformation, + redirectUrl, scope, state, resource, + addClientAuthentication, }: { metadata?: OAuthMetadata; clientInformation: OAuthClientInformation; + redirectUrl: string | URL; scope?: string; state?: string; resource?: URL; + addClientAuthentication?: OAuthClientProvider["addClientAuthentication"]; }, - provider: OAuthClientProvider, ): Promise<{ authorizationUrl: URL; codeVerifier: string }> { const responseType = "code"; const codeChallengeMethod = "S256"; @@ -567,7 +576,7 @@ export async function startAuthorization( "code_challenge_method", codeChallengeMethod, ); - authorizationUrl.searchParams.set("redirect_uri", String(provider.redirectUrl)); + authorizationUrl.searchParams.set("redirect_uri", String(redirectUrl)); if (state) { authorizationUrl.searchParams.set("state", state); @@ -602,14 +611,19 @@ export async function exchangeAuthorization( metadata, clientInformation, authorizationCode, + codeVerifier, + redirectUri, resource, + addClientAuthentication }: { metadata?: OAuthMetadata; clientInformation: OAuthClientInformation; authorizationCode: string; + codeVerifier: string; + redirectUri: string | URL; resource?: URL; + addClientAuthentication?: OAuthClientProvider["addClientAuthentication"]; }, - provider: OAuthClientProvider ): Promise { const grantType = "authorization_code"; @@ -630,16 +644,15 @@ export async function exchangeAuthorization( const headers = new Headers({ "Content-Type": "application/x-www-form-urlencoded", }); - const codeVerifier = await provider.codeVerifier(); const params = new URLSearchParams({ grant_type: grantType, code: authorizationCode, code_verifier: codeVerifier, - redirect_uri: String(provider.redirectUrl), + redirect_uri: String(redirectUri), }); - if (provider?.addClientAuthentication) { - provider.addClientAuthentication(tokenUrl, headers, params); + if (addClientAuthentication) { + addClientAuthentication(tokenUrl, headers, params); } else { // Determine and apply client authentication method const supportedMethods = metadata?.token_endpoint_auth_methods_supported ?? []; @@ -684,13 +697,14 @@ export async function refreshAuthorization( clientInformation, refreshToken, resource, + addClientAuthentication, }: { metadata?: OAuthMetadata; clientInformation: OAuthClientInformation; refreshToken: string; resource?: URL; - }, - provider: OAuthClientProvider, + addClientAuthentication?: OAuthClientProvider["addClientAuthentication"]; + } ): Promise { const grantType = "refresh_token"; @@ -719,8 +733,8 @@ export async function refreshAuthorization( refresh_token: refreshToken, }); - if (provider?.addClientAuthentication) { - provider.addClientAuthentication(tokenUrl, headers, params); + if (addClientAuthentication) { + addClientAuthentication(tokenUrl, headers, params); } else { // Determine and apply client authentication method const supportedMethods = metadata?.token_endpoint_auth_methods_supported ?? []; From 3ccff1c4b751aa2526ed196a42d22cfbad324fae Mon Sep 17 00:00:00 2001 From: Olivier Chafik Date: Tue, 1 Jul 2025 18:08:18 +0100 Subject: [PATCH 13/19] cleanup --- src/client/auth.test.ts | 7 +++++-- src/client/auth.ts | 2 -- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/client/auth.test.ts b/src/client/auth.test.ts index 36420dfc..e4f5bc9a 100644 --- a/src/client/auth.test.ts +++ b/src/client/auth.test.ts @@ -690,8 +690,8 @@ describe("OAuth Authorization", () => { exchangeAuthorization("https://auth.example.com", { clientInformation: validClientInfo, authorizationCode: "code123", - redirectUri: "http://localhost:3000/callback", codeVerifier: "verifier123", + redirectUri: "http://localhost:3000/callback", }) ).rejects.toThrow(); }); @@ -705,9 +705,9 @@ describe("OAuth Authorization", () => { await expect( exchangeAuthorization("https://auth.example.com", { clientInformation: validClientInfo, - redirectUri: "http://localhost:3000/callback", authorizationCode: "code123", codeVerifier: "verifier123", + redirectUri: "http://localhost:3000/callback", }) ).rejects.toThrow("Token exchange failed"); }); @@ -751,6 +751,9 @@ describe("OAuth Authorization", () => { }), expect.objectContaining({ method: "POST", + headers: new Headers({ + "Content-Type": "application/x-www-form-urlencoded", + }), }) ); diff --git a/src/client/auth.ts b/src/client/auth.ts index bb5d8b0d..c4b3c920 100644 --- a/src/client/auth.ts +++ b/src/client/auth.ts @@ -528,7 +528,6 @@ export async function startAuthorization( scope, state, resource, - addClientAuthentication, }: { metadata?: OAuthMetadata; clientInformation: OAuthClientInformation; @@ -536,7 +535,6 @@ export async function startAuthorization( scope?: string; state?: string; resource?: URL; - addClientAuthentication?: OAuthClientProvider["addClientAuthentication"]; }, ): Promise<{ authorizationUrl: URL; codeVerifier: string }> { const responseType = "code"; From 8875e212b666383fb3f959a3cb66326409cac338 Mon Sep 17 00:00:00 2001 From: Jared Hanson Date: Tue, 1 Jul 2025 20:39:40 -0700 Subject: [PATCH 14/19] Add authorizationServerUrl and metadata arguments to addClientAuthentication. --- src/client/auth.ts | 42 +++++++++++++++++++++--------------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/src/client/auth.ts b/src/client/auth.ts index c4b3c920..87a8423a 100644 --- a/src/client/auth.ts +++ b/src/client/auth.ts @@ -72,25 +72,25 @@ export interface OAuthClientProvider { * the authorization result. */ codeVerifier(): string | Promise; - + /** * Adds custom client authentication to OAuth token requests. - * + * * This optional method allows implementations to customize how client credentials * are included in token exchange and refresh requests. When provided, this method * is called instead of the default authentication logic, giving full control over * the authentication mechanism. - * + * * Common use cases include: * - Supporting authentication methods beyond the standard OAuth 2.0 methods * - Adding custom headers for proprietary authentication schemes * - Implementing client assertion-based authentication (e.g., JWT bearer tokens) - * + * * @param url - The token endpoint URL being called * @param headers - The request headers (can be modified to add authentication) * @param params - The request body parameters (can be modified to add credentials) */ - addClientAuthentication?(url: URL, headers: Headers, params: URLSearchParams): void | Promise; + addClientAuthentication?(headers: Headers, params: URLSearchParams, url: string | URL, metadata?: OAuthMetadata): void | Promise; /** * If defined, overrides the selection and validation of the @@ -112,12 +112,12 @@ export class UnauthorizedError extends Error { /** * Determines the best client authentication method to use based on server support and client configuration. - * + * * Priority order (highest to lowest): * 1. client_secret_basic (if client secret is available) * 2. client_secret_post (if client secret is available) * 3. none (for public clients) - * + * * @param clientInformation - OAuth client information containing credentials * @param supportedMethods - Authentication methods supported by the authorization server * @returns The selected authentication method @@ -127,7 +127,7 @@ function selectClientAuthMethod( supportedMethods: string[] ): string { const hasClientSecret = !!clientInformation.client_secret; - + // If server doesn't specify supported methods, use RFC 6749 defaults if (supportedMethods.length === 0) { return hasClientSecret ? "client_secret_post" : "none"; @@ -137,11 +137,11 @@ function selectClientAuthMethod( if (hasClientSecret && supportedMethods.includes("client_secret_basic")) { return "client_secret_basic"; } - + if (hasClientSecret && supportedMethods.includes("client_secret_post")) { return "client_secret_post"; } - + if (supportedMethods.includes("none")) { return "none"; } @@ -152,12 +152,12 @@ function selectClientAuthMethod( /** * Applies client authentication to the request based on the specified method. - * + * * Implements OAuth 2.1 client authentication methods: * - client_secret_basic: HTTP Basic authentication (RFC 6749 Section 2.3.1) * - client_secret_post: Credentials in request body (RFC 6749 Section 2.3.1) * - none: Public client authentication (RFC 6749 Section 2.1) - * + * * @param method - The authentication method to use * @param clientInformation - OAuth client information containing credentials * @param headers - HTTP headers object to modify @@ -197,7 +197,7 @@ function applyBasicAuth(clientId: string, clientSecret: string | undefined, head if (!clientSecret) { throw new Error("client_secret_basic authentication requires a client_secret"); } - + const credentials = btoa(`${clientId}:${clientSecret}`); headers.set("Authorization", `Basic ${credentials}`); } @@ -593,11 +593,11 @@ export async function startAuthorization( /** * Exchanges an authorization code for an access token with the given server. - * + * * Supports multiple client authentication methods as specified in OAuth 2.1: * - Automatically selects the best authentication method based on server support * - Falls back to appropriate defaults when server metadata is unavailable - * + * * @param authorizationServerUrl - The authorization server's base URL * @param options - Configuration object containing client info, auth code, etc. * @returns Promise resolving to OAuth tokens @@ -650,12 +650,12 @@ export async function exchangeAuthorization( }); if (addClientAuthentication) { - addClientAuthentication(tokenUrl, headers, params); + addClientAuthentication(headers, params, authorizationServerUrl, metadata); } else { // Determine and apply client authentication method const supportedMethods = metadata?.token_endpoint_auth_methods_supported ?? []; const authMethod = selectClientAuthMethod(clientInformation, supportedMethods); - + applyClientAuthentication(authMethod, clientInformation, headers, params); } @@ -678,11 +678,11 @@ export async function exchangeAuthorization( /** * Exchange a refresh token for an updated access token. - * + * * Supports multiple client authentication methods as specified in OAuth 2.1: * - Automatically selects the best authentication method based on server support * - Preserves the original refresh token if a new one is not returned - * + * * @param authorizationServerUrl - The authorization server's base URL * @param options - Configuration object containing client info, refresh token, etc. * @returns Promise resolving to OAuth tokens (preserves original refresh_token if not replaced) @@ -732,12 +732,12 @@ export async function refreshAuthorization( }); if (addClientAuthentication) { - addClientAuthentication(tokenUrl, headers, params); + addClientAuthentication(headers, params, authorizationServerUrl, metadata); } else { // Determine and apply client authentication method const supportedMethods = metadata?.token_endpoint_auth_methods_supported ?? []; const authMethod = selectClientAuthMethod(clientInformation, supportedMethods); - + applyClientAuthentication(authMethod, clientInformation, headers, params); } From 0bd770b68ee06d8da8a31f24b1e46a895dd79b06 Mon Sep 17 00:00:00 2001 From: Jared Hanson Date: Tue, 1 Jul 2025 20:47:59 -0700 Subject: [PATCH 15/19] Fix tests. --- src/client/auth.test.ts | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/src/client/auth.test.ts b/src/client/auth.test.ts index e4f5bc9a..eb09a2ad 100644 --- a/src/client/auth.test.ts +++ b/src/client/auth.test.ts @@ -232,7 +232,7 @@ describe("OAuth Authorization", () => { ok: false, status: 404, }); - + // Second call (root fallback) succeeds mockFetch.mockResolvedValueOnce({ ok: true, @@ -242,17 +242,17 @@ describe("OAuth Authorization", () => { const metadata = await discoverOAuthMetadata("https://auth.example.com/path/name"); expect(metadata).toEqual(validMetadata); - + const calls = mockFetch.mock.calls; expect(calls.length).toBe(2); - + // First call should be path-aware const [firstUrl, firstOptions] = calls[0]; expect(firstUrl.toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server/path/name"); expect(firstOptions.headers).toEqual({ "MCP-Protocol-Version": LATEST_PROTOCOL_VERSION }); - + // Second call should be root fallback const [secondUrl, secondOptions] = calls[1]; expect(secondUrl.toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server"); @@ -267,7 +267,7 @@ describe("OAuth Authorization", () => { ok: false, status: 404, }); - + // Second call (root fallback) also returns 404 mockFetch.mockResolvedValueOnce({ ok: false, @@ -276,7 +276,7 @@ describe("OAuth Authorization", () => { const metadata = await discoverOAuthMetadata("https://auth.example.com/path/name"); expect(metadata).toBeUndefined(); - + const calls = mockFetch.mock.calls; expect(calls.length).toBe(2); }); @@ -290,10 +290,10 @@ describe("OAuth Authorization", () => { const metadata = await discoverOAuthMetadata("https://auth.example.com/"); expect(metadata).toBeUndefined(); - + const calls = mockFetch.mock.calls; expect(calls.length).toBe(1); // Should not attempt fallback - + const [url] = calls[0]; expect(url.toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server"); }); @@ -307,10 +307,10 @@ describe("OAuth Authorization", () => { const metadata = await discoverOAuthMetadata("https://auth.example.com"); expect(metadata).toBeUndefined(); - + const calls = mockFetch.mock.calls; expect(calls.length).toBe(1); // Should not attempt fallback - + const [url] = calls[0]; expect(url.toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server"); }); @@ -318,13 +318,13 @@ describe("OAuth Authorization", () => { it("falls back when path-aware discovery encounters CORS error", async () => { // First call (path-aware) fails with TypeError (CORS) mockFetch.mockImplementationOnce(() => Promise.reject(new TypeError("CORS error"))); - + // Retry path-aware without headers (simulating CORS retry) mockFetch.mockResolvedValueOnce({ ok: false, status: 404, }); - + // Second call (root fallback) succeeds mockFetch.mockResolvedValueOnce({ ok: true, @@ -334,10 +334,10 @@ describe("OAuth Authorization", () => { const metadata = await discoverOAuthMetadata("https://auth.example.com/deep/path"); expect(metadata).toEqual(validMetadata); - + const calls = mockFetch.mock.calls; expect(calls.length).toBe(3); - + // Final call should be root fallback const [lastUrl, lastOptions] = calls[2]; expect(lastUrl.toString()).toBe("https://auth.example.com/.well-known/oauth-authorization-server"); @@ -645,9 +645,9 @@ describe("OAuth Authorization", () => { authorizationCode: "code123", codeVerifier: "verifier123", redirectUri: "http://localhost:3000/callback", - addClientAuthentication: (url: URL, headers: Headers, params: URLSearchParams) => { + addClientAuthentication: (headers: Headers, params: URLSearchParams, url: string | URL) => { headers.set("Authorization", "Basic " + btoa(validClientInfo.client_id + ":" + validClientInfo.client_secret)); - params.set("example_url", url.toString()); + params.set("example_url", typeof url === 'string' ? url : url.toString()); params.set("example_param", "example_value"); }, }); @@ -671,7 +671,7 @@ describe("OAuth Authorization", () => { expect(body.get("code_verifier")).toBe("verifier123"); expect(body.get("client_id")).toBeNull(); expect(body.get("redirect_uri")).toBe("http://localhost:3000/callback"); - expect(body.get("example_url")).toBe("https://auth.example.com/token"); + expect(body.get("example_url")).toBe("https://auth.example.com"); expect(body.get("example_param")).toBe("example_value"); expect(body.get("client_secret")).toBeNull(); }); @@ -775,9 +775,9 @@ describe("OAuth Authorization", () => { const tokens = await refreshAuthorization("https://auth.example.com", { clientInformation: validClientInfo, refreshToken: "refresh123", - addClientAuthentication: (url: URL, headers: Headers, params: URLSearchParams) => { + addClientAuthentication: (headers: Headers, params: URLSearchParams, url: string | URL) => { headers.set("Authorization", "Basic " + btoa(validClientInfo.client_id + ":" + validClientInfo.client_secret)); - params.set("example_url", url.toString()); + params.set("example_url", typeof url === 'string' ? url : url.toString()); params.set("example_param", "example_value"); }, }); @@ -799,7 +799,7 @@ describe("OAuth Authorization", () => { expect(body.get("grant_type")).toBe("refresh_token"); expect(body.get("refresh_token")).toBe("refresh123"); expect(body.get("client_id")).toBeNull(); - expect(body.get("example_url")).toBe("https://auth.example.com/token"); + expect(body.get("example_url")).toBe("https://auth.example.com"); expect(body.get("example_param")).toBe("example_value"); expect(body.get("client_secret")).toBeNull(); }); From b797e9cedaac5bc3642acbc7250f639fd8e98bb4 Mon Sep 17 00:00:00 2001 From: Jared Hanson Date: Tue, 1 Jul 2025 20:58:21 -0700 Subject: [PATCH 16/19] Update addClientAuthentication tests for metadata. --- src/client/auth.test.ts | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/src/client/auth.test.ts b/src/client/auth.test.ts index eb09a2ad..3285c37b 100644 --- a/src/client/auth.test.ts +++ b/src/client/auth.test.ts @@ -11,6 +11,7 @@ import { auth, type OAuthClientProvider, } from "./auth.js"; +import { OAuthMetadata } from 'src/shared/auth.js'; // Mock fetch globally const mockFetch = jest.fn(); @@ -588,6 +589,13 @@ describe("OAuth Authorization", () => { refresh_token: "refresh123", }; + const validMetadata = { + issuer: "https://auth.example.com", + authorization_endpoint: "https://auth.example.com/authorize", + token_endpoint: "https://auth.example.com/token", + response_types_supported: ["code"] + }; + const validClientInfo = { client_id: "client123", client_secret: "secret123", @@ -641,13 +649,15 @@ describe("OAuth Authorization", () => { }); const tokens = await exchangeAuthorization("https://auth.example.com", { + metadata: validMetadata, clientInformation: validClientInfo, authorizationCode: "code123", codeVerifier: "verifier123", redirectUri: "http://localhost:3000/callback", - addClientAuthentication: (headers: Headers, params: URLSearchParams, url: string | URL) => { + addClientAuthentication: (headers: Headers, params: URLSearchParams, url: string | URL, metadata: OAuthMetadata) => { headers.set("Authorization", "Basic " + btoa(validClientInfo.client_id + ":" + validClientInfo.client_secret)); params.set("example_url", typeof url === 'string' ? url : url.toString()); + params.set("example_metadata", metadata.authorization_endpoint); params.set("example_param", "example_value"); }, }); @@ -672,6 +682,7 @@ describe("OAuth Authorization", () => { expect(body.get("client_id")).toBeNull(); expect(body.get("redirect_uri")).toBe("http://localhost:3000/callback"); expect(body.get("example_url")).toBe("https://auth.example.com"); + expect(body.get("example_metadata")).toBe("https://auth.example.com/authorize"); expect(body.get("example_param")).toBe("example_value"); expect(body.get("client_secret")).toBeNull(); }); @@ -724,6 +735,13 @@ describe("OAuth Authorization", () => { refresh_token: "newrefresh123", }; + const validMetadata = { + issuer: "https://auth.example.com", + authorization_endpoint: "https://auth.example.com/authorize", + token_endpoint: "https://auth.example.com/token", + response_types_supported: ["code"] + }; + const validClientInfo = { client_id: "client123", client_secret: "secret123", @@ -773,11 +791,13 @@ describe("OAuth Authorization", () => { }); const tokens = await refreshAuthorization("https://auth.example.com", { + metadata: validMetadata, clientInformation: validClientInfo, refreshToken: "refresh123", - addClientAuthentication: (headers: Headers, params: URLSearchParams, url: string | URL) => { + addClientAuthentication: (headers: Headers, params: URLSearchParams, url: string | URL, metadata: OAuthMetadata) => { headers.set("Authorization", "Basic " + btoa(validClientInfo.client_id + ":" + validClientInfo.client_secret)); params.set("example_url", typeof url === 'string' ? url : url.toString()); + params.set("example_metadata", metadata.authorization_endpoint); params.set("example_param", "example_value"); }, }); @@ -800,6 +820,7 @@ describe("OAuth Authorization", () => { expect(body.get("refresh_token")).toBe("refresh123"); expect(body.get("client_id")).toBeNull(); expect(body.get("example_url")).toBe("https://auth.example.com"); + expect(body.get("example_metadata")).toBe("https://auth.example.com/authorize"); expect(body.get("example_param")).toBe("example_value"); expect(body.get("client_secret")).toBeNull(); }); From 2568a21c9c4f3ca88289e7e998cfee43051bc43f Mon Sep 17 00:00:00 2001 From: Olivier Chafik Date: Thu, 3 Jul 2025 21:21:55 +0100 Subject: [PATCH 17/19] Fix optional metadata arg type signature in auth.test.ts --- src/client/auth.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/client/auth.test.ts b/src/client/auth.test.ts index 3285c37b..e3537aa7 100644 --- a/src/client/auth.test.ts +++ b/src/client/auth.test.ts @@ -794,10 +794,10 @@ describe("OAuth Authorization", () => { metadata: validMetadata, clientInformation: validClientInfo, refreshToken: "refresh123", - addClientAuthentication: (headers: Headers, params: URLSearchParams, url: string | URL, metadata: OAuthMetadata) => { + addClientAuthentication: (headers: Headers, params: URLSearchParams, url: string | URL, metadata?: OAuthMetadata) => { headers.set("Authorization", "Basic " + btoa(validClientInfo.client_id + ":" + validClientInfo.client_secret)); params.set("example_url", typeof url === 'string' ? url : url.toString()); - params.set("example_metadata", metadata.authorization_endpoint); + params.set("example_metadata", metadata?.authorization_endpoint ?? '?'); params.set("example_param", "example_value"); }, }); From 1600a566371bce65643bdddc21c14ef2a879d37a Mon Sep 17 00:00:00 2001 From: Olivier Chafik Date: Mon, 7 Jul 2025 13:32:19 +0100 Subject: [PATCH 18/19] test: fix lint --- src/client/auth.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/client/auth.test.ts b/src/client/auth.test.ts index e3537aa7..df2377e4 100644 --- a/src/client/auth.test.ts +++ b/src/client/auth.test.ts @@ -1,4 +1,3 @@ -import { mock, Mock } from 'node:test'; import { LATEST_PROTOCOL_VERSION } from '../types.js'; import { discoverOAuthMetadata, From 906005c78548ae23fa82b3145c6af5340668267c Mon Sep 17 00:00:00 2001 From: Olivier Chafik Date: Wed, 9 Jul 2025 18:22:28 +0100 Subject: [PATCH 19/19] apply suggested updates --- src/client/auth.test.ts | 40 +++++++++++++++++++++++++++++++++++++--- src/client/auth.ts | 38 +++++++++++++++++++------------------- 2 files changed, 56 insertions(+), 22 deletions(-) diff --git a/src/client/auth.test.ts b/src/client/auth.test.ts index 79edef9b..75cf20b9 100644 --- a/src/client/auth.test.ts +++ b/src/client/auth.test.ts @@ -10,7 +10,7 @@ import { auth, type OAuthClientProvider, } from "./auth.js"; -import { OAuthMetadata } from 'src/shared/auth.js'; +import { OAuthMetadata } from '../shared/auth.js'; // Mock fetch globally const mockFetch = jest.fn(); @@ -1615,6 +1615,11 @@ describe("OAuth Authorization", () => { token_endpoint_auth_methods_supported: ["none"], }; + const metadataWithAllBuiltinMethods = { + ...metadataWithBasicOnly, + token_endpoint_auth_methods_supported: ["client_secret_basic", "client_secret_post", "none"], + }; + it("uses HTTP Basic authentication when client_secret_basic is supported", async () => { mockFetch.mockResolvedValueOnce({ ok: true, @@ -1639,8 +1644,8 @@ describe("OAuth Authorization", () => { expect(authHeader).toBe(expected); const body = request.body as URLSearchParams; - expect(body.get("client_id")).toBeNull(); // should not be in body - expect(body.get("client_secret")).toBeNull(); // should not be in body + expect(body.get("client_id")).toBeNull(); + expect(body.get("client_secret")).toBeNull(); }); it("includes credentials in request body when client_secret_post is supported", async () => { @@ -1669,6 +1674,35 @@ describe("OAuth Authorization", () => { expect(body.get("client_secret")).toBe("secret123"); }); + it("it picks client_secret_basic when all builtin methods are supported", async () => { + mockFetch.mockResolvedValueOnce({ + ok: true, + status: 200, + json: async () => validTokens, + }); + + const tokens = await exchangeAuthorization("https://auth.example.com", { + metadata: metadataWithAllBuiltinMethods, + clientInformation: validClientInfo, + authorizationCode: "code123", + redirectUri: "http://localhost:3000/callback", + codeVerifier: "verifier123", + }); + + expect(tokens).toEqual(validTokens); + const request = mockFetch.mock.calls[0][1]; + + // Check Authorization header - should use Basic auth as it's the most secure + const authHeader = request.headers.get("Authorization"); + const expected = "Basic " + btoa("client123:secret123"); + expect(authHeader).toBe(expected); + + // Credentials should not be in body when using Basic auth + const body = request.body as URLSearchParams; + expect(body.get("client_id")).toBeNull(); + expect(body.get("client_secret")).toBeNull(); + }); + it("uses public client authentication when none method is specified", async () => { mockFetch.mockResolvedValueOnce({ ok: true, diff --git a/src/client/auth.ts b/src/client/auth.ts index 4fd57213..8e32dc2f 100644 --- a/src/client/auth.ts +++ b/src/client/auth.ts @@ -86,9 +86,10 @@ export interface OAuthClientProvider { * - Adding custom headers for proprietary authentication schemes * - Implementing client assertion-based authentication (e.g., JWT bearer tokens) * - * @param url - The token endpoint URL being called * @param headers - The request headers (can be modified to add authentication) * @param params - The request body parameters (can be modified to add credentials) + * @param url - The token endpoint URL being called + * @param metadata - Optional OAuth metadata for the server, which may include supported authentication methods */ addClientAuthentication?(headers: Headers, params: URLSearchParams, url: string | URL, metadata?: OAuthMetadata): void | Promise; @@ -110,6 +111,8 @@ export class UnauthorizedError extends Error { } } +type ClientAuthMethod = 'client_secret_basic' | 'client_secret_post' | 'none'; + /** * Determines the best client authentication method to use based on server support and client configuration. * @@ -125,8 +128,8 @@ export class UnauthorizedError extends Error { function selectClientAuthMethod( clientInformation: OAuthClientInformation, supportedMethods: string[] -): string { - const hasClientSecret = !!clientInformation.client_secret; +): ClientAuthMethod { + const hasClientSecret = clientInformation.client_secret !== undefined; // If server doesn't specify supported methods, use RFC 6749 defaults if (supportedMethods.length === 0) { @@ -165,29 +168,26 @@ function selectClientAuthMethod( * @throws {Error} When required credentials are missing */ function applyClientAuthentication( - method: string, + method: ClientAuthMethod, clientInformation: OAuthClientInformation, headers: Headers, params: URLSearchParams ): void { const { client_id, client_secret } = clientInformation; - if (method === "client_secret_basic") { - applyBasicAuth(client_id, client_secret, headers); - return; - } - - if (method === "client_secret_post") { - applyPostAuth(client_id, client_secret, params); - return; - } - - if (method === "none") { - applyPublicAuth(client_id, params); - return; + switch (method) { + case "client_secret_basic": + applyBasicAuth(client_id, client_secret, headers); + return; + case "client_secret_post": + applyPostAuth(client_id, client_secret, params); + return; + case "none": + applyPublicAuth(client_id, params); + return; + default: + throw new Error(`Unsupported client authentication method: ${method}`); } - - throw new Error(`Unsupported client authentication method: ${method}`); } /**