From 6e5a88e57b0b5253bc49797cede415ff27a82931 Mon Sep 17 00:00:00 2001 From: Gustavo Valverde Date: Thu, 15 Oct 2026 07:57:45 +0000 Subject: [PATCH] fix(oauth-provider)!: store pairwise sectors and isolate native clients (#11650) --- .changeset/pairwise-stored-sector.md | 7 ++ docs/content/docs/plugins/oauth-provider.mdx | 20 ++- packages/cimd/src/flows.test.ts | 40 ++++++ .../src/backchannel-logout.test.ts | 4 +- .../oauth-provider/src/client-metadata.ts | 2 + .../src/oauthClient/endpoints.ts | 15 ++- packages/oauth-provider/src/pairwise.test.ts | 115 +++++++++++++++++- packages/oauth-provider/src/register.ts | 21 +++- packages/oauth-provider/src/schema.ts | 4 + packages/oauth-provider/src/types/index.ts | 6 + packages/oauth-provider/src/utils/index.ts | 30 ++++- 11 files changed, 251 insertions(+), 13 deletions(-) create mode 100644 .changeset/pairwise-stored-sector.md diff --git a/.changeset/pairwise-stored-sector.md b/.changeset/pairwise-stored-sector.md new file mode 100644 index 0000000000..df8bec4e3f --- /dev/null +++ b/.changeset/pairwise-stored-sector.md @@ -0,0 +1,7 @@ +--- +"@better-auth/oauth-provider": minor +--- + +Pairwise clients with authority-free private-use redirect URIs, such as `com.example.app:/callback`, or loopback redirect URIs, such as `http://localhost:8787/callback`, now each receive a `sub` of their own. Previously, unrelated clients of either kind could receive the same pairwise `sub`. HTTPS clients on the same host still share a `sub`. + +This adds a `sectorIdentifier` column to the `oauthClient` table. Run `npx auth@latest migrate`, or `npx auth@latest generate` for your ORM, before upgrading. Existing clients keep their current `sub`; the new rule applies to clients created after the upgrade. diff --git a/docs/content/docs/plugins/oauth-provider.mdx b/docs/content/docs/plugins/oauth-provider.mdx index 0b92874b13..2bf3d91870 100644 --- a/docs/content/docs/plugins/oauth-provider.mdx +++ b/docs/content/docs/plugins/oauth-provider.mdx @@ -2152,11 +2152,18 @@ const response = await auth.api.createOAuthClient({ #### How It Works -Pairwise identifiers are computed using HMAC-SHA256 over the **sector identifier** (the host of the client's first redirect URI) and the user ID, keyed with `pairwiseSecret`. This means: +Pairwise identifiers are computed using HMAC-SHA256 over the **sector identifier** and the user ID, keyed with `pairwiseSecret`. Better Auth assigns the sector when the client is created: -* Two clients with different redirect URI hosts always receive different `sub` values for the same user -* Two clients sharing the same redirect URI host receive the **same** pairwise `sub` (per OIDC Core Section 8.1) +* A client whose redirect URIs all use `https` gets the host of its first redirect URI, including any port. Clients on the same `https` host share a sector and receive the **same** pairwise `sub` (per OIDC Core Section 8.1). +* Any other client, such as one with an authority-free private-use (`com.example.app:/callback`) or loopback (`http://localhost:8787/callback`) redirect URI, gets a sector of its own. Any app can register a custom scheme, and any local process can listen on a loopback port, so those redirect URIs do not identify the client. + +This means: + +* Two clients with different sectors always receive different `sub` values for the same user * The same client always receives the same `sub` for the same user (deterministic) +* Changing a client's redirect URIs recomputes its sector, so moving to another host changes the `sub` its users receive + +Clients created before Better Auth stored sectors keep the sector of their first redirect URI host, so their users' `sub` does not change. Unrelated custom-scheme or loopback clients created before then can share a sector. Pairwise `sub` appears in: @@ -2240,6 +2247,13 @@ export const oauthClientTableFields = [ 'Subject identifier type for this client. Set to "pairwise" to receive unique, unlinkable sub claims per user. Requires pairwiseSecret to be configured on the server.', isOptional: true, }, + { + name: "sectorIdentifier", + type: "string", + description: + "Pairwise sector assigned by the server when the client is created. Empty for clients created before sectors were stored, which use the host of their first redirect URI.", + isOptional: true, + }, { name: "scopes", type: "string[]", diff --git a/packages/cimd/src/flows.test.ts b/packages/cimd/src/flows.test.ts index e9c1cdee38..97def19a34 100644 --- a/packages/cimd/src/flows.test.ts +++ b/packages/cimd/src/flows.test.ts @@ -918,6 +918,46 @@ describe("CIMD - refresh preserves admin-set flags", async () => { }), ); }); + + /** + * @see https://github.com/better-auth/better-auth/issues/11649 + */ + it("keeps the pairwise sector a client was stored with across a refresh", async () => { + const sectorClientUrl = + "https://mcp-client-refresh.example.com/sector-client.json"; + stubMetadataFetch(sectorClientUrl, { + ...metadataDocument, + client_id: sectorClientUrl, + }); + const { headers } = await signInWithTestUser(); + const ctx = await authorizationServer.$context; + const authorize = () => + customFetchImpl( + buildAuthorizeUrl(authServerBaseUrl, sectorClientUrl, redirectUri), + { headers }, + ); + const sectorOf = async () => + ( + await ctx.adapter.findOne>({ + model: "oauthClient", + where: [{ field: "clientId", value: sectorClientUrl }], + }) + )?.sectorIdentifier ?? null; + + await authorize(); + expect(await sectorOf()).toBe(`client/${sectorClientUrl}`); + await authorize(); + expect(await sectorOf()).toBe(`client/${sectorClientUrl}`); + + // A client stored before sectors were recorded keeps the host rule. + await ctx.adapter.update({ + model: "oauthClient", + where: [{ field: "clientId", value: sectorClientUrl }], + update: { sectorIdentifier: null }, + }); + await authorize(); + expect(await sectorOf()).toBeNull(); + }); }); describe("CIMD - metadata document URL policy", async () => { diff --git a/packages/oauth-provider/src/backchannel-logout.test.ts b/packages/oauth-provider/src/backchannel-logout.test.ts index a772275202..4a928e192b 100644 --- a/packages/oauth-provider/src/backchannel-logout.test.ts +++ b/packages/oauth-provider/src/backchannel-logout.test.ts @@ -664,7 +664,9 @@ describe("oauth back-channel logout", async () => { await ctx.adapter.update({ model: "oauthClient", where: [{ field: "clientId", value: malformedClient.client_id }], - update: { redirectUris: [] }, + // Without a stored sector or a redirect URI, the pairwise sub cannot + // be derived. + update: { redirectUris: [], sectorIdentifier: null }, }); await client.signOut({ fetchOptions: { headers } }); diff --git a/packages/oauth-provider/src/client-metadata.ts b/packages/oauth-provider/src/client-metadata.ts index 527eacc5ad..add0a3362c 100644 --- a/packages/oauth-provider/src/client-metadata.ts +++ b/packages/oauth-provider/src/client-metadata.ts @@ -43,6 +43,7 @@ const OAUTH_CLIENT_RECORD_FIELDS = { skipConsent: true, enableEndSession: true, subjectType: true, + sectorIdentifier: true, referenceId: true, metadata: true, } as const satisfies Record; @@ -114,6 +115,7 @@ const CIMD_FORBIDDEN_SERVER_FIELD_NAMES = new Set([ "resources", "clientSecret", "clientDiscoveryId", + "sectorIdentifier", "skipConsent", "enableEndSession", "requirePKCE", diff --git a/packages/oauth-provider/src/oauthClient/endpoints.ts b/packages/oauth-provider/src/oauthClient/endpoints.ts index 587461ec94..1fc6c99b48 100644 --- a/packages/oauth-provider/src/oauthClient/endpoints.ts +++ b/packages/oauth-provider/src/oauthClient/endpoints.ts @@ -11,7 +11,12 @@ import type { Scope, } from "../types"; import type { GrantType } from "../types/oauth"; -import { getClient, parseClientMetadata, storeClientSecret } from "../utils"; +import { + deriveSectorIdentifier, + getClient, + parseClientMetadata, + storeClientSecret, +} from "../utils"; import { normalizeClientCredentialsScopes, validateClientCredentialsScopes, @@ -364,6 +369,14 @@ export async function updateClientEndpoint( const schemaUpdates: Record = { ...oauthToSchema(updates), }; + // A client created before sectors were stored keeps deriving its sector + // from its redirect URI host, so only recompute a stored sector. + if (updates.redirect_uris && client.sectorIdentifier) { + schemaUpdates.sectorIdentifier = deriveSectorIdentifier( + client.clientId, + updates.redirect_uris, + ); + } if (clearsBackchannelLogout) { schemaUpdates.backchannelLogoutUri = null; schemaUpdates.backchannelLogoutSessionRequired = null; diff --git a/packages/oauth-provider/src/pairwise.test.ts b/packages/oauth-provider/src/pairwise.test.ts index f40790aeb7..140fb6b4b4 100644 --- a/packages/oauth-provider/src/pairwise.test.ts +++ b/packages/oauth-provider/src/pairwise.test.ts @@ -11,12 +11,15 @@ import { decodeJwt } from "jose"; import { beforeAll, describe, expect, it } from "vitest"; import { oauthProviderClient } from "./client"; import { oauthProvider } from "./oauth"; +import type { OAuthOptions, SchemaClient, Scope } from "./types"; import type { OAuthClient } from "./types/oauth"; +import { resolveSubjectIdentifier } from "./utils"; describe("pairwise subject identifiers", async () => { const authServerBaseUrl = "http://localhost:3000"; - const rpBaseUrl = "http://localhost:5000"; - const rpBaseUrl2 = "http://localhost:6000"; + // HTTPS hosts share a pairwise sector; loopback clients each get their own. + const rpBaseUrl = "https://rp-a.example.com"; + const rpBaseUrl2 = "https://rp-b.example.com"; const validResource = "https://myapi.example.com"; const { auth, signInWithTestUser, customFetchImpl } = await getTestInstance({ @@ -222,7 +225,7 @@ describe("pairwise subject identifiers", async () => { const idTokenA = decodeJwt(tokensA.data!.id_token!); const idTokenSameHost = decodeJwt(tokensSameHost.data!.id_token!); - // Same host (localhost) → same sector → same pairwise sub + // Same HTTPS host → same sector → same pairwise sub expect(idTokenA.sub).toBe(idTokenSameHost.sub); }); @@ -486,6 +489,112 @@ describe("pairwise DCR validation", async () => { }); }); +/** + * @see https://github.com/better-auth/better-auth/issues/11649 + */ +describe("stored pairwise sector", async () => { + const opts: OAuthOptions = { + loginPage: "/login", + consentPage: "/consent", + pairwiseSecret: "test-pairwise-secret-key-32chars!!", + }; + const { auth, signInWithTestUser } = await getTestInstance({ + baseURL: "http://localhost:3000", + plugins: [jwt(), oauthProvider(opts)], + }); + const { headers, user } = await signInWithTestUser(); + const adapter = (await auth.$context).adapter; + + const storedClient = async (clientId: string) => { + const client = await adapter.findOne>({ + model: "oauthClient", + where: [{ field: "clientId", value: clientId }], + }); + if (!client) throw new Error(`client ${clientId} not found`); + return client; + }; + const sub = async (clientId: string) => + resolveSubjectIdentifier("user-1", await storedClient(clientId), opts); + const createPairwise = async ( + redirectUris: string[], + applicationType: "web" | "native" = "native", + ) => { + const client = await auth.api.adminCreateOAuthClient({ + headers, + body: { + redirect_uris: redirectUris, + application_type: applicationType, + subject_type: "pairwise", + }, + }); + return client.client_id; + }; + const insertLegacyClient = async (clientId: string, redirectUri: string) => { + await adapter.create({ + model: "oauthClient", + data: { + clientId, + redirectUris: [redirectUri], + subjectType: "pairwise", + userId: user.id, + createdAt: new Date(), + updatedAt: new Date(), + }, + }); + }; + const updateRedirectUris = (clientId: string, redirectUris: string[]) => + auth.api.adminUpdateOAuthClient({ + headers, + body: { client_id: clientId, update: { redirect_uris: redirectUris } }, + }); + const sectorOf = async (clientId: string) => + (await storedClient(clientId)).sectorIdentifier ?? null; + + it.each([ + ["authority-free", "com.example.app:/callback", "com.other.app:/callback"], + [ + "same-port loopback", + "http://localhost:8787/callback", + "http://localhost:8787/callback", + ], + ])( + "gives new %s native clients a sector of their own", + async (_, uriA, uriB) => { + const a = await createPairwise([uriA]); + const b = await createPairwise([uriB]); + expect(await sub(a)).not.toBe(await sub(b)); + }, + ); + + it("keeps a shared sector for new HTTPS clients on the same host", async () => { + const a = await createPairwise(["https://rp.example.com/a"], "web"); + const b = await createPairwise(["https://rp.example.com/b"], "web"); + expect(await sectorOf(a)).toBe("rp.example.com"); + expect(await sub(a)).toBe(await sub(b)); + }); + + it("keeps the host-derived sector for clients stored without one", async () => { + await insertLegacyClient("legacy-a", "com.example.app:/callback"); + await insertLegacyClient("legacy-b", "com.other.app:/callback"); + expect(await sectorOf("legacy-a")).toBeNull(); + expect(await sub("legacy-a")).toBe(await sub("legacy-b")); + }); + + it("recomputes the sector when redirect URIs move to another HTTPS host", async () => { + const clientId = await createPairwise(["https://rp.example.com/cb"], "web"); + await updateRedirectUris(clientId, ["https://other.example.com/cb"]); + expect(await sectorOf(clientId)).toBe("other.example.com"); + }); + + it("does not assign a sector to a client stored without one when it is updated", async () => { + await insertLegacyClient("legacy-update", "https://legacy.example.com/cb"); + await updateRedirectUris("legacy-update", [ + "https://legacy.example.com/other", + ]); + expect(await sectorOf("legacy-update")).toBeNull(); + }); +}); + /** * A custom URI scheme is not bound to the host in its authority, so that host * cannot identify a pairwise sector. diff --git a/packages/oauth-provider/src/register.ts b/packages/oauth-provider/src/register.ts index 1b5ec398e6..d615de8ab9 100644 --- a/packages/oauth-provider/src/register.ts +++ b/packages/oauth-provider/src/register.ts @@ -36,7 +36,11 @@ import type { } from "./types"; import type { GrantType, OAuthClient } from "./types/oauth"; import { clientRegistrationRequestSchema } from "./types/zod"; -import { parseClientMetadata, storeClientSecret } from "./utils"; +import { + deriveSectorIdentifier, + parseClientMetadata, + storeClientSecret, +} from "./utils"; import { isPrivateHostname } from "./utils/client-assertion"; import { authorizeInitialAccessToken } from "./utils/initial-access-token"; @@ -589,9 +593,10 @@ export async function checkOAuthClient( "pairwise subject_type requires server pairwiseSecret configuration", }); } - // The pairwise sector is the redirect URI host. Any app can register a - // custom scheme, so the host in `app://rp.example.com/callback` does not - // identify the client and would share an HTTPS client's sector. + // A client stored without a sector still uses its redirect URI host as + // the pairwise sector, and this check cannot tell those clients apart. + // Any app can register a custom scheme, so the host in + // `app://rp.example.com/callback` does not identify the client. if ( clientWithDefaults.subject_type === "pairwise" && clientWithDefaults.redirect_uris?.some(isCustomSchemeWithHost) @@ -1013,6 +1018,10 @@ async function persistOAuthClientRegistration( input.registrationSource === "clientMetadataDocument" ? input.clientDiscoveryId : null; + schema.sectorIdentifier = deriveSectorIdentifier( + clientId, + body.redirect_uris, + ); if ( input.registrationSource === "clientMetadataDocument" && body.application_type === undefined @@ -1090,6 +1099,10 @@ async function persistOAuthClientRegistration( disabled: existingClient.disabled, skipConsent: existingClient.skipConsent, enableEndSession: existingClient.enableEndSession, + // A client stored without a sector keeps the host-derived one. + sectorIdentifier: existingClient.sectorIdentifier + ? schema.sectorIdentifier + : null, createdAt: existingClient.createdAt, updatedAt: new Date(iat * 1000), }, diff --git a/packages/oauth-provider/src/schema.ts b/packages/oauth-provider/src/schema.ts index a59a3d24b9..910ef1daf3 100644 --- a/packages/oauth-provider/src/schema.ts +++ b/packages/oauth-provider/src/schema.ts @@ -35,6 +35,10 @@ export const schema = { type: "string", required: false, }, + sectorIdentifier: { + type: "string", + required: false, + }, scopes: { type: "string[]", required: false, diff --git a/packages/oauth-provider/src/types/index.ts b/packages/oauth-provider/src/types/index.ts index f221379cbd..29419bc69a 100644 --- a/packages/oauth-provider/src/types/index.ts +++ b/packages/oauth-provider/src/types/index.ts @@ -2079,6 +2079,12 @@ export interface SchemaClient< enableEndSession?: boolean; /** Subject identifier type: "public" (default) or "pairwise" */ subjectType?: "public" | "pairwise"; + /** + * Pairwise sector, set by the server when the client is created. `null` + * for clients created before sectors were stored; those keep the sector + * derived from their first redirect URI host. + */ + sectorIdentifier?: string | null; /** Reference to the owner of this client. Eg. Organization, Team, Profile */ referenceId?: string; /** diff --git a/packages/oauth-provider/src/utils/index.ts b/packages/oauth-provider/src/utils/index.ts index 75877217aa..e4a971b6ad 100644 --- a/packages/oauth-provider/src/utils/index.ts +++ b/packages/oauth-provider/src/utils/index.ts @@ -1053,12 +1053,40 @@ export function parsePrompt(prompt: string) { } /** - * Extracts the sector identifier (hostname) from a client's first redirect URI. + * Derives the pairwise sector to store for a client. + * + * Only an `https` host identifies who controls a redirect URI, so clients + * whose redirect URIs are all `https` share the sector of that host. Any app + * can register a custom scheme, and any local process can listen on a + * loopback port, so every other client gets a sector of its own. `/` cannot + * appear in a URL host, so a client sector never equals a host sector. + * + * @see https://openid.net/specs/openid-connect-core-1_0.html#PairwiseAlg + * @internal + */ +export function deriveSectorIdentifier( + clientId: string, + redirectUris: readonly string[] | undefined, +): string { + // Callers validate every redirect URI first, so parsing cannot throw. + const urls = (redirectUris ?? []).map((uri) => new URL(uri)); + const first = urls[0]; + return first && urls.every((url) => url.protocol === "https:") + ? first.host + : `client/${clientId}`; +} + +/** + * Returns the client's stored pairwise sector, or for a client created before + * sectors were stored, the host of its first redirect URI. * * @see https://openid.net/specs/openid-connect-core-1_0.html#PairwiseAlg * @internal */ function getSectorIdentifier(client: SchemaClient): string { + if (client.sectorIdentifier) { + return client.sectorIdentifier; + } const uri = client.redirectUris?.[0]; if (!uri) { throw new BetterAuthError(