diff --git a/.changeset/oauth-revoke-introspect-token-lookup.md b/.changeset/oauth-revoke-introspect-token-lookup.md new file mode 100644 index 0000000000..3c6d14c7e7 --- /dev/null +++ b/.changeset/oauth-revoke-introspect-token-lookup.md @@ -0,0 +1,15 @@ +--- +"@better-auth/oauth-provider": minor +--- + +`/oauth2/revoke` and `/oauth2/introspect` now follow RFC 7009 and RFC 7662 when they look up the presented token. Check your clients and tests for these changes: + +- `/oauth2/revoke` returns `200` for an unknown or already revoked token. Before, it returned `400 invalid_request`, including for a second revoke of the same refresh token and for a revoke that raced a refresh. +- A token sent with the wrong `token_type_hint` is found. Before, `/oauth2/revoke` returned `400` and left the token active, and `/oauth2/introspect` reported `{ "active": false }` for a valid token. +- Revoking a token issued to a different client returns `400 invalid_request` when the caller is a confidential client. Before, it returned `200` without revoking anything, or `400 unsupported_token_type` for a JWT access token. A public client gets `200`, and the token stays valid. +- A JWT-shaped token that fails verification (for example `alg: none`, a forged signature, or an unknown `kid`) is treated as an invalid token: `200` from revoke, `{ "active": false }` from introspect, and `401 invalid_token` from userinfo. Before, all three returned `500`. +- At `/oauth2/token`, a refresh token without the configured `prefix.refreshToken` now gets `400 invalid_grant`, like any other unknown refresh token. Before, it got `400 invalid_token`. +- An async `formatRefreshToken.encrypt`, such as the JWE example in the docs, now works. Before, the issued refresh token was the string `[object Promise]`. +- In an extension grant handler, `provider.validateAccessToken` returns `{ active: false }` for an unknown token instead of throwing. `provider.requireActiveAccessToken` still throws. + +A valid JWT access token still returns `400 unsupported_token_type` from revoke, and failed client authentication still returns `401`. diff --git a/docs/content/docs/plugins/oauth-provider.mdx b/docs/content/docs/plugins/oauth-provider.mdx index 2bf3d91870..70fb445953 100644 --- a/docs/content/docs/plugins/oauth-provider.mdx +++ b/docs/content/docs/plugins/oauth-provider.mdx @@ -730,7 +730,7 @@ Post login must be [configured](#post-login-screen) to perform post login select [RFC7662](https://datatracker.ietf.org/doc/html/rfc7662)-compliant Introspection. A request that sends a parameter more than once returns `400 invalid_request`; an empty value counts as omitted. -This endpoint provides details of the provided token. If the token is additionally tied to a session, the endpoint will ensure the session is `active`. +This endpoint provides details of the provided token. If the token is additionally tied to a session, the endpoint will ensure the session is `active`. `token_type_hint` only sets which token type is checked first: a token sent with the wrong hint is still found. Use the `resources` field in `customAccessTokenClaims` to add claims based on the protected resource. @@ -755,10 +755,14 @@ The two formats differ in one way. An opaque token is recomputed on each call, s [RFC7009](https://datatracker.ietf.org/doc/html/rfc7009)-compliant Revocation. A request that sends a parameter more than once returns `400 invalid_request`; an empty value counts as omitted. +An unknown, expired, or already revoked token returns `200` instead of an error ([RFC7009 §2.2](https://datatracker.ietf.org/doc/html/rfc7009#section-2.2)). `token_type_hint` only sets which token type is checked first: a token sent with the wrong hint is still revoked. + +A client can only revoke tokens issued to it ([RFC7009 §2.1](https://datatracker.ietf.org/doc/html/rfc7009#section-2.1)). A confidential client that sends another client's token gets `400 invalid_request`. A public client gets `200` and the token stays valid, so a public client can't use the endpoint to test whether a token exists. + What the endpoint does depends on the token type: * opaque `access_token`: deleted from the database immediately. A `refresh_token` from the same grant stays valid. -* `refresh_token`: deletes every `access_token` it minted and removes the `refresh_token`, so it can no longer issue tokens. +* `refresh_token`: marks the `refresh_token` revoked and deletes every opaque `access_token` it minted, so it can no longer issue tokens. Revoking a `refresh_token` that was already revoked or rotated is treated as replay: it still returns `200`, and it deletes the client's `refresh_token`s for that user, along with the opaque `access_token`s minted from them. An `access_token` issued without a `refresh_token`, and any JWT `access_token`, stays valid until it expires. * JWT `access_token`: cannot be revoked server-side. A JWT is self-contained and is never stored, so there is nothing to delete. A token that still verifies for this server responds with `400 unsupported_token_type` ([RFC7009 §2.2.1](https://datatracker.ietf.org/doc/html/rfc7009#section-2.2.1)), making clear that no server-side revocation happened. A JWT that is already expired or carries an audience rejected by the OAuth resource model is treated as an invalid token and returns a `200` no-op. Because a JWT `access_token` cannot be revoked individually, plan for it: @@ -2037,40 +2041,37 @@ oauthProvider({ }) ``` -Pseudocode for a token encryption method: +Example that encrypts the refresh token as a JWE: ```ts title="auth.ts" import { betterAuth } from "better-auth"; -import { CompactEncrypt, compactDecrypt } from 'jose' -import { oauthProvider } from "@better-auth/oauth-provider"; +import { base64url, CompactEncrypt, compactDecrypt } from "jose"; +import { oauthProvider } from "@better-auth/oauth-provider"; -const secret = "SOME_SECRET_OR_KEY" -const alg = "A256KW" -const enc = "A256GCM" +// A random 256-bit key kept in your secret store, base64url-encoded. +// Generate one with `openssl rand -base64 32 | tr '+/' '-_' | tr -d '='`. +const key = base64url.decode(process.env.REFRESH_TOKEN_KEY!); +const alg = "A256KW"; +const enc = "A256GCM"; const auth = betterAuth({ plugins: [ oauthProvider({ - formatRefreshToken: { - encrypt: (token, sessionId) { - const value = JSON.stringify({ - sessionId, - token, - }); - const jwe = await new CompactEncrypt(Buffer.from(value)) - .setProtectedHeader({ alg, enc }) - .encrypt(secret); - return jwe; + formatRefreshToken: { + encrypt: async (token, sessionId) => { + const value = JSON.stringify({ sessionId, token }); + return new CompactEncrypt(new TextEncoder().encode(value)) + .setProtectedHeader({ alg, enc }) + .encrypt(key); + }, + decrypt: async (token) => { + const { plaintext } = await compactDecrypt(token, key); + return JSON.parse(new TextDecoder().decode(plaintext)); + }, }, - decrypt: (token) { - const { plaintext } = await compactDecrypt(token, secret); - const payload = new TextDecoder().decode(plaintext); - return JSON.parse(payload); - }, - } - }) -] -}) + }), + ], +}); ``` ### Advertised Metadata diff --git a/packages/oauth-provider/src/introspect.test.ts b/packages/oauth-provider/src/introspect.test.ts index da1a545601..1ab4f1a5e4 100644 --- a/packages/oauth-provider/src/introspect.test.ts +++ b/packages/oauth-provider/src/introspect.test.ts @@ -230,7 +230,10 @@ describe("oauth introspect", async () => { }); }); - it("should fail with token_type_hint access_token and sent refresh_token", async () => { + /** + * @see https://datatracker.ietf.org/doc/html/rfc7662#section-2.1 + */ + it("finds a refresh_token sent with token_type_hint access_token", async () => { const tokens = await getTokens(); const introspection = await client.oauth2.introspect( { @@ -246,7 +249,11 @@ describe("oauth introspect", async () => { }, }, ); - expect(introspection.data).toEqual({ active: false }); + expect(introspection.data).toMatchObject({ + active: true, + client_id: oauthClient?.client_id, + scope: "openid profile email offline_access", + }); }); /** @@ -271,6 +278,26 @@ describe("oauth introspect", async () => { expect(introspection.data).toEqual({ active: false }); }); + /** + * @see https://github.com/better-auth/better-auth/issues/11595 + */ + it("returns inactive for a JWT-shaped token that fails verification", async () => { + const introspection = await client.oauth2.introspect( + { + client_id: oauthClient?.client_id, + client_secret: oauthClient?.client_secret, + token: "eyJhbGciOiJub25lIn0.eyJzdWIiOiJ4In0.", + }, + { + headers: { + accept: "application/json", + "content-type": "application/x-www-form-urlencoded", + }, + }, + ); + expect(introspection.data).toEqual({ active: false }); + }); + it("should pass with token_type_hint refresh_token and sent refresh_token", async () => { const tokens = await getTokens(); const introspection = await client.oauth2.introspect( @@ -299,7 +326,10 @@ describe("oauth introspect", async () => { }); }); - it("should fail with token_type_hint refresh_token and sent access_token", async () => { + /** + * @see https://datatracker.ietf.org/doc/html/rfc7662#section-2.1 + */ + it("finds an opaque access_token sent with token_type_hint refresh_token", async () => { const tokens = await getTokens(); const introspection = await client.oauth2.introspect( { @@ -315,7 +345,10 @@ describe("oauth introspect", async () => { }, }, ); - expect(introspection.data?.active).toBeFalsy(); + expect(introspection.data).toMatchObject({ + active: true, + client_id: oauthClient?.client_id, + }); }); it("should pass without token_type_hint and sent jwt access_token", async () => { diff --git a/packages/oauth-provider/src/introspect.ts b/packages/oauth-provider/src/introspect.ts index 359e9e6b1e..d2f394bdba 100644 --- a/packages/oauth-provider/src/introspect.ts +++ b/packages/oauth-provider/src/introspect.ts @@ -1,26 +1,28 @@ import type { GenericEndpointContext } from "@better-auth/core"; import { logger } from "@better-auth/core/env"; -import { - getJwks, - stripAccessTokenAuthorizationScheme, -} from "better-auth/oauth2"; +import { stripAccessTokenAuthorizationScheme } from "better-auth/oauth2"; import type { Session, User } from "better-auth/types"; import { APIError } from "better-call"; -import type { JSONWebKeySet, JWTPayload } from "jose"; -import { createLocalJWKSet, jwtVerify } from "jose"; +import type { JWTPayload } from "jose"; import { resolveAccessTokenClaims } from "./claims"; import { getResourceCustomClaims, isAudienceClaimAllowed, isClientLinkedToAnyResource, + userInfoResource, } from "./resources"; -import { confirmationTokenType, decodeRefreshToken } from "./token"; +import { confirmationTokenType } from "./token"; +import type { + FoundAccessToken, + JwtAccessTokenPayload, + OpaqueAccessTokenRecord, + RefreshTokenRecord, +} from "./token-lookup"; +import { findAccessToken, findToken } from "./token-lookup"; import type { ActiveAccessTokenPayload, Confirmation, - OAuthOpaqueAccessToken, OAuthOptions, - OAuthRefreshToken, SchemaClient, Scope, } from "./types"; @@ -29,7 +31,6 @@ import { extractClientCredentials, getClient, getJwtPlugin, - getStoredToken, parseClientMetadata, resolveSubjectIdentifier, toAudienceClaim, @@ -57,11 +58,11 @@ function inactiveAccessToken(): ValidatedAccessToken { } /** - * IMPORTANT NOTES: - * Introspection follows RFC7662 + * Introspection follows RFC 7662 * https://datatracker.ietf.org/doc/html/rfc7662 - * - APIError: Continue catches (returnable to client) - * - Error: Should immediately stop catches (internal error) + * + * A token this server cannot find, or one the caller may not see, resolves as + * `{ active: false }` (§2.2, §4). Only protocol errors surface. */ /** @@ -110,69 +111,17 @@ async function isIntrospectionAuthorized( } /** - * Validates a JWT access token against the configured JWKs. + * Validates a JWT access token whose signature and issuer are verified. * * @returns RFC7662 introspection format */ async function validateJwtAccessToken( ctx: GenericEndpointContext, opts: OAuthOptions, - token: string, + jwtPayload: JwtAccessTokenPayload, clientId?: string, ) { - const jwtPlugin = opts.disableJwtPlugin - ? undefined - : getJwtPlugin(ctx.context); - const jwtPluginOptions = jwtPlugin?.options; - const baseURL = ctx.context.baseURL ?? ""; - const userInfoAud = `${baseURL}/oauth2/userinfo`; - const expectedIssuer = jwtPluginOptions?.jwt?.issuer ?? ctx.context.baseURL; - let jwtPayload: JWTPayload & { - sid?: string; - azp?: string; - }; - - try { - // Do NOT pass `audience` to jose's verifier. Verify signature + issuer - // here, then validate `aud` manually against the resource model below. - const jwksFetch = jwtPluginOptions?.jwks?.remoteUrl - ? jwtPluginOptions.jwks.remoteUrl - : async (): Promise => { - const jwksRes = await jwtPlugin?.endpoints.getJwks(ctx); - // @ts-expect-error response is a JSONWebKeySet but within the response field - return jwksRes?.response as JSONWebKeySet | undefined; - }; - const jwks = await getJwks(token, { jwksFetch, jwksCacheKey: jwtPlugin }); - const verified = await jwtVerify( - token, - createLocalJWKSet(jwks), - { - issuer: expectedIssuer, - }, - ); - jwtPayload = verified.payload; - } catch (error) { - if (error instanceof Error) { - if (error.name === "TypeError" || error.name === "JWSInvalid") { - // likely an opaque token - throw new APIError("BAD_REQUEST", { - error_description: "invalid JWT signature", - error: "invalid_request", - }); - } else if (error.name === "JWTExpired") { - return { - active: false, - }; - } else if (error.name === "JWTInvalid") { - // issuer or other JWT claim validation failure - return { - active: false, - }; - } - throw error; - } - throw new Error(error as unknown as string); - } + const userInfoAud = userInfoResource(ctx.context.baseURL ?? ""); // Manual `aud` validation (RFC 7662 §2.2 + RFC 8707 §3). EVERY value in // the `aud` claim must resolve to a legitimate resource target: @@ -191,19 +140,8 @@ async function validateJwtAccessToken( return { active: false }; } - // An OAuth access token issued by this provider always carries an `azp` - // (authorized party = client) claim, stamped by `createJwtAccessToken`. The - // JWT plugin shares the same issuer, audience convention, and signing keys, so - // a plain session JWT (e.g. from its `/token` endpoint) can otherwise satisfy - // the signature/issuer/audience checks above. Such a token was never issued - // through the OAuth token endpoint and has no client or consent binding, so - // it must not be reported as an active access token. Require `azp` and a - // matching, enabled client before considering the token active. - if (!jwtPayload.azp) { - return { - active: false, - }; - } + // `findToken` only returns JWTs carrying `azp`; require that client to + // exist and be enabled before considering the token active. const client = await getClient(ctx, opts, jwtPayload.azp); if (!client || client?.disabled) { return { @@ -249,49 +187,16 @@ async function validateJwtAccessToken( } /** - * Searches for an opaque access token in the database and validates it + * Validates an opaque access token record. * * @returns RFC7662 introspection format */ async function validateOpaqueAccessToken( ctx: GenericEndpointContext, opts: OAuthOptions, - token: string, + accessToken: OpaqueAccessTokenRecord, clientId?: string, ): Promise { - let tokenValue = token; - if (opts.prefix?.opaqueAccessToken) { - if (tokenValue.startsWith(opts.prefix.opaqueAccessToken)) { - tokenValue = tokenValue.replace(opts.prefix.opaqueAccessToken, ""); - } else { - throw new APIError("BAD_REQUEST", { - error_description: "opaque access token not found", - error: "invalid_request", - }); - } - } - const accessToken = await ctx.context.adapter.findOne< - OAuthOpaqueAccessToken - >({ - model: "oauthAccessToken", - where: [ - { - field: "token", - value: await getStoredToken( - opts.storeTokens, - tokenValue, - "access_token", - ), - }, - ], - }); - if (!accessToken) { - // Pass through, may be other token type - throw new APIError("BAD_REQUEST", { - error_description: "opaque access token not found", - error: "invalid_token", - }); - } if (!accessToken.expiresAt || accessToken.expiresAt < new Date()) { return inactiveAccessToken(); } @@ -423,34 +328,16 @@ async function validateOpaqueAccessToken( } /** - * Validates a refresh token in the session store. + * Validates a refresh token record. * * @returns payload in RFC7662 introspection format */ async function validateRefreshToken( ctx: GenericEndpointContext, opts: OAuthOptions, - token: string, + refreshToken: RefreshTokenRecord, clientId: string, ) { - const refreshToken = await ctx.context.adapter.findOne | null>({ - model: "oauthRefreshToken", - where: [ - { - field: "token", - value: await getStoredToken(opts.storeTokens, token, "refresh_token"), - }, - ], - }); - if (!refreshToken) { - // Pass through may be other token type - throw new APIError("BAD_REQUEST", { - error_description: "token not found", - error: "invalid_token", - }); - } if (!refreshToken.clientId || refreshToken.clientId !== clientId) { return { active: false, @@ -524,17 +411,26 @@ function createInvalidAccessTokenError() { ); } -function isInactiveTokenError(error: APIError) { - return ( - error.status === "BAD_REQUEST" || error.body?.error === "invalid_token" - ); +async function validateFoundAccessToken( + ctx: GenericEndpointContext, + opts: OAuthOptions, + found: FoundAccessToken, + clientId?: string, +): Promise { + if (found.type === "opaque") { + return validateOpaqueAccessToken(ctx, opts, found.record, clientId); + } + return { + payload: await validateJwtAccessToken(ctx, opts, found.payload, clientId), + requestedUserInfoClaims: [], + }; } /** - * We don't know the access token format so we try to validate it as a JWT - * first, then as an opaque token. Returns the RFC 7662 introspection payload - * alongside the per-issuance UserInfo claims the opaque row persisted (empty for - * a JWT access token, which resolves UserInfo claims by scope). + * Validates an access token (JWT or opaque). Returns the RFC 7662 + * introspection payload, inactive for an unknown token, alongside the + * per-issuance UserInfo claims the opaque row persisted (empty for a JWT access + * token, which resolves UserInfo claims by scope). */ async function resolveAccessTokenValidation( ctx: GenericEndpointContext, @@ -542,30 +438,9 @@ async function resolveAccessTokenValidation( token: string, clientId?: string, ): Promise { - try { - const payload = await validateJwtAccessToken(ctx, opts, token, clientId); - return { payload, requestedUserInfoClaims: [] }; - } catch (err) { - if (err instanceof APIError) { - // continue - } else if (err instanceof Error) { - throw err; - } else { - throw new Error(err as unknown as string); - } - } - try { - return await validateOpaqueAccessToken(ctx, opts, token, clientId); - } catch (err) { - if (err instanceof APIError) { - // nothing - } else if (err instanceof Error) { - throw err; - } else { - throw new Error("Unknown error validating access token"); - } - } - throw createInvalidAccessTokenError(); + const found = await findAccessToken(ctx, opts, token); + if (!found) return inactiveAccessToken(); + return validateFoundAccessToken(ctx, opts, found, clientId); } /** @@ -665,15 +540,6 @@ export async function introspectEndpoint( token_type_hint?: string; }; - // RFC 7662 §2.1: unknown hints are ignored and detection falls back to - // trying both supported token types. - if ( - token_type_hint !== "access_token" && - token_type_hint !== "refresh_token" - ) { - token_type_hint = undefined; - } - const credentials = await extractClientCredentials( ctx, opts, @@ -717,69 +583,27 @@ export async function introspectEndpoint( ); try { - if (token_type_hint === undefined || token_type_hint === "access_token") { - try { - const payload = await validateAccessToken( - ctx, - opts, - token, - client.clientId, - ); - return resolveIntrospectionSub(ctx, opts, payload, client); - } catch (error) { - if (error instanceof APIError) { - if (token_type_hint === "access_token") { - throw error; - } // else continue - } else if (error instanceof Error) { - throw error; - } else { - throw new Error(error as unknown as string); - } - } + const found = await findToken(ctx, opts, token, token_type_hint); + if (!found) return { active: false }; + if (found.type === "refresh") { + const payload = await validateRefreshToken( + ctx, + opts, + found.record, + client.clientId, + ); + return resolveIntrospectionSub(ctx, opts, payload, client); } - - if (token_type_hint === undefined || token_type_hint === "refresh_token") { - try { - const refreshToken = await decodeRefreshToken(opts, token); - const payload = await validateRefreshToken( - ctx, - opts, - refreshToken.token, - client.clientId, - ); - return resolveIntrospectionSub(ctx, opts, payload, client); - } catch (error) { - if (error instanceof APIError) { - if (token_type_hint === "refresh_token") { - throw error; - } // else continue - } else if (error instanceof Error) { - throw error; - } else { - throw new Error(error as unknown as string); - } - } - } - - throw new APIError("BAD_REQUEST", { - error_description: "token not found", - error: "invalid_request", - }); + const { payload } = await validateFoundAccessToken( + ctx, + opts, + found, + client.clientId, + ); + return resolveIntrospectionSub(ctx, opts, payload, client); } catch (error) { - if (error instanceof APIError) { - if (isInactiveTokenError(error)) { - return { - active: false, - }; - } - throw error; - } else if (error instanceof Error) { - logger.error("Introspection error:", error.message, error.stack); - throw new APIError("INTERNAL_SERVER_ERROR"); - } else { - logger.error("Introspection error:", error); - throw new APIError("INTERNAL_SERVER_ERROR"); - } + if (error instanceof APIError) throw error; + logger.error("Introspection error:", error); + throw new APIError("INTERNAL_SERVER_ERROR"); } } diff --git a/packages/oauth-provider/src/resources.ts b/packages/oauth-provider/src/resources.ts index 585f913edc..1b3d0d831f 100644 --- a/packages/oauth-provider/src/resources.ts +++ b/packages/oauth-provider/src/resources.ts @@ -202,7 +202,8 @@ export interface ResolvedResourcePolicy { * implicit `aud` value when `openid` is in scope — not looked up against * `oauthResource` rows. */ -const userInfoResource = (baseURL: string) => `${baseURL}/oauth2/userinfo`; +export const userInfoResource = (baseURL: string) => + `${baseURL}/oauth2/userinfo`; /** * Normalizes a `resource` parameter into an array. Accepts the RFC 8707 diff --git a/packages/oauth-provider/src/revoke.test.ts b/packages/oauth-provider/src/revoke.test.ts index 7f6e9f98af..61091e4736 100644 --- a/packages/oauth-provider/src/revoke.test.ts +++ b/packages/oauth-provider/src/revoke.test.ts @@ -6,6 +6,7 @@ import { } from "better-auth/oauth2"; import { jwt } from "better-auth/plugins/jwt"; import { getTestInstance } from "better-auth/test"; +import { CompactEncrypt, compactDecrypt } from "jose"; import { beforeAll, describe, expect, it } from "vitest"; import { oauthProviderClient } from "./client"; import { oauthProvider } from "./oauth"; @@ -211,25 +212,6 @@ describe("oauth revoke", async () => { expect(revocation.error).toBe(null); }); - it("should fail with token_type_hint access_token and sent refresh_token", async () => { - const tokens = await getTokens(); - const revocation = await client.oauth2.revoke( - { - client_id: oauthClient?.client_id, - client_secret: oauthClient?.client_secret, - token: tokens.data?.refresh_token!, - token_type_hint: "access_token", - }, - { - headers: { - accept: "application/json", - "content-type": "application/x-www-form-urlencoded", - }, - }, - ); - expect(revocation.error?.status).toBe(400); - }); - it("should pass verification with token_type_hint refresh_token and sent refresh_token", async () => { const tokens = await getTokens(); const revocation = await client.oauth2.revoke( @@ -250,25 +232,6 @@ describe("oauth revoke", async () => { expect(revocation.error).toBe(null); }); - it("should fail verification with token_type_hint refresh_token and sent access_token", async () => { - const tokens = await getTokens(); - const revocation = await client.oauth2.revoke( - { - client_id: oauthClient?.client_id, - client_secret: oauthClient?.client_secret, - token: tokens.data?.access_token!, - token_type_hint: "refresh_token", - }, - { - headers: { - accept: "application/json", - "content-type": "application/x-www-form-urlencoded", - }, - }, - ); - expect(revocation.error?.status).toBe(400); - }); - it("reports unsupported_token_type for a jwt access_token without token_type_hint", async () => { const tokens = await getTokens(undefined, validResource); const revocation = await client.oauth2.revoke( @@ -309,6 +272,177 @@ describe("oauth revoke", async () => { expect(revocation.error).toBe(null); }); + /** + * @see https://datatracker.ietf.org/doc/html/rfc7009#section-2.1 + * @see https://datatracker.ietf.org/doc/html/rfc7009#section-2.2 + */ + describe("RFC 7009 token handling", () => { + function revoke( + token: string, + token_type_hint?: string, + caller: Pick< + OAuthClient, + "client_id" | "client_secret" + > | null = oauthClient, + ) { + return client.oauth2.revoke( + { + client_id: caller?.client_id, + ...(caller?.client_secret + ? { client_secret: caller.client_secret } + : {}), + token, + token_type_hint, + }, + { + headers: { + accept: "application/json", + "content-type": "application/x-www-form-urlencoded", + }, + }, + ); + } + + async function isActive(token: string) { + const introspection = await client.oauth2.introspect( + { + client_id: oauthClient?.client_id, + client_secret: oauthClient?.client_secret, + token, + }, + { + headers: { + accept: "application/json", + "content-type": "application/x-www-form-urlencoded", + }, + }, + ); + return introspection.data?.active; + } + + it("revokes a refresh_token sent with token_type_hint access_token", async () => { + const tokens = await getTokens(); + const refreshToken = tokens.data?.refresh_token!; + expect(await isActive(refreshToken)).toBe(true); + + const revocation = await revoke(refreshToken, "access_token"); + expect(revocation.error).toBe(null); + expect(await isActive(refreshToken)).toBe(false); + }); + + it("revokes an opaque access_token sent with token_type_hint refresh_token", async () => { + const tokens = await getTokens(); + const accessToken = tokens.data?.access_token!; + expect(await isActive(accessToken)).toBe(true); + + const revocation = await revoke(accessToken, "refresh_token"); + expect(revocation.error).toBe(null); + expect(await isActive(accessToken)).toBe(false); + }); + + it.each([undefined, "access_token", "refresh_token"])( + "returns 200 for an unknown token (token_type_hint %s)", + async (hint) => { + const revocation = await revoke("unknown-token", hint); + expect(revocation.error).toBe(null); + }, + ); + + function createOtherClient( + token_endpoint_auth_method: "client_secret_post" | "none", + ) { + return auth.api.adminCreateOAuthClient({ + headers, + body: { + redirect_uris: [redirectUri], + application_type: "native", + token_endpoint_auth_method, + skip_consent: true, + }, + }); + } + + it.each(["access_token", "refresh_token"] as const)( + "refuses a confidential client revoking another client's %s", + async (type) => { + const tokens = await getTokens(); + const token = tokens.data?.[type]!; + const other = await createOtherClient("client_secret_post"); + + const revocation = await revoke(token, undefined, other); + expect(revocation.error).toMatchObject({ + status: 400, + error: "invalid_request", + }); + expect(await isActive(token)).toBe(true); + }, + ); + + it.each(["access_token", "refresh_token"] as const)( + "answers a public client revoking another client's %s with 200 and keeps the token", + async (type) => { + const tokens = await getTokens(); + const token = tokens.data?.[type]!; + const other = await createOtherClient("none"); + + const revocation = await revoke(token, undefined, { + client_id: other.client_id, + }); + expect(revocation.error).toBe(null); + expect(await isActive(token)).toBe(true); + }, + ); + + it("applies the same ownership rule to another client's JWT access_token", async () => { + const tokens = await getTokens(undefined, validResource); + const jwtAccessToken = tokens.data?.access_token!; + expect(jwtAccessToken.split(".")).toHaveLength(3); + + const confidential = await createOtherClient("client_secret_post"); + expect( + (await revoke(jwtAccessToken, undefined, confidential)).error, + ).toMatchObject({ status: 400, error: "invalid_request" }); + + const publicClient = await createOtherClient("none"); + expect( + ( + await revoke(jwtAccessToken, undefined, { + client_id: publicClient.client_id, + }) + ).error, + ).toBe(null); + }); + + it("refuses another client's revoked refresh_token before replay handling", async () => { + const tokens = await getTokens(); + const refreshToken = tokens.data?.refresh_token!; + expect((await revoke(refreshToken)).error).toBe(null); + + const other = await createOtherClient("client_secret_post"); + const revocation = await revoke(refreshToken, undefined, other); + expect(revocation.error).toMatchObject({ + status: 400, + error: "invalid_request", + }); + }); + + /** + * @see https://github.com/better-auth/better-auth/issues/11595 + */ + it("returns 200 for a JWT-shaped token that fails verification", async () => { + const revocation = await revoke("eyJhbGciOiJub25lIn0.eyJzdWIiOiJ4In0."); + expect(revocation.error).toBe(null); + }); + + it("returns 200 when revoking an already revoked refresh_token", async () => { + const tokens = await getTokens(); + const refreshToken = tokens.data?.refresh_token!; + expect((await revoke(refreshToken, "refresh_token")).error).toBe(null); + expect((await revoke(refreshToken, "refresh_token")).error).toBe(null); + expect(await isActive(refreshToken)).toBe(false); + }); + }); + it("should pass verification without token_type_hint and sent refresh_token", async () => { const tokens = await getTokens(); const revocation = await client.oauth2.revoke( @@ -439,6 +573,109 @@ describe("oauth revoke - config", async () => { }; } + /** + * The JWE refresh-token format from the `formatRefreshToken` docs, on a + * deployment without the JWT plugin. + * @see https://datatracker.ietf.org/doc/html/rfc7009#section-2.1 + */ + describe("encrypted refresh tokens without the JWT plugin", () => { + const key = new Uint8Array(32).fill(7); + const formatRefreshToken = { + encrypt: async (token: string, sessionId?: string) => + new CompactEncrypt( + new TextEncoder().encode(JSON.stringify({ token, sessionId })), + ) + .setProtectedHeader({ alg: "A256KW", enc: "A256GCM" }) + .encrypt(key), + decrypt: async (token: string) => { + const { plaintext } = await compactDecrypt(token, key); + return JSON.parse(new TextDecoder().decode(plaintext)); + }, + }; + + async function setup() { + const testScopes = ["openid", "offline_access"]; + const { client, oauthClient } = await createTestInstance({ + oauthProviderConfig: { + disableJwtPlugin: true, + formatRefreshToken, + scopes: testScopes, + }, + }); + if (!oauthClient) expect.unreachable(); + const { url: authUrl, codeVerifier } = await createAuthUrl(oauthClient, { + scopes: testScopes, + }); + let callbackRedirectUrl = ""; + await client.$fetch(authUrl.toString(), { + onError(context) { + callbackRedirectUrl = context.response.headers.get("Location") || ""; + }, + }); + const tokens = await client.$fetch<{ + access_token: string; + refresh_token: string; + }>("/oauth2/token", { + method: "POST", + body: new URLSearchParams({ + grant_type: "authorization_code", + code: new URL(callbackRedirectUrl).searchParams.get("code")!, + code_verifier: codeVerifier, + client_id: oauthClient.client_id, + client_secret: oauthClient.client_secret!, + redirect_uri: redirectUri, + }), + headers: { "content-type": "application/x-www-form-urlencoded" }, + }); + const call = (path: string, body: Record) => + client.$fetch<{ active?: boolean }>(path, { + method: "POST", + body: new URLSearchParams({ + client_id: oauthClient.client_id, + client_secret: oauthClient.client_secret!, + ...body, + }), + headers: { "content-type": "application/x-www-form-urlencoded" }, + }); + return { tokens: tokens.data!, call }; + } + + it("revokes the encrypted refresh_token", async () => { + const { tokens, call } = await setup(); + expect(tokens.refresh_token.split(".")).toHaveLength(5); + const revoked = await call("/oauth2/revoke", { + token: tokens.refresh_token, + }); + expect(revoked.error).toBe(null); + const introspection = await call("/oauth2/introspect", { + token: tokens.refresh_token, + }); + expect(introspection.data?.active).toBe(false); + }); + + it("revokes an opaque access_token sent with token_type_hint refresh_token", async () => { + const { tokens, call } = await setup(); + const revoked = await call("/oauth2/revoke", { + token: tokens.access_token, + token_type_hint: "refresh_token", + }); + expect(revoked.error).toBe(null); + const introspection = await call("/oauth2/introspect", { + token: tokens.access_token, + }); + expect(introspection.data?.active).toBe(false); + }); + + it("returns 200 for an unknown token the decoder rejects", async () => { + const { call } = await setup(); + const revoked = await call("/oauth2/revoke", { + token: "not-a-jwe", + token_type_hint: "refresh_token", + }); + expect(revoked.error).toBe(null); + }); + }); + it("should pass with the correct opaqueAccessTokenPrefix", async () => { const prefix = "hello_"; const testScopes = ["read:profile"]; diff --git a/packages/oauth-provider/src/revoke.ts b/packages/oauth-provider/src/revoke.ts index 4cabbb6d4f..39c750b577 100644 --- a/packages/oauth-provider/src/revoke.ts +++ b/packages/oauth-provider/src/revoke.ts @@ -1,114 +1,52 @@ import type { GenericEndpointContext } from "@better-auth/core"; import { logger } from "@better-auth/core/env"; -import { - getJwks, - stripAccessTokenAuthorizationScheme, -} from "better-auth/oauth2"; +import { stripAccessTokenAuthorizationScheme } from "better-auth/oauth2"; import { APIError } from "better-call"; -import type { JSONWebKeySet, JWTPayload } from "jose"; -import { createLocalJWKSet, jwtVerify } from "jose"; -import { isAudienceClaimAllowed } from "./resources"; -import { decodeRefreshToken, invalidateRefreshFamily } from "./token"; +import { isAudienceClaimAllowed, userInfoResource } from "./resources"; +import { invalidateRefreshFamily } from "./token"; import type { - OAuthOpaqueAccessToken, - OAuthOptions, - OAuthRefreshToken, - Scope, -} from "./types"; + JwtAccessTokenPayload, + OpaqueAccessTokenRecord, + RefreshTokenRecord, +} from "./token-lookup"; +import { findToken } from "./token-lookup"; +import type { OAuthOptions, Scope } from "./types"; import { destructureCredentials, extractClientCredentials, - getJwtPlugin, - getStoredToken, validateClientCredentials, } from "./utils"; /** - * IMPORTANT NOTES: - * Revocation follows RFC7009 + * Revocation follows RFC 7009 * https://datatracker.ietf.org/doc/html/rfc7009 - * - APIError: Continue catches (returnable to client) - * - Error: Should immediately stop catches (internal error) + * + * A token this server cannot find is already unusable, so it resolves as a + * successful no-op (§2.2). Only explicit protocol errors surface. */ /** - * Revokes a JWT access token against the configured JWKs. - * * A JWT access token is self-contained and never stored, so there is nothing to * delete. Once the token is confirmed to be a valid JWT for this server, the * endpoint reports `unsupported_token_type` (RFC 7009 §2.2.1) instead of a * silent success, so callers can tell that no server-side revocation happened. - * An expired JWT or a JWT with an audience rejected by the OAuth resource model - * is already inactive and still resolves as a successful no-op. Session-bound - * tokens (carrying `sid`) are cut off early by the session-liveness check in - * introspection and userinfo. + * A JWT with an audience rejected by the OAuth resource model is already + * inactive and resolves as a successful no-op. Session-bound tokens (carrying + * `sid`) are cut off early by the session-liveness check in introspection and + * userinfo. */ async function revokeJwtAccessToken( ctx: GenericEndpointContext, opts: OAuthOptions, - token: string, + payload: JwtAccessTokenPayload, ) { - const jwtPlugin = opts.disableJwtPlugin - ? undefined - : getJwtPlugin(ctx.context); - const jwtPluginOptions = jwtPlugin?.options; - - // Verify signature + issuer first, then validate `aud` against the OAuth - // resource model. Do not pass jose's `audience` option here: access-token - // audiences are resource identifiers, not a single global authorization-server - // audience. - try { - const jwksFetch = jwtPluginOptions?.jwks?.remoteUrl - ? jwtPluginOptions.jwks.remoteUrl - : async (): Promise => { - const jwksRes = await jwtPlugin?.endpoints.getJwks(ctx); - // @ts-expect-error response is a JSONWebKeySet but within the response field - return jwksRes?.response as JSONWebKeySet | undefined; - }; - const jwks = await getJwks(token, { - jwksFetch, - // The plugin instance is stable across requests, so the key set - // fetched by the per-request closure above is cached under it. - jwksCacheKey: jwtPlugin, - }); - const verified = await jwtVerify( - token, - createLocalJWKSet(jwks), - { - issuer: jwtPluginOptions?.jwt?.issuer ?? ctx.context.baseURL, - }, - ); - const userInfoAudience = `${ctx.context.baseURL}/oauth2/userinfo`; - if ( - !verified.payload.azp || - !(await isAudienceClaimAllowed(ctx, opts, verified.payload.aud, [ - userInfoAudience, - ])) - ) { - return null; - } - } catch (error) { - if (error instanceof Error) { - if (error.name === "TypeError" || error.name === "JWSInvalid") { - // likely an opaque token - throw new APIError("BAD_REQUEST", { - error_description: "invalid JWT signature", - error: "invalid_request", - }); - } else if (error.name === "JWTExpired") { - return null; - } else if (error.name === "JWTInvalid") { - // issuer or other JWT claim validation failure - return null; - } - throw error; - } - throw new Error(error as unknown as string); + if ( + !(await isAudienceClaimAllowed(ctx, opts, payload.aud, [ + userInfoResource(ctx.context.baseURL), + ])) + ) { + return null; } - - // Verified: a valid JWT access token for this server. It is self-contained - // and not stored, so it cannot be revoked. Report that rather than implying - // success (RFC 7009 §2.2.1). throw new APIError("BAD_REQUEST", { error_description: "JWT access tokens are self-contained and cannot be revoked server-side", @@ -116,99 +54,29 @@ async function revokeJwtAccessToken( }); } -/** - * Searches for an opaque access token in the database and validates it - */ async function revokeOpaqueAccessToken( ctx: GenericEndpointContext, - opts: OAuthOptions, - token: string, - clientId: string, + accessToken: OpaqueAccessTokenRecord, ) { - let tokenValue = token; - if (opts.prefix?.opaqueAccessToken) { - if (tokenValue.startsWith(opts.prefix.opaqueAccessToken)) { - tokenValue = tokenValue.replace(opts.prefix.opaqueAccessToken, ""); - } else { - throw new APIError("BAD_REQUEST", { - error_description: "opaque access token not found", - error: "invalid_request", - }); - } - } - const accessToken: - | (OAuthOpaqueAccessToken & { id?: string }) - | null = await ctx.context.adapter.findOne>( - { - model: "oauthAccessToken", - where: [ - { - field: "token", - value: await getStoredToken( - opts.storeTokens, - tokenValue, - "access_token", - ), - }, - ], - }, - ); - if (!accessToken) { - throw new APIError("BAD_REQUEST", { - error_description: "opaque access token not found", - error: "invalid_request", - }); - } - if (!accessToken.clientId || accessToken.clientId !== clientId) { - return null; - } - - accessToken.id - ? await ctx.context.adapter.delete({ - model: "oauthAccessToken", - where: [{ field: "id", value: accessToken.id }], - }) - : await ctx.context.adapter.delete({ - model: "oauthAccessToken", - where: [{ field: "token", value: accessToken.token }], - }); + await ctx.context.adapter.delete({ + model: "oauthAccessToken", + where: accessToken.id + ? [{ field: "id", value: accessToken.id }] + : [{ field: "token", value: accessToken.token }], + }); } -/** - * Validates a refresh token in the session store. - */ async function revokeRefreshToken( ctx: GenericEndpointContext, - opts: OAuthOptions, - token: string, + refreshToken: RefreshTokenRecord, clientId: string, ) { - const refreshToken = await ctx.context.adapter.findOne< - OAuthRefreshToken & { id: string } - >({ - model: "oauthRefreshToken", - where: [ - { - field: "token", - value: await getStoredToken(opts.storeTokens, token, "refresh_token"), - }, - ], - }); - if (!refreshToken) { - throw new APIError("BAD_REQUEST", { - error_description: "token not found", - error: "invalid_request", - }); - } + const { userId } = refreshToken; + // Presenting an already revoked refresh token is treated as replay: tear + // down the whole family, and still answer 200 since the token is unusable. if (refreshToken.revoked) { - await invalidateRefreshFamily(ctx, clientId, refreshToken.userId); - throw new APIError("BAD_REQUEST", { - error_description: "refresh token revoked", - error: "invalid_request", - }); - } - if (!refreshToken.clientId || refreshToken.clientId !== clientId) { - return null; + await invalidateRefreshFamily(ctx, clientId, userId); + return; } const iat = Math.floor(Date.now() / 1000); @@ -234,11 +102,8 @@ async function revokeRefreshToken( }, }); if (!won) { - await invalidateRefreshFamily(ctx, clientId, refreshToken.userId); - throw new APIError("BAD_REQUEST", { - error_description: "refresh token revoked", - error: "invalid_request", - }); + await invalidateRefreshFamily(ctx, clientId, userId); + return; } await ctx.context.adapter.deleteMany({ model: "oauthAccessToken", @@ -246,49 +111,6 @@ async function revokeRefreshToken( }); } -/** - * We don't know the access token format so we try to validate it - * as a JWT first, then as an opaque token. - */ -async function revokeAccessToken( - ctx: GenericEndpointContext, - opts: OAuthOptions, - clientId: string, - token: string, -) { - try { - return await revokeJwtAccessToken(ctx, opts, token); - } catch (err) { - if (err instanceof APIError) { - // A confirmed JWT access token cannot be revoked: surface that rather - // than falling through to the opaque-token lookup. - if (err.body?.error === "unsupported_token_type") { - throw err; - } - // otherwise not a JWT, continue to opaque - } else if (err instanceof Error) { - throw err; - } else { - throw new Error(err as unknown as string); - } - } - try { - return await revokeOpaqueAccessToken(ctx, opts, token, clientId); - } catch (err) { - if (err instanceof APIError) { - // nothing - } else if (err instanceof Error) { - throw err; - } else { - throw new Error("Unknown error validating access token"); - } - } - throw new APIError("BAD_REQUEST", { - error_description: "Invalid access token", - error: "invalid_request", - }); -} - export async function revokeEndpoint( ctx: GenericEndpointContext, opts: OAuthOptions, @@ -298,15 +120,6 @@ export async function revokeEndpoint( token_type_hint?: string; }; - // RFC 7009 §2.2.1: unknown hints are ignored and the server extends its - // search across all supported token types. - if ( - token_type_hint !== "access_token" && - token_type_hint !== "refresh_token" - ) { - token_type_hint = undefined; - } - const credentials = await extractClientCredentials( ctx, opts, @@ -350,68 +163,35 @@ export async function revokeEndpoint( ); try { - if (token_type_hint === undefined || token_type_hint === "access_token") { - try { - return await revokeAccessToken(ctx, opts, client.clientId, token); - } catch (error) { - if (error instanceof APIError) { - if ( - token_type_hint === "access_token" || - error.body?.error === "unsupported_token_type" - ) { - throw error; - } // else continue - } else if (error instanceof Error) { - throw error; - } else { - throw new Error(error as unknown as string); - } - } + const found = await findToken(ctx, opts, token, token_type_hint); + if (!found) return null; + + // RFC 7009 §2.1: refuse to revoke a token issued to another client. A + // public client gets a silent 200 instead, so it cannot probe which + // token values exist. + const ownerClientId = + found.type === "jwt" ? found.payload.azp : found.record.clientId; + if (ownerClientId !== client.clientId) { + if (client.tokenEndpointAuthMethod === "none") return null; + throw new APIError("BAD_REQUEST", { + error_description: + "client is not authorized to revoke the presented token", + error: "invalid_request", + }); } - if (token_type_hint === undefined || token_type_hint === "refresh_token") { - try { - const refreshToken = await decodeRefreshToken(opts, token); - return await revokeRefreshToken( - ctx, - opts, - refreshToken.token, - client.clientId, - ); - } catch (error) { - if (error instanceof APIError) { - if (token_type_hint === "refresh_token") { - throw error; - } // else continue - } else if (error instanceof Error) { - throw error; - } else { - throw new Error(error as unknown as string); - } - } + if (found.type === "jwt") { + return await revokeJwtAccessToken(ctx, opts, found.payload); } - - throw new APIError("BAD_REQUEST", { - error_description: "token not found", - error: "invalid_request", - }); - } catch (error) { - if (error instanceof APIError) { - // RFC 7009 §2.2.1: an explicit protocol error (unsupported_token_type) - // must surface; an unknown or already-invalid token still succeeds. - if (error.body?.error === "unsupported_token_type") { - throw error; - } - if (error.name === "BAD_REQUEST") { - return null; - } - throw error; - } else if (error instanceof Error) { - logger.error("Introspection error:", error.message, error.stack); - throw new APIError("INTERNAL_SERVER_ERROR"); + if (found.type === "opaque") { + await revokeOpaqueAccessToken(ctx, found.record); } else { - logger.error("Introspection error:", error); - throw new APIError("INTERNAL_SERVER_ERROR"); + await revokeRefreshToken(ctx, found.record, client.clientId); } + return null; + } catch (error) { + if (error instanceof APIError) throw error; + logger.error("Revocation error:", error); + throw new APIError("INTERNAL_SERVER_ERROR"); } } diff --git a/packages/oauth-provider/src/token-lookup.ts b/packages/oauth-provider/src/token-lookup.ts new file mode 100644 index 0000000000..a13287a5d9 --- /dev/null +++ b/packages/oauth-provider/src/token-lookup.ts @@ -0,0 +1,223 @@ +import type { GenericEndpointContext } from "@better-auth/core"; +import { getJwks } from "better-auth/oauth2"; +import { APIError } from "better-call"; +import type { JSONWebKeySet, JWTPayload } from "jose"; +import { + createLocalJWKSet, + decodeProtectedHeader, + errors as joseErrors, + jwtVerify, +} from "jose"; +import type { + OAuthOpaqueAccessToken, + OAuthOptions, + OAuthRefreshToken, + Scope, +} from "./types"; +import { getJwtPlugin, getStoredToken } from "./utils"; + +/** Signature, issuer, and `azp` are verified; audience and client are not. */ +export type JwtAccessTokenPayload = JWTPayload & { azp: string; sid?: string }; +export type OpaqueAccessTokenRecord = OAuthOpaqueAccessToken & { + id?: string; +}; +export type RefreshTokenRecord = OAuthRefreshToken & { id: string }; + +/** An access token issued by this server, identified by its format. */ +export type FoundAccessToken = + | { type: "jwt"; payload: JwtAccessTokenPayload } + | { type: "opaque"; record: OpaqueAccessTokenRecord }; + +/** A token presented to revocation or introspection, identified by type. */ +export type FoundToken = + | FoundAccessToken + | { type: "refresh"; record: RefreshTokenRecord }; + +const jwksInfrastructureErrorCodes = new Set([ + joseErrors.JWKSTimeout.code, + joseErrors.JWKSInvalid.code, + joseErrors.JWKSMultipleMatchingKeys.code, +]); + +/** + * Verifies a JWT access token issued by this server. Returns `"inactive"` for + * one that is expired or fails claim validation, so the lookup can stop: it + * cannot be another token type. Returns `null` for anything else (an opaque + * token, a malformed or forged JWT, a JWT that is not an access token) so the + * lookup can continue. Key-set failures still throw. + */ +async function verifyJwtAccessToken( + ctx: GenericEndpointContext, + opts: OAuthOptions, + token: string, +): Promise { + if (opts.disableJwtPlugin) return null; + // Only a compact JWS (three segments) can be a JWT access token; a JWE such + // as an encrypted refresh token has five, and must not wait on the key set. + if (token.split(".").length !== 3) return null; + try { + decodeProtectedHeader(token); + } catch { + return null; + } + const jwtPlugin = getJwtPlugin(ctx.context); + const jwtPluginOptions = jwtPlugin.options; + const jwksFetch = jwtPluginOptions?.jwks?.remoteUrl + ? jwtPluginOptions.jwks.remoteUrl + : async (): Promise => { + const jwksRes = await jwtPlugin.endpoints.getJwks(ctx); + // @ts-expect-error response is a JSONWebKeySet but within the response field + return jwksRes?.response as JSONWebKeySet | undefined; + }; + // The plugin instance is stable across requests, so the key set fetched by + // the per-request closure above is cached under it. + const jwks = await getJwks(token, { jwksFetch, jwksCacheKey: jwtPlugin }); + let payload: JWTPayload; + try { + // Do not pass jose's `audience` option: access-token audiences are + // resource identifiers, which each endpoint validates itself. + ({ payload } = await jwtVerify(token, createLocalJWKSet(jwks), { + issuer: jwtPluginOptions?.jwt?.issuer ?? ctx.context.baseURL, + })); + } catch (error) { + // jose checks claims only after the signature verifies. + if (error instanceof joseErrors.JWTClaimValidationFailed) { + return isAccessTokenPayload(error.payload) ? "inactive" : null; + } + if ( + error instanceof joseErrors.JOSEError && + !jwksInfrastructureErrorCodes.has(error.code) + ) { + return null; + } + throw error; + } + return isAccessTokenPayload(payload) ? payload : null; +} + +/** + * The OAuth token endpoint stamps `azp` (the client) on every JWT access token. + * The JWT plugin signs other JWTs with the same issuer and keys, such as its + * `/token` session JWT, so one without `azp` is not an access token. + */ +function isAccessTokenPayload( + payload: JWTPayload, +): payload is JwtAccessTokenPayload { + return typeof payload.azp === "string" && payload.azp.length > 0; +} + +async function findOpaqueAccessToken( + ctx: GenericEndpointContext, + opts: OAuthOptions, + token: string, +) { + const prefix = opts.prefix?.opaqueAccessToken; + if (prefix && !token.startsWith(prefix)) return null; + return ctx.context.adapter.findOne({ + model: "oauthAccessToken", + where: [ + { + field: "token", + value: await getStoredToken( + opts.storeTokens, + prefix ? token.slice(prefix.length) : token, + "access_token", + ), + }, + ], + }); +} + +/** + * Looks up a refresh token by its presented value, after removing the + * configured prefix and applying `formatRefreshToken.decrypt`. Returns `null` + * when the value is not a refresh token this server stored. + */ +export async function findRefreshToken( + ctx: GenericEndpointContext, + opts: OAuthOptions, + token: string, +) { + const prefix = opts.prefix?.refreshToken; + if (prefix && !token.startsWith(prefix)) return null; + const value = prefix ? token.slice(prefix.length) : token; + let decoded: { token: string }; + try { + decoded = opts.formatRefreshToken?.decrypt + ? await opts.formatRefreshToken.decrypt(value) + : { token: value }; + } catch (error) { + // A `decrypt` that rejects the format, with a 400 or a JOSE error such as + // `compactDecrypt` throws, means this is not a refresh token. Any other + // failure is real. + if ( + (error instanceof APIError && error.statusCode === 400) || + error instanceof joseErrors.JOSEError + ) { + return null; + } + throw error; + } + return ctx.context.adapter.findOne({ + model: "oauthRefreshToken", + where: [ + { + field: "token", + value: await getStoredToken( + opts.storeTokens, + decoded.token, + "refresh_token", + ), + }, + ], + }); +} + +async function identifyAccessToken( + ctx: GenericEndpointContext, + opts: OAuthOptions, + token: string, +): Promise { + const payload = await verifyJwtAccessToken(ctx, opts, token); + if (payload === "inactive") return payload; + if (payload) return { type: "jwt", payload }; + const record = await findOpaqueAccessToken(ctx, opts, token); + return record ? { type: "opaque", record } : null; +} + +/** + * Identifies an access token as a JWT or an opaque token issued by this + * server. Returns `null` when it is neither, including an expired JWT. + */ +export async function findAccessToken( + ctx: GenericEndpointContext, + opts: OAuthOptions, + token: string, +): Promise { + const found = await identifyAccessToken(ctx, opts, token); + return found === "inactive" ? null : found; +} + +/** + * Identifies a token presented to revocation or introspection. Every token + * type is searched; `token_type_hint` only decides which one is tried first + * (RFC 7009 §2.1, RFC 7662 §2.1). Returns `null` when no type matches. Only + * infrastructure failures (key set, database, refresh-token decryption) throw. + */ +export async function findToken( + ctx: GenericEndpointContext, + opts: OAuthOptions, + token: string, + tokenTypeHint?: string, +): Promise { + const asRefreshToken = async (): Promise => { + const record = await findRefreshToken(ctx, opts, token); + return record ? { type: "refresh", record } : null; + }; + if (tokenTypeHint === "refresh_token") { + return (await asRefreshToken()) ?? findAccessToken(ctx, opts, token); + } + const found = await identifyAccessToken(ctx, opts, token); + if (found === "inactive") return null; + return found ?? asRefreshToken(); +} diff --git a/packages/oauth-provider/src/token.test.ts b/packages/oauth-provider/src/token.test.ts index 46ab3d87c0..0fc893c981 100644 --- a/packages/oauth-provider/src/token.test.ts +++ b/packages/oauth-provider/src/token.test.ts @@ -1593,26 +1593,17 @@ describe("oauth token - refresh_token", async () => { }); /** - * V1 from variants.md: revokeRefreshToken racing against - * handleRefreshTokenGrant on the same parent row. Both paths must run - * through the same CAS so exactly one wins; the loser fails closed and - * the parent stays revoked. + * `revokeRefreshToken` racing `handleRefreshTokenGrant` on the same parent + * row. Both run through the same CAS, so exactly one wins and the loser + * fails closed. Each ordering is forced by holding the first CAS until the + * other request has finished, instead of leaving it to scheduling. * * @see https://github.com/better-auth/better-auth/security/advisories/GHSA-392p-2q2v-4372 */ - it("concurrent revoke + rotate against the same refresh token: exactly one wins", async () => { - if (!oauthClient?.client_id || !oauthClient?.client_secret) { - throw Error("beforeAll not run properly"); - } - - const scopes = ["openid", "profile", "offline_access"]; - const tokens = await authorizeForRefreshToken(scopes); - expect(tokens?.refresh_token).toBeDefined(); - const parentToken = tokens!.refresh_token!; - - const refresh = async () => { + describe("concurrent revoke + rotate against the same refresh token", () => { + async function refresh(refreshToken: string) { const { body, headers } = await refreshAccessTokenRequest({ - refreshToken: parentToken, + refreshToken, options: { clientId: oauthClient!.client_id, clientSecret: oauthClient!.client_secret!, @@ -1622,19 +1613,18 @@ describe("oauth token - refresh_token", async () => { return client.$fetch<{ access_token?: string; refresh_token?: string; - error?: string; }>("/oauth2/token", { method: "POST", body, headers, }); - }; + } - const revoke = () => - client.$fetch("/oauth2/revoke", { + function revoke(refreshToken: string) { + return client.$fetch("/oauth2/revoke", { method: "POST", body: new URLSearchParams({ - token: parentToken, + token: refreshToken, token_type_hint: "refresh_token", client_id: oauthClient!.client_id, client_secret: oauthClient!.client_secret!, @@ -1643,29 +1633,90 @@ describe("oauth token - refresh_token", async () => { "content-type": "application/x-www-form-urlencoded", }, }); - - const [refreshResult, revokeResult] = await Promise.all([ - refresh(), - revoke(), - ]); - - // /oauth2/revoke is RFC 7009 §2.2 always-200, so the racer outcome is - // observable through the refresh result alone: - // - refresh won the CAS → mints a fresh refresh_token; revoke is a no-op - // - revoke won the CAS → refresh returns invalid_grant - expect(revokeResult.error).toBeNull(); - if (refreshResult.error === null) { - expect(refreshResult.data?.refresh_token).toBeDefined(); - } else { - const errPayload = refreshResult.error as { error?: string } | null; - expect(errPayload?.error).toBe("invalid_grant"); } - // In both orderings, replaying the parent must now fail closed - // (either revoked by the explicit revoke or rotated out by the refresh). - const replay = await refresh(); - const replayErr = replay.error as { error?: string } | null; - expect(replayErr?.error).toBe("invalid_grant"); + /** Pauses the next refresh-token CAS until `release` is called. */ + async function holdNextRefreshTokenCas() { + const context = await authorizationServer.$context; + const incrementOne = context.adapter.incrementOne.bind(context.adapter); + const reached = Promise.withResolvers(); + const released = Promise.withResolvers(); + const spy = vi.spyOn(context.adapter, "incrementOne"); + spy.mockImplementation(async (...args) => { + if (args[0].model === "oauthRefreshToken") { + spy.mockRestore(); + reached.resolve(); + await released.promise; + } + return incrementOne(...args); + }); + return { + reached: reached.promise, + release: released.resolve, + // Release too, so a test that fails early does not leave the held + // request pending. + restore: () => { + released.resolve(); + spy.mockRestore(); + }, + }; + } + + async function issueParentRefreshToken() { + const tokens = await authorizeForRefreshToken([ + "openid", + "profile", + "offline_access", + ]); + expect(tokens?.refresh_token).toBeDefined(); + return tokens!.refresh_token!; + } + + it("fails the revoke closed when the rotation wins", async () => { + const parentToken = await issueParentRefreshToken(); + const cas = await holdNextRefreshTokenCas(); + try { + const revoking = revoke(parentToken); + await cas.reached; + const rotated = await refresh(parentToken); + cas.release(); + const revoked = await revoking; + + expect(rotated.error).toBeNull(); + expect(rotated.data?.refresh_token).toBeDefined(); + // RFC 7009 §2.2: the losing revoke still returns 200 and tears down + // the family, including the new token. + expect(revoked.error).toBeNull(); + expect( + (await refresh(rotated.data!.refresh_token!)).error, + ).toMatchObject({ error: "invalid_grant" }); + expect((await refresh(parentToken)).error).toMatchObject({ + error: "invalid_grant", + }); + } finally { + cas.restore(); + } + }); + + it("fails the rotation with invalid_grant when the revoke wins", async () => { + const parentToken = await issueParentRefreshToken(); + const cas = await holdNextRefreshTokenCas(); + try { + const rotating = refresh(parentToken); + await cas.reached; + const revoked = await revoke(parentToken); + cas.release(); + const rotated = await rotating; + + expect(revoked.error).toBeNull(); + expect(rotated.error).toMatchObject({ error: "invalid_grant" }); + expect((await refresh(parentToken)).error).toMatchObject({ + error: "invalid_grant", + }); + } finally { + cas.restore(); + } + }); }); }); diff --git a/packages/oauth-provider/src/token.ts b/packages/oauth-provider/src/token.ts index 05ad685095..32a96c766d 100644 --- a/packages/oauth-provider/src/token.ts +++ b/packages/oauth-provider/src/token.ts @@ -32,6 +32,7 @@ import { isUserDelegatedScope } from "./oauthClient/client-credentials"; import type { ResolvedResourcePolicy } from "./resources"; import { resolveResourcePolicy } from "./resources"; import { getSupportedClaims, STANDARD_CLAIM_NAMES } from "./standard-claims"; +import { findRefreshToken } from "./token-lookup"; import type { Confirmation, OAuthAuthenticatedClient, @@ -58,7 +59,6 @@ import { extractClientCredentials, getClient, getJwtPlugin, - getStoredToken, isPKCERequired, normalizeTimestampValue, parseClientMetadata, @@ -449,36 +449,11 @@ async function encodeRefreshToken( return ( (opts.prefix?.refreshToken ?? "") + (opts.formatRefreshToken?.encrypt - ? opts.formatRefreshToken.encrypt(token, sessionId) + ? await opts.formatRefreshToken.encrypt(token, sessionId) : token) ); } -/** - * Decodes a refresh token for a client - * - * @internal - */ -export async function decodeRefreshToken( - opts: OAuthOptions, - token: string, -) { - if (opts.prefix?.refreshToken) { - if (token.startsWith(opts.prefix.refreshToken)) { - token = token.replace(opts.prefix.refreshToken, ""); - } else { - throw new APIError("BAD_REQUEST", { - error_description: "refresh token not found", - error: "invalid_token", - }); - } - } - - return opts.formatRefreshToken?.decrypt - ? opts.formatRefreshToken?.decrypt(token) - : { token }; -} - async function createOpaqueAccessToken( ctx: GenericEndpointContext, opts: OAuthOptions, @@ -1838,23 +1813,7 @@ async function handleRefreshTokenGrant( error: "invalid_request", }); } - const decodedRefresh = await decodeRefreshToken(opts, refresh_token); - - const refreshToken = await ctx.context.adapter.findOne< - OAuthRefreshToken & { id: string } - >({ - model: "oauthRefreshToken", - where: [ - { - field: "token", - value: await getStoredToken( - opts.storeTokens, - decodedRefresh.token, - "refresh_token", - ), - }, - ], - }); + const refreshToken = await findRefreshToken(ctx, opts, refresh_token); // Check refresh if (!refreshToken) { diff --git a/packages/oauth-provider/src/userinfo.test.ts b/packages/oauth-provider/src/userinfo.test.ts index ac001b0830..756ff71905 100644 --- a/packages/oauth-provider/src/userinfo.test.ts +++ b/packages/oauth-provider/src/userinfo.test.ts @@ -240,28 +240,36 @@ describe("oauth userinfo", async () => { /** * @see https://github.com/better-auth/better-auth/issues/9949 + * @see https://github.com/better-auth/better-auth/issues/11595 */ - it("should return an invalid_token challenge for an unknown bearer token", async () => { - let wwwAuthenticate = ""; - const userinfo = await client.$fetch>( - "/oauth2/userinfo", - { - headers: { - authorization: "Bearer this-is-not-a-valid-access-token", + it.each([ + "this-is-not-a-valid-access-token", + "eyJhbGciOiJub25lIn0.eyJzdWIiOiJ4In0.", + ])( + "should return an invalid_token challenge for an unknown bearer token %s", + async (token) => { + let wwwAuthenticate = ""; + const userinfo = await client.$fetch>( + "/oauth2/userinfo", + { + headers: { + authorization: `Bearer ${token}`, + }, + onError(context) { + wwwAuthenticate = + context.response.headers.get("WWW-Authenticate") ?? ""; + }, }, - onError(context) { - wwwAuthenticate = - context.response.headers.get("WWW-Authenticate") ?? ""; - }, - }, - ); + ); - expect(userinfo.error?.status).toBe(401); - expect( - (userinfo.error as { error?: string; error_description?: string }).error, - ).toBe("invalid_token"); - expect(wwwAuthenticate).toBe(INVALID_ACCESS_TOKEN_CHALLENGE); - }); + expect(userinfo.error?.status).toBe(401); + expect( + (userinfo.error as { error?: string; error_description?: string }) + .error, + ).toBe("invalid_token"); + expect(wwwAuthenticate).toBe(INVALID_ACCESS_TOKEN_CHALLENGE); + }, + ); it("should fail without the openid scope", async () => { const tokens = await getTokens({