From e5091ee1e64fcbe69bdeb4ed86e774e32ca85d7d Mon Sep 17 00:00:00 2001 From: Gustavo Valverde Date: Mon, 6 Apr 2026 15:14:39 +0100 Subject: [PATCH] fix(oauth-provider): scope loss on PAR, loopback redirect matching, DCR skip_consent (#8632) --- .../fix-oauth-provider-par-loopback-dcr.md | 10 + packages/oauth-provider/src/authorize.test.ts | 116 +++++++ packages/oauth-provider/src/authorize.ts | 74 ++++- packages/oauth-provider/src/oauth.ts | 14 +- packages/oauth-provider/src/register.test.ts | 40 +++ packages/oauth-provider/src/register.ts | 8 + packages/oauth-provider/src/token.test.ts | 283 ++++++++++++++++++ packages/oauth-provider/src/token.ts | 2 +- packages/oauth-provider/src/types/index.ts | 21 +- 9 files changed, 555 insertions(+), 13 deletions(-) create mode 100644 .changeset/fix-oauth-provider-par-loopback-dcr.md diff --git a/.changeset/fix-oauth-provider-par-loopback-dcr.md b/.changeset/fix-oauth-provider-par-loopback-dcr.md new file mode 100644 index 0000000000..0eb29ecb73 --- /dev/null +++ b/.changeset/fix-oauth-provider-par-loopback-dcr.md @@ -0,0 +1,10 @@ +--- +"@better-auth/oauth-provider": patch +--- + +fix PAR scope loss, loopback redirect matching, and DCR skip_consent + +- **PAR (RFC 9126)**: resolve `request_uri` into stored params before processing; discard front-channel URL params per §4 to prevent prompt/scope injection +- **Loopback (RFC 8252 §7.3)**: port-agnostic redirect URI matching for `127.0.0.1` and `[::1]`; scheme, host, path, and query must still match +- **DCR**: accept `skip_consent` in schema but reject it during dynamic registration to prevent privilege escalation +- **Serialization**: fix `oAuthState` query serialization and preserve non-string values like `max_age` diff --git a/packages/oauth-provider/src/authorize.test.ts b/packages/oauth-provider/src/authorize.test.ts index d0cb415b19..045b4ff432 100644 --- a/packages/oauth-provider/src/authorize.test.ts +++ b/packages/oauth-provider/src/authorize.test.ts @@ -181,6 +181,122 @@ describe("oauth authorize - unauthenticated", async () => { }); }); +describe("oauth authorize - request_uri resolution", async () => { + const authServerBaseUrl = "http://localhost:3000"; + const rpBaseUrl = "http://localhost:5000"; + const providerId = "test"; + const redirectUri = `${rpBaseUrl}/api/auth/oauth2/callback/${providerId}`; + const requestUri = "urn:better-auth:par:test"; + + const { auth, signInWithTestUser, customFetchImpl } = await getTestInstance({ + baseURL: authServerBaseUrl, + plugins: [ + oauthProvider({ + loginPage: "/login", + consentPage: "/consent", + requestUriResolver: async ({ requestUri: receivedRequestUri }) => { + if (receivedRequestUri !== requestUri) { + return null; + } + + return { + response_type: "code", + redirect_uri: redirectUri, + scope: "openid", + state: "par-state", + code_challenge: "a".repeat(43), + code_challenge_method: "S256", + }; + }, + silenceWarnings: { + oauthAuthServerConfig: true, + openidConfig: true, + }, + }), + jwt(), + ], + }); + const { headers } = await signInWithTestUser(); + const unauthenticatedClient = createAuthClient({ + plugins: [oauthProviderClient()], + baseURL: authServerBaseUrl, + fetchOptions: { + customFetchImpl, + }, + }); + + let oauthClient: OAuthClient | null; + beforeAll(async () => { + const response = await auth.api.adminCreateOAuthClient({ + headers, + body: { + redirect_uris: [redirectUri], + skip_consent: true, + }, + }); + expect(response?.client_id).toBeDefined(); + oauthClient = response; + }); + + it("should sign the resolved PAR parameters for the login redirect", async () => { + if (!oauthClient?.client_id) { + throw Error("beforeAll not run properly"); + } + + const authUrl = new URL(`${authServerBaseUrl}/api/auth/oauth2/authorize`); + authUrl.searchParams.set("client_id", oauthClient.client_id); + authUrl.searchParams.set("request_uri", requestUri); + + let loginRedirectUrl = ""; + await unauthenticatedClient.$fetch(authUrl.toString(), { + onError(context) { + loginRedirectUrl = context.response.headers.get("Location") || ""; + }, + }); + + expect(loginRedirectUrl).toContain("/login"); + expect(loginRedirectUrl).toContain("response_type=code"); + expect(loginRedirectUrl).toContain(`client_id=${oauthClient.client_id}`); + expect(loginRedirectUrl).toContain("scope=openid"); + expect(loginRedirectUrl).toContain( + `redirect_uri=${encodeURIComponent(redirectUri)}`, + ); + expect(loginRedirectUrl).toContain("state=par-state"); + expect(loginRedirectUrl).not.toContain("request_uri="); + }); + + /** + * RFC 9126 §4: params must come from the stored request, not the URL. + * Extra URL params like prompt or scope must not leak into the signed redirect. + */ + it("should discard front-channel params not in the stored PAR request", async () => { + if (!oauthClient?.client_id) { + throw Error("beforeAll not run properly"); + } + + const authUrl = new URL(`${authServerBaseUrl}/api/auth/oauth2/authorize`); + authUrl.searchParams.set("client_id", oauthClient.client_id); + authUrl.searchParams.set("request_uri", requestUri); + // These params are NOT in the PAR payload — must be discarded + authUrl.searchParams.set("prompt", "none"); + authUrl.searchParams.set("scope", "openid profile admin"); + + let loginRedirectUrl = ""; + await unauthenticatedClient.$fetch(authUrl.toString(), { + onError(context) { + loginRedirectUrl = context.response.headers.get("Location") || ""; + }, + }); + + expect(loginRedirectUrl).toContain("/login"); + // PAR-resolved scope must win, not the URL-injected one + expect(loginRedirectUrl).toContain("scope=openid"); + expect(loginRedirectUrl).not.toContain("admin"); + // prompt=none was not in the PAR payload — must not appear + expect(loginRedirectUrl).not.toContain("prompt=none"); + }); +}); + describe("oauth authorize - authenticated", async () => { const authServerBaseUrl = "http://localhost:3000"; const rpBaseUrl = "http://localhost:5000"; diff --git a/packages/oauth-provider/src/authorize.ts b/packages/oauth-provider/src/authorize.ts index ec97a07ae2..2f4b5a698b 100644 --- a/packages/oauth-provider/src/authorize.ts +++ b/packages/oauth-provider/src/authorize.ts @@ -168,10 +168,41 @@ export async function authorizeEndpoint( }); } - // Check request - const query: OAuthAuthorizationQuery = ctx.query; + // Resolve request_uri (PAR) before processing + let query: OAuthAuthorizationQuery = ctx.query; + if (query.request_uri) { + if (!opts.requestUriResolver) { + return handleRedirect( + ctx, + getErrorURL(ctx, "invalid_request_uri", "request_uri not supported"), + ); + } + const resolvedParams = await opts.requestUriResolver({ + requestUri: query.request_uri, + clientId: query.client_id ?? "", + ctx, + }); + if (!resolvedParams) { + return handleRedirect( + ctx, + getErrorURL( + ctx, + "invalid_request_uri", + "request_uri is invalid or expired", + ), + ); + } + // RFC 9126 §4: all params come from the stored request, not the URL. + // Only client_id is carried from the authorization URL. + const urlClientId = query.client_id; + query = resolvedParams as unknown as OAuthAuthorizationQuery; + if (urlClientId) { + query.client_id = urlClientId; + } + } + ctx.query = query; await oAuthState.set({ - query: query.toString(), + query: serializeAuthorizationQuery(query).toString(), }); if (!query.client_id) { @@ -229,9 +260,24 @@ export async function authorizeEndpoint( ); } - const redirectUri = client.redirectUris?.find( - (url) => url === query.redirect_uri, - ); + const redirectUri = client.redirectUris?.find((url) => { + if (url === query.redirect_uri) return true; + try { + const registered = new URL(url); + const requested = new URL(query.redirect_uri); + // RFC 8252 §7.3: loopback IPs match on scheme+host+path+query, ignoring port + if ( + (registered.hostname === "127.0.0.1" || + registered.hostname === "[::1]") && + registered.hostname === requested.hostname && + registered.pathname === requested.pathname && + registered.protocol === requested.protocol && + registered.search === requested.search + ) + return true; + } catch {} + return false; + }); if (!redirectUri || !query.redirect_uri) { return handleRedirect( ctx, @@ -483,6 +529,16 @@ export async function authorizeEndpoint( }); } +function serializeAuthorizationQuery(query: OAuthAuthorizationQuery) { + const params = new URLSearchParams(); + for (const [key, value] of Object.entries(query)) { + if (value != null) { + params.set(key, String(value)); + } + } + return params; +} + async function redirectWithAuthorizationCode( ctx: GenericEndpointContext, opts: OAuthOptions, @@ -505,7 +561,7 @@ async function redirectWithAuthorizationCode( expiresAt: new Date(exp * 1000), value: JSON.stringify({ type: "authorization_code", - query: ctx.query, + query: verificationValue.query, userId: verificationValue.userId, sessionId: verificationValue?.sessionId, referenceId: verificationValue.referenceId, @@ -561,7 +617,9 @@ async function signParams( // Add expiration to query parameters const iat = Math.floor(Date.now() / 1000); const exp = iat + (opts.codeExpiresIn ?? 600); - const params = new URLSearchParams(ctx.query); + const params = serializeAuthorizationQuery( + ctx.query as OAuthAuthorizationQuery, + ); params.set("exp", String(exp)); const signature = await makeSignature(params.toString(), ctx.context.secret); diff --git a/packages/oauth-provider/src/oauth.ts b/packages/oauth-provider/src/oauth.ts index b4d5a53bc7..4d2708b49a 100644 --- a/packages/oauth-provider/src/oauth.ts +++ b/packages/oauth-provider/src/oauth.ts @@ -372,11 +372,12 @@ export const oauthProvider = >(options: O) => { { method: "GET", query: z.object({ - response_type: z.enum(["code"]), + response_type: z.enum(["code"]).optional(), client_id: z.string(), redirect_uri: SafeUrlSchema.optional(), scope: z.string().optional(), state: z.string().optional(), + request_uri: z.string().optional(), code_challenge: z.string().optional(), code_challenge_method: z.enum(["S256"]).optional(), nonce: z.string().optional(), @@ -399,7 +400,7 @@ export const oauthProvider = >(options: O) => { { name: "response_type", in: "query", - required: true, + required: false, schema: { type: "string" }, description: "OAuth2 response type (e.g., 'code')", }, @@ -431,6 +432,14 @@ export const oauthProvider = >(options: O) => { schema: { type: "string" }, description: "OAuth2 state parameter", }, + { + name: "request_uri", + in: "query", + required: false, + schema: { type: "string" }, + description: + "Pushed Authorization Request URI referencing stored parameters", + }, { name: "code_challenge", in: "query", @@ -1168,6 +1177,7 @@ export const oauthProvider = >(options: O) => { .optional(), type: z.enum(["web", "native", "user-agent-based"]).optional(), subject_type: z.enum(["public", "pairwise"]).optional(), + skip_consent: z.boolean().optional(), }), metadata: { openapi: { diff --git a/packages/oauth-provider/src/register.test.ts b/packages/oauth-provider/src/register.test.ts index 0c9a47a5b8..0c5876d3dd 100644 --- a/packages/oauth-provider/src/register.test.ts +++ b/packages/oauth-provider/src/register.test.ts @@ -363,3 +363,43 @@ describe("oauth register - organization", async () => { expect(client.data?.reference_id).toBe(org.id); }); }); + +describe("oauth register - skip_consent blocked", async () => { + const baseUrl = "http://localhost:3000"; + const { signInWithTestUser, customFetchImpl } = await getTestInstance({ + baseURL: baseUrl, + plugins: [ + oauthProvider({ + loginPage: "/login", + consentPage: "/consent", + allowDynamicClientRegistration: true, + silenceWarnings: { + oauthAuthServerConfig: true, + openidConfig: true, + }, + }), + jwt(), + ], + }); + const { headers } = await signInWithTestUser(); + const serverClient = createAuthClient({ + plugins: [oauthProviderClient()], + baseURL: baseUrl, + fetchOptions: { customFetchImpl, headers }, + }); + + it("should reject skip_consent during dynamic registration", async () => { + const res = await serverClient.oauth2.register({ + redirect_uris: ["http://localhost:5000/callback"], + skip_consent: true, + }); + expect(res.error?.status).toBe(400); + }); + + it("should allow registration without skip_consent", async () => { + const res = await serverClient.oauth2.register({ + redirect_uris: ["http://localhost:5000/callback"], + }); + expect(res.data?.client_id).toBeDefined(); + }); +}); diff --git a/packages/oauth-provider/src/register.ts b/packages/oauth-provider/src/register.ts index f5e048657a..944fa0ddd7 100644 --- a/packages/oauth-provider/src/register.ts +++ b/packages/oauth-provider/src/register.ts @@ -174,6 +174,14 @@ export async function checkOAuthClient( error_description: `pkce is required for registered clients.`, }); } + + if (settings?.isRegister && client.skip_consent) { + throw new APIError("BAD_REQUEST", { + error: "invalid_client_metadata", + error_description: + "skip_consent cannot be set during dynamic client registration", + }); + } } export async function createOAuthClientEndpoint( diff --git a/packages/oauth-provider/src/token.test.ts b/packages/oauth-provider/src/token.test.ts index 77c6d6af1f..bbf63f8615 100644 --- a/packages/oauth-provider/src/token.test.ts +++ b/packages/oauth-provider/src/token.test.ts @@ -1952,3 +1952,286 @@ describe("id token claim override security", async () => { expect(claims.sid).not.toBe("evil-sid"); }); }); + +describe("loopback redirect URI matching", async () => { + const authServerBaseUrl = "http://localhost:3000"; + const rpBaseUrl = "http://localhost:5000"; + const { auth, signInWithTestUser, customFetchImpl } = await getTestInstance({ + baseURL: authServerBaseUrl, + plugins: [ + jwt({ jwt: { issuer: authServerBaseUrl } }), + oauthProvider({ + loginPage: "/login", + consentPage: "/consent", + silenceWarnings: { + oauthAuthServerConfig: true, + openidConfig: true, + }, + }), + ], + }); + + const { headers } = await signInWithTestUser(); + const client = createAuthClient({ + plugins: [oauthProviderClient(), jwtClient()], + baseURL: authServerBaseUrl, + fetchOptions: { customFetchImpl, headers }, + }); + + const providerId = "test"; + const state = "123"; + + it("127.0.0.1 with different ports should succeed", async ({ expect }) => { + const registeredUri = "http://127.0.0.1:8080/callback"; + const requestedUri = "http://127.0.0.1:9090/callback"; + + const oauthClient = await auth.api.adminCreateOAuthClient({ + headers, + body: { redirect_uris: [registeredUri], skip_consent: true }, + }); + + const codeVerifier = generateRandomString(32); + const url = await createAuthorizationURL({ + id: providerId, + options: { + clientId: oauthClient!.client_id!, + clientSecret: oauthClient!.client_secret!, + redirectURI: requestedUri, + }, + redirectURI: "", + authorizationEndpoint: `${authServerBaseUrl}/api/auth/oauth2/authorize`, + state, + scopes: ["openid"], + codeVerifier, + }); + + let callbackRedirectUrl = ""; + await client.$fetch(url.toString(), { + onError(context) { + callbackRedirectUrl = context.response.headers.get("Location") || ""; + }, + }); + expect(callbackRedirectUrl).toContain("code="); + + const code = new URL(callbackRedirectUrl).searchParams.get("code")!; + const { body, headers: reqHeaders } = createAuthorizationCodeRequest({ + code, + codeVerifier, + redirectURI: requestedUri, + options: { + clientId: oauthClient!.client_id!, + clientSecret: oauthClient!.client_secret!, + redirectURI: requestedUri, + }, + }); + + const tokens = await client.$fetch<{ access_token?: string }>( + "/oauth2/token", + { method: "POST", body, headers: reqHeaders }, + ); + expect(tokens.data?.access_token).toBeDefined(); + }); + + it("[::1] with different ports should succeed", async ({ expect }) => { + const registeredUri = "http://[::1]:8080/callback"; + const requestedUri = "http://[::1]:3000/callback"; + + const oauthClient = await auth.api.adminCreateOAuthClient({ + headers, + body: { redirect_uris: [registeredUri], skip_consent: true }, + }); + + const codeVerifier = generateRandomString(32); + const url = await createAuthorizationURL({ + id: providerId, + options: { + clientId: oauthClient!.client_id!, + clientSecret: oauthClient!.client_secret!, + redirectURI: requestedUri, + }, + redirectURI: "", + authorizationEndpoint: `${authServerBaseUrl}/api/auth/oauth2/authorize`, + state, + scopes: ["openid"], + codeVerifier, + }); + + let callbackRedirectUrl = ""; + await client.$fetch(url.toString(), { + onError(context) { + callbackRedirectUrl = context.response.headers.get("Location") || ""; + }, + }); + expect(callbackRedirectUrl).toContain("code="); + + const code = new URL(callbackRedirectUrl).searchParams.get("code")!; + const { body, headers: reqHeaders } = createAuthorizationCodeRequest({ + code, + codeVerifier, + redirectURI: requestedUri, + options: { + clientId: oauthClient!.client_id!, + clientSecret: oauthClient!.client_secret!, + redirectURI: requestedUri, + }, + }); + + const tokens = await client.$fetch<{ access_token?: string }>( + "/oauth2/token", + { method: "POST", body, headers: reqHeaders }, + ); + expect(tokens.data?.access_token).toBeDefined(); + }); + + it("non-loopback with different ports should be rejected", async ({ + expect, + }) => { + const registeredUri = `${rpBaseUrl}/api/auth/oauth2/callback/${providerId}`; + const requestedUri = "http://localhost:9999/api/auth/oauth2/callback/test"; + + const oauthClient = await auth.api.adminCreateOAuthClient({ + headers, + body: { redirect_uris: [registeredUri], skip_consent: true }, + }); + + const codeVerifier = generateRandomString(32); + const url = await createAuthorizationURL({ + id: providerId, + options: { + clientId: oauthClient!.client_id!, + clientSecret: oauthClient!.client_secret!, + redirectURI: requestedUri, + }, + redirectURI: "", + authorizationEndpoint: `${authServerBaseUrl}/api/auth/oauth2/authorize`, + state, + scopes: ["openid"], + codeVerifier, + }); + + let callbackRedirectUrl = ""; + await client.$fetch(url.toString(), { + onError(context) { + callbackRedirectUrl = context.response.headers.get("Location") || ""; + }, + }); + expect(callbackRedirectUrl).toContain("invalid_redirect"); + expect(callbackRedirectUrl).not.toContain("code="); + }); + + it("loopback with different path should be rejected", async ({ expect }) => { + const registeredUri = "http://127.0.0.1:8080/callback"; + const requestedUri = "http://127.0.0.1:8080/other-path"; + + const oauthClient = await auth.api.adminCreateOAuthClient({ + headers, + body: { redirect_uris: [registeredUri], skip_consent: true }, + }); + + const codeVerifier = generateRandomString(32); + const url = await createAuthorizationURL({ + id: providerId, + options: { + clientId: oauthClient!.client_id!, + clientSecret: oauthClient!.client_secret!, + redirectURI: requestedUri, + }, + redirectURI: "", + authorizationEndpoint: `${authServerBaseUrl}/api/auth/oauth2/authorize`, + state, + scopes: ["openid"], + codeVerifier, + }); + + let callbackRedirectUrl = ""; + await client.$fetch(url.toString(), { + onError(context) { + callbackRedirectUrl = context.response.headers.get("Location") || ""; + }, + }); + expect(callbackRedirectUrl).toContain("invalid_redirect"); + expect(callbackRedirectUrl).not.toContain("code="); + }); +}); + +describe("scope preservation through authorization code flow", async () => { + const authServerBaseUrl = "http://localhost:3000"; + const rpBaseUrl = "http://localhost:5000"; + const { auth, signInWithTestUser, customFetchImpl } = await getTestInstance({ + baseURL: authServerBaseUrl, + plugins: [ + jwt({ jwt: { issuer: authServerBaseUrl } }), + oauthProvider({ + loginPage: "/login", + consentPage: "/consent", + silenceWarnings: { + oauthAuthServerConfig: true, + openidConfig: true, + }, + }), + ], + }); + + const { headers } = await signInWithTestUser(); + const client = createAuthClient({ + plugins: [oauthProviderClient(), jwtClient()], + baseURL: authServerBaseUrl, + fetchOptions: { customFetchImpl, headers }, + }); + + const providerId = "test"; + const redirectUri = `${rpBaseUrl}/api/auth/oauth2/callback/${providerId}`; + const state = "123"; + + it("scopes from authorization request should survive into token response", async ({ + expect, + }) => { + const oauthClient = await auth.api.adminCreateOAuthClient({ + headers, + body: { redirect_uris: [redirectUri], skip_consent: true }, + }); + + const requestedScopes = ["openid", "profile", "email"]; + const codeVerifier = generateRandomString(32); + const url = await createAuthorizationURL({ + id: providerId, + options: { + clientId: oauthClient!.client_id!, + clientSecret: oauthClient!.client_secret!, + redirectURI: redirectUri, + }, + redirectURI: "", + authorizationEndpoint: `${authServerBaseUrl}/api/auth/oauth2/authorize`, + state, + scopes: requestedScopes, + codeVerifier, + }); + + let callbackRedirectUrl = ""; + await client.$fetch(url.toString(), { + onError(context) { + callbackRedirectUrl = context.response.headers.get("Location") || ""; + }, + }); + expect(callbackRedirectUrl).toContain("code="); + + const code = new URL(callbackRedirectUrl).searchParams.get("code")!; + const { body, headers: reqHeaders } = createAuthorizationCodeRequest({ + code, + codeVerifier, + redirectURI: redirectUri, + options: { + clientId: oauthClient!.client_id!, + clientSecret: oauthClient!.client_secret!, + redirectURI: redirectUri, + }, + }); + + const tokens = await client.$fetch<{ scope?: string }>("/oauth2/token", { + method: "POST", + body, + headers: reqHeaders, + }); + expect(tokens.data?.scope).toBe(requestedScopes.join(" ")); + }); +}); diff --git a/packages/oauth-provider/src/token.ts b/packages/oauth-provider/src/token.ts index 82724f3143..249800fa28 100644 --- a/packages/oauth-provider/src/token.ts +++ b/packages/oauth-provider/src/token.ts @@ -568,7 +568,7 @@ async function checkVerificationValue( verificationValue.query?.redirect_uri !== redirect_uri ) { throw new APIError("BAD_REQUEST", { - error_description: "missing verification redirect_uri", + error_description: "redirect_uri mismatch", error: "invalid_request", }); } diff --git a/packages/oauth-provider/src/types/index.ts b/packages/oauth-provider/src/types/index.ts index 3c6e5a7467..cad67d7c25 100644 --- a/packages/oauth-provider/src/types/index.ts +++ b/packages/oauth-provider/src/types/index.ts @@ -1,4 +1,4 @@ -import type { LiteralString } from "@better-auth/core"; +import type { GenericEndpointContext, LiteralString } from "@better-auth/core"; import type { InferOptionSchema, Session, User } from "better-auth/types"; import type { JWTPayload } from "jose"; import type { schema } from "../schema"; @@ -677,15 +677,32 @@ export interface OAuthOptions< * @see https://openid.net/specs/openid-connect-core-1_0.html#PairwiseAlg */ pairwiseSecret?: string; + /** + * Resolves a `request_uri` at the authorize endpoint (PAR support). + * + * When the authorize endpoint receives a `request_uri` parameter, this callback + * resolves it to the original authorization parameters. Return null if the URI + * is invalid or expired. + */ + requestUriResolver?: (input: { + requestUri: string; + clientId: string; + ctx: GenericEndpointContext; + }) => Promise | null>; } export interface OAuthAuthorizationQuery { /** * The response type. * - "code": authorization code flow. + * Optional in the query when using request_uri (PAR) — resolved from stored params. */ // NEVER SUPPORT "token" or "id_token" - depreciated in oAuth2.1 - response_type: "code"; + response_type?: "code"; + /** + * PAR request_uri. When present, other params are resolved from the stored request. + */ + request_uri?: string; /** * The redirect URI for the client. Must be one of the registered redirect URLs for the client. */