fix(plugin-auth)!: implicit account linking requires the standard local-ownership condition; unlink is honoured

.changeset/21846-implicit-account-linking-ownership.md

@@ -0,0 +1,25 @@
+---
+'@objectstack/plugin-auth': minor
+---
+
+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 (narrowing)
+
+<!-- adr-0087: not-required (no-migration-prescription) No authorable key, export or config field is removed or renamed: the change narrows when an external sign-in links implicitly to an existing local user, and nothing an author wrote needs rewriting. The one config key it reads, account.accountLinking.requireLocalEmailVerified, keeps its name and gains an explicit opt-out meaning. -->
+
+**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.
+- 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.**
+
+- 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.

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)

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

packages/plugins/plugin-auth/src/auth-manager.ts

@@ -30,6 +30,14 @@ import {
   type VerificationDeclarant,
 } from './audience-posture.js';
 import { shouldStampOwnerVerifiedAtCreation } from './walled-owner-operator-stamp.js';
+import {
+  PLATFORM_IDP_PROVIDER_ID,
+  clearUnlinkTombstone,
+  clearUserUnlinkTombstones,
+  recordUnlinkTombstone,
+  type LinkingAuthContext,
+  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,47 +1598,64 @@ 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(),
+            resolveContext: () => this.linkingContext(),
+            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 ?? []),
           ])),
         },
       },
       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
@@ -4473,6 +4498,31 @@ 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;
+  }
+
+  /**
+   * 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 linkingContext(): Promise<LinkingAuthContext | undefined> {
+    try {
+      const auth: any = await this.getOrCreateAuth();
+      const context = await auth?.$context;
+      return context?.adapter && context?.internalAdapter ? (context as LinkingAuthContext) : undefined;
+    } catch {
+      return undefined;
+    }
+  }
+
   /** OAuth providerIds that are OPERATOR-REGISTERED identity authorities (enterprise `oidcProviders`, incl. the cloud platform IdP). */
   private enterpriseOAuthProviderIds(): ReadonlySet<string> {
     const ids = new Set<string>();
@@ -7325,7 +7375,27 @@ 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. It runs FIRST, ahead
+    // of the identity-source stamp, so a stamp failure can never leave a
+    // landed link still refused.
+    const resolveLinkingAdapter = () => this.linkingContext();
+    const clearUnlink = async (account: any, ctx: any) => {
+      try {
+        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,
+        });
+      }
+    };
+    const stamp = async (account: any, ctx: any) => {
+      await clearUnlink(account, ctx);
+      await this.stampIdentitySource(account, ctx);
+    };
     const hostAccountAfter = (host as any)?.account?.create?.after;
     const after = hostAccountAfter
       ? async (account: any, ctx: any) => {
@@ -7334,6 +7404,53 @@ export class AuthManager {
         }
       : stamp;
 
+    // A user's unlink is recorded so the provider cannot re-link implicitly
+    // (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, resolveLinkingAdapter);
+      } catch (e) {
+        this.audienceLogError(
+          '[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)
@@ -7537,6 +7654,10 @@ export class AuthManager {
           ...((host as any)?.account?.create ?? {}),
           after,
         },
+        delete: {
+          ...((host as any)?.account?.delete ?? {}),
+          before: accountDeleteBefore,
+        },
       },
       user: {
         ...((host as any)?.user ?? {}),
@@ -7545,6 +7666,10 @@ export class AuthManager {
           before: userBefore,
           after: userAfter,
         },
+        delete: {
+          ...((host as any)?.user?.delete ?? {}),
+          after: userDeleteAfter,
+        },
       },
       session: {
         ...((host as any)?.session ?? {}),