Skip to content

Commit 598e7a7

Browse files
committed
fix(plugin-auth): fail an unlink closed, bind explicit links to their user, clear records on user delete
Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018zT8d8NpiQ1ExhuNd5TxY6
1 parent 6b0f00e commit 598e7a7

3 files changed

Lines changed: 195 additions & 58 deletions

File tree

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,23 @@
11
---
2-
'@objectstack/plugin-auth': patch
2+
'@objectstack/plugin-auth': minor
33
---
44

5-
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.
5+
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
66

7-
Clause-②: no
7+
Clause-②: no (narrowing)
88

9-
- **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.
10-
- **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.
11-
- **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.
12-
- **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.
9+
<!-- 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. -->
10+
11+
**BREAKING for deployments that relied on external sign-in (OAuth, OIDC, SSO) linking implicitly to a local user whose email is not verified.**
12+
13+
**What changed.**
14+
15+
- 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.
16+
- 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.
17+
- 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.
18+
- `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.
19+
20+
**What to do after upgrading.**
21+
22+
- A user refused this way signs in with their existing method, then links the provider from account settings, or verifies their email first.
1323
- To let unverified local users link implicitly again, set `account.accountLinking.requireLocalEmailVerified: false`. Before you do, read the library's warning about account takeover.

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

Lines changed: 66 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,9 @@ import { shouldStampOwnerVerifiedAtCreation } from './walled-owner-operator-stam
3333
import {
3434
PLATFORM_IDP_PROVIDER_ID,
3535
clearUnlinkTombstone,
36+
clearUserUnlinkTombstones,
3637
recordUnlinkTombstone,
38+
type LinkingInternalAdapter,
3739
refuseImplicitAccountLink,
3840
} from './implicit-account-linking.js';
3941
import type { IDataEngine } from '@objectstack/core';
@@ -1605,6 +1607,7 @@ export class AuthManager {
16051607
validateUserInfo: async (data: any, ctx: any) =>
16061608
(await refuseImplicitAccountLink(data, ctx, {
16071609
requireLocalEmailVerified: this.implicitLinkRequiresLocalEmailVerified(),
1610+
resolveAdapter: () => this.linkingAdapter(),
16081611
logInfo: (m, meta) => this.config.logger?.info?.(m, meta),
16091612
})) ?? this.validateAudienceAdmission(data, ctx),
16101613
},
@@ -4496,6 +4499,21 @@ export class AuthManager {
44964499
return (this.config as any)?.account?.accountLinking?.requireLocalEmailVerified !== false;
44974500
}
44984501

4502+
/**
4503+
* better-auth's internal adapter read off the auth instance — the store for
4504+
* the implicit-linking hooks when a call carries no endpoint context (a
4505+
* server-side `auth.api.*` call or an internal-adapter write).
4506+
*/
4507+
private async linkingAdapter(): Promise<LinkingInternalAdapter | undefined> {
4508+
try {
4509+
const auth: any = await this.getOrCreateAuth();
4510+
const context = await auth?.$context;
4511+
return context?.internalAdapter as LinkingInternalAdapter | undefined;
4512+
} catch {
4513+
return undefined;
4514+
}
4515+
}
4516+
44994517
/** OAuth providerIds that are OPERATOR-REGISTERED identity authorities (enterprise `oidcProviders`, incl. the cloud platform IdP). */
45004518
private enterpriseOAuthProviderIds(): ReadonlySet<string> {
45014519
const ids = new Set<string>();
@@ -7352,19 +7370,22 @@ export class AuthManager {
73527370
// rule 3). While the record stands an implicit link is refused, so a link
73537371
// that lands here is an explicit one. Failure to clear is functional (the
73547372
// user sees a refusal on their next implicit sign-in and can re-link), so
7355-
// it is reported at `warn` and never fails the link.
7373+
// it is reported at `warn` and never fails the link. It runs FIRST, ahead
7374+
// of the identity-source stamp, so a stamp failure can never leave a
7375+
// landed link still refused.
7376+
const resolveLinkingAdapter = () => this.linkingAdapter();
73567377
const clearUnlink = async (account: any, ctx: any) => {
73577378
try {
7358-
await clearUnlinkTombstone(account, ctx);
7379+
await clearUnlinkTombstone(account, ctx, resolveLinkingAdapter);
73597380
} catch (e) {
73607381
this.config.logger?.warn?.('[auth] could not clear the unlink record after a link', {
73617382
error: (e as Error)?.message,
73627383
});
73637384
}
73647385
};
73657386
const stamp = async (account: any, ctx: any) => {
7366-
await this.stampIdentitySource(account, ctx);
73677387
await clearUnlink(account, ctx);
7388+
await this.stampIdentitySource(account, ctx);
73687389
};
73697390
const hostAccountAfter = (host as any)?.account?.create?.after;
73707391
const after = hostAccountAfter
@@ -7375,26 +7396,52 @@ export class AuthManager {
73757396
: stamp;
73767397

73777398
// A user's unlink is recorded so the provider cannot re-link implicitly
7378-
// (implicit-account-linking.ts, rule 3). A record that does not land
7379-
// leaves the unlink answering 200 while the provider still re-links on
7380-
// the next sign-in — the protection the user asked for is silently
7381-
// absent — so the failure is reported at `error`, once per unlink.
7382-
const hostAccountDeleteAfter = (host as any)?.account?.delete?.after;
7383-
const accountDeleteAfter = async (account: any, ctx: any) => {
7384-
const result = hostAccountDeleteAfter ? await hostAccountDeleteAfter(account, ctx) : undefined;
7399+
// (implicit-account-linking.ts, rule 3) — BEFORE the account row goes, so
7400+
// that a record which cannot be written aborts the unlink: a throw from a
7401+
// `delete.before` hook propagates out of better-auth's delete, the unlink
7402+
// answers an error and the identity stays linked. Fail closed: an unlink
7403+
// that answered success without its record would leave the provider free
7404+
// to re-link on the next sign-in. Reported at `error`, once per refusal.
7405+
// A host `delete.before` runs first; its `false` (abort) is honoured
7406+
// before anything is recorded.
7407+
const hostAccountDeleteBefore = (host as any)?.account?.delete?.before;
7408+
const accountDeleteBefore = async (account: any, ctx: any) => {
7409+
const result = hostAccountDeleteBefore ? await hostAccountDeleteBefore(account, ctx) : undefined;
7410+
if (result === false) return false;
73857411
try {
7386-
await recordUnlinkTombstone(account, ctx);
7412+
await recordUnlinkTombstone(account, ctx, resolveLinkingAdapter);
73877413
} catch (e) {
73887414
this.audienceLogError(
7389-
'[auth] unlink record NOT written: the provider can still re-link this account implicitly ' +
7390-
'on its next sign-in although the unlink answered success. Check the auth store ' +
7391-
'(sys_verification) is writable, then have the user unlink again.',
7415+
'[auth] unlink refused: its record could not be written, so the provider stays linked. ' +
7416+
'Without the record the provider could re-link this account implicitly on its next sign-in. ' +
7417+
'Check the auth store (sys_verification) is writable, then unlink again.',
73927418
{ providerId: account?.providerId, error: (e as Error)?.message },
73937419
);
7420+
throw e;
73947421
}
73957422
return result;
73967423
};
73977424

7425+
// A deleted user leaves no unlink record behind (implicit-account-linking.ts).
7426+
// A record that outlives its user is inert — a new user never has the
7427+
// deleted user's id — so a failure is reported at `warn` and never fails
7428+
// the deletion.
7429+
const clearUserUnlinks = async (user: any, ctx: any) => {
7430+
try {
7431+
await clearUserUnlinkTombstones(user, ctx, resolveLinkingAdapter);
7432+
} catch (e) {
7433+
this.config.logger?.warn?.('[auth] could not clear the unlink record of a deleted user', {
7434+
error: (e as Error)?.message,
7435+
});
7436+
}
7437+
};
7438+
const hostUserDeleteAfter = (host as any)?.user?.delete?.after;
7439+
const userDeleteAfter = async (user: any, ctx: any) => {
7440+
const result = hostUserDeleteAfter ? await hostUserDeleteAfter(user, ctx) : undefined;
7441+
await clearUserUnlinks(user, ctx);
7442+
return result;
7443+
};
7444+
73987445
// ADR-0093 D9 — default active-org on session create. Without it, a user
73997446
// with memberships logs in with `activeOrganizationId = null`: better-auth
74007447
// org endpoints can't resolve an active org (single-org invite dead-end)
@@ -7600,7 +7647,7 @@ export class AuthManager {
76007647
},
76017648
delete: {
76027649
...((host as any)?.account?.delete ?? {}),
7603-
after: accountDeleteAfter,
7650+
before: accountDeleteBefore,
76047651
},
76057652
},
76067653
user: {
@@ -7610,6 +7657,10 @@ export class AuthManager {
76107657
before: userBefore,
76117658
after: userAfter,
76127659
},
7660+
delete: {
7661+
...((host as any)?.user?.delete ?? {}),
7662+
after: userDeleteAfter,
7663+
},
76137664
},
76147665
session: {
76157666
...((host as any)?.session ?? {}),

0 commit comments

Comments
 (0)