mirror of
https://github.com/better-auth/better-auth.git
synced 2026-10-20 20:26:55 -05:00
fix(oauth-provider)!: store pairwise sectors and isolate native clients (#11650)
This commit is contained in:
@@ -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.
|
||||
@@ -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[]",
|
||||
|
||||
@@ -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<SchemaClient<Scope[]>>({
|
||||
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 () => {
|
||||
|
||||
@@ -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 } });
|
||||
|
||||
@@ -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<keyof SchemaClient, true>;
|
||||
@@ -114,6 +115,7 @@ const CIMD_FORBIDDEN_SERVER_FIELD_NAMES = new Set([
|
||||
"resources",
|
||||
"clientSecret",
|
||||
"clientDiscoveryId",
|
||||
"sectorIdentifier",
|
||||
"skipConsent",
|
||||
"enableEndSession",
|
||||
"requirePKCE",
|
||||
|
||||
@@ -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<string, unknown> = {
|
||||
...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;
|
||||
|
||||
@@ -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<Scope[]> = {
|
||||
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<SchemaClient<Scope[]>>({
|
||||
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.
|
||||
|
||||
@@ -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),
|
||||
},
|
||||
|
||||
@@ -35,6 +35,10 @@ export const schema = {
|
||||
type: "string",
|
||||
required: false,
|
||||
},
|
||||
sectorIdentifier: {
|
||||
type: "string",
|
||||
required: false,
|
||||
},
|
||||
scopes: {
|
||||
type: "string[]",
|
||||
required: false,
|
||||
|
||||
@@ -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;
|
||||
/**
|
||||
|
||||
@@ -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<Scope[]>): string {
|
||||
if (client.sectorIdentifier) {
|
||||
return client.sectorIdentifier;
|
||||
}
|
||||
const uri = client.redirectUris?.[0];
|
||||
if (!uri) {
|
||||
throw new BetterAuthError(
|
||||
|
||||
Reference in New Issue
Block a user