Skip to content

fix(plugin-auth)!: under local sign-in off, the last identity provider is the last way in; the direct sign-in door keeps ?redirect= - #33623

Queued
objectstack-fleet[bot] wants to merge 6 commits into
mainfrom
claude/area-A1-33358
Queued

objectstack-fleet[bot] wants to merge 6 commits into
mainfrom
claude/area-A1-33358

Conversation

@objectstack-fleet

@objectstack-fleet objectstack-fleet Bot commented Nov 15, 2026 •

Copy link
Copy Markdown
Contributor

Part of #33358

Clause-②: yes (narrowing) — narrowed: with direct sign-in on and its local sign-in path off (directSignIn.localSignIn: 'off', or OS_AUTH_DIRECT_SIGN_IN_LOCAL_PATH=off), POST /api/v1/auth/sso/delete-provider on the last identity provider answers 409 DELETE_RESTRICTED and deletes nothing while email and password is switched on, where it deleted the row; email and password, and phone sign-in, no longer count as a way in there, because the sign-in page does not offer them. Widened: module exports (not on the package entry) pageOffersLocalSignIn, pageOffersEmailPassword and carriedRedirect, an optional localSignIn on SignInMethodConfigView.features, and an asThePageOffers option on otherSignInMethods; the refusal under off answers with a new message, the copy key enterpriseSsoLocalOff, in all 11 locales.
Metadata-protocol: none
Skills-docs: none — no skill under skills/** teaches direct sign-in, the provider delete or the boot report; content/docs/permissions/sso.mdx gains the rule in this change.

维护者速读

  • 改了什么:直接登录把本地登录路径设为 off 时,登录页不再显示邮箱/密码和手机登录;现在"保留最后一种登录方式"的检查也不再把它们算作登录方式,所以删除最后一个身份提供方会被拒绝(409),启动时也会报错提示。另外,直接登录入口把人送回登录页时(身份提供方报错、被限流、无法发起)会带上原来的 ?redirect=。
  • 为什么改:之前在 off 下删掉最后一个身份提供方会成功,随后登录页只显示"没有可用的登录方式",所有人(包括管理员)被锁在外面;深链接用户在出错后重试会落到首页而不是原来要去的页面。
  • 风险与回滚:只影响开启直接登录且本地登录路径为 off 的自托管部署(之前允许的删除现在被拒,属于收窄,已标 BREAKING);hidden(默认)和未开启直接登录的部署行为不变。回滚即 revert 本 PR。
  • 先看:packages/plugins/plugin-auth/src/sign-in-method-floor.ts(ssoProviderDeleteRefusal 与 resolveNoSignInMethodReport 两处判断)、packages/plugins/plugin-auth/src/direct-sign-in.ts(carriedRedirect 与两个落地地址)、.changeset/33358-local-sign-in-off-floor.md。
  • 两分钟验证:pnpm --filter @objectstack/plugin-auth exec vitest run src/sign-in-method-floor.test.ts src/direct-sign-in.test.ts,预期全部通过,其中包含 under the local sign-in path off, deleting the last provider is refused 与 keeps the page's ?redirect= 几条。
  • 需要你做的:无,席位落地。

What changes

Items covered from #33358: 1 (N7, the sign-in method floor and the boot report), 3 (the provider-error return address keeps ?redirect=) and 4 (the rate-limited landing keeps ?redirect=). Items 2 (the IP allow-list landing on the direct sign-in door) and 5 (the N13 sign-out note) remain on the card for a later round.

Item 1 — the floor counts what the sign-in page offers. The page decides what it draws from features.localSignIn, which getPublicConfig() serves only while direct sign-in is on: under 'off' LoginForm draws no email and password and no phone sign-in. The floor now reads the same served value with the same predicate (pageOffersLocalSignIn(config) is config.features?.localSignIn !== 'off', exactly the page's test), so:

  • ssoProviderDeleteRefusal (the /sso/delete-provider door) and resolveNoSignInMethodReport (the boot report) count email and password only when it is on and the page offers it (pageOffersEmailPassword), and count phone sign-in only when the page offers it (otherSignInMethods(..., { asThePageOffers: true })).
  • The 409 under off carries a new localized copy, enterpriseSsoLocalOff (11 locales), chosen by !pageOffersLocalSignIn(config) whatever email and password says, because the existing copy's remedy, "turn email and password sign-in back on", draws nothing under off. Its remedy: register another identity provider, set up another sign-in method, or set the local sign-in path back to hidden with email and password sign-in on.
  • The boot report under off (chosen the same way) names OS_AUTH_DIRECT_SIGN_IN_LOCAL_PATH=hidden as the recovery, plus OS_AUTH_EMAIL_PASSWORD_ENABLED=true when email and password is off too; it no longer says "no phone-number sign-in" there.

Phone sign-in is included as a bounded in-place fix: it is the same defect class (a local way in the page does not draw under off, counted anyway), a mechanical change in the same file, held by this claim, in the same gate family. Without it a deployment with phone sign-in on and off would pass the same delete and reach the same "No sign-in method is available" page.

Items 3 and 4 — the door's landings keep ?redirect=. The Console hands the door its own path and query as callbackURL (for example /_console/login?redirect=%2Fapps%2Fcrm&direct=returned). carriedRedirect(kept) reads redirect from the already-kept return path, carries it only when it is sent once and is itself a kept return path (keptReturnPath, the door's existing same-origin rule), and both landings now use it: the errorCallbackURL handed to the vendor (/login?redirect=...; better-auth appends its error to it) and the door's own landing (/login?error=...&redirect=..., the rate-limited one and the unavailable one alike). With no ?redirect=, or a foreign one, both are the bare page exactly as before.

Docs: content/docs/permissions/sso.mdx says both rules, names the two checks this PR changes (the provider delete and the boot report, email and password and phone sign-in alike), and says the settings save is still open on #33358.

Tests

  • src/sign-in-method-floor.test.ts, the composition (real kernel, sqlite, AuthPlugin and settings): under directSignIn: { localSignIn: 'off' } with email and password on, deleting the last provider answers 409 DELETE_RESTRICTED with enterpriseSsoLocalOff and the row stays; CONTROL: under hidden the same delete is 200 and the row is gone; a boot under off with no provider reports no_sign_in_method_at_boot naming OS_AUTH_DIRECT_SIGN_IN_LOCAL_PATH=hidden; CONTROL: the same boot under hidden says nothing.
  • Same file, the judge over a fake: under off, neither email and password nor phone sign-in counts (delete refused, boot report said); CONTROL under hidden: both count as before; phone alone under off with email and password off is refused with enterpriseSsoLocalOff and its boot report names both settings without "no phone-number sign-in"; CONTROL: email and password off under hidden keeps the enterpriseSso copy and the OS_AUTH_EMAIL_PASSWORD_ENABLED=true report. Composition: a boot under off with email and password off names OS_AUTH_DIRECT_SIGN_IN_LOCAL_PATH=hidden and OS_AUTH_EMAIL_PASSWORD_ENABLED=true.
  • src/direct-sign-in.test.ts: the vendor's errorCallbackURL keeps ?redirect=; the rate-limited landing keeps it; CONTROL: with no ?redirect= both landings are the bare page, as before; a foreign ?redirect= is dropped and the landings are the bare page, pinned over every spelling in FOREIGN_RETURN_PATHS (24: absolute, protocol-relative, backslash, javascript:, control characters, percent-encoded forms, /..//, empty, over-long).

Acceptance notes

  • The card names the setting OS_AUTH_LOCAL_SIGN_IN=off; the variable in the code is OS_AUTH_DIRECT_SIGN_IN_LOCAL_PATH (directSignIn.localSignIn). The premise holds otherwise.
  • Not changed here, same family: the auth settings save door (signInMethodFloorPredicate) still counts email and password on under off, so under off a save that switches off Google as the last social provider passes. Its refusal copy speaks of turning email and password back on, so a fix there needs its own copy. Reported to the seat for the card rather than widened here.
  • Not changed here: the 503 copy (enterpriseSsoUnverified) still says "with email and password off"; under off with email and password on it is slightly inaccurate, but it is still the right refusal.

Verification

Review#3 delta: head cc741c8ef3 answers M1 and N1 to N4 of contract record 6888499485; the runs below the dogfood paragraph were taken at 2a03b651b6 and the delta's runs are in the first fold. Earlier head 2a03b651b6 (merge base c32bb6ccb = origin/main at the time; origin/main had not moved when re-fetched before opening).

Dogfood battery (the title carries !): pnpm --filter @objectstack/dogfood test under the verify lock, dependency closure built first, on head 2a03b651b6. It was still running when this PR opened; its verdict (Test Files, Tests, every failure named) goes into the os-dev-report comment on #33358, and the PR's own Dogfood Regression Gate runs it in the queue.

Review#3 delta, local runs on head cc741c8
  • pnpm --filter @objectstack/plugin-auth test (verify lock): Test Files 274 passed (274), Tests 5336 passed | 10 skipped (5346), VERDICT command-exit 0.
  • pnpm --filter @objectstack/plugin-auth typecheck: VERDICT command-exit 0, check:test-typecheck: OK (ledger unchanged). eslint --no-inline-config on the three changed .ts files: exit 0. check-nul-bytes: OK.
  • M1 ablations (scripts/ablation-replace.mjs, landed and restored, blob equal to HEAD): delete copy chosen by email and password again: 1 failed / 41; boot report branched by email and password again: 2 failed / 41.
  • Narrowed gate re-run (declared): check-adr-0087-registration, check-empty-changeset, check-changeset-no-major, check:doc-authoring, check:issue-citations, check-closing-keyword-parity, check:doc-anchors, check-affected-docs: all exit 0. The full derived set was not re-run for the delta; CI runs it.
  • Dogfood battery not re-run: the delta changes copy and report-text selection and test pins only; the runtime decision (refuse or allow, report or not) is unchanged from 2a03b651b6, where the battery was green.
Local runs on head 2a03b65
  • pnpm --filter @objectstack/plugin-auth test (verify lock): Test Files 274 passed (274), Tests 5312 passed | 10 skipped (5322), VERDICT command-exit 0.
  • pnpm --filter @objectstack/plugin-auth build && pnpm --filter @objectstack/plugin-auth typecheck: VERDICT command-exit 0; check:test-typecheck: OK (10 files / 72 errors / 23 pinned signatures held in the existing ledger, unchanged).
  • eslint --no-inline-config on the four changed source files: exit 0. node scripts/check-nul-bytes.mjs: OK.
  • Single files: src/sign-in-method-floor.test.ts 40/40, src/direct-sign-in.test.ts 87/87.
Ablations (one per fix; each mutation landed on disk via scripts/ablation-replace.mjs and was restored, blob equal to HEAD and git diff HEAD empty)
  • Delete door back to config.emailPassword?.enabled !== false: 2 failed / 38 (the composition delete under off, the judge case).
  • Boot report back to config.emailPassword?.enabled !== false: 2 failed / 38 (the composition boot under off, the judge case).
  • errorCallbackURL back to the bare /login: 1 failed / 87 (keeps the page's ?redirect=).
  • The door's landing without the carried redirect: 1 failed / 87 (the rate-limited landing).

The tests import src/ directly (no dist/ in the path), so no rebuild leg was needed.

Derived gate families (dispatch-gates --commands, 97 families)

97 run: 94 exit 0; 3 exit 3 PREREQUISITE NOT MET, read as NOT MEASURED and left to CI: check:skill-examples (no packages/client-react/dist in this worktree), check:dual-build-cjs-loads (several packages unbuilt), check:type-check-debt (its re-measure prerequisite). dispatch-gates --ran: 97 derived, 97 run, 0 UNRUN.


Generated by Claude Code

…nly when the sign-in page offers it

Claude-Session: https://claude.ai/code/session_016WV4d1NJYFGVi2FRztyBNo
Co-authored-by: Claude <noreply@anthropic.com>
… off, with controls

Claude-Session: https://claude.ai/code/session_016WV4d1NJYFGVi2FRztyBNo
Co-authored-by: Claude <noreply@anthropic.com>
…-in door's error landings

Claude-Session: https://claude.ai/code/session_016WV4d1NJYFGVi2FRztyBNo
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling Fix lands in tooling, not a product package — enters pm:queue only with Unblocks: or a named surface labels Nov 15, 2026
@github-actions

github-actions Bot commented Nov 15, 2026 •

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-auth, touching 13 documentable anchor(s).

2 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/permissions/authentication.mdx (via callbackURL (literal, a string literal in runDirectSignIn), phone_number (literal, a string literal in otherSignInMethods))
  • content/docs/permissions/sso.mdx (via callbackURL (literal, a string literal in runDirectSignIn))

⛔ 2 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v16.mdx (via phone_number (literal, a string literal in otherSignInMethods))
  • content/docs/releases/v17/17-5.mdx (via callbackURL (literal, a string literal in runDirectSignIn))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 65 of 247 client-bound route-ledger rows — the other 182 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 182: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 59 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 123 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 22 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 4c32b9694b7b248646b2053b6eafea2cc212693b → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 9f8e2fc6d8ac8c79bb110b727cc583c1a5ee23cb — the merge of head 4cf17caabd606319a63ffc071e6061f75361156f into base 4c32b9694b7b248646b2053b6eafea2cc212693b, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 9f8e2fc6d8ac8c79bb110b727cc583c1a5ee23cb && git checkout 9f8e2fc6d8ac8c79bb110b727cc583c1a5ee23cb
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 4c32b9694b7b248646b2053b6eafea2cc212693b 4cf17caabd606319a63ffc071e6061f75361156f && git checkout -B drift-repro 4c32b9694b7b248646b2053b6eafea2cc212693b && git merge --no-ff 4cf17caabd606319a63ffc071e6061f75361156f

node scripts/docs-audit/affected-docs.mjs --json 4c32b9694b7b248646b2053b6eafea2cc212693b

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 4c32b9694b7b248646b2053b6eafea2cc212693b → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#5 · os-elon-musk · session_016WV4d1NJYFGVi2FRztyBNo

Request: review · area A1 · Part of #33358 (items 1, 3, 4) · head 2a03b65 · requested at 2026-11-15T03:15Z

Local (os-dev report, #33358 comment 6888421655): plugin-auth full suite 274 files / 5312 passed, 10 skipped; build + typecheck exit 0; eslint exit 0; four ablations red (delete door 2/38, boot report 2/38, errorCallbackURL 1/87, rate-limited landing 1/87); dogfood battery at 2a03b65: 467 files / 3950 passed, 0 failed; derived gates 97: 94 exit 0, 3 NOT MEASURED (check:skill-examples, check:dual-build-cjs-loads, check:type-check-debt: prerequisite builds). Declarations: !, Clause-② yes (narrowing: the last-provider delete is refused under localSignIn: 'off'), BREAKING patch, adr-0087 not-required (no-migration-prescription), Metadata-protocol none. Beyond the claim's file surface: content/docs/permissions/sso.mdx (two paragraphs stating both rules). Env var correction: the card's OS_AUTH_LOCAL_SIGN_IN is OS_AUTH_DIRECT_SIGN_IN_LOCAL_PATH in the code. Remaining on the card: items 2 and 5, and the settings-save half of item 1 (os-dev out-of-scope finding, noted on the card).

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract-review-claim: session_016WV4d1PGptBW3QRQNmy5e6 · head 2a03b65 · 2026-11-15T03:17Z

review#3.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 2a03b651b6bf0df1bb20a4ea831111617662fb7a
Local-runs: none
Scope: .changeset/33358-local-sign-in-off-floor.md, content/docs/permissions/sso.mdx, packages/plugins/plugin-auth/src/direct-sign-in.ts, packages/plugins/plugin-auth/src/direct-sign-in.test.ts, packages/plugins/plugin-auth/src/sign-in-method-floor.ts, packages/plugins/plugin-auth/src/sign-in-method-floor.test.ts

Read: the PR's four commits on top of c32bb6cc (623d63fa, a72de75a, 773e81c7, 2a03b651): 6 files, +295/−22. I compared the floor's predicate with the page's own (ui/packages/auth/src/LoginForm.tsx:349-354), the served value (auth-manager.ts:5928) and the Console's retry logic (ui/apps/console/src/pages/auth/directSignIn.ts:79). I read how better-auth 1.7.12 and @better-auth/sso 1.7.12 add error to errorCallbackURL. I ran no repo test or build. I ran a scratch copy of keptReturnPath and carriedRedirect against the foreign spellings listed below.

① Derived judgments

Items 3 and 4: the carried redirect uses the door's own return-path rule. I found no open redirect.

  • Same validator. carriedRedirect (direct-sign-in.ts:221-229) reads redirect from the return path that keptReturnPath already accepted. It then passes that value through keptReturnPath again, the same function the door uses for callbackURL (:187-212). That rule needs one leading /, and rejects //, a backslash, a control character, any of these behind up to three layers of percent-encoding, and a parser-resolved path that starts with //. A scheme such as javascript: fails the leading-/ test.
  • Checked again before use. The value is read with URL.searchParams, which decodes it once, and is checked by the rule again at that point. If redirect appears more than once, nothing is carried.
  • Encoded, never concatenated. signInPageUrl (:233-238) builds the query with URLSearchParams.set. Both the vendor's errorCallbackURL (:432) and the door's own landing (:412) go through it. The base is host.consolePageUrl('/login'), so the origin is fixed by the server. better-auth adds its error with appendQueryParams (oauth2/errors.mjs, api/routes/callback.mjs, and the SSO plugin's OIDC callback), so an existing ?redirect= keeps a correct & separator.
  • No new surface. Anyone can already open /login?redirect=<x> directly. The server only carries a value its stricter rule accepts, and the Console still applies its own handling. A landing that carries error is a hold, so the page does not start again (directSignIn.ts:79). The retry drops only error and error_description.
  • Scratch probe (a copy of the two functions, not a repo test). These were all dropped: //evil.example/x, https://evil.example/, /%2F%2Fevil.example, /\evil.example, \\evil.example, /%5cevil, javascript:alert(1), /%09/evil, /..//evil and /%252f%252fevil. A redirect of /apps/crm?view=all came back as redirect=%2Fapps%2Fcrm%3Fview%3Dall.

Item 1: the floor counts what the page draws.

  • pageOffersLocalSignIn (sign-in-method-floor.ts:130) is features.localSignIn !== 'off'. The page uses the same test (LoginForm.tsx:349). It hides email and password, phone with a password, and phone OTP under off, whether or not directSignIn is on. The floor reads the same served config (auth-plugin.ts:2335, getPublicConfig()), and localSignIn is served only while the direct sign-in setting is on. So "phone is excluded only under off" is exactly the page's rule. Under hidden, the page still offers both at /login?local=1.
  • Too strict? No. The refusal fires only under off, with enterprise SSO on, on the last provider row, and with no social or OIDC provider and no host handoff. In that state, deleting the row leaves the page with nothing to draw. A refused delete locks nobody out, and every way to recover stays open.
  • Too loose? No longer. Before this PR, off with phone on passed the delete and the boot report. That is fixed.
  • 409 DELETE_RESTRICTED is a standard code (packages/spec/src/api/errors.zod.ts:97; the ledger synonym is in error-code-ledger.test.ts:313). The PR does not add a new code.
  • enterpriseSsoLocalOff is in all 11 locales (:284, :304, :326, :352, :373, :394, :418, :444, :469, :494, :518). It is a required field of SignInFloorCopy, so typecheck holds the set.

must-fix M1: under off with email and password switched off, the new refusal and the new boot report give a remedy that does not work, and a pin locks it in.

  • The message is chosen by config.emailPassword?.enabled !== false (sign-in-method-floor.ts:763). The boot report is branched the same way (:787). Under off with email and password off, the delete gets enterpriseSso, whose first remedy is "First turn email and password sign-in back on". The boot report gets the original text, which says "no phone-number sign-in" and "Recover: set OS_AUTH_EMAIL_PASSWORD_ENABLED=true and restart" (:799-806).
  • Under off, neither remedy helps. Turning email and password on only moves the next attempt to the enterpriseSsoLocalOff refusal. With phone on, "no phone-number sign-in" is false. This PR newly refuses that state (phone on, email and password off, off) and newly reports it. The pin at sign-in-method-floor.test.ts:811-815 asserts the enterpriseSso copy there.
  • The PR body gives the reason for the new copy as "turn email and password sign-in back on does nothing there". The same reason applies to this case.
  • sso.mdx:722 ("Recover by setting OS_AUTH_DIRECT_SIGN_IN_LOCAL_PATH=hidden") is also not enough in this state.
  • Fix:
    • Choose the copy and the report branch by !pageOffersLocalSignIn(config).
    • Give the off texts a remedy that holds in both states. For example: set the local sign-in path back to hidden, and also turn email and password on if it is off. Or add a third variant.
    • Re-pin the phone-only-under-off case and add a boot case for off with email and password off.
    • Word the docs sentence to match.

② Semver level

  • Clause-②: yes (narrowing) is right. A delete that returned 200 now returns 409 DELETE_RESTRICTED under off. Two pins hold this, each with its CONTROL (sign-in-method-floor.test.ts:660-685). The title carries !.
  • The changeset (:13) has a BREAKING paragraph that names who is affected (direct sign-in with the local path off) and the fix (:17). It has exactly one <!-- adr-0087: not-required (no-migration-prescription) ... --> marker (:11). No stored key or metadata moves, so Metadata-protocol: none and patch are right.
  • The redirect change only widens: a landing that was the bare page now carries a value the page already had. It is described in prose under "Also fixed" (:19).

③ Boundary flags

  • The diff touches only plugin-auth sources, tests, one changeset and one docs page. It touches no packages/spec, protocol, generated file or release-owned page. sso.mdx is outside the claim's file surface, as declared.
  • The dogfood battery is reported green at this head (467 files / 3950 passed). I did not run it.
  • Still open on the card: items 2 and 5, and the settings save (signInMethodFloorPredicate, sign-in-method-floor.ts:631), which still counts email and password under off.

Nits (none blocks).

  • N1. The changeset's "Widened" list (:7) does not name the new served copy enterpriseSsoLocalOff (11 locales) or the asThePageOffers option of otherSignInMethods. It also calls pageOffersLocalSignIn, pageOffersEmailPassword and carriedRedirect "public functions". They are module exports only: src/index.ts:97 exports just two types from direct-sign-in, and nothing from sign-in-method-floor. Say "module exports", and add the copy key in prose.
  • N2. The foreign-redirect pin (direct-sign-in.test.ts:310) loops over 3 spellings. Loop it over FOREIGN_RETURN_PATHS (:56-80) instead, so the backslash, javascript:, control-character and /..// spellings are pinned through carriedRedirect too. Today they hold only because carriedRedirect delegates to keptReturnPath.
  • N3. sso.mdx:717-718 says "the checks that keep a way to sign in do not count it under off". The settings save still counts it. Name the two checks this PR changes, and mention phone sign-in too.
  • N4. The BREAKING paragraph (:13) does not name phone sign-in. A deployment under off with email and password off and phone on is also newly refused and newly reported. Also, the exits it lists under off (another provider row, a social or OIDC provider, a host handoff) have no pin under off. In the code, asThePageOffers gates only phone, so they hold today.

Implemented-by: claude/area-A1-33358
Reviewed-by: session_016WV4d1PGptBW3QRQNmy5e6

VERDICT: FAIL

… boot report give the local-path remedy whatever email and password says

Review#3 M1 and N1-N4: the copy and the report branch are chosen by
pageOffersLocalSignIn, the off copy names email and password on with the
local path back to hidden, the foreign-redirect pin loops over every
foreign spelling, and the changeset and the docs say what changed.

Claude-Session: https://claude.ai/code/session_016WV4d1NJYFGVi2FRztyBNo
Co-authored-by: Claude <noreply@anthropic.com>
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract-review-claim: session_016WV4d1PGptBW3QRQNmy5e6 · head cc741c8 · 2026-11-15T03:35Z

review#3 (delta).

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: cc741c8ef32ceca989c36dbe42bbcad2c74fe798
Local-runs: none
Scope: .changeset/33358-local-sign-in-off-floor.md, content/docs/permissions/sso.mdx, packages/plugins/plugin-auth/src/direct-sign-in.test.ts, packages/plugins/plugin-auth/src/sign-in-method-floor.ts, packages/plugins/plugin-auth/src/sign-in-method-floor.test.ts

This reviews the delta 2a03b651...cc741c8e (one commit, cc741c8e, 5 files, +61/−38) against my FAIL at 2a03b651 (comment 6888499485). I read the diff and the code it relies on at the head: resolveDirectSignIn (direct-sign-in.ts:141-162), keptReturnPath and carriedRedirect (:187-229), pageOffersLocalSignIn and pageOffersEmailPassword (sign-in-method-floor.ts:130-137), and otherSignInMethods (:228-238). I ran no repo test or build. CI at this head was still running when I posted (25 passed, 11 in progress, 2 queued, none failed).

① Derived judgments

M1: closed.

  • Selection. The delete copy is now pageOffersLocalSignIn(config) ? enterpriseSso : enterpriseSsoLocalOff (sign-in-method-floor.ts:763). The boot report branches on !pageOffersLocalSignIn(config) (:787). So under off the delete always gets enterpriseSsoLocalOff, and the boot report always gets the local-path text, whatever email and password says.
  • The off remedy is true. resolveDirectSignIn reads OS_AUTH_DIRECT_SIGN_IN_LOCAL_PATH before directSignIn.localSignIn (direct-sign-in.ts:147), so =hidden replaces an off set in code. getPublicConfig() then serves localSignIn: 'hidden' (auth-manager.ts:5928), pageOffersLocalSignIn is true, and with email and password on, pageOffersEmailPassword is true. The page offers it at /login?local=1, and both checks stop firing. When email and password is off, the report adds OS_AUTH_EMAIL_PASSWORD_ENABLED=true (:796-798). That is the settings environment override, which wins over the stored value (:71-73), the same remedy the email-off branch already gave. The copy now says "set the local sign-in path back to hidden with email and password sign-in on", which holds in both states.
  • The other branch is now true too. It is reached only when the page offers local sign-in and email and password is off. There, asThePageOffers counts phone (:238), so "no phone-number sign-in" is true whenever this text is printed.
  • Locales. enterpriseSsoLocalOff changed in all 11 locales (en, zh-Hans, ja, es, zh-Hant, ar, fr, pt, ru, de, ko). I checked zh-Hans ("改回 hidden 并开启邮箱/密码登录"), zh-Hant, ja ("hidden に戻してメール/パスワードログインをオンにして"), es, fr and de. Each one adds "with email and password sign-in on" to the hidden remedy, as the English does, and no other sentence changed.
  • Pins. The phone-only-under-off pin now expects enterpriseSsoLocalOff. It also expects the report to name both settings and not to say "no phone-number sign-in" (sign-in-method-floor.test.ts:818-827). A CONTROL with email and password off under hidden keeps enterpriseSso and the OS_AUTH_EMAIL_PASSWORD_ENABLED=true and restart report (:828-833). A real-composition boot under off with email and password off expects one report that names both settings (:694-699). Each assertion matches the strings the code builds.
  • Docs. sso.mdx:717-726 now gives the same two-part remedy.

No refuse/allow decision changed. In both ssoProviderDeleteRefusal and resolveNoSignInMethodReport, every line that returns undefined or null is unchanged. The delta changes only which message a refusal or report carries. The decisions the earlier dogfood run covered are the same, so no new dogfood run is owed.

Redirect: still safe. direct-sign-in.ts is not in the delta. carriedRedirect still runs the value through keptReturnPath again, and signInPageUrl still builds the query with URLSearchParams. My earlier reasoning still holds.

N1–N4: addressed.

  • N1. The changeset now says "module exports (not on the package entry)". src/index.ts:97 still exports only the two direct-sign-in types. It also names asThePageOffers and the copy key enterpriseSsoLocalOff in 11 locales.
  • N2. The pin is now it.each(FOREIGN_RETURN_PATHS), over all 24 spellings (direct-sign-in.test.ts:309).
  • N3. sso.mdx now names the two checks, includes phone sign-in, and says the settings save still counts email and password under off.
  • N4. The BREAKING paragraph now says "and even with phone sign-in switched on".

Nits (none blocks).

  • N5. One of the 24 spellings, /${'a'.repeat(2048)}, makes the whole callbackURL longer than RETURN_PATH_MAX (2048). keptReturnPath then drops the whole page path, so this case never reaches carriedRedirect. The assertion still holds, and the other 23 spellings do go through carriedRedirect.
  • N6, carried over from the second half of N4. The exits under off (another provider row, a social or OIDC provider, a host handoff) still have no pin under localSignIn: 'off'. The passes the delete with $name table uses OFF (email and password off, no localSignIn). In the code, asThePageOffers gates only phone, so they hold today.

② Semver level

Unchanged from my earlier record. patch with fix(auth)! and Clause-②: yes (narrowing) is right. The delta changes only refusal and report text, docs and pins. The new copy key existed before the delta, and only its text changed. The BREAKING paragraph and the "What to do" line now give the two-part remedy. There is still exactly one adr-0087: not-required marker. Metadata-protocol: none still holds.

③ Boundary flags

  • The delta touches only plugin-auth source and tests, the changeset and sso.mdx. It does not touch packages/spec, any protocol or generated file, or any release-owned page.
  • Still open on the card and outside this PR: items 2 and 5, and the settings save (signInMethodFloorPredicate, sign-in-method-floor.ts:631), which still counts email and password under off. sso.mdx now says so.

④ Design judgment

The delta makes the requested fix with the predicate the floor already uses, so the remedy is chosen by the rule the page uses to draw local sign-in. One copy key with a two-part remedy is enough. A third variant would add a locale set without telling the reader anything more. The boot report names the second setting only when it is needed, which keeps the common case short. The CONTROL under hidden makes sure the email-off copy cannot drift into the off text.

Implemented-by: claude/area-A1-33358
Reviewed-by: session_016WV4d1PGptBW3QRQNmy5e6

VERDICT: PASS

…s/permissions/sso.mdx)

Claude-Session: https://claude.ai/code/session_016WV4d1NJYFGVi2FRztyBNo
Co-authored-by: Claude <noreply@anthropic.com>
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract review — PASS carries

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 4cf17caabd606319a63ffc071e6061f75361156f
Local-runs: none
Scope: .changeset/33358-local-sign-in-off-floor.md, content/docs/permissions/sso.mdx, packages/plugins/plugin-auth/src/direct-sign-in.ts, packages/plugins/plugin-auth/src/direct-sign-in.test.ts, packages/plugins/plugin-auth/src/sign-in-method-floor.ts, packages/plugins/plugin-auth/src/sign-in-method-floor.test.ts

This carries my PASS at cc741c8e (comment 6888701895) to 4cf17caa. The one new commit, 4cf17caa, merges main (c32bb6cc → 4c32b969) into the branch, and the PR adds nothing else.

① Derived judgments

  • Only the merge changed. I compared the PR's own diff before the merge (c32bb6cc...cc741c8e) with its diff after it (4c32b969...4cf17caa). Every added and removed line is the same, as is every context line, across all six files. Only the blob ids and the @@ offsets in sso.mdx differ, by +44, because main added text above the PR's hunks. The merge needed no conflict resolution.
  • No coupling to what main brought. sso.mdx is the only file both sides touched. On main, plugin-auth changed only the SAML account key (fix(auth)!: a SAML provider keys its accounts on a stable attribute (Entra object id by default), so the app and the IdP portal reach one account #33490: sso-saml-*, sso-sign-in-options.ts, auth-manager.ts, register-sso-provider.ts, objectql-adapter.ts). direct-sign-in.ts imports only the type SsoProviderReader from sso-sign-in-options.ts, and that type did not change. Nothing on main touches sign-in-method-floor.ts, direct-sign-in.ts, the floor predicates or the redirect carry. So every refuse or allow decision, and every message I judged at cc741c8e, is the same at this head.

② Semver level

Unchanged. The changeset is byte-identical: patch, fix(auth)!, Clause-②: yes (narrowing), one adr-0087: not-required marker, Metadata-protocol: none.

③ Boundary flags

Unchanged from 6888701895. Items 2 and 5 and the settings save's floor predicate are still open on the card, outside this PR.

④ Design judgment

Unchanged from 6888701895, since no reviewed line moved. Nits N5 and N6 still stand, and neither blocks.

CI at this head when I posted: 24 passed, 15 skipped, 12 in progress, none failed.

Implemented-by: claude/area-A1-33358
Reviewed-by: session_016WV4czb1FQB8j27FVgnMye

VERDICT: PASS

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#5 · os-elon-musk · session_016WV4d1NJYFGVi2FRztyBNo

ACCEPT · area A1 · Part of #33358 (items 1, 3, 4) · head 4cf17ca · 2026-11-15T04:22Z

  • Reviewer: review#3 (session_016WV4d1PGptBW3QRQNmy5e6) FAIL at 2a03b65 (record 6888499485), delta PASS at cc741c8 (record 6888701895): M1 and N1–N4 closed; no refuse/allow decision moved, so no new dogfood run owed.
  • Regen-provenance: 2a03b65 → cc741c8 one commit answering M1 (the delete copy and the boot report chosen by !pageOffersLocalSignIn(config); enterpriseSsoLocalOff remedy in 11 locales; pins re-pointed and added) and N1–N4 (changeset wording and copy key, it.each over FOREIGN_RETURN_PATHS, sso.mdx scope, BREAKING names phone sign-in). cc741c8 → 4cf17ca merges origin/main (22 commits; main touched content/docs/permissions/sso.mdx), auto-merged without conflict; the two touched test files 151/151 after the merge.
  • Declarations: fix!, Clause-② yes (narrowing), BREAKING patch, adr-0087 not-required (no-migration-prescription), Metadata-protocol none.
  • Governed: check-governed-merges.mjs --pr 33623 → NOT governed, 340 changed lines.
  • Checks at 4cf17ca: 53 check runs completed, none red (38 success, 15 skipped). Local: plugin-auth full suite 274 files / 5336 passed at cc741c8; typecheck + eslint exit 0; M1 ablations red; dogfood battery 467 files / 3950 passed at 2a03b65 (the last refuse/allow change). plugin-auth is not a shared package under the dependents-before-arming rule.
  • Remaining on plugin-auth: with local sign-in off the sign-in floor still counts email and password, and the direct sign-in door's landings lose ?redirect= and the IP allow-list page #33358 after this lands: items 2 and 5 and the settings-save half of item 1 (card scope note).
  • Landing: ready + auto-merge (maintainer 2026-11-14 15:21Z).

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#5 · os-elon-musk · session_016WV4cytdN5jRpgKX58AY4d

ACCEPT · area A1 · Part of #33358 (items 1 (the floor), 3, 4) · head 4cf17ca · 2026-11-15T04:23Z

  • Reviewer: review#3 (session_016WV4d1PGptBW3QRQNmy5e6) FAIL at 2a03b65 (record 6888499485, must-fix M1: the off remedy text) → delta PASS at cc741c8 (record 6888701895), nits only; PASS carried to 4cf17ca (record 6888853480, PR side line-identical). Clause-② yes (narrowing) upheld: a delete that returned 200 now returns 409 DELETE_RESTRICTED under local sign-in off; title carries !; one adr-0087: not-required marker; Metadata-protocol none; patch.
  • Regen-provenance: 2a03b65 → cc741c8 (delta, reviewed) → 4cf17ca = clean merge of origin/main 4c32b96 (main touched content/docs/permissions/sso.mdx only; no conflict). The PR has no generated file. PASS survives a clean main merge (pilot rule).
  • Governed: check-governed-merges.mjs --pr 33623 → NOT governed, 0 of 6 paths, 340 changed lines.
  • Checks at 4cf17ca: 53 check runs completed, none red (38 success, 15 skipped).
  • Main since merge base 4c32b96: 24 commits; none touches the PR's 6 files (plugin-auth overlap is fix(plugin-auth): create-user reports the membership the store holds for the new account #33587's admin-user-endpoints* and test-typecheck-debt.json only). The queue builds on current main.
  • Card stays open: item 2 (IP allow-list landing), item 5 (N13 sign-out note) and the settings-save half of item 1 (signInMethodFloorPredicate still counts email and password under off).
  • Landing: ready + auto-merge (maintainer 2026-11-14 15:21Z, restated for all areas 2026-11-15 03:57Z).

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review November 15, 2026 04:23
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Nov 15, 2026
Any commits made after this event will not be merged.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling Fix lands in tooling, not a product package — enters pm:queue only with Unblocks: or a named surface

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants