fix(plugin-auth)!: implicit account linking requires the standard local-ownership condition; unlink is honoured - #21872
Conversation
Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018zT8d8NpiQ1ExhuNd5TxY6
Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018zT8d8NpiQ1ExhuNd5TxY6
Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018zT8d8NpiQ1ExhuNd5TxY6
Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018zT8d8NpiQ1ExhuNd5TxY6
Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018zT8d8NpiQ1ExhuNd5TxY6
📓 Docs Drift CheckThis PR changes 1 package(s): 24 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: ⛔ 9 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails. What this run could not see
Coarse fallback — 15 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin f1fe29a0182b07c5cf126d3792ce6581bb1dd2e8 && git checkout f1fe29a0182b07c5cf126d3792ce6581bb1dd2e8
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 25eb7de8ad49ca5c943a677e5ee22034afb8ba8e f7814a1b914e3ea2b8b7656d59d066cdff803b6a && git checkout -B drift-repro 25eb7de8ad49ca5c943a677e5ee22034afb8ba8e && git merge --no-ff f7814a1b914e3ea2b8b7656d59d066cdff803b6a
node scripts/docs-audit/affected-docs.mjs --json 25eb7de8ad49ca5c943a677e5ee22034afb8ba8e
|
… user, clear records on user delete Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018zT8d8NpiQ1ExhuNd5TxY6
…OIDC discovery, fail-closed unlink, user delete Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018zT8d8NpiQ1ExhuNd5TxY6
Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018zT8d8NpiQ1ExhuNd5TxY6
Contract reviewServed-tier: Inputs: card #21846 (body + 5 comments: triage, claim, two os-dev reports, the Clause-② correction), PR #21872 body and its 7-file list, the net diff ① Derived judgments
② Semver levelThe diff publishes a behaviour narrowing in Clause-②: no (narrowing) — correct. There is no widening (no new export, no new authorable key), and the narrowing arm comes with banner, ③ Boundary flags
Gates: 43 success, 9 skipped (Auto Label, Check PR Size, Packed-tarball smoke, Console Pin Gate: all opt-in or not applicable), and 1 Check Changeset run still in_progress. Three sibling Check Changeset runs on this head are green. Landing waits for every check to be green. Implemented-by: VERDICT: PASS |
…atform exception bound to the OAuth method Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018zT8d8NpiQ1ExhuNd5TxY6
…alues for hosts with secondaryStorage Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018zT8d8NpiQ1ExhuNd5TxY6
Contract reviewServed-tier: Inputs read: card #21846 body and all 7 comments (triage 5991270306, claim 5991690180, dev reports 5994181249 / 5996242400 / 5999082055, Clause-② correction 5996304603, cross-lane note 5996720856); PR #21872 body and file list (7 files, none on a governed surface); net diff ① Derived judgmentsAccept-set changes the diff implies, each judged:
Public surface (package
② Semver level
Clause-②: no (narrowing) ③ Boundary flags
Implemented-by: VERDICT: PASS |
⛔ merge queue 构建失败 — 先分诊,再决定要不要重排队列构建 37371558473 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集), 失败的 job(日志抽取,best effort):
跨 PR 相同签名(24h,按失败测试文件聚合):
历史信号:
分诊清单:
Generated by Claude Code · merge-queue-triage workflow (#4859) |
⛔ merge queue 构建失败 — 先分诊,再决定要不要重排队列构建 37374282440 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集), 失败的 job(日志抽取,best effort):
跨 PR 相同签名(24h,按失败测试文件聚合):
历史信号:
分诊清单:
Generated by Claude Code · merge-queue-triage workflow (#4859) |
Fixes #21846
Clause-②: no (narrowing)
What changes
Implicit account linking on external sign-in (OAuth, OIDC, SSO) now requires the library's standard local-ownership condition. The platform identity provider keeps its documented exception, and a user's unlink is honoured. This follows the ruling recorded on the card (「算漏洞,收紧」).
objectstack-cloud) links implicitly only to a local user whose email is verified. Otherwise the callback answerserror=account_not_linked, the same code better-auth's own refusal produces. No link is written and the local row stays unverified, so the link no longer setsemailVerifiedon an unverified row.emailVerified=false). The exception applies only to its OAuth sign-in path (source.method === 'oauth'), so an SSO provider registered under the same id gets no exception./link-socialis still allowed and ends the refusal.account.accountLinking.requireLocalEmailVerified:true: also handed to better-auth, so the strict form applies to every provider, the platform one included;false: turns off only the local-verification check; the unlink rule stays.Mechanism (better-auth 1.7.3, measured in the installed
dist/)requireLocalEmailVerifiedis one global boolean. It has no per-provider form, andtrustedProvidersdoes not relax it, so it cannot carry the platform exception. The vendor flag therefore staysfalseby default, and the requirement is enforced at theuser.validateUserInfoseam.handleOAuthUserInfocallsuser.validateUserInfowithaction: 'link-account'and the provider id, right beforelinkAccountand theemailVerifiedflip. Every implicit-link entry goes through it: the OAuth callback, id-token sign-in, one-tap, oauth-proxy and SSO.linkin the parsed OAuth state (getOAuthState()), and only when that state'slink.userIdequals the user being linked.generateStatewriteslinkafter the client'sadditionalData, so a client cannot forge it.sys_verificationrow per user and provider (account-unlinked:<user id>:<provider id>), created inaccount.delete.before, scoped to the/unlink-accountpath. A row is only ever created or deleted, never rewritten, so no write passes through a state with less protection and concurrent unlinks each keep their own row. A failed create is logged aterrorand rethrown, so the unlink fails and the provider stays linked (fail-closed). A landed link deletes only that provider's row, before the identity source is stamped; deleting the user deletes all of that user's rows by prefix. Records go through the database adapter. When a host configures better-authsecondaryStorage, the auth manager now also setsverification.storeInDatabase: true, so the record stays a database row behind the cache and survives eviction; hosts withoutsecondaryStorageare unchanged.New module:
packages/plugins/plugin-auth/src/implicit-account-linking.ts. Wiring:auth-manager.ts(validateUserInfo,account.accountLinking,composeDatabaseHooks).Docs:
content/docs/permissions/sso.mdxgains a "Linking to an existing account" section (the verified-email rule, the platform-provider exception, unlink and explicit re-link, the operator override, and whattrustedProvidersdoes and does not relax);content/docs/permissions/authentication.mdxpoints to it from the OAuth callback step.auth-service.mdxandservices-checklist.mdx, also named by the docs drift check, say nothing about linking and are unchanged.Tests
All at head
93ed0240e1unless noted.pnpm --filter @objectstack/plugin-auth exec vitest run --maxWorkers=2: 121 files, 2538 passed, 10 skipped.src/implicit-account-linking.test.ts: 23 passed. Besides the decision table, the vendor config and the end-to-end OAuth round trips over the real better-auth pipeline (stubbed IdP, in-memory engine), it covers:/sign-in/social: refused for an unverified user, linked for a verified one;linkinadditionalDatais refused;secondaryStorage, the record is a database row and survives evicting every verification cache entry;pnpm --filter @objectstack/plugin-auth typecheck(src, examples,check:test-typecheck): exit 0.scripts/ablation-replace.mjs, each restored to the HEAD blob withgit diff HEADempty:account.delete.beforeremoved: the store-fault test fails;dispatch-gates --ranat83010a685e(round 2): 105 derived, 104 run with exit 0,check:dual-build-cjs-loadsNOT MEASURED (exit 3, it needs a whole-workspace build; declared to CI). Also exit 0:check:adr-0087-registration,check:error-code-casing,check:durability-log-level,check:startup-registry-verdict.scripts/engine-double-contract.pinned.jsonis regenerated (--write) for the new test file's pinned double, a coverage-only addition.eslint --no-inline-config --format jsonover the 3 changed TS files: 0 errors, 0 warnings. The config has no type-aware linting, so this diff cannot change a verdict on an untouched file.Acceptance notes
account_not_linked. The vendor's own refusal on those paths answers 401OAUTH_LINK_ERROR. On the browser callback the two are identical (error=account_not_linked). No in-repo or objectui consumer reads either code.objectstack-cloudmust re-link from account settings before platform SSO signs them in to that environment again. That follows the ruling's wording, and the platform exception is about the verification precondition only.requireLocalEmailVerifiedbecomes unconditional, the platform-provider exception needs a new carrier, for example the owner seed. Carrier: the PR that bumps better-auth to that minor, where the pinned end-to-end test turns red.link.userIdbinding cannot be reached through a real flow today (the explicit-link callback always passes the linking user), so no test turns it red without a synthetic state.Generated by Claude Code