From d46a6b4e260741dd9e54293f372e6d58b0f97534 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 5 Oct 2026 11:14:28 +0000 Subject: [PATCH 01/10] wip(plugin-auth): implicit account linking gate Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_018zT8d8NpiQ1ExhuNd5TxY6 --- .../plugins/plugin-auth/src/auth-manager.ts | 115 ++++++-- .../src/implicit-account-linking.ts | 262 ++++++++++++++++++ 2 files changed, 352 insertions(+), 25 deletions(-) create mode 100644 packages/plugins/plugin-auth/src/implicit-account-linking.ts diff --git a/packages/plugins/plugin-auth/src/auth-manager.ts b/packages/plugins/plugin-auth/src/auth-manager.ts index 9d24bb3fe3..b01d4e73a3 100644 --- a/packages/plugins/plugin-auth/src/auth-manager.ts +++ b/packages/plugins/plugin-auth/src/auth-manager.ts @@ -30,6 +30,12 @@ import { type VerificationDeclarant, } from './audience-posture.js'; import { shouldStampOwnerVerifiedAtCreation } from './walled-owner-operator-stamp.js'; +import { + PLATFORM_IDP_PROVIDER_ID, + clearUnlinkTombstone, + recordUnlinkTombstone, + refuseImplicitAccountLink, +} from './implicit-account-linking.js'; import type { IDataEngine } from '@objectstack/core'; // [#10348] The ONE id-shaped platform-admin predicate (ADR-0068 D2). // `auth-manager` used to re-derive that standing itself, in two spellings @@ -1590,41 +1596,48 @@ export class AuthManager { // by vendor construction. One seam, one owner — see // `audience-posture.ts` for the decision semantics, the pinned // domain-matching rules, and the creation-class table. - validateUserInfo: (data: any, ctx: any) => this.validateAudienceAdmission(data, ctx), + // + // The same seam also carries the implicit-account-linking gate: the + // vendor calls it with `action: 'link-account'` right before an + // implicit link is written — see `implicit-account-linking.ts`. The + // audience gate judges only `create-user`, so the two never answer + // the same call. + validateUserInfo: async (data: any, ctx: any) => + (await refuseImplicitAccountLink(data, ctx, { + requireLocalEmailVerified: this.implicitLinkRequiresLocalEmailVerified(), + logInfo: (m, meta) => this.config.logger?.info?.(m, meta), + })) ?? this.validateAudienceAdmission(data, ctx), }, account: { ...AUTH_ACCOUNT_CONFIG, - // Allow OIDC/OAuth callbacks to implicitly link the incoming - // identity to a pre-existing local user when the emails match. - // - // ObjectStack's platform SSO ("objectstack-cloud" provider) is the - // canonical case: cloud is the IdP for every project, so a user - // arriving via SSO is — by construction — the same person who was - // auto-seeded as the project owner when the project was created. - // Without trusting the provider, better-auth's safety check rejects - // the link with `error=account_not_linked` because the seeded user - // row has `emailVerified=false` (no actual verification ever runs - // in the IdP-mediated flow). See packages/plugins/plugin-auth/ - // node_modules/better-auth/dist/oauth2/link-account.mjs:22. + // Implicit account linking: an OAuth / OIDC / SSO sign-in whose + // identity is not linked yet, but whose email matches an existing + // local user, links to that user. The rules — every provider needs a + // VERIFIED local row, except the platform's own cloud IdP + // (`PLATFORM_IDP_PROVIDER_ID`, whose owner-seeded rows are born + // `emailVerified=false`), and a user's unlink is honoured — are + // enforced in `refuseImplicitAccountLink` (the `validateUserInfo` + // seam above). Why there, and why the vendor flag below is pinned + // `false` by default: `implicit-account-linking.ts`, module header. // // Custom-deployment consumers can extend the trusted set via // `config.account.accountLinking.trustedProviders`; we always // include `objectstack-cloud` because it is the platform IdP. + // Trust relaxes only the IdP-side `email_verified` clause; it never + // relaxes the local-verification requirement. accountLinking: { enabled: true, - // better-auth's account-linking gate has TWO independent clauses - // (see link-account.mjs:22). Trusting the provider only satisfies - // the first clause; the second — `requireLocalEmailVerified && - // !dbUser.user.emailVerified` — still blocks linking when the - // pre-existing local user row has `emailVerified=false` (the - // default for owner-seeded rows). Disabling the local-email gate - // is safe here because the OAuth side is what we actually trust: - // the incoming identity was verified by the IdP. Consumers who - // need the stricter behavior can override via config. - requireLocalEmailVerified: false, ...((this.config as any)?.account?.accountLinking ?? {}), + // better-auth's flag is ONE global boolean with no per-provider + // form, so it cannot carry the cloud exception: `true` would refuse + // the cloud owner row before our gate runs. It is therefore `false` + // unless the operator explicitly asked for the strict form for + // EVERY provider (`true`), and the requirement itself is enforced + // by our gate (`implicitLinkRequiresLocalEmailVerified`). + requireLocalEmailVerified: + (this.config as any)?.account?.accountLinking?.requireLocalEmailVerified === true, trustedProviders: Array.from(new Set([ - 'objectstack-cloud', + PLATFORM_IDP_PROVIDER_ID, ...((this.config as any)?.account?.accountLinking?.trustedProviders ?? []), ])), }, @@ -4473,6 +4486,16 @@ export class AuthManager { else logger?.warn?.(message, meta); } + /** + * The effective local-verification requirement for an implicit account + * link — `true` unless the operator explicitly set + * `account.accountLinking.requireLocalEmailVerified: false`. See + * `implicit-account-linking.ts` for the rules and the operator override. + */ + private implicitLinkRequiresLocalEmailVerified(): boolean { + return (this.config as any)?.account?.accountLinking?.requireLocalEmailVerified !== false; + } + /** OAuth providerIds that are OPERATOR-REGISTERED identity authorities (enterprise `oidcProviders`, incl. the cloud platform IdP). */ private enterpriseOAuthProviderIds(): ReadonlySet { const ids = new Set(); @@ -7325,7 +7348,24 @@ export class AuthManager { private composeDatabaseHooks( host?: BetterAuthOptions['databaseHooks'], ): BetterAuthOptions['databaseHooks'] { - const stamp = (account: any, ctx: any) => this.stampIdentitySource(account, ctx); + // A landed link ends a standing unlink record (implicit-account-linking.ts, + // rule 3). While the record stands an implicit link is refused, so a link + // that lands here is an explicit one. Failure to clear is functional (the + // user sees a refusal on their next implicit sign-in and can re-link), so + // it is reported at `warn` and never fails the link. + const clearUnlink = async (account: any, ctx: any) => { + try { + await clearUnlinkTombstone(account, ctx); + } catch (e) { + this.config.logger?.warn?.('[auth] could not clear the unlink record after a link', { + error: (e as Error)?.message, + }); + } + }; + const stamp = async (account: any, ctx: any) => { + await this.stampIdentitySource(account, ctx); + await clearUnlink(account, ctx); + }; const hostAccountAfter = (host as any)?.account?.create?.after; const after = hostAccountAfter ? async (account: any, ctx: any) => { @@ -7334,6 +7374,27 @@ export class AuthManager { } : stamp; + // A user's unlink is recorded so the provider cannot re-link implicitly + // (implicit-account-linking.ts, rule 3). A record that does not land + // leaves the unlink answering 200 while the provider still re-links on + // the next sign-in — the protection the user asked for is silently + // absent — so the failure is reported at `error`, once per unlink. + const hostAccountDeleteAfter = (host as any)?.account?.delete?.after; + const accountDeleteAfter = async (account: any, ctx: any) => { + const result = hostAccountDeleteAfter ? await hostAccountDeleteAfter(account, ctx) : undefined; + try { + await recordUnlinkTombstone(account, ctx); + } catch (e) { + this.audienceLogError( + '[auth] unlink record NOT written: the provider can still re-link this account implicitly ' + + 'on its next sign-in although the unlink answered success. Check the auth store ' + + '(sys_verification) is writable, then have the user unlink again.', + { providerId: account?.providerId, error: (e as Error)?.message }, + ); + } + return result; + }; + // ADR-0093 D9 — default active-org on session create. Without it, a user // with memberships logs in with `activeOrganizationId = null`: better-auth // org endpoints can't resolve an active org (single-org invite dead-end) @@ -7537,6 +7598,10 @@ export class AuthManager { ...((host as any)?.account?.create ?? {}), after, }, + delete: { + ...((host as any)?.account?.delete ?? {}), + after: accountDeleteAfter, + }, }, user: { ...((host as any)?.user ?? {}), diff --git a/packages/plugins/plugin-auth/src/implicit-account-linking.ts b/packages/plugins/plugin-auth/src/implicit-account-linking.ts new file mode 100644 index 0000000000..88dee0a69a --- /dev/null +++ b/packages/plugins/plugin-auth/src/implicit-account-linking.ts @@ -0,0 +1,262 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Implicit account linking — when an external sign-in may attach itself to a + * pre-existing local user. + * + * "Implicit" linking is better-auth's behaviour on an OAuth / OIDC / SSO + * sign-in whose identity is not yet linked to anyone but whose email matches + * an existing local user: the library links the identity to that user and + * signs the caller in as them. It is the opposite of an EXPLICIT link, which a + * signed-in user starts themselves (`POST /link-social`) and which is + * therefore authenticated by the user's own session. + * + * ## The rules (maintainer ruling recorded on the card: 「算漏洞，收紧」) + * + * 1. **Every provider meets the library's standard local-ownership + * requirement** before an implicit link: the existing local row must be + * `emailVerified: true`. Without it, anyone who can register an + * UNVERIFIED local row at a victim's address (open self-registration) + * gets the victim's external identity linked into the row they control — + * and the link then flips that row to verified. better-auth names this + * exact case as the reason `requireLocalEmailVerified` defaults to `true`. + * 2. **The platform's own cloud identity provider keeps its documented + * exception** ({@link PLATFORM_IDP_PROVIDER_ID}). The cloud is the IdP + * for every environment, and the environment's owner row is seeded by the + * cloud itself with `emailVerified: false` (no mailbox round-trip ever + * runs in that IdP-mediated flow). Requiring local verification there + * would lock every owner out of their own environment. + * 3. **A user's unlink is honoured.** After a user unlinks provider P, an + * implicit sign-in through P must not quietly re-create the link — that + * would make unlinking decorative. The unlink is recorded + * ({@link unlinkTombstoneIdentifier}); implicit linking for that + * user + provider is refused while the record stands; an EXPLICIT, + * session-authenticated link-social is still allowed and clears it. + * This applies to every provider, the cloud one included: the cloud + * exception is about rule 1's verification precondition, not about + * overriding what the user asked for. + * + * ## Why the enforcement point is `user.validateUserInfo` + * + * Measured against the installed better-auth `1.7.3` + * (`dist/oauth2/link-account.mjs`, `handleOAuthUserInfo`): + * + * - The library's own gate refuses an implicit link when + * `(!trusted && !idp.emailVerified) || (requireLocalEmailVerified && + * !local.emailVerified) || enabled === false || disableImplicitLinking`. + * `requireLocalEmailVerified` is ONE global boolean: there is no + * per-provider form of it, and `trustedProviders` does not relax it. So + * the vendor flag cannot express rule 2 — `true` locks the cloud owner + * out, `false` (what this platform used to ship) drops rule 1 for every + * provider. + * - Immediately after that gate, and BEFORE `internalAdapter.linkAccount` + * and the `emailVerified: true` flip, the same function calls + * `assertValidUserInfo` with `source.action: 'link-account'` and the + * provider id. A refusal there aborts the link (the error is an + * `APIError`, rethrown; the OAuth callback turns it into the error + * redirect). That is the narrowest seam the library offers that sees the + * provider id: one call site per implicit link, on every entry that links + * implicitly (`/callback/:id`, `/sign-in/social` with an id token, + * one-tap, oauth-proxy, and the SSO plugin, which share the function). + * - The EXPLICIT link (`/link-social` → `/callback/:id` with `link` in the + * OAuth state) passes the same `action: 'link-account'`. It is told apart + * by the OAuth state the callback parsed (`getOAuthState().link`), which + * the server writes from the session at `/link-social`; `generateState` + * places `link` AFTER the client-supplied `additionalData`, so a client + * cannot forge it. + * + * So the vendor flag is pinned `false` (it would otherwise refuse the cloud + * owner before our gate runs) and rule 1 is enforced here for everyone else. + * ⛔ better-auth marks `requireLocalEmailVerified` deprecated, "the gate will + * become unconditional" on its next minor. When that lands the vendor gate + * refuses the cloud owner row on its own, and rule 2 needs a different + * carrier (the owner seed); the cloud-exception test in + * `implicit-account-linking.test.ts` goes red on that upgrade, on purpose. + * + * ## Operator override + * + * `config.account.accountLinking.requireLocalEmailVerified`: + * - unset (default) → rules 1 + 2 as above; + * - `true` → handed to better-auth as well: the vendor gate applies to EVERY + * provider, the cloud one included (the operator asked for the strict + * form); + * - `false` → rule 1 is switched off (the pre-tightening behaviour, which + * the library documents as a takeover risk). Rule 3 still applies. + */ + +/** + * The provider id of the platform's own identity provider (cloud-as-IdP). + * Always in `trustedProviders`, and the one provider exempt from the + * local-verification precondition — see the module header, rule 2. + */ +export const PLATFORM_IDP_PROVIDER_ID = 'objectstack-cloud'; + +/** + * The refusal code. Byte-identical to the code better-auth's own implicit-link + * refusal produces on the OAuth callback redirect (`handleOAuthUserInfo` + * answers `"account not linked"`, which the callback rewrites to + * `error=account_not_linked`), so a sign-in page cannot tell the library's + * refusal and this one apart — they ARE the same rule. + */ +export const IMPLICIT_LINK_REFUSED = 'account_not_linked'; + +/** The `sys_verification.identifier` namespace for unlink records. */ +const UNLINK_TOMBSTONE_PREFIX = 'account-unlinked'; + +/** + * The unlink record's lifetime. A record must outlive any realistic gap + * between an unlink and the next sign-in; it ends earlier only when the user + * re-links explicitly. better-auth's verification store sweeps rows by + * `expiresAt`, so the value has to be finite. + */ +const UNLINK_TOMBSTONE_TTL_MS = 100 * 365 * 24 * 60 * 60 * 1000; + +/** The provider that never takes part in linking (email + password). */ +const CREDENTIAL_PROVIDER_ID = 'credential'; + +export function unlinkTombstoneIdentifier(userId: string, providerId: string): string { + return `${UNLINK_TOMBSTONE_PREFIX}:${userId}:${providerId}`; +} + +export interface ImplicitLinkInput { + providerId: string; + /** The EXISTING local user row's `emailVerified`. */ + localEmailVerified: boolean; + /** The effective local-verification requirement (default `true`). */ + requireLocalEmailVerified: boolean; + /** Whether the user unlinked this provider and has not re-linked it explicitly. */ + unlinkedByUser: boolean; +} + +export type ImplicitLinkVerdict = + | { allow: true } + | { allow: false; reason: 'unlinked-by-user' | 'local-email-unverified' }; + +/** Pure decision for one implicit link. See the module header for the rules. */ +export function decideImplicitLink(input: ImplicitLinkInput): ImplicitLinkVerdict { + if (input.unlinkedByUser) return { allow: false, reason: 'unlinked-by-user' }; + if ( + input.requireLocalEmailVerified && + !input.localEmailVerified && + input.providerId !== PLATFORM_IDP_PROVIDER_ID + ) { + return { allow: false, reason: 'local-email-unverified' }; + } + return { allow: true }; +} + +/** The provider id a `validateUserInfo` source names, for any linking method. */ +export function linkSourceProviderId(source: unknown): string | undefined { + const s = source as { oauth?: { providerId?: unknown }; sso?: { providerId?: unknown } } | undefined; + const id = s?.oauth?.providerId ?? s?.sso?.providerId; + return typeof id === 'string' && id.length > 0 ? id : undefined; +} + +/** The slice of better-auth's `internalAdapter` this module uses. */ +interface LinkingInternalAdapter { + findUserById(id: string): Promise<{ emailVerified?: boolean } | null>; + findVerificationValue(identifier: string): Promise; + createVerificationValue(data: { identifier: string; value: string; expiresAt: Date }): Promise; + deleteVerificationByIdentifier(identifier: string): Promise; +} + +const internalAdapterOf = (ctx: unknown): LinkingInternalAdapter | undefined => + (ctx as { context?: { internalAdapter?: LinkingInternalAdapter } } | undefined)?.context?.internalAdapter; + +/** + * Is the current request the callback of an EXPLICIT link (`/link-social`)? + * Reads the OAuth state the callback parsed; see the module header for why + * `link` cannot be supplied by the client. + */ +async function isExplicitLinkFlow(): Promise { + try { + const { getOAuthState } = await import('better-auth/api'); + const state = (await getOAuthState()) as { link?: { userId?: unknown } } | null; + return typeof state?.link?.userId === 'string'; + } catch { + // No request state (not inside an OAuth flow) ⇒ not an explicit link. + return false; + } +} + +export interface ImplicitLinkGateOptions { + requireLocalEmailVerified: boolean; + logInfo?: (message: string, meta?: Record) => void; +} + +/** + * The `validateUserInfo` half: answers a refusal for an implicit link the + * rules forbid, `undefined` otherwise (including for every action that is not + * `link-account`, and for an explicit link). Throws — and better-auth's + * `assertValidUserInfo` turns a throw into a FORBIDDEN refusal, failing closed + * — when the store cannot answer. + */ +export async function refuseImplicitAccountLink( + data: { user?: Record; source?: { action?: string } } | undefined, + ctx: unknown, + options: ImplicitLinkGateOptions, +): Promise<{ error: string; errorDescription: string } | undefined> { + if (data?.source?.action !== 'link-account') return undefined; + if (await isExplicitLinkFlow()) return undefined; + const providerId = linkSourceProviderId(data.source); + const userId = typeof data.user?.id === 'string' ? (data.user.id as string) : undefined; + const adapter = internalAdapterOf(ctx); + if (!providerId || !userId || !adapter) { + throw new Error('implicit account link: provider, user or store unavailable — refusing'); + } + const tombstone = await adapter.findVerificationValue(unlinkTombstoneIdentifier(userId, providerId)); + const local = await adapter.findUserById(userId); + if (!local) throw new Error('implicit account link: local user not found — refusing'); + const verdict = decideImplicitLink({ + providerId, + localEmailVerified: local.emailVerified === true, + requireLocalEmailVerified: options.requireLocalEmailVerified, + unlinkedByUser: tombstone != null, + }); + if (verdict.allow) return undefined; + options.logInfo?.('[auth] implicit account link refused', { providerId, reason: verdict.reason }); + return { + error: IMPLICIT_LINK_REFUSED, + errorDescription: + verdict.reason === 'unlinked-by-user' + ? 'This sign-in method was unlinked from the account. Sign in another way and link it again from your account settings.' + : 'An account with this email already exists and its email address is not verified. Sign in to that account and link this sign-in method from your account settings.', + }; +} + +/** + * `account.delete.after` half: a user's own unlink (`/unlink-account`) + * leaves a record that keeps the provider from re-linking implicitly. Other + * deletions (user removal, admin tooling) leave none. + */ +export async function recordUnlinkTombstone(account: unknown, ctx: unknown): Promise { + const a = account as { userId?: unknown; providerId?: unknown } | null; + const path = (ctx as { path?: unknown } | undefined)?.path; + if (path !== '/unlink-account') return; + if (typeof a?.userId !== 'string' || typeof a?.providerId !== 'string') return; + if (a.providerId === CREDENTIAL_PROVIDER_ID) return; + const adapter = internalAdapterOf(ctx); + if (!adapter) throw new Error('unlink record: store unavailable'); + const identifier = unlinkTombstoneIdentifier(a.userId, a.providerId); + await adapter.deleteVerificationByIdentifier(identifier); + await adapter.createVerificationValue({ + identifier, + value: JSON.stringify({ userId: a.userId, providerId: a.providerId, unlinkedAt: new Date().toISOString() }), + expiresAt: new Date(Date.now() + UNLINK_TOMBSTONE_TTL_MS), + }); +} + +/** + * `account.create.after` half: a link that lands clears the unlink record. + * While the record stands an implicit link is refused, so a link that lands + * is an explicit one (or an operator act) — exactly the "until the user + * re-links" end the ruling sets. + */ +export async function clearUnlinkTombstone(account: unknown, ctx: unknown): Promise { + const a = account as { userId?: unknown; providerId?: unknown } | null; + if (typeof a?.userId !== 'string' || typeof a?.providerId !== 'string') return; + if (a.providerId === CREDENTIAL_PROVIDER_ID) return; + const adapter = internalAdapterOf(ctx); + if (!adapter) return; + await adapter.deleteVerificationByIdentifier(unlinkTombstoneIdentifier(a.userId, a.providerId)); +} From 2debeefece91aece02b8f97eea734c5f7c3f71f0 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 5 Oct 2026 11:21:07 +0000 Subject: [PATCH 02/10] test(plugin-auth): pin implicit account linking conditions Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_018zT8d8NpiQ1ExhuNd5TxY6 --- .../src/implicit-account-linking.test.ts | 383 ++++++++++++++++++ 1 file changed, 383 insertions(+) create mode 100644 packages/plugins/plugin-auth/src/implicit-account-linking.test.ts diff --git a/packages/plugins/plugin-auth/src/implicit-account-linking.test.ts b/packages/plugins/plugin-auth/src/implicit-account-linking.test.ts new file mode 100644 index 0000000000..f00115f85d --- /dev/null +++ b/packages/plugins/plugin-auth/src/implicit-account-linking.test.ts @@ -0,0 +1,383 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// Implicit account linking on external sign-in — see +// `implicit-account-linking.ts` for the rules this file pins. +// +// Two layers: +// +// 1. PURE decision — `decideImplicitLink` over each condition. +// 2. END of the chain — a real better-auth pipeline over the in-memory +// engine, driving a real OAuth round trip (`/sign-in/social` → +// `/callback/:id`) through generic-OAuth providers whose token and +// userinfo endpoints are stubbed. The assertions read what landed: the +// `sys_account` rows, the local row's `email_verified`, and the redirect +// the callback answered — never only a status. + +import { afterEach, beforeEach, describe, it, expect, vi } from 'vitest'; +import { assertEngineDeleteDispatch, assertEngineUpdateDispatch, assertEngineFindOnePredicate } from '@objectstack/objectql'; +import { AuthManager } from './auth-manager'; +import { + IMPLICIT_LINK_REFUSED, + PLATFORM_IDP_PROVIDER_ID, + decideImplicitLink, + linkSourceProviderId, + unlinkTombstoneIdentifier, +} from './implicit-account-linking'; + +// ── In-memory IDataEngine (the audience-posture harness shape) ─────────────── + +const createMemoryEngine = () => { + const tables = new Map(); + const rows = (name: string) => { + if (!tables.has(name)) tables.set(name, []); + return tables.get(name)!; + }; + const eq = (a: any, b: any) => + a instanceof Date || b instanceof Date + ? new Date(a as any).getTime() === new Date(b as any).getTime() + : a === b; + const matches = (row: any, where: Record = {}) => + Object.entries(where).every(([k, v]) => { + if (k.startsWith('$')) throw new Error(`fake driver: unsupported operator ${k}`); + const actual = row[k]; + if (v && typeof v === 'object' && !Array.isArray(v) && !(v instanceof Date)) { + if ('$ne' in v) return !eq(actual, v.$ne); + if ('$in' in v) return (v.$in as any[]).some((x) => eq(actual, x)); + if ('$gt' in v) return actual > v.$gt; + if ('$gte' in v) return actual >= v.$gte; + if ('$lt' in v) return actual < v.$lt; + if ('$lte' in v) return actual <= v.$lte; + } + return eq(actual, v); + }); + const project = (row: any, fields?: string[]) => { + if (!Array.isArray(fields) || fields.length === 0) return { ...row }; + const out: any = {}; + for (const f of ['id', ...fields]) if (f in row) out[f] = row[f]; + return out; + }; + let seq = 0; + return { + tables, + async insert(name: string, data: any) { + const row = { id: data.id ?? `row_${++seq}`, ...data }; + rows(name).push(row); + return { ...row }; + }, + async findOne(name: string, q: any = {}) { + assertEngineFindOnePredicate(name, q); + const row = rows(name).find((r) => matches(r, q.where)); + return row ? project(row, q.fields) : null; + }, + async find(name: string, q: any = {}) { + let out = rows(name).filter((r) => matches(r, q.where)); + if (q.offset) out = out.slice(q.offset); + if (typeof q.limit === 'number') out = out.slice(0, q.limit); + return out.map((r) => project(r, q.fields)); + }, + async count(name: string, q: any = {}) { + return rows(name).filter((r) => matches(r, q.where)).length; + }, + async update(name: string, patch: any, options?: any) { + assertEngineUpdateDispatch(patch, options); + const row = rows(name).find((r) => r.id === patch.id); + if (!row) return null; + Object.assign(row, patch); + return { ...row }; + }, + async delete(name: string, q: any = {}) { + assertEngineDeleteDispatch(q); + const table = rows(name); + const keep = table.filter((r) => !matches(r, q.where)); + tables.set(name, keep); + return table.length - keep.length; + }, + }; +}; + +const SECRET = 'test-secret-at-least-32-chars-long!!'; +const PASSWORD = 'S3cure!Passw0rd-link'; +const BASE = 'http://localhost:3000'; +const AFTER = `${BASE}/after-sign-in`; +const IDP = 'https://idp.example.test'; +const EXTERNAL = 'acme-idp'; + +const provider = (providerId: string) => ({ + providerId, + clientId: `${providerId}-client`, + clientSecret: `${providerId}-secret`, + authorizationUrl: `${IDP}/${providerId}/authorize`, + tokenUrl: `${IDP}/${providerId}/token`, + userInfoUrl: `${IDP}/${providerId}/userinfo`, + pkce: false, +}); + +const makeManager = (engine: any, config: Record = {}) => + new AuthManager({ + secret: SECRET, + baseUrl: BASE, + dataEngine: engine, + oidcProviders: [provider(EXTERNAL), provider(PLATFORM_IDP_PROVIDER_ID)], + ...config, + } as any); + +// ── The stubbed identity provider ──────────────────────────────────────────── + +/** What the stubbed IdP asserts about the person signing in, per provider. */ +let idpProfile: Record = {}; + +const realFetch = globalThis.fetch; +beforeEach(() => { + idpProfile = {}; + vi.stubGlobal('fetch', async (input: any, init?: any) => { + const url = typeof input === 'string' ? input : input instanceof URL ? input.href : input.url; + if (!url.startsWith(IDP)) return realFetch(input, init); + const providerId = new URL(url).pathname.split('/')[1]!; + if (url.endsWith('/token')) { + return new Response( + JSON.stringify({ access_token: `at-${providerId}`, token_type: 'Bearer', expires_in: 3600 }), + { status: 200, headers: { 'Content-Type': 'application/json' } }, + ); + } + if (url.endsWith('/userinfo')) { + const profile = idpProfile[providerId]; + if (!profile) return new Response('{}', { status: 401 }); + return new Response(JSON.stringify({ ...profile, id: profile.sub, name: 'Synthetic Person' }), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }); + } + return new Response('not found', { status: 404 }); + }); +}); +afterEach(() => { + vi.unstubAllGlobals(); +}); + +// ── HTTP helpers ───────────────────────────────────────────────────────────── + +const cookiesFrom = (response: Response): string[] => + (response.headers.getSetCookie?.() ?? [response.headers.get('set-cookie') ?? '']) + .map((c) => c.split(';')[0]!) + .filter(Boolean); + +const joinCookies = (...sets: string[][]) => sets.flat().filter(Boolean).join('; '); + +const post = (manager: AuthManager, path: string, body: unknown, cookie?: string) => + manager.handleRequest( + new Request(`${BASE}/api/v1/auth/${path}`, { + method: 'POST', + headers: { 'Content-Type': 'application/json', ...(cookie ? { cookie } : {}) }, + body: JSON.stringify(body), + }), + ); + +/** Sign up with email + password; answers the session cookies. */ +const signUp = async (manager: AuthManager, email: string): Promise => { + const res = await post(manager, 'sign-up/email', { email, password: PASSWORD, name: 'Synthetic Person' }); + expect(res.status).toBe(200); + return cookiesFrom(res); +}; + +/** + * One OAuth round trip. `start` is `sign-in/social` (an implicit sign-in) or + * `link-social` (an explicit link, which needs `sessionCookie`). Answers the + * callback's redirect target. + */ +const oauthRoundTrip = async ( + manager: AuthManager, + providerId: string, + start: 'sign-in/social' | 'link-social', + sessionCookie: string[] = [], +): Promise => { + const begin = await post( + manager, + start, + { provider: providerId, callbackURL: AFTER, disableRedirect: true }, + joinCookies(sessionCookie), + ); + expect(begin.status).toBe(200); + const { url } = (await begin.json()) as { url: string }; + const state = new URL(url).searchParams.get('state'); + expect(state).toBeTruthy(); + const callback = await manager.handleRequest( + new Request(`${BASE}/api/v1/auth/callback/${providerId}?code=code-1&state=${encodeURIComponent(state!)}`, { + headers: { cookie: joinCookies(sessionCookie, cookiesFrom(begin)) }, + }), + ); + expect(callback.status).toBe(302); + return new URL(callback.headers.get('location')!, BASE); +}; + +const userRow = (engine: any, email: string) => + (engine.tables.get('sys_user') ?? []).find((u: any) => u.email === email); +const accountsOf = (engine: any, userId: string, providerId: string) => + (engine.tables.get('sys_account') ?? []).filter((a: any) => a.user_id === userId && a.provider_id === providerId); +const tombstones = (engine: any, userId: string, providerId: string) => + (engine.tables.get('sys_verification') ?? []).filter( + (v: any) => v.identifier === unlinkTombstoneIdentifier(userId, providerId), + ); +const setVerified = (engine: any, email: string, verified: boolean) => { + userRow(engine, email).email_verified = verified; +}; + +// ───────────────────────────────────────────────────────────────────────────── + +describe('implicit link decision', () => { + const base = { + providerId: EXTERNAL, + localEmailVerified: false, + requireLocalEmailVerified: true, + unlinkedByUser: false, + }; + + it('refuses an unverified local row for an external provider', () => { + expect(decideImplicitLink(base)).toEqual({ allow: false, reason: 'local-email-unverified' }); + }); + + it('allows a verified local row', () => { + expect(decideImplicitLink({ ...base, localEmailVerified: true })).toEqual({ allow: true }); + }); + + it('exempts the platform identity provider from the local-verification precondition', () => { + expect(decideImplicitLink({ ...base, providerId: PLATFORM_IDP_PROVIDER_ID })).toEqual({ allow: true }); + }); + + it('honours an unlink for every provider, the platform identity provider included', () => { + for (const providerId of [EXTERNAL, PLATFORM_IDP_PROVIDER_ID]) { + expect(decideImplicitLink({ ...base, providerId, localEmailVerified: true, unlinkedByUser: true })).toEqual({ + allow: false, + reason: 'unlinked-by-user', + }); + } + }); + + it('an explicit operator opt-out drops only the verification precondition', () => { + expect(decideImplicitLink({ ...base, requireLocalEmailVerified: false })).toEqual({ allow: true }); + expect(decideImplicitLink({ ...base, requireLocalEmailVerified: false, unlinkedByUser: true }).allow).toBe(false); + }); + + it('reads the provider id from oauth and sso sources alike', () => { + expect(linkSourceProviderId({ oauth: { providerId: 'p1' } })).toBe('p1'); + expect(linkSourceProviderId({ sso: { providerId: 'p2' } })).toBe('p2'); + expect(linkSourceProviderId({})).toBeUndefined(); + }); +}); + +describe('vendor account-linking configuration', () => { + const capture = async (config: Record = {}) => { + const manager = makeManager(createMemoryEngine(), config); + const auth: any = await (manager as any).getOrCreateAuth(); + return auth.options.account.accountLinking; + }; + + it('pins the vendor flag off by default and always trusts the platform identity provider', async () => { + const linking = await capture(); + expect(linking.requireLocalEmailVerified).toBe(false); + expect(linking.trustedProviders).toContain(PLATFORM_IDP_PROVIDER_ID); + }); + + it('hands an explicit strict operator value to the vendor for every provider', async () => { + const linking = await capture({ account: { accountLinking: { requireLocalEmailVerified: true, trustedProviders: ['p9'] } } }); + expect(linking.requireLocalEmailVerified).toBe(true); + expect(linking.trustedProviders).toEqual(expect.arrayContaining([PLATFORM_IDP_PROVIDER_ID, 'p9'])); + }); +}); + +describe('implicit link on external sign-in, end to end', () => { + it('refuses an unverified local row, writes no link and leaves the row unverified', async () => { + const engine = createMemoryEngine(); + const manager = makeManager(engine); + const email = 'member@example.test'; + await signUp(manager, email); + const user = userRow(engine, email); + expect(Boolean(user.email_verified)).toBe(false); + idpProfile[EXTERNAL] = { sub: 'ext-1', email, email_verified: true }; + + const target = await oauthRoundTrip(manager, EXTERNAL, 'sign-in/social'); + + expect(target.searchParams.get('error')).toBe(IMPLICIT_LINK_REFUSED); + expect(accountsOf(engine, user.id, EXTERNAL)).toHaveLength(0); + expect(Boolean(userRow(engine, email).email_verified)).toBe(false); + }); + + it('links a verified local row and signs the caller in', async () => { + const engine = createMemoryEngine(); + const manager = makeManager(engine); + const email = 'verified@example.test'; + await signUp(manager, email); + setVerified(engine, email, true); + const user = userRow(engine, email); + idpProfile[EXTERNAL] = { sub: 'ext-2', email, email_verified: true }; + + const target = await oauthRoundTrip(manager, EXTERNAL, 'sign-in/social'); + + expect(target.searchParams.get('error')).toBeNull(); + expect(target.href).toBe(AFTER); + expect(accountsOf(engine, user.id, EXTERNAL)).toHaveLength(1); + }); + + it('keeps the platform identity provider exception for an unverified owner-seeded row', async () => { + const engine = createMemoryEngine(); + const manager = makeManager(engine); + const email = 'owner@example.test'; + await signUp(manager, email); + const user = userRow(engine, email); + expect(Boolean(user.email_verified)).toBe(false); + idpProfile[PLATFORM_IDP_PROVIDER_ID] = { sub: 'cloud-1', email, email_verified: true }; + + const target = await oauthRoundTrip(manager, PLATFORM_IDP_PROVIDER_ID, 'sign-in/social'); + + expect(target.searchParams.get('error')).toBeNull(); + expect(accountsOf(engine, user.id, PLATFORM_IDP_PROVIDER_ID)).toHaveLength(1); + }); + + it('an explicit operator opt-out restores the unverified-row link', async () => { + const engine = createMemoryEngine(); + const manager = makeManager(engine, { account: { accountLinking: { requireLocalEmailVerified: false } } }); + const email = 'optout@example.test'; + await signUp(manager, email); + const user = userRow(engine, email); + idpProfile[EXTERNAL] = { sub: 'ext-3', email, email_verified: true }; + + const target = await oauthRoundTrip(manager, EXTERNAL, 'sign-in/social'); + + expect(target.searchParams.get('error')).toBeNull(); + expect(accountsOf(engine, user.id, EXTERNAL)).toHaveLength(1); + }); + + it('after an unlink, an implicit sign-in is refused; an explicit link is allowed and lifts the refusal', async () => { + const engine = createMemoryEngine(); + const manager = makeManager(engine); + const email = 'unlinker@example.test'; + const session = await signUp(manager, email); + setVerified(engine, email, true); + const user = userRow(engine, email); + idpProfile[EXTERNAL] = { sub: 'ext-4', email, email_verified: true }; + + // Linked implicitly (verified row). + expect((await oauthRoundTrip(manager, EXTERNAL, 'sign-in/social')).searchParams.get('error')).toBeNull(); + const [linked] = accountsOf(engine, user.id, EXTERNAL); + expect(linked).toBeTruthy(); + + // The user unlinks it. + const unlink = await post(manager, 'unlink-account', { accountId: linked.id }, joinCookies(session)); + expect(unlink.status).toBe(200); + expect(accountsOf(engine, user.id, EXTERNAL)).toHaveLength(0); + expect(tombstones(engine, user.id, EXTERNAL)).toHaveLength(1); + + // An implicit sign-in through the same provider no longer re-links. + const refused = await oauthRoundTrip(manager, EXTERNAL, 'sign-in/social'); + expect(refused.searchParams.get('error')).toBe(IMPLICIT_LINK_REFUSED); + expect(accountsOf(engine, user.id, EXTERNAL)).toHaveLength(0); + + // An explicit, session-authenticated link is allowed and clears the record. + const explicit = await oauthRoundTrip(manager, EXTERNAL, 'link-social', session); + expect(explicit.searchParams.get('error')).toBeNull(); + expect(accountsOf(engine, user.id, EXTERNAL)).toHaveLength(1); + expect(tombstones(engine, user.id, EXTERNAL)).toHaveLength(0); + + // …and the provider signs the user in again. + expect((await oauthRoundTrip(manager, EXTERNAL, 'sign-in/social')).searchParams.get('error')).toBeNull(); + }); +}); From 80f8b066ef2036be113f34344b0662523ed1160c Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 5 Oct 2026 11:33:00 +0000 Subject: [PATCH 03/10] chore(changeset): implicit account linking ownership Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_018zT8d8NpiQ1ExhuNd5TxY6 --- .../21846-implicit-account-linking-ownership.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 .changeset/21846-implicit-account-linking-ownership.md diff --git a/.changeset/21846-implicit-account-linking-ownership.md b/.changeset/21846-implicit-account-linking-ownership.md new file mode 100644 index 0000000000..97f15f7da5 --- /dev/null +++ b/.changeset/21846-implicit-account-linking-ownership.md @@ -0,0 +1,13 @@ +--- +'@objectstack/plugin-auth': patch +--- + +Implicit account linking on external sign-in (OAuth, OIDC, SSO) now requires the library's standard local-ownership condition: an external identity links implicitly to an existing local user only when that local user's email is verified. The platform's own identity provider (`objectstack-cloud`) keeps its documented exception, and a user's unlink is honoured. + +Clause-②: no + +- **Local ownership.** An external sign-in whose email matches an existing local user whose email is not verified is refused with `error=account_not_linked`. That is the same code better-auth's own refusal produces. No link is written and the local user stays unverified. A verified local user links as before. +- **Platform identity provider.** `objectstack-cloud` still links to an unverified local user, because it seeds the environment owner's row without a mailbox round-trip. +- **Unlink is honoured.** After a user unlinks a provider, an implicit sign-in through that provider no longer links the identity again, for any provider. An explicit, signed-in link from account settings (`/link-social`) is still allowed and ends the refusal. +- **Operator override.** `account.accountLinking.requireLocalEmailVerified` is now read as follows. Unset (the default) means the rules above. `true` applies the strict check to every provider, including `objectstack-cloud`. `false` turns off only the local-verification check and keeps the unlink rule. +- To let unverified local users link implicitly again, set `account.accountLinking.requireLocalEmailVerified: false`. Before you do, read the library's warning about account takeover. From 3532f099ae771649fa89f28d4a17abb92f43ebdd Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 5 Oct 2026 11:41:45 +0000 Subject: [PATCH 04/10] chore(gates): record the new pinned engine double Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_018zT8d8NpiQ1ExhuNd5TxY6 --- scripts/engine-double-contract.pinned.json | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/scripts/engine-double-contract.pinned.json b/scripts/engine-double-contract.pinned.json index ff74f36941..98eb20f8d4 100644 --- a/scripts/engine-double-contract.pinned.json +++ b/scripts/engine-double-contract.pinned.json @@ -2751,6 +2751,21 @@ "verb": "update", "pinned": 1 }, + { + "file": "packages/plugins/plugin-auth/src/implicit-account-linking.test.ts", + "verb": "delete", + "pinned": 1 + }, + { + "file": "packages/plugins/plugin-auth/src/implicit-account-linking.test.ts", + "verb": "findOne", + "pinned": 1 + }, + { + "file": "packages/plugins/plugin-auth/src/implicit-account-linking.test.ts", + "verb": "update", + "pinned": 1 + }, { "file": "packages/plugins/plugin-auth/src/list-user-invitations-verification.test.ts", "verb": "delete", From 6b0f00e9b24241dc78ecff12b5717b72f0f55c04 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 5 Oct 2026 12:02:14 +0000 Subject: [PATCH 05/10] docs(plugin-auth): state the linking precondition at class level Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_018zT8d8NpiQ1ExhuNd5TxY6 --- .../plugins/plugin-auth/src/implicit-account-linking.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/plugins/plugin-auth/src/implicit-account-linking.ts b/packages/plugins/plugin-auth/src/implicit-account-linking.ts index 88dee0a69a..02593f7605 100644 --- a/packages/plugins/plugin-auth/src/implicit-account-linking.ts +++ b/packages/plugins/plugin-auth/src/implicit-account-linking.ts @@ -15,11 +15,10 @@ * * 1. **Every provider meets the library's standard local-ownership * requirement** before an implicit link: the existing local row must be - * `emailVerified: true`. Without it, anyone who can register an - * UNVERIFIED local row at a victim's address (open self-registration) - * gets the victim's external identity linked into the row they control — - * and the link then flips that row to verified. better-auth names this - * exact case as the reason `requireLocalEmailVerified` defaults to `true`. + * `emailVerified: true` — the account-ownership precondition better-auth + * documents as the reason `requireLocalEmailVerified` defaults to `true`. + * A refused link writes nothing, so it also never flips the local row to + * verified. * 2. **The platform's own cloud identity provider keeps its documented * exception** ({@link PLATFORM_IDP_PROVIDER_ID}). The cloud is the IdP * for every environment, and the environment's owner row is seeded by the From 598e7a7bf9fcecb3bd8992eab26a8ed1db106224 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 5 Oct 2026 13:22:26 +0000 Subject: [PATCH 06/10] fix(plugin-auth): fail an unlink closed, bind explicit links to their user, clear records on user delete Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_018zT8d8NpiQ1ExhuNd5TxY6 --- ...1846-implicit-account-linking-ownership.md | 24 ++- .../plugins/plugin-auth/src/auth-manager.ts | 81 ++++++++-- .../src/implicit-account-linking.ts | 148 +++++++++++++----- 3 files changed, 195 insertions(+), 58 deletions(-) diff --git a/.changeset/21846-implicit-account-linking-ownership.md b/.changeset/21846-implicit-account-linking-ownership.md index 97f15f7da5..6b47e4903c 100644 --- a/.changeset/21846-implicit-account-linking-ownership.md +++ b/.changeset/21846-implicit-account-linking-ownership.md @@ -1,13 +1,23 @@ --- -'@objectstack/plugin-auth': patch +'@objectstack/plugin-auth': minor --- -Implicit account linking on external sign-in (OAuth, OIDC, SSO) now requires the library's standard local-ownership condition: an external identity links implicitly to an existing local user only when that local user's email is verified. The platform's own identity provider (`objectstack-cloud`) keeps its documented exception, and a user's unlink is honoured. +fix(plugin-auth)!: implicit account linking on external sign-in requires the library's standard local-ownership condition; the platform identity provider keeps its documented exception; an unlink is honoured -Clause-②: no +Clause-②: no (narrowing) -- **Local ownership.** An external sign-in whose email matches an existing local user whose email is not verified is refused with `error=account_not_linked`. That is the same code better-auth's own refusal produces. No link is written and the local user stays unverified. A verified local user links as before. -- **Platform identity provider.** `objectstack-cloud` still links to an unverified local user, because it seeds the environment owner's row without a mailbox round-trip. -- **Unlink is honoured.** After a user unlinks a provider, an implicit sign-in through that provider no longer links the identity again, for any provider. An explicit, signed-in link from account settings (`/link-social`) is still allowed and ends the refusal. -- **Operator override.** `account.accountLinking.requireLocalEmailVerified` is now read as follows. Unset (the default) means the rules above. `true` applies the strict check to every provider, including `objectstack-cloud`. `false` turns off only the local-verification check and keeps the unlink rule. + + +**BREAKING for deployments that relied on external sign-in (OAuth, OIDC, SSO) linking implicitly to a local user whose email is not verified.** + +**What changed.** + +- An external sign-in links implicitly to an existing local user only when that local user's email is verified. Otherwise the sign-in is refused with `error=account_not_linked`, the same code better-auth's own refusal produces. No link is written and the local user stays unverified. A verified local user links as before. +- The platform's own identity provider (`objectstack-cloud`) keeps its documented exception and still links to an unverified local user, because it seeds the environment owner's row without a mailbox round-trip. +- After a user unlinks a provider, an implicit sign-in through it no longer links the identity again, for any provider. An explicit, signed-in link from account settings (`/link-social`) is still allowed and ends the refusal. If the unlink cannot be recorded, the unlink itself is refused and the provider stays linked. Deleting a user removes the user's unlink records. +- `account.accountLinking.requireLocalEmailVerified` now reads as follows. Unset (the default) means the rules above. `true` applies the strict check to every provider, including `objectstack-cloud`. `false` turns off only the local-verification check and keeps the unlink rule. + +**What to do after upgrading.** + +- A user refused this way signs in with their existing method, then links the provider from account settings, or verifies their email first. - To let unverified local users link implicitly again, set `account.accountLinking.requireLocalEmailVerified: false`. Before you do, read the library's warning about account takeover. diff --git a/packages/plugins/plugin-auth/src/auth-manager.ts b/packages/plugins/plugin-auth/src/auth-manager.ts index b01d4e73a3..863a8ab4cb 100644 --- a/packages/plugins/plugin-auth/src/auth-manager.ts +++ b/packages/plugins/plugin-auth/src/auth-manager.ts @@ -33,7 +33,9 @@ import { shouldStampOwnerVerifiedAtCreation } from './walled-owner-operator-stam import { PLATFORM_IDP_PROVIDER_ID, clearUnlinkTombstone, + clearUserUnlinkTombstones, recordUnlinkTombstone, + type LinkingInternalAdapter, refuseImplicitAccountLink, } from './implicit-account-linking.js'; import type { IDataEngine } from '@objectstack/core'; @@ -1605,6 +1607,7 @@ export class AuthManager { validateUserInfo: async (data: any, ctx: any) => (await refuseImplicitAccountLink(data, ctx, { requireLocalEmailVerified: this.implicitLinkRequiresLocalEmailVerified(), + resolveAdapter: () => this.linkingAdapter(), logInfo: (m, meta) => this.config.logger?.info?.(m, meta), })) ?? this.validateAudienceAdmission(data, ctx), }, @@ -4496,6 +4499,21 @@ export class AuthManager { return (this.config as any)?.account?.accountLinking?.requireLocalEmailVerified !== false; } + /** + * better-auth's internal adapter read off the auth instance — the store for + * the implicit-linking hooks when a call carries no endpoint context (a + * server-side `auth.api.*` call or an internal-adapter write). + */ + private async linkingAdapter(): Promise { + try { + const auth: any = await this.getOrCreateAuth(); + const context = await auth?.$context; + return context?.internalAdapter as LinkingInternalAdapter | undefined; + } catch { + return undefined; + } + } + /** OAuth providerIds that are OPERATOR-REGISTERED identity authorities (enterprise `oidcProviders`, incl. the cloud platform IdP). */ private enterpriseOAuthProviderIds(): ReadonlySet { const ids = new Set(); @@ -7352,10 +7370,13 @@ export class AuthManager { // rule 3). While the record stands an implicit link is refused, so a link // that lands here is an explicit one. Failure to clear is functional (the // user sees a refusal on their next implicit sign-in and can re-link), so - // it is reported at `warn` and never fails the link. + // it is reported at `warn` and never fails the link. It runs FIRST, ahead + // of the identity-source stamp, so a stamp failure can never leave a + // landed link still refused. + const resolveLinkingAdapter = () => this.linkingAdapter(); const clearUnlink = async (account: any, ctx: any) => { try { - await clearUnlinkTombstone(account, ctx); + await clearUnlinkTombstone(account, ctx, resolveLinkingAdapter); } catch (e) { this.config.logger?.warn?.('[auth] could not clear the unlink record after a link', { error: (e as Error)?.message, @@ -7363,8 +7384,8 @@ export class AuthManager { } }; const stamp = async (account: any, ctx: any) => { - await this.stampIdentitySource(account, ctx); await clearUnlink(account, ctx); + await this.stampIdentitySource(account, ctx); }; const hostAccountAfter = (host as any)?.account?.create?.after; const after = hostAccountAfter @@ -7375,26 +7396,52 @@ export class AuthManager { : stamp; // A user's unlink is recorded so the provider cannot re-link implicitly - // (implicit-account-linking.ts, rule 3). A record that does not land - // leaves the unlink answering 200 while the provider still re-links on - // the next sign-in — the protection the user asked for is silently - // absent — so the failure is reported at `error`, once per unlink. - const hostAccountDeleteAfter = (host as any)?.account?.delete?.after; - const accountDeleteAfter = async (account: any, ctx: any) => { - const result = hostAccountDeleteAfter ? await hostAccountDeleteAfter(account, ctx) : undefined; + // (implicit-account-linking.ts, rule 3) — BEFORE the account row goes, so + // that a record which cannot be written aborts the unlink: a throw from a + // `delete.before` hook propagates out of better-auth's delete, the unlink + // answers an error and the identity stays linked. Fail closed: an unlink + // that answered success without its record would leave the provider free + // to re-link on the next sign-in. Reported at `error`, once per refusal. + // A host `delete.before` runs first; its `false` (abort) is honoured + // before anything is recorded. + const hostAccountDeleteBefore = (host as any)?.account?.delete?.before; + const accountDeleteBefore = async (account: any, ctx: any) => { + const result = hostAccountDeleteBefore ? await hostAccountDeleteBefore(account, ctx) : undefined; + if (result === false) return false; try { - await recordUnlinkTombstone(account, ctx); + await recordUnlinkTombstone(account, ctx, resolveLinkingAdapter); } catch (e) { this.audienceLogError( - '[auth] unlink record NOT written: the provider can still re-link this account implicitly ' + - 'on its next sign-in although the unlink answered success. Check the auth store ' + - '(sys_verification) is writable, then have the user unlink again.', + '[auth] unlink refused: its record could not be written, so the provider stays linked. ' + + 'Without the record the provider could re-link this account implicitly on its next sign-in. ' + + 'Check the auth store (sys_verification) is writable, then unlink again.', { providerId: account?.providerId, error: (e as Error)?.message }, ); + throw e; } return result; }; + // A deleted user leaves no unlink record behind (implicit-account-linking.ts). + // A record that outlives its user is inert — a new user never has the + // deleted user's id — so a failure is reported at `warn` and never fails + // the deletion. + const clearUserUnlinks = async (user: any, ctx: any) => { + try { + await clearUserUnlinkTombstones(user, ctx, resolveLinkingAdapter); + } catch (e) { + this.config.logger?.warn?.('[auth] could not clear the unlink record of a deleted user', { + error: (e as Error)?.message, + }); + } + }; + const hostUserDeleteAfter = (host as any)?.user?.delete?.after; + const userDeleteAfter = async (user: any, ctx: any) => { + const result = hostUserDeleteAfter ? await hostUserDeleteAfter(user, ctx) : undefined; + await clearUserUnlinks(user, ctx); + return result; + }; + // ADR-0093 D9 — default active-org on session create. Without it, a user // with memberships logs in with `activeOrganizationId = null`: better-auth // org endpoints can't resolve an active org (single-org invite dead-end) @@ -7600,7 +7647,7 @@ export class AuthManager { }, delete: { ...((host as any)?.account?.delete ?? {}), - after: accountDeleteAfter, + before: accountDeleteBefore, }, }, user: { @@ -7610,6 +7657,10 @@ export class AuthManager { before: userBefore, after: userAfter, }, + delete: { + ...((host as any)?.user?.delete ?? {}), + after: userDeleteAfter, + }, }, session: { ...((host as any)?.session ?? {}), diff --git a/packages/plugins/plugin-auth/src/implicit-account-linking.ts b/packages/plugins/plugin-auth/src/implicit-account-linking.ts index 02593f7605..735f3e2b1d 100644 --- a/packages/plugins/plugin-auth/src/implicit-account-linking.ts +++ b/packages/plugins/plugin-auth/src/implicit-account-linking.ts @@ -28,7 +28,8 @@ * 3. **A user's unlink is honoured.** After a user unlinks provider P, an * implicit sign-in through P must not quietly re-create the link — that * would make unlinking decorative. The unlink is recorded - * ({@link unlinkTombstoneIdentifier}); implicit linking for that + * ({@link unlinkTombstoneIdentifier}, written before the unlink and + * failing it closed); implicit linking for that * user + provider is refused while the record stands; an EXPLICIT, * session-authenticated link-social is still allowed and clears it. * This applies to every provider, the cloud one included: the cloud @@ -105,16 +106,22 @@ const UNLINK_TOMBSTONE_PREFIX = 'account-unlinked'; /** * The unlink record's lifetime. A record must outlive any realistic gap * between an unlink and the next sign-in; it ends earlier only when the user - * re-links explicitly. better-auth's verification store sweeps rows by - * `expiresAt`, so the value has to be finite. + * re-links explicitly or the user is deleted. better-auth's verification + * store sweeps rows by `expiresAt`, so the value has to be finite. */ const UNLINK_TOMBSTONE_TTL_MS = 100 * 365 * 24 * 60 * 60 * 1000; /** The provider that never takes part in linking (email + password). */ const CREDENTIAL_PROVIDER_ID = 'credential'; -export function unlinkTombstoneIdentifier(userId: string, providerId: string): string { - return `${UNLINK_TOMBSTONE_PREFIX}:${userId}:${providerId}`; +/** + * ONE record per user, listing every provider the user unlinked and has not + * re-linked explicitly. Per user rather than per user + provider so that a + * user's deletion can clear all of them by identifier — the verification + * store offers no prefix delete. + */ +export function unlinkTombstoneIdentifier(userId: string): string { + return `${UNLINK_TOMBSTONE_PREFIX}:${userId}`; } export interface ImplicitLinkInput { @@ -152,26 +159,62 @@ export function linkSourceProviderId(source: unknown): string | undefined { } /** The slice of better-auth's `internalAdapter` this module uses. */ -interface LinkingInternalAdapter { +export interface LinkingInternalAdapter { findUserById(id: string): Promise<{ emailVerified?: boolean } | null>; - findVerificationValue(identifier: string): Promise; + findVerificationValue(identifier: string): Promise<{ value?: unknown } | null>; createVerificationValue(data: { identifier: string; value: string; expiresAt: Date }): Promise; deleteVerificationByIdentifier(identifier: string): Promise; } -const internalAdapterOf = (ctx: unknown): LinkingInternalAdapter | undefined => +/** + * Resolves the store for a hook or gate call. The endpoint context carries it + * on a request; a server-side call (`auth.api.*` without a request, or an + * internal-adapter write) may carry none, so the caller supplies a fallback + * read off the auth instance itself. + */ +export type LinkingAdapterResolver = (ctx: unknown) => Promise; + +export const internalAdapterOf = (ctx: unknown): LinkingInternalAdapter | undefined => (ctx as { context?: { internalAdapter?: LinkingInternalAdapter } } | undefined)?.context?.internalAdapter; /** - * Is the current request the callback of an EXPLICIT link (`/link-social`)? - * Reads the OAuth state the callback parsed; see the module header for why - * `link` cannot be supplied by the client. + * The providers a user's unlink record lists. A record whose value cannot be + * read is an ERROR, not an empty list: answering "nothing unlinked" for a + * record that exists would re-open exactly what the record closes. */ -async function isExplicitLinkFlow(): Promise { +async function readUnlinkedProviders(adapter: LinkingInternalAdapter, userId: string): Promise { + const row = await adapter.findVerificationValue(unlinkTombstoneIdentifier(userId)); + if (!row) return []; + const parsed = JSON.parse(String(row.value)) as { providers?: unknown }; + if (!Array.isArray(parsed?.providers) || !parsed.providers.every((p) => typeof p === 'string')) { + throw new Error('unlink record: unreadable value'); + } + return parsed.providers as string[]; +} + +async function writeUnlinkedProviders(adapter: LinkingInternalAdapter, userId: string, providers: string[]): Promise { + const identifier = unlinkTombstoneIdentifier(userId); + await adapter.deleteVerificationByIdentifier(identifier); + if (providers.length === 0) return; + await adapter.createVerificationValue({ + identifier, + value: JSON.stringify({ userId, providers, updatedAt: new Date().toISOString() }), + expiresAt: new Date(Date.now() + UNLINK_TOMBSTONE_TTL_MS), + }); +} + +/** + * Is the current request the callback of an EXPLICIT link (`/link-social`) + * for THIS user? Reads the OAuth state the callback parsed; see the module + * header for why `link` cannot be supplied by the client. The state's user + * must also be the user the link is being written for — a state naming + * anyone else is not an explicit link of this account. + */ +async function isExplicitLinkFlow(userId: string): Promise { try { const { getOAuthState } = await import('better-auth/api'); const state = (await getOAuthState()) as { link?: { userId?: unknown } } | null; - return typeof state?.link?.userId === 'string'; + return typeof state?.link?.userId === 'string' && state.link.userId === userId; } catch { // No request state (not inside an OAuth flow) ⇒ not an explicit link. return false; @@ -180,6 +223,7 @@ async function isExplicitLinkFlow(): Promise { export interface ImplicitLinkGateOptions { requireLocalEmailVerified: boolean; + resolveAdapter?: LinkingAdapterResolver; logInfo?: (message: string, meta?: Record) => void; } @@ -196,21 +240,21 @@ export async function refuseImplicitAccountLink( options: ImplicitLinkGateOptions, ): Promise<{ error: string; errorDescription: string } | undefined> { if (data?.source?.action !== 'link-account') return undefined; - if (await isExplicitLinkFlow()) return undefined; const providerId = linkSourceProviderId(data.source); const userId = typeof data.user?.id === 'string' ? (data.user.id as string) : undefined; - const adapter = internalAdapterOf(ctx); + if (userId && (await isExplicitLinkFlow(userId))) return undefined; + const adapter = internalAdapterOf(ctx) ?? (await options.resolveAdapter?.(ctx)); if (!providerId || !userId || !adapter) { throw new Error('implicit account link: provider, user or store unavailable — refusing'); } - const tombstone = await adapter.findVerificationValue(unlinkTombstoneIdentifier(userId, providerId)); + const unlinked = await readUnlinkedProviders(adapter, userId); const local = await adapter.findUserById(userId); if (!local) throw new Error('implicit account link: local user not found — refusing'); const verdict = decideImplicitLink({ providerId, localEmailVerified: local.emailVerified === true, requireLocalEmailVerified: options.requireLocalEmailVerified, - unlinkedByUser: tombstone != null, + unlinkedByUser: unlinked.includes(providerId), }); if (verdict.allow) return undefined; options.logInfo?.('[auth] implicit account link refused', { providerId, reason: verdict.reason }); @@ -224,38 +268,70 @@ export async function refuseImplicitAccountLink( } /** - * `account.delete.after` half: a user's own unlink (`/unlink-account`) - * leaves a record that keeps the provider from re-linking implicitly. Other - * deletions (user removal, admin tooling) leave none. + * `account.delete.before` half: a user's own unlink (`/unlink-account`) + * records the provider BEFORE the account row is deleted. It throws when the + * record cannot be written, and a throw from a `delete.before` hook aborts + * the delete, so the unlink answers an error and the identity stays linked — + * fail closed. An unlink that succeeded without its record would leave the + * provider free to re-link implicitly while the user believes it gone. (A + * record written for a delete that then fails is harmless: the record is read + * only when NO account for the provider is linked.) Other deletions (user + * removal, admin tooling) record nothing. + * + * The record lives in better-auth's verification store. On a deployment that + * configures a `secondaryStorage` without `verification.storeInDatabase`, + * that store is the secondary storage: the record then lasts only as long as + * the cache keeps it, and an evicted record re-opens implicit linking for + * that provider. ObjectStack wires no `secondaryStorage`, so the record is a + * database row. */ -export async function recordUnlinkTombstone(account: unknown, ctx: unknown): Promise { +export async function recordUnlinkTombstone( + account: unknown, + ctx: unknown, + resolveAdapter?: LinkingAdapterResolver, +): Promise { const a = account as { userId?: unknown; providerId?: unknown } | null; const path = (ctx as { path?: unknown } | undefined)?.path; if (path !== '/unlink-account') return; if (typeof a?.userId !== 'string' || typeof a?.providerId !== 'string') return; if (a.providerId === CREDENTIAL_PROVIDER_ID) return; - const adapter = internalAdapterOf(ctx); + const adapter = internalAdapterOf(ctx) ?? (await resolveAdapter?.(ctx)); if (!adapter) throw new Error('unlink record: store unavailable'); - const identifier = unlinkTombstoneIdentifier(a.userId, a.providerId); - await adapter.deleteVerificationByIdentifier(identifier); - await adapter.createVerificationValue({ - identifier, - value: JSON.stringify({ userId: a.userId, providerId: a.providerId, unlinkedAt: new Date().toISOString() }), - expiresAt: new Date(Date.now() + UNLINK_TOMBSTONE_TTL_MS), - }); + const providers = await readUnlinkedProviders(adapter, a.userId); + if (providers.includes(a.providerId)) return; + await writeUnlinkedProviders(adapter, a.userId, [...providers, a.providerId]); } /** - * `account.create.after` half: a link that lands clears the unlink record. - * While the record stands an implicit link is refused, so a link that lands - * is an explicit one (or an operator act) — exactly the "until the user - * re-links" end the ruling sets. + * `account.create.after` half: a link that lands removes its provider from + * the unlink record. While the provider is listed an implicit link is + * refused, so a link that lands is an explicit one (or an operator act) — + * exactly the "until the user re-links" end the ruling sets. */ -export async function clearUnlinkTombstone(account: unknown, ctx: unknown): Promise { +export async function clearUnlinkTombstone( + account: unknown, + ctx: unknown, + resolveAdapter?: LinkingAdapterResolver, +): Promise { const a = account as { userId?: unknown; providerId?: unknown } | null; if (typeof a?.userId !== 'string' || typeof a?.providerId !== 'string') return; if (a.providerId === CREDENTIAL_PROVIDER_ID) return; - const adapter = internalAdapterOf(ctx); + const adapter = internalAdapterOf(ctx) ?? (await resolveAdapter?.(ctx)); if (!adapter) return; - await adapter.deleteVerificationByIdentifier(unlinkTombstoneIdentifier(a.userId, a.providerId)); + const providers = await readUnlinkedProviders(adapter, a.userId); + if (!providers.includes(a.providerId)) return; + await writeUnlinkedProviders(adapter, a.userId, providers.filter((p) => p !== a.providerId)); +} + +/** `user.delete.after` half: a deleted user leaves no unlink record behind. */ +export async function clearUserUnlinkTombstones( + user: unknown, + ctx: unknown, + resolveAdapter?: LinkingAdapterResolver, +): Promise { + const id = (user as { id?: unknown } | null)?.id; + if (typeof id !== 'string') return; + const adapter = internalAdapterOf(ctx) ?? (await resolveAdapter?.(ctx)); + if (!adapter) throw new Error('unlink record: store unavailable'); + await adapter.deleteVerificationByIdentifier(unlinkTombstoneIdentifier(id)); } From 07ba1e9378ec51c0259688903c1153967bde2d51 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 5 Oct 2026 13:28:26 +0000 Subject: [PATCH 07/10] test(plugin-auth): id-token, forged state, explicit unverified link, OIDC discovery, fail-closed unlink, user delete Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_018zT8d8NpiQ1ExhuNd5TxY6 --- .../src/implicit-account-linking.test.ts | 182 +++++++++++++++++- 1 file changed, 175 insertions(+), 7 deletions(-) diff --git a/packages/plugins/plugin-auth/src/implicit-account-linking.test.ts b/packages/plugins/plugin-auth/src/implicit-account-linking.test.ts index f00115f85d..763fab69af 100644 --- a/packages/plugins/plugin-auth/src/implicit-account-linking.test.ts +++ b/packages/plugins/plugin-auth/src/implicit-account-linking.test.ts @@ -57,9 +57,13 @@ const createMemoryEngine = () => { return out; }; let seq = 0; + /** Tables whose inserts fail — the store-fault seam for the fail-closed cases. */ + const failInserts = new Set(); return { tables, + failInserts, async insert(name: string, data: any) { + if (failInserts.has(name)) throw new Error(`fake driver: insert into ${name} refused`); const row = { id: data.id ?? `row_${++seq}`, ...data }; rows(name).push(row); return { ...row }; @@ -101,6 +105,10 @@ const BASE = 'http://localhost:3000'; const AFTER = `${BASE}/after-sign-in`; const IDP = 'https://idp.example.test'; const EXTERNAL = 'acme-idp'; +/** A generic OIDC provider configured by discovery (subject = `sub`). */ +const OIDC = 'corp-oidc'; +/** A built-in social provider, for the id-token sign-in path. */ +const SOCIAL = 'google'; const provider = (providerId: string) => ({ providerId, @@ -117,7 +125,34 @@ const makeManager = (engine: any, config: Record = {}) => secret: SECRET, baseUrl: BASE, dataEngine: engine, - oidcProviders: [provider(EXTERNAL), provider(PLATFORM_IDP_PROVIDER_ID)], + oidcProviders: [ + provider(EXTERNAL), + provider(PLATFORM_IDP_PROVIDER_ID), + { + providerId: OIDC, + clientId: `${OIDC}-client`, + clientSecret: `${OIDC}-secret`, + discoveryUrl: `${IDP}/${OIDC}/.well-known/openid-configuration`, + pkce: false, + }, + ], + socialProviders: { + [SOCIAL]: { + clientId: `${SOCIAL}-client`, + clientSecret: `${SOCIAL}-secret`, + // The id token is synthetic: verification and the profile come from + // the stubbed IdP below, never from a real issuer. + verifyIdToken: async () => true, + getUserInfo: async () => { + const profile = idpProfile[SOCIAL]; + if (!profile) return null; + return { + user: { id: profile.sub, email: profile.email, emailVerified: profile.email_verified, name: 'Synthetic Person' }, + data: { ...profile }, + }; + }, + }, + }, ...config, } as any); @@ -133,6 +168,18 @@ beforeEach(() => { const url = typeof input === 'string' ? input : input instanceof URL ? input.href : input.url; if (!url.startsWith(IDP)) return realFetch(input, init); const providerId = new URL(url).pathname.split('/')[1]!; + if (url.endsWith('/.well-known/openid-configuration')) { + return new Response( + JSON.stringify({ + issuer: `${IDP}/${providerId}`, + authorization_endpoint: `${IDP}/${providerId}/authorize`, + token_endpoint: `${IDP}/${providerId}/token`, + userinfo_endpoint: `${IDP}/${providerId}/userinfo`, + id_token_signing_alg_values_supported: ['RS256'], + }), + { status: 200, headers: { 'Content-Type': 'application/json' } }, + ); + } if (url.endsWith('/token')) { return new Response( JSON.stringify({ access_token: `at-${providerId}`, token_type: 'Bearer', expires_in: 3600 }), @@ -189,11 +236,12 @@ const oauthRoundTrip = async ( providerId: string, start: 'sign-in/social' | 'link-social', sessionCookie: string[] = [], + extraBody: Record = {}, ): Promise => { const begin = await post( manager, start, - { provider: providerId, callbackURL: AFTER, disableRedirect: true }, + { provider: providerId, callbackURL: AFTER, disableRedirect: true, ...extraBody }, joinCookies(sessionCookie), ); expect(begin.status).toBe(200); @@ -213,10 +261,14 @@ const userRow = (engine: any, email: string) => (engine.tables.get('sys_user') ?? []).find((u: any) => u.email === email); const accountsOf = (engine: any, userId: string, providerId: string) => (engine.tables.get('sys_account') ?? []).filter((a: any) => a.user_id === userId && a.provider_id === providerId); -const tombstones = (engine: any, userId: string, providerId: string) => - (engine.tables.get('sys_verification') ?? []).filter( - (v: any) => v.identifier === unlinkTombstoneIdentifier(userId, providerId), +/** The providers the user's unlink record lists (none when there is no record). */ +const unlinkedProviders = (engine: any, userId: string): string[] => { + const rows = (engine.tables.get('sys_verification') ?? []).filter( + (v: any) => v.identifier === unlinkTombstoneIdentifier(userId), ); + expect(rows.length).toBeLessThanOrEqual(1); + return rows.length ? JSON.parse(rows[0].value).providers : []; +}; const setVerified = (engine: any, email: string, verified: boolean) => { userRow(engine, email).email_verified = verified; }; @@ -364,7 +416,7 @@ describe('implicit link on external sign-in, end to end', () => { const unlink = await post(manager, 'unlink-account', { accountId: linked.id }, joinCookies(session)); expect(unlink.status).toBe(200); expect(accountsOf(engine, user.id, EXTERNAL)).toHaveLength(0); - expect(tombstones(engine, user.id, EXTERNAL)).toHaveLength(1); + expect(unlinkedProviders(engine, user.id)).toEqual([EXTERNAL]); // An implicit sign-in through the same provider no longer re-links. const refused = await oauthRoundTrip(manager, EXTERNAL, 'sign-in/social'); @@ -375,9 +427,125 @@ describe('implicit link on external sign-in, end to end', () => { const explicit = await oauthRoundTrip(manager, EXTERNAL, 'link-social', session); expect(explicit.searchParams.get('error')).toBeNull(); expect(accountsOf(engine, user.id, EXTERNAL)).toHaveLength(1); - expect(tombstones(engine, user.id, EXTERNAL)).toHaveLength(0); + expect(unlinkedProviders(engine, user.id)).toEqual([]); // …and the provider signs the user in again. expect((await oauthRoundTrip(manager, EXTERNAL, 'sign-in/social')).searchParams.get('error')).toBeNull(); }); + it('refuses on the id-token sign-in path for an unverified local row', async () => { + const engine = createMemoryEngine(); + const manager = makeManager(engine); + const email = 'idtoken@example.test'; + await signUp(manager, email); + const user = userRow(engine, email); + idpProfile[SOCIAL] = { sub: 'social-1', email, email_verified: true }; + + const res = await post(manager, 'sign-in/social', { + provider: SOCIAL, + idToken: { token: 'synthetic-id-token' }, + }); + + expect(res.status).toBe(403); + expect(((await res.json()) as any).code).toBe(IMPLICIT_LINK_REFUSED); + expect(accountsOf(engine, user.id, SOCIAL)).toHaveLength(0); + expect(Boolean(userRow(engine, email).email_verified)).toBe(false); + + // The same path links a verified row. + setVerified(engine, email, true); + const ok = await post(manager, 'sign-in/social', { provider: SOCIAL, idToken: { token: 'synthetic-id-token' } }); + expect(ok.status).toBe(200); + expect(accountsOf(engine, user.id, SOCIAL)).toHaveLength(1); + }); + + it('a client-supplied link in additionalData does not make a sign-in an explicit link', async () => { + const engine = createMemoryEngine(); + const manager = makeManager(engine); + const email = 'forged@example.test'; + await signUp(manager, email); + const user = userRow(engine, email); + idpProfile[EXTERNAL] = { sub: 'ext-5', email, email_verified: true }; + + const target = await oauthRoundTrip(manager, EXTERNAL, 'sign-in/social', [], { + additionalData: { link: { userId: user.id, email } }, + }); + + expect(target.searchParams.get('error')).toBe(IMPLICIT_LINK_REFUSED); + expect(accountsOf(engine, user.id, EXTERNAL)).toHaveLength(0); + }); + + it('allows an explicit link-social for an unverified user, without marking the email verified', async () => { + const engine = createMemoryEngine(); + const manager = makeManager(engine); + const email = 'explicit@example.test'; + const session = await signUp(manager, email); + const user = userRow(engine, email); + expect(Boolean(user.email_verified)).toBe(false); + idpProfile[EXTERNAL] = { sub: 'ext-6', email, email_verified: true }; + + const target = await oauthRoundTrip(manager, EXTERNAL, 'link-social', session); + + expect(target.searchParams.get('error')).toBeNull(); + expect(accountsOf(engine, user.id, EXTERNAL)).toHaveLength(1); + expect(Boolean(userRow(engine, email).email_verified)).toBe(false); + }); + + it('applies to a generic OIDC provider configured by discovery', async () => { + const engine = createMemoryEngine(); + const manager = makeManager(engine); + const email = 'oidc@example.test'; + await signUp(manager, email); + const user = userRow(engine, email); + idpProfile[OIDC] = { sub: 'oidc-1', email, email_verified: true }; + + const refused = await oauthRoundTrip(manager, OIDC, 'sign-in/social'); + expect(refused.searchParams.get('error')).toBe(IMPLICIT_LINK_REFUSED); + expect(accountsOf(engine, user.id, OIDC)).toHaveLength(0); + + setVerified(engine, email, true); + const linked = await oauthRoundTrip(manager, OIDC, 'sign-in/social'); + expect(linked.searchParams.get('error')).toBeNull(); + expect(accountsOf(engine, user.id, OIDC)).toHaveLength(1); + }); + + it('refuses the unlink when its record cannot be written, leaving the provider linked', async () => { + const engine = createMemoryEngine(); + const manager = makeManager(engine); + const email = 'storefault@example.test'; + const session = await signUp(manager, email); + setVerified(engine, email, true); + const user = userRow(engine, email); + idpProfile[EXTERNAL] = { sub: 'ext-7', email, email_verified: true }; + expect((await oauthRoundTrip(manager, EXTERNAL, 'sign-in/social')).searchParams.get('error')).toBeNull(); + const [linked] = accountsOf(engine, user.id, EXTERNAL); + + engine.failInserts.add('sys_verification'); + const unlink = await post(manager, 'unlink-account', { accountId: linked.id }, joinCookies(session)); + engine.failInserts.delete('sys_verification'); + + expect(unlink.status).toBeGreaterThanOrEqual(400); + expect(accountsOf(engine, user.id, EXTERNAL)).toHaveLength(1); + expect(unlinkedProviders(engine, user.id)).toEqual([]); + }); + + it('deleting the user removes its unlink record', async () => { + const engine = createMemoryEngine(); + const manager = makeManager(engine); + const email = 'deleted@example.test'; + const session = await signUp(manager, email); + setVerified(engine, email, true); + const user = userRow(engine, email); + idpProfile[EXTERNAL] = { sub: 'ext-8', email, email_verified: true }; + expect((await oauthRoundTrip(manager, EXTERNAL, 'sign-in/social')).searchParams.get('error')).toBeNull(); + const [linked] = accountsOf(engine, user.id, EXTERNAL); + expect((await post(manager, 'unlink-account', { accountId: linked.id }, joinCookies(session))).status).toBe(200); + expect(unlinkedProviders(engine, user.id)).toEqual([EXTERNAL]); + + // A server-side deletion: no endpoint context, so the hook resolves the + // store from the auth instance. + const auth: any = await (manager as any).getOrCreateAuth(); + await (await auth.$context).internalAdapter.deleteUser(user.id); + + expect(userRow(engine, email)).toBeUndefined(); + expect(unlinkedProviders(engine, user.id)).toEqual([]); + }); }); From 83010a685e5d5a526a00409208475469a4120c78 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 5 Oct 2026 13:31:13 +0000 Subject: [PATCH 08/10] docs(auth): state the implicit linking and unlink rules Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_018zT8d8NpiQ1ExhuNd5TxY6 --- content/docs/permissions/authentication.mdx | 6 +++++ content/docs/permissions/sso.mdx | 29 +++++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/content/docs/permissions/authentication.mdx b/content/docs/permissions/authentication.mdx index d213b3cede..1b02544f13 100644 --- a/content/docs/permissions/authentication.mdx +++ b/content/docs/permissions/authentication.mdx @@ -527,6 +527,12 @@ window.location.href = data.url ?? data.data.url; Better-Auth automatically handles the OAuth callback at `/api/v1/auth/callback/google` and redirects the user back to your application with a session. +When the provider's email matches an existing local account that is not linked to this +provider yet, the sign-in links to that account only if its email is verified. Otherwise +the callback redirects with `error=account_not_linked`. The platform identity provider +(`objectstack-cloud`) is the one exception, and an unlinked provider never re-links +itself. See [Linking to an existing account](/docs/permissions/sso#linking-to-an-existing-account). + --- ## Enterprise Authentication Hardening (ADR-0069) diff --git a/content/docs/permissions/sso.mdx b/content/docs/permissions/sso.mdx index 6c79ebab8d..ab8d04229a 100644 --- a/content/docs/permissions/sso.mdx +++ b/content/docs/permissions/sso.mdx @@ -385,6 +385,35 @@ The Console `/login` and `/register` pages will now show a button for each enabl 3. Browser redirects to the provider's authorization endpoint. 4. Provider redirects back; better-auth validates the OIDC token and creates a session. +## Linking to an existing account + +A social, OIDC or enterprise SSO sign-in may arrive with an email that already belongs to a +local account, one that is not linked to this provider yet. The platform then links the +identity to that account automatically (an *implicit* link) only under these rules: + +- **The local account's email must be verified.** If it is not, no link is written and the + callback redirects with `error=account_not_linked`. The user signs in with their existing + method and links the provider from account settings, or verifies their email first. +- **The platform identity provider is the exception.** `objectstack-cloud` links to an + unverified local account, because it creates the environment owner's account itself, + without a mailbox round-trip. +- **An unlink is honoured.** After a user unlinks a provider, signing in with that provider + no longer re-links it, for any provider, `objectstack-cloud` included. The user links it + again explicitly from account settings (`POST /api/v1/auth/link-social` while signed in), + which also ends the refusal. If the unlink cannot be recorded, the unlink itself fails and + the provider stays linked. + +An explicit link from account settings does not depend on whether the signed-in user's +email is verified. It still requires the provider to vouch for the email (a trusted +provider or an `email_verified` claim) and the email to match the account's. + +`account.accountLinking.requireLocalEmailVerified` in the auth plugin config changes the first +rule. Unset (the default) gives the rules above. `true` applies the verified-email +requirement to every provider, `objectstack-cloud` included. `false` turns the requirement +off, which the library warns is an account-takeover risk; the unlink rule still applies. +`account.accountLinking.trustedProviders` relaxes only the provider-side `email_verified` +claim and never the local requirement. + --- ## Notes From 93ed0240e1c5734c48847e035ced02147735b54c Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 5 Oct 2026 16:10:48 +0000 Subject: [PATCH 09/10] fix(plugin-auth): unlink records are one durable row per provider; platform exception bound to the OAuth method Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_018zT8d8NpiQ1ExhuNd5TxY6 --- ...1846-implicit-account-linking-ownership.md | 1 + .../plugins/plugin-auth/src/auth-manager.ts | 23 +- .../src/implicit-account-linking.test.ts | 114 +++++++++- .../src/implicit-account-linking.ts | 214 ++++++++++-------- 4 files changed, 248 insertions(+), 104 deletions(-) diff --git a/.changeset/21846-implicit-account-linking-ownership.md b/.changeset/21846-implicit-account-linking-ownership.md index 6b47e4903c..d40545929e 100644 --- a/.changeset/21846-implicit-account-linking-ownership.md +++ b/.changeset/21846-implicit-account-linking-ownership.md @@ -15,6 +15,7 @@ Clause-②: no (narrowing) - An external sign-in links implicitly to an existing local user only when that local user's email is verified. Otherwise the sign-in is refused with `error=account_not_linked`, the same code better-auth's own refusal produces. No link is written and the local user stays unverified. A verified local user links as before. - The platform's own identity provider (`objectstack-cloud`) keeps its documented exception and still links to an unverified local user, because it seeds the environment owner's row without a mailbox round-trip. - After a user unlinks a provider, an implicit sign-in through it no longer links the identity again, for any provider. An explicit, signed-in link from account settings (`/link-social`) is still allowed and ends the refusal. If the unlink cannot be recorded, the unlink itself is refused and the provider stays linked. Deleting a user removes the user's unlink records. +- A deployment that passes `secondaryStorage` to the auth plugin now also keeps verification values in the database (`verification.storeInDatabase: true`). The cache still fronts them. This keeps the unlink records durable when the cache evicts entries. Deployments without `secondaryStorage` are unchanged. - `account.accountLinking.requireLocalEmailVerified` now reads as follows. Unset (the default) means the rules above. `true` applies the strict check to every provider, including `objectstack-cloud`. `false` turns off only the local-verification check and keeps the unlink rule. **What to do after upgrading.** diff --git a/packages/plugins/plugin-auth/src/auth-manager.ts b/packages/plugins/plugin-auth/src/auth-manager.ts index 863a8ab4cb..eeca37282f 100644 --- a/packages/plugins/plugin-auth/src/auth-manager.ts +++ b/packages/plugins/plugin-auth/src/auth-manager.ts @@ -35,7 +35,7 @@ import { clearUnlinkTombstone, clearUserUnlinkTombstones, recordUnlinkTombstone, - type LinkingInternalAdapter, + type LinkingAuthContext, refuseImplicitAccountLink, } from './implicit-account-linking.js'; import type { IDataEngine } from '@objectstack/core'; @@ -1607,7 +1607,7 @@ export class AuthManager { validateUserInfo: async (data: any, ctx: any) => (await refuseImplicitAccountLink(data, ctx, { requireLocalEmailVerified: this.implicitLinkRequiresLocalEmailVerified(), - resolveAdapter: () => this.linkingAdapter(), + resolveContext: () => this.linkingContext(), logInfo: (m, meta) => this.config.logger?.info?.(m, meta), })) ?? this.validateAudienceAdmission(data, ctx), }, @@ -1647,6 +1647,15 @@ export class AuthManager { }, verification: { ...AUTH_VERIFICATION_CONFIG, + // A host `secondaryStorage` would otherwise make the cache the ONLY + // home of every verification value (better-auth drops the + // `verification` model from the schema), and the implicit-linking + // unlink records (`implicit-account-linking.ts`) must be durable: an + // evicted record re-opens implicit re-linking without a sound. With + // `storeInDatabase` every verification value is a `sys_verification` + // row — exactly what it is when no `secondaryStorage` is configured, + // ObjectStack's default — and the cache only fronts it. + ...(this.config.secondaryStorage ? { storeInDatabase: true } : {}), }, // Social / OAuth providers @@ -4500,15 +4509,15 @@ export class AuthManager { } /** - * better-auth's internal adapter read off the auth instance — the store for - * the implicit-linking hooks when a call carries no endpoint context (a + * better-auth's auth context read off the auth instance — the store for the + * implicit-linking hooks when a call carries no endpoint context (a * server-side `auth.api.*` call or an internal-adapter write). */ - private async linkingAdapter(): Promise { + private async linkingContext(): Promise { try { const auth: any = await this.getOrCreateAuth(); const context = await auth?.$context; - return context?.internalAdapter as LinkingInternalAdapter | undefined; + return context?.adapter && context?.internalAdapter ? (context as LinkingAuthContext) : undefined; } catch { return undefined; } @@ -7373,7 +7382,7 @@ export class AuthManager { // it is reported at `warn` and never fails the link. It runs FIRST, ahead // of the identity-source stamp, so a stamp failure can never leave a // landed link still refused. - const resolveLinkingAdapter = () => this.linkingAdapter(); + const resolveLinkingAdapter = () => this.linkingContext(); const clearUnlink = async (account: any, ctx: any) => { try { await clearUnlinkTombstone(account, ctx, resolveLinkingAdapter); diff --git a/packages/plugins/plugin-auth/src/implicit-account-linking.test.ts b/packages/plugins/plugin-auth/src/implicit-account-linking.test.ts index 763fab69af..7bbd177776 100644 --- a/packages/plugins/plugin-auth/src/implicit-account-linking.test.ts +++ b/packages/plugins/plugin-auth/src/implicit-account-linking.test.ts @@ -22,6 +22,7 @@ import { decideImplicitLink, linkSourceProviderId, unlinkTombstoneIdentifier, + unlinkTombstoneUserPrefix, } from './implicit-account-linking'; // ── In-memory IDataEngine (the audience-posture harness shape) ─────────────── @@ -47,6 +48,7 @@ const createMemoryEngine = () => { if ('$gte' in v) return actual >= v.$gte; if ('$lt' in v) return actual < v.$lt; if ('$lte' in v) return actual <= v.$lte; + if ('$startsWith' in v) return typeof actual === 'string' && actual.startsWith(v.$startsWith); } return eq(actual, v); }); @@ -261,14 +263,16 @@ const userRow = (engine: any, email: string) => (engine.tables.get('sys_user') ?? []).find((u: any) => u.email === email); const accountsOf = (engine: any, userId: string, providerId: string) => (engine.tables.get('sys_account') ?? []).filter((a: any) => a.user_id === userId && a.provider_id === providerId); -/** The providers the user's unlink record lists (none when there is no record). */ -const unlinkedProviders = (engine: any, userId: string): string[] => { - const rows = (engine.tables.get('sys_verification') ?? []).filter( - (v: any) => v.identifier === unlinkTombstoneIdentifier(userId), - ); - expect(rows.length).toBeLessThanOrEqual(1); - return rows.length ? JSON.parse(rows[0].value).providers : []; -}; +/** The providers the user has an unlink record for, sorted (none when there is no record). */ +const unlinkedProviders = (engine: any, userId: string): string[] => + (engine.tables.get('sys_verification') ?? []) + .filter((v: any) => typeof v.identifier === 'string' && v.identifier.startsWith(unlinkTombstoneUserPrefix(userId))) + .map((v: any) => { + const providerId = JSON.parse(v.value).providerId as string; + expect(v.identifier).toBe(unlinkTombstoneIdentifier(userId, providerId)); + return providerId; + }) + .sort(); const setVerified = (engine: any, email: string, verified: boolean) => { userRow(engine, email).email_verified = verified; }; @@ -278,6 +282,7 @@ const setVerified = (engine: any, email: string, verified: boolean) => { describe('implicit link decision', () => { const base = { providerId: EXTERNAL, + sourceMethod: 'oauth', localEmailVerified: false, requireLocalEmailVerified: true, unlinkedByUser: false, @@ -295,6 +300,15 @@ describe('implicit link decision', () => { expect(decideImplicitLink({ ...base, providerId: PLATFORM_IDP_PROVIDER_ID })).toEqual({ allow: true }); }); + it('binds the platform exception to the OAuth sign-in method, not the provider id alone', () => { + for (const sourceMethod of ['sso-oidc', 'sso-saml', undefined]) { + expect(decideImplicitLink({ ...base, providerId: PLATFORM_IDP_PROVIDER_ID, sourceMethod })).toEqual({ + allow: false, + reason: 'local-email-unverified', + }); + } + }); + it('honours an unlink for every provider, the platform identity provider included', () => { for (const providerId of [EXTERNAL, PLATFORM_IDP_PROVIDER_ID]) { expect(decideImplicitLink({ ...base, providerId, localEmailVerified: true, unlinkedByUser: true })).toEqual({ @@ -548,4 +562,88 @@ describe('implicit link on external sign-in, end to end', () => { expect(userRow(engine, email)).toBeUndefined(); expect(unlinkedProviders(engine, user.id)).toEqual([]); }); + /** A verified user signed up with a password and linked to two external providers. */ + const twoLinkedProviders = async (manager: AuthManager, engine: any, email: string) => { + const session = await signUp(manager, email); + setVerified(engine, email, true); + const user = userRow(engine, email); + idpProfile[EXTERNAL] = { sub: `${email}-ext`, email, email_verified: true }; + idpProfile[OIDC] = { sub: `${email}-oidc`, email, email_verified: true }; + expect((await oauthRoundTrip(manager, EXTERNAL, 'sign-in/social')).searchParams.get('error')).toBeNull(); + expect((await oauthRoundTrip(manager, OIDC, 'sign-in/social')).searchParams.get('error')).toBeNull(); + const [first] = accountsOf(engine, user.id, EXTERNAL); + const [second] = accountsOf(engine, user.id, OIDC); + return { session, user, first, second }; + }; + + it('a later unlink that cannot be recorded keeps every earlier unlink in force', async () => { + const engine = createMemoryEngine(); + const manager = makeManager(engine); + const { session, user, first, second } = await twoLinkedProviders(manager, engine, 'twice@example.test'); + + expect((await post(manager, 'unlink-account', { accountId: first.id }, joinCookies(session))).status).toBe(200); + expect(unlinkedProviders(engine, user.id)).toEqual([EXTERNAL]); + + engine.failInserts.add('sys_verification'); + const failed = await post(manager, 'unlink-account', { accountId: second.id }, joinCookies(session)); + engine.failInserts.delete('sys_verification'); + + expect(failed.status).toBeGreaterThanOrEqual(400); + expect(accountsOf(engine, user.id, OIDC)).toHaveLength(1); + expect(unlinkedProviders(engine, user.id)).toEqual([EXTERNAL]); + const refused = await oauthRoundTrip(manager, EXTERNAL, 'sign-in/social'); + expect(refused.searchParams.get('error')).toBe(IMPLICIT_LINK_REFUSED); + expect(accountsOf(engine, user.id, EXTERNAL)).toHaveLength(0); + }); + + it('concurrent unlinks of two providers both stay recorded', async () => { + const engine = createMemoryEngine(); + const manager = makeManager(engine); + const { session, user, first, second } = await twoLinkedProviders(manager, engine, 'concurrent@example.test'); + + const [a, b] = await Promise.all([ + post(manager, 'unlink-account', { accountId: first.id }, joinCookies(session)), + post(manager, 'unlink-account', { accountId: second.id }, joinCookies(session)), + ]); + + expect([a.status, b.status]).toEqual([200, 200]); + expect(unlinkedProviders(engine, user.id)).toEqual([OIDC, EXTERNAL].sort()); + for (const providerId of [EXTERNAL, OIDC]) { + const target = await oauthRoundTrip(manager, providerId, 'sign-in/social'); + expect(target.searchParams.get('error')).toBe(IMPLICIT_LINK_REFUSED); + } + }); + it('keeps the unlink record in the database when a secondaryStorage cache is configured', async () => { + const engine = createMemoryEngine(); + const cache = new Map(); + const manager = makeManager(engine, { + // The embedded OAuth provider refuses a cache-held session store; it is + // not what this case is about. + plugins: { oidcProvider: false }, + secondaryStorage: { + get: async (key: string) => cache.get(key) ?? null, + set: async (key: string, value: string) => { + cache.set(key, value); + }, + delete: async (key: string) => { + cache.delete(key); + }, + }, + }); + const email = 'cached@example.test'; + const session = await signUp(manager, email); + setVerified(engine, email, true); + const user = userRow(engine, email); + idpProfile[EXTERNAL] = { sub: 'ext-cache', email, email_verified: true }; + expect((await oauthRoundTrip(manager, EXTERNAL, 'sign-in/social')).searchParams.get('error')).toBeNull(); + const [linked] = accountsOf(engine, user.id, EXTERNAL); + expect((await post(manager, 'unlink-account', { accountId: linked.id }, joinCookies(session))).status).toBe(200); + + // The record is a database row, not a cache entry: evicting every + // verification value from the cache leaves the refusal in force. + expect(unlinkedProviders(engine, user.id)).toEqual([EXTERNAL]); + for (const key of [...cache.keys()]) if (key.startsWith('verification:')) cache.delete(key); + const refused = await oauthRoundTrip(manager, EXTERNAL, 'sign-in/social'); + expect(refused.searchParams.get('error')).toBe(IMPLICIT_LINK_REFUSED); + }); }); diff --git a/packages/plugins/plugin-auth/src/implicit-account-linking.ts b/packages/plugins/plugin-auth/src/implicit-account-linking.ts index 735f3e2b1d..be82116f00 100644 --- a/packages/plugins/plugin-auth/src/implicit-account-linking.ts +++ b/packages/plugins/plugin-auth/src/implicit-account-linking.ts @@ -24,13 +24,17 @@ * for every environment, and the environment's owner row is seeded by the * cloud itself with `emailVerified: false` (no mailbox round-trip ever * runs in that IdP-mediated flow). Requiring local verification there - * would lock every owner out of their own environment. + * would lock every owner out of their own environment. The exception + * is bound to the provider id AND the OAuth sign-in method (the cloud + * provider is a generic-OAuth provider): an enterprise SSO provider + * (`sso-oidc` / `sso-saml`) registered under the same id gets no + * exception. * 3. **A user's unlink is honoured.** After a user unlinks provider P, an * implicit sign-in through P must not quietly re-create the link — that * would make unlinking decorative. The unlink is recorded - * ({@link unlinkTombstoneIdentifier}, written before the unlink and - * failing it closed); implicit linking for that - * user + provider is refused while the record stands; an EXPLICIT, + * ({@link unlinkTombstoneIdentifier}: one row per user + provider, + * written before the unlink and failing it closed); implicit linking for + * that user + provider is refused while the record stands; an EXPLICIT, * session-authenticated link-social is still allowed and clears it. * This applies to every provider, the cloud one included: the cloud * exception is about rule 1's verification precondition, not about @@ -107,7 +111,7 @@ const UNLINK_TOMBSTONE_PREFIX = 'account-unlinked'; * The unlink record's lifetime. A record must outlive any realistic gap * between an unlink and the next sign-in; it ends earlier only when the user * re-links explicitly or the user is deleted. better-auth's verification - * store sweeps rows by `expiresAt`, so the value has to be finite. + * sweep deletes rows by `expiresAt`, so the value has to be finite. */ const UNLINK_TOMBSTONE_TTL_MS = 100 * 365 * 24 * 60 * 60 * 1000; @@ -115,17 +119,25 @@ const UNLINK_TOMBSTONE_TTL_MS = 100 * 365 * 24 * 60 * 60 * 1000; const CREDENTIAL_PROVIDER_ID = 'credential'; /** - * ONE record per user, listing every provider the user unlinked and has not - * re-linked explicitly. Per user rather than per user + provider so that a - * user's deletion can clear all of them by identifier — the verification - * store offers no prefix delete. + * ONE row per user + provider. A record is only ever created or deleted, + * never rewritten, so no write passes through a state with less protection + * than before it: a failed create leaves every earlier record standing, and + * two concurrent unlinks each create their own row. Every row of one user + * shares the {@link unlinkTombstoneUserPrefix}, which is what a user's + * deletion clears. */ -export function unlinkTombstoneIdentifier(userId: string): string { - return `${UNLINK_TOMBSTONE_PREFIX}:${userId}`; +export function unlinkTombstoneIdentifier(userId: string, providerId: string): string { + return `${unlinkTombstoneUserPrefix(userId)}${providerId}`; +} + +export function unlinkTombstoneUserPrefix(userId: string): string { + return `${UNLINK_TOMBSTONE_PREFIX}:${userId}:`; } export interface ImplicitLinkInput { providerId: string; + /** The `validateUserInfo` source's `method` (`oauth`, `sso-oidc`, `sso-saml`, …). */ + sourceMethod: string | undefined; /** The EXISTING local user row's `emailVerified`. */ localEmailVerified: boolean; /** The effective local-verification requirement (default `true`). */ @@ -138,13 +150,23 @@ export type ImplicitLinkVerdict = | { allow: true } | { allow: false; reason: 'unlinked-by-user' | 'local-email-unverified' }; +/** + * Is this the platform identity provider's own sign-in? The provider id alone + * is not enough: the cloud provider is a generic-OAuth provider, so its + * sign-ins carry `method: 'oauth'`; an SSO provider registered under the same + * id carries `sso-oidc` / `sso-saml` and is not the platform IdP. + */ +export function isPlatformIdpSource(providerId: string, sourceMethod: string | undefined): boolean { + return providerId === PLATFORM_IDP_PROVIDER_ID && sourceMethod === 'oauth'; +} + /** Pure decision for one implicit link. See the module header for the rules. */ export function decideImplicitLink(input: ImplicitLinkInput): ImplicitLinkVerdict { if (input.unlinkedByUser) return { allow: false, reason: 'unlinked-by-user' }; if ( input.requireLocalEmailVerified && !input.localEmailVerified && - input.providerId !== PLATFORM_IDP_PROVIDER_ID + !isPlatformIdpSource(input.providerId, input.sourceMethod) ) { return { allow: false, reason: 'local-email-unverified' }; } @@ -158,49 +180,53 @@ export function linkSourceProviderId(source: unknown): string | undefined { return typeof id === 'string' && id.length > 0 ? id : undefined; } -/** The slice of better-auth's `internalAdapter` this module uses. */ -export interface LinkingInternalAdapter { - findUserById(id: string): Promise<{ emailVerified?: boolean } | null>; - findVerificationValue(identifier: string): Promise<{ value?: unknown } | null>; - createVerificationValue(data: { identifier: string; value: string; expiresAt: Date }): Promise; - deleteVerificationByIdentifier(identifier: string): Promise; -} - /** - * Resolves the store for a hook or gate call. The endpoint context carries it - * on a request; a server-side call (`auth.api.*` without a request, or an - * internal-adapter write) may carry none, so the caller supplies a fallback - * read off the auth instance itself. + * The slice of better-auth's DATABASE adapter (`AuthContext.adapter`) this + * module uses. + * + * The record is always a `sys_verification` row, written and read through + * the database adapter, never through `internalAdapter.*VerificationValue`. + * Those helpers would put it in a host's `secondaryStorage` cache, where an + * eviction silently re-opens implicit re-linking. A host `secondaryStorage` + * on its own would also drop the `verification` model from better-auth's + * schema, so the auth manager sets `verification.storeInDatabase: true` + * whenever one is configured: every verification value then stays a + * database row (as it is with no cache, the default), and the cache only + * fronts it. */ -export type LinkingAdapterResolver = (ctx: unknown) => Promise; +export interface LinkingDbAdapter { + findOne(data: { model: string; where: Array> }): Promise; + create(data: { model: string; data: Record; forceAllowId?: boolean }): Promise; + deleteMany(data: { model: string; where: Array> }): Promise; +} -export const internalAdapterOf = (ctx: unknown): LinkingInternalAdapter | undefined => - (ctx as { context?: { internalAdapter?: LinkingInternalAdapter } } | undefined)?.context?.internalAdapter; +/** The slice of better-auth's `AuthContext` this module uses. */ +export interface LinkingAuthContext { + adapter: LinkingDbAdapter; + internalAdapter: { findUserById(id: string): Promise<{ emailVerified?: boolean } | null> }; +} /** - * The providers a user's unlink record lists. A record whose value cannot be - * read is an ERROR, not an empty list: answering "nothing unlinked" for a - * record that exists would re-open exactly what the record closes. + * Resolves the auth context for a hook or gate call. The endpoint context + * carries it on a request; a server-side call (`auth.api.*` without a + * request, or an internal-adapter write) may carry none, so the caller + * supplies a fallback read off the auth instance itself. */ -async function readUnlinkedProviders(adapter: LinkingInternalAdapter, userId: string): Promise { - const row = await adapter.findVerificationValue(unlinkTombstoneIdentifier(userId)); - if (!row) return []; - const parsed = JSON.parse(String(row.value)) as { providers?: unknown }; - if (!Array.isArray(parsed?.providers) || !parsed.providers.every((p) => typeof p === 'string')) { - throw new Error('unlink record: unreadable value'); - } - return parsed.providers as string[]; -} +export type LinkingContextResolver = (ctx: unknown) => Promise; + +export const authContextOf = (ctx: unknown): LinkingAuthContext | undefined => { + const c = (ctx as { context?: Partial } | undefined)?.context; + return c?.adapter && c?.internalAdapter ? (c as LinkingAuthContext) : undefined; +}; -async function writeUnlinkedProviders(adapter: LinkingInternalAdapter, userId: string, providers: string[]): Promise { - const identifier = unlinkTombstoneIdentifier(userId); - await adapter.deleteVerificationByIdentifier(identifier); - if (providers.length === 0) return; - await adapter.createVerificationValue({ - identifier, - value: JSON.stringify({ userId, providers, updatedAt: new Date().toISOString() }), - expiresAt: new Date(Date.now() + UNLINK_TOMBSTONE_TTL_MS), +const VERIFICATION_MODEL = 'verification'; + +async function hasUnlinkRecord(adapter: LinkingDbAdapter, userId: string, providerId: string): Promise { + const row = await adapter.findOne({ + model: VERIFICATION_MODEL, + where: [{ field: 'identifier', value: unlinkTombstoneIdentifier(userId, providerId) }], }); + return row != null; } /** @@ -223,7 +249,7 @@ async function isExplicitLinkFlow(userId: string): Promise { export interface ImplicitLinkGateOptions { requireLocalEmailVerified: boolean; - resolveAdapter?: LinkingAdapterResolver; + resolveContext?: LinkingContextResolver; logInfo?: (message: string, meta?: Record) => void; } @@ -235,7 +261,7 @@ export interface ImplicitLinkGateOptions { * — when the store cannot answer. */ export async function refuseImplicitAccountLink( - data: { user?: Record; source?: { action?: string } } | undefined, + data: { user?: Record; source?: { action?: string; method?: string } } | undefined, ctx: unknown, options: ImplicitLinkGateOptions, ): Promise<{ error: string; errorDescription: string } | undefined> { @@ -243,18 +269,19 @@ export async function refuseImplicitAccountLink( const providerId = linkSourceProviderId(data.source); const userId = typeof data.user?.id === 'string' ? (data.user.id as string) : undefined; if (userId && (await isExplicitLinkFlow(userId))) return undefined; - const adapter = internalAdapterOf(ctx) ?? (await options.resolveAdapter?.(ctx)); - if (!providerId || !userId || !adapter) { + const auth = authContextOf(ctx) ?? (await options.resolveContext?.(ctx)); + if (!providerId || !userId || !auth) { throw new Error('implicit account link: provider, user or store unavailable — refusing'); } - const unlinked = await readUnlinkedProviders(adapter, userId); - const local = await adapter.findUserById(userId); + const unlinked = await hasUnlinkRecord(auth.adapter, userId, providerId); + const local = await auth.internalAdapter.findUserById(userId); if (!local) throw new Error('implicit account link: local user not found — refusing'); const verdict = decideImplicitLink({ providerId, + sourceMethod: typeof data.source.method === 'string' ? data.source.method : undefined, localEmailVerified: local.emailVerified === true, requireLocalEmailVerified: options.requireLocalEmailVerified, - unlinkedByUser: unlinked.includes(providerId), + unlinkedByUser: unlinked, }); if (verdict.allow) return undefined; options.logInfo?.('[auth] implicit account link refused', { providerId, reason: verdict.reason }); @@ -269,69 +296,78 @@ export async function refuseImplicitAccountLink( /** * `account.delete.before` half: a user's own unlink (`/unlink-account`) - * records the provider BEFORE the account row is deleted. It throws when the - * record cannot be written, and a throw from a `delete.before` hook aborts - * the delete, so the unlink answers an error and the identity stays linked — - * fail closed. An unlink that succeeded without its record would leave the - * provider free to re-link implicitly while the user believes it gone. (A - * record written for a delete that then fails is harmless: the record is read - * only when NO account for the provider is linked.) Other deletions (user - * removal, admin tooling) record nothing. + * creates the provider's record BEFORE the account row is deleted. It throws + * when the record cannot be written, and a throw from a `delete.before` hook + * aborts the delete, so the unlink answers an error and the identity stays + * linked — fail closed. An unlink that succeeded without its record would + * leave the provider free to re-link implicitly while the user believes it + * gone. A record written for a delete that then fails is harmless: the record + * is read only when NO account for the provider is linked. Other deletions + * (user removal, admin tooling) record nothing. * - * The record lives in better-auth's verification store. On a deployment that - * configures a `secondaryStorage` without `verification.storeInDatabase`, - * that store is the secondary storage: the record then lasts only as long as - * the cache keeps it, and an evicted record re-opens implicit linking for - * that provider. ObjectStack wires no `secondaryStorage`, so the record is a - * database row. + * The record is a `sys_verification` row whether or not a `secondaryStorage` + * cache is configured (see {@link LinkingDbAdapter}). */ export async function recordUnlinkTombstone( account: unknown, ctx: unknown, - resolveAdapter?: LinkingAdapterResolver, + resolveContext?: LinkingContextResolver, ): Promise { const a = account as { userId?: unknown; providerId?: unknown } | null; const path = (ctx as { path?: unknown } | undefined)?.path; if (path !== '/unlink-account') return; if (typeof a?.userId !== 'string' || typeof a?.providerId !== 'string') return; if (a.providerId === CREDENTIAL_PROVIDER_ID) return; - const adapter = internalAdapterOf(ctx) ?? (await resolveAdapter?.(ctx)); - if (!adapter) throw new Error('unlink record: store unavailable'); - const providers = await readUnlinkedProviders(adapter, a.userId); - if (providers.includes(a.providerId)) return; - await writeUnlinkedProviders(adapter, a.userId, [...providers, a.providerId]); + const auth = authContextOf(ctx) ?? (await resolveContext?.(ctx)); + if (!auth) throw new Error('unlink record: store unavailable'); + if (await hasUnlinkRecord(auth.adapter, a.userId, a.providerId)) return; + const now = new Date(); + await auth.adapter.create({ + model: VERIFICATION_MODEL, + data: { + identifier: unlinkTombstoneIdentifier(a.userId, a.providerId), + value: JSON.stringify({ userId: a.userId, providerId: a.providerId, unlinkedAt: now.toISOString() }), + expiresAt: new Date(now.getTime() + UNLINK_TOMBSTONE_TTL_MS), + createdAt: now, + updatedAt: now, + }, + }); } /** - * `account.create.after` half: a link that lands removes its provider from - * the unlink record. While the provider is listed an implicit link is - * refused, so a link that lands is an explicit one (or an operator act) — - * exactly the "until the user re-links" end the ruling sets. + * `account.create.after` half: a link that lands deletes its provider's + * record. While the record stands an implicit link is refused, so a link that + * lands is an explicit one (or an operator act) — exactly the "until the user + * re-links" end the ruling sets. Only that provider's rows are touched. */ export async function clearUnlinkTombstone( account: unknown, ctx: unknown, - resolveAdapter?: LinkingAdapterResolver, + resolveContext?: LinkingContextResolver, ): Promise { const a = account as { userId?: unknown; providerId?: unknown } | null; if (typeof a?.userId !== 'string' || typeof a?.providerId !== 'string') return; if (a.providerId === CREDENTIAL_PROVIDER_ID) return; - const adapter = internalAdapterOf(ctx) ?? (await resolveAdapter?.(ctx)); - if (!adapter) return; - const providers = await readUnlinkedProviders(adapter, a.userId); - if (!providers.includes(a.providerId)) return; - await writeUnlinkedProviders(adapter, a.userId, providers.filter((p) => p !== a.providerId)); + const auth = authContextOf(ctx) ?? (await resolveContext?.(ctx)); + if (!auth) throw new Error('unlink record: store unavailable'); + await auth.adapter.deleteMany({ + model: VERIFICATION_MODEL, + where: [{ field: 'identifier', value: unlinkTombstoneIdentifier(a.userId, a.providerId) }], + }); } /** `user.delete.after` half: a deleted user leaves no unlink record behind. */ export async function clearUserUnlinkTombstones( user: unknown, ctx: unknown, - resolveAdapter?: LinkingAdapterResolver, + resolveContext?: LinkingContextResolver, ): Promise { const id = (user as { id?: unknown } | null)?.id; - if (typeof id !== 'string') return; - const adapter = internalAdapterOf(ctx) ?? (await resolveAdapter?.(ctx)); - if (!adapter) throw new Error('unlink record: store unavailable'); - await adapter.deleteVerificationByIdentifier(unlinkTombstoneIdentifier(id)); + if (typeof id !== 'string' || id.length === 0) return; + const auth = authContextOf(ctx) ?? (await resolveContext?.(ctx)); + if (!auth) throw new Error('unlink record: store unavailable'); + await auth.adapter.deleteMany({ + model: VERIFICATION_MODEL, + where: [{ field: 'identifier', operator: 'starts_with', value: unlinkTombstoneUserPrefix(id) }], + }); } From f7814a1b914e3ea2b8b7656d59d066cdff803b6a Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 5 Oct 2026 18:05:08 +0000 Subject: [PATCH 10/10] docs(changeset): note the one-time effect on in-flight verification values for hosts with secondaryStorage Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_018zT8d8NpiQ1ExhuNd5TxY6 --- .changeset/21846-implicit-account-linking-ownership.md | 1 + 1 file changed, 1 insertion(+) diff --git a/.changeset/21846-implicit-account-linking-ownership.md b/.changeset/21846-implicit-account-linking-ownership.md index d40545929e..e065393f9f 100644 --- a/.changeset/21846-implicit-account-linking-ownership.md +++ b/.changeset/21846-implicit-account-linking-ownership.md @@ -22,3 +22,4 @@ Clause-②: no (narrowing) - A user refused this way signs in with their existing method, then links the provider from account settings, or verifies their email first. - To let unverified local users link implicitly again, set `account.accountLinking.requireLocalEmailVerified: false`. Before you do, read the library's warning about account takeover. +- If you pass `secondaryStorage`: verification values written to the cache alone before the upgrade (password-reset links, one-time codes, magic links and email-verification links that were in flight at deploy time) can no longer be consumed afterwards. Users who hit this request a fresh link or code once.