From 9fea0af99800647c7e9d0e7fca3fc42469cc8258 Mon Sep 17 00:00:00 2001 From: Justin Spahr-Summers Date: Tue, 11 Feb 2025 15:59:16 +0000 Subject: [PATCH] Correctly provide client ID (+ secret) to every auth step --- src/client/auth.test.ts | 37 ++++++++++++++- src/client/auth.ts | 101 +++++++++++++++++++++++----------------- 2 files changed, 95 insertions(+), 43 deletions(-) diff --git a/src/client/auth.test.ts b/src/client/auth.test.ts index 39be8216..7332ae36 100644 --- a/src/client/auth.test.ts +++ b/src/client/auth.test.ts @@ -96,10 +96,18 @@ describe("OAuth Authorization", () => { code_challenge_methods_supported: ["S256"], }; + const validClientInfo = { + client_id: "client123", + client_secret: "secret123", + redirect_uris: ["http://localhost:3000/callback"], + client_name: "Test Client", + }; + it("generates authorization URL with PKCE challenge", async () => { const { authorizationUrl, codeVerifier } = await startAuthorization( "https://auth.example.com", { + clientInformation: validClientInfo, redirectUrl: "http://localhost:3000/callback", } ); @@ -123,6 +131,7 @@ describe("OAuth Authorization", () => { "https://auth.example.com", { metadata: validMetadata, + clientInformation: validClientInfo, redirectUrl: "http://localhost:3000/callback", } ); @@ -141,6 +150,7 @@ describe("OAuth Authorization", () => { await expect( startAuthorization("https://auth.example.com", { metadata, + clientInformation: validClientInfo, redirectUrl: "http://localhost:3000/callback", }) ).rejects.toThrow(/does not support response type/); @@ -156,6 +166,7 @@ describe("OAuth Authorization", () => { await expect( startAuthorization("https://auth.example.com", { metadata, + clientInformation: validClientInfo, redirectUrl: "http://localhost:3000/callback", }) ).rejects.toThrow(/does not support code challenge method/); @@ -170,6 +181,13 @@ describe("OAuth Authorization", () => { refresh_token: "refresh123", }; + const validClientInfo = { + client_id: "client123", + client_secret: "secret123", + redirect_uris: ["http://localhost:3000/callback"], + client_name: "Test Client", + }; + it("exchanges code for tokens", async () => { mockFetch.mockResolvedValueOnce({ ok: true, @@ -178,6 +196,7 @@ describe("OAuth Authorization", () => { }); const tokens = await exchangeAuthorization("https://auth.example.com", { + clientInformation: validClientInfo, authorizationCode: "code123", codeVerifier: "verifier123", }); @@ -199,6 +218,8 @@ 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_secret")).toBe("secret123"); }); it("validates token response schema", async () => { @@ -213,6 +234,7 @@ describe("OAuth Authorization", () => { await expect( exchangeAuthorization("https://auth.example.com", { + clientInformation: validClientInfo, authorizationCode: "code123", codeVerifier: "verifier123", }) @@ -227,6 +249,7 @@ describe("OAuth Authorization", () => { await expect( exchangeAuthorization("https://auth.example.com", { + clientInformation: validClientInfo, authorizationCode: "code123", codeVerifier: "verifier123", }) @@ -242,6 +265,13 @@ describe("OAuth Authorization", () => { refresh_token: "newrefresh123", }; + const validClientInfo = { + client_id: "client123", + client_secret: "secret123", + redirect_uris: ["http://localhost:3000/callback"], + client_name: "Test Client", + }; + it("exchanges refresh token for new tokens", async () => { mockFetch.mockResolvedValueOnce({ ok: true, @@ -250,6 +280,7 @@ describe("OAuth Authorization", () => { }); const tokens = await refreshAuthorization("https://auth.example.com", { + clientInformation: validClientInfo, refreshToken: "refresh123", }); @@ -269,6 +300,8 @@ 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_secret")).toBe("secret123"); }); it("validates token response schema", async () => { @@ -283,6 +316,7 @@ describe("OAuth Authorization", () => { await expect( refreshAuthorization("https://auth.example.com", { + clientInformation: validClientInfo, refreshToken: "refresh123", }) ).rejects.toThrow(); @@ -296,9 +330,10 @@ describe("OAuth Authorization", () => { await expect( refreshAuthorization("https://auth.example.com", { + clientInformation: validClientInfo, refreshToken: "refresh123", }) - ).rejects.toThrow("Token exchange failed"); + ).rejects.toThrow("Token refresh failed"); }); }); diff --git a/src/client/auth.ts b/src/client/auth.ts index b5b043f6..6fef1439 100644 --- a/src/client/auth.ts +++ b/src/client/auth.ts @@ -89,9 +89,6 @@ export type OAuthClientInformation = z.infer; + clientInformation(): OAuthClientInformation | undefined | Promise; /** * If implemented, this permits the OAuth client to dynamically register with * the server. Client information saved this way should later be read via * `clientInformation()`. * - * This method is not required to be implemented if redirecting to - * `localhost`, or if client information is statically known (e.g., - * pre-registered). + * This method is not required to be implemented if client information is + * statically known (e.g., pre-registered). */ saveClientInformation?(clientInformation: OAuthClientInformation): void | Promise; @@ -164,31 +158,22 @@ export async function auth( const metadata = await discoverOAuthMetadata(serverUrl); // Handle client registration if needed - const hostname = new URL(provider.redirectUrl).hostname; - if (hostname !== "localhost" && hostname !== "127.0.0.1") { - if (!provider.clientInformation) { - throw new Error("OAuth client information is required when not redirecting to localhost") + let clientInformation = await Promise.resolve(provider.clientInformation()); + if (!clientInformation) { + if (authorizationCode !== undefined) { + throw new Error("Existing OAuth client information is required when exchanging an authorization code"); } - let clientInformation = await Promise.resolve(provider.clientInformation()); - if (!clientInformation) { - if (authorizationCode !== undefined) { - throw new Error("Existing OAuth client information is required when exchanging an authorization code"); - } - - if (!provider.saveClientInformation) { - throw new Error("OAuth client information must be saveable when not provided and not redirecting to localhost"); - } - - clientInformation = await registerClient(serverUrl, { - metadata, - clientMetadata: provider.clientMetadata, - }); - - await provider.saveClientInformation(clientInformation); + if (!provider.saveClientInformation) { + throw new Error("OAuth client information must be saveable for dynamic registration"); } - // TODO: Send clientInformation into auth flow + clientInformation = await registerClient(serverUrl, { + metadata, + clientMetadata: provider.clientMetadata, + }); + + await provider.saveClientInformation(clientInformation); } // Exchange authorization code for tokens @@ -196,6 +181,7 @@ export async function auth( const codeVerifier = await provider.codeVerifier(); const tokens = await exchangeAuthorization(serverUrl, { metadata, + clientInformation, authorizationCode, codeVerifier, }); @@ -212,6 +198,7 @@ export async function auth( // Attempt to refresh the token const newTokens = await refreshAuthorization(serverUrl, { metadata, + clientInformation, refreshToken: tokens.refresh_token, }); @@ -223,7 +210,12 @@ export async function auth( } // Start new authorization flow - const { authorizationUrl, codeVerifier } = await startAuthorization(serverUrl, { metadata, redirectUrl: provider.redirectUrl }); + const { authorizationUrl, codeVerifier } = await startAuthorization(serverUrl, { + metadata, + clientInformation, + redirectUrl: provider.redirectUrl + }); + await provider.saveCodeVerifier(codeVerifier); await provider.redirectToAuthorization(authorizationUrl); return "REDIRECT"; @@ -260,8 +252,13 @@ export async function startAuthorization( serverUrl: string | URL, { metadata, + clientInformation, redirectUrl, - }: { metadata?: OAuthMetadata; redirectUrl: string | URL }, + }: { + metadata?: OAuthMetadata; + clientInformation: OAuthClientInformation; + redirectUrl: string | URL; + }, ): Promise<{ authorizationUrl: URL; codeVerifier: string }> { const responseType = "code"; const codeChallengeMethod = "S256"; @@ -294,6 +291,7 @@ export async function startAuthorization( const codeChallenge = challenge.code_challenge; authorizationUrl.searchParams.set("response_type", responseType); + authorizationUrl.searchParams.set("client_id", clientInformation.client_id); authorizationUrl.searchParams.set("code_challenge", codeChallenge); authorizationUrl.searchParams.set( "code_challenge_method", @@ -311,10 +309,12 @@ export async function exchangeAuthorization( serverUrl: string | URL, { metadata, + clientInformation, authorizationCode, codeVerifier, }: { metadata?: OAuthMetadata; + clientInformation: OAuthClientInformation; authorizationCode: string; codeVerifier: string; }, @@ -338,16 +338,23 @@ export async function exchangeAuthorization( } // Exchange code for tokens + const params = new URLSearchParams({ + grant_type: grantType, + client_id: clientInformation.client_id, + code: authorizationCode, + code_verifier: codeVerifier, + }); + + 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", }, - body: new URLSearchParams({ - grant_type: grantType, - code: authorizationCode, - code_verifier: codeVerifier, - }), + body: params, }); if (!response.ok) { @@ -364,9 +371,11 @@ export async function refreshAuthorization( serverUrl: string | URL, { metadata, + clientInformation, refreshToken, }: { metadata?: OAuthMetadata; + clientInformation: OAuthClientInformation; refreshToken: string; }, ): Promise { @@ -388,19 +397,27 @@ export async function refreshAuthorization( tokenUrl = new URL("/token", serverUrl); } + // 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); + } + const response = await fetch(tokenUrl, { method: "POST", headers: { "Content-Type": "application/x-www-form-urlencoded", }, - body: new URLSearchParams({ - grant_type: grantType, - refresh_token: refreshToken, - }), + body: params, }); if (!response.ok) { - throw new Error(`Token exchange failed: HTTP ${response.status}`); + throw new Error(`Token refresh failed: HTTP ${response.status}`); } return OAuthTokensSchema.parse(await response.json());