Skip to content

fix(plugin-auth): a correct-password sign-in refused by an organization IP list no longer counts toward the account lockout - #31262

Merged
objectstack-fleet[bot] merged 7 commits into
mainfrom
claude/project-thread-t94vm3
Nov 7, 2026
Merged

objectstack-fleet[bot] merged 7 commits into
mainfrom
claude/project-thread-t94vm3

Conversation

@objectstack-fleet

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

Copy link
Copy Markdown
Contributor

Requested by Leehom · project thread

Fixes #31251

Clause-②: no
Metadata-protocol: none

维护者速读

  • 改了什么:auth-manager-after-hook.ts 的 ADR-0069 D2 锁定计数器,遇到三种「密码已通过、之后才被拒」的登录时不再计数,也不清零:组织 IP 名单拒绝(403 IP_NOT_ALLOWED)、账号被封禁(403 BANNED_USER)、邮箱未验证(403 EMAIL_NOT_VERIFIED)。其余拒绝照旧计数,成功照旧清零。
  • 为什么改:密码正确的本人会把自己锁住:在组织网络外登录三次,回到网络内被拒 ACCOUNT_LOCKED;封禁解除后仍被锁;等验证邮件期间登录三次,验证后仍被锁。
  • 风险与回滚:只少计「密码已正确」的三种拒绝,错误密码在任何情况下仍然计数,所以不给猜密码者多留尝试次数;调用方看到的 403 状态、错误码、文字和审计行都不变。回滚即还原本 PR。
  • 先看:packages/plugins/plugin-auth/src/auth-manager-after-hook.ts(refusedAfterTheCredential、VENDOR_BANNED_USER_CODE 与计数调用处),再看 account-lockout-resolved-account.test.ts 末尾新增的两个 describe。
  • 两分钟验证:pnpm --filter @objectstack/plugin-auth exec vitest run src/account-lockout-resolved-account.test.ts,预期 11 个用例全绿。
  • 需要你做的:无,席位落地。

What changes

  • packages/plugins/plugin-auth/src/auth-manager-after-hook.ts: in the SIGN_IN_EMAIL_PATH branch, a refused sign-in whose code refusedAfterTheCredential names skips recordSignInOutcome: no increment, no reset. The set, per the claim and its amendment, each code named by a constant: SESSION_SCOPING_REFUSAL.code (imported from organization-access-policy.ts); BASE_ERROR_CODES.EMAIL_NOT_VERIFIED.code (@better-auth/core/error, loaded lazily as the before-hook does); and VENDOR_BANNED_USER_CODE, one module-level constant whose comment names its vendor source (ADMIN_ERROR_CODES.BANNED_USER in better-auth's admin plugin, which the package exports on no public path). auditRefusedSignIn still runs, so the refusal's login_failed row is written exactly as before (and as feat(plugin-auth, plugin-audit): a sign-in the IP allow-list refuses writes one login_failed row, bounded per address #31249 then shapes it). The status, code, message and timing the caller sees are untouched: the after-hook only ever observed the returned error.
  • The predicate is an async function evaluated per sign-in, not a module-level Set: organization-access-policy.ts imports auth-manager.ts, which reaches the after-hook through auth-manager-auth-instance.ts, so on that import order a module-level constant built from SESSION_SCOPING_REFUSAL would read it before it is initialised.
  • Why these codes can only arrive after the password passed: better-auth 1.7.12's signInEmail throws INVALID_EMAIL_OR_PASSWORD for a wrong password, then EMAIL_NOT_VERIFIED (when the deployment requires a verified address), then calls createSession, whose session.create.before hooks raise the organization list's IP_NOT_ALLOWED (refuseUnadmittedScoping) and the admin plugin's BANNED_USER. A wrong password never reaches any of the three. The deployment's own list (allowed_ip_ranges on the deployment) refuses at the auth-route middleware in auth-plugin.ts, before better-auth runs, so it never reached this counter.
  • .changeset/31251-org-ip-refusal-lockout-counter.md: @objectstack/plugin-auth patch (the after-hook ships in the package's dist/), naming all three refusals.

Tests

New top-level describe in src/account-lockout-resolved-account.test.ts (the lockout's shared harness: real AuthManager.handleRequest over the in-memory engine; the file's post gains an optional remoteAddress). The account is a member of an organization whose own list admits only 10.1.0.0/16; deployment threshold 3.

  1. Three correct-password sign-ins from 10.2.0.7 each answer 403 IP_NOT_ALLOWED (code + status) and leave failed_login_count at 0; no account_locked event; the next sign-in from 10.1.0.7 answers 200 and the count is 0.
  2. Two wrong passwords from inside raise the count to 2; a correct password from outside (403 IP_NOT_ALLOWED) leaves it at 2 and the account unlocked: the refusal neither counts nor clears.
  3. Controls: a wrong password from outside answers 401 INVALID_EMAIL_OR_PASSWORD and counts (the credential refuses first, the organization list is never reached); a wrong password from inside counts; a correct password from inside clears to 0; three wrong passwords from outside lock, and the correct password from inside is then 403 ACCOUNT_LOCKED.

A second new describe on the same harness (deployment takes config extras):

  1. Ban (plugins.admin: true, the account's banned set): two wrong passwords count to 2 (401 INVALID_EMAIL_OR_PASSWORD, the wrong-password control); three correct passwords each answer 403 BANNED_USER (code + status) and leave the count at 2, no lock, no account_locked event; the ban lifted, the correct password answers 200 and clears to 0.
  2. Unverified address (emailAndPassword.requireEmailVerification, email_verified false): two wrong passwords count to 2; three correct passwords each answer 403 EMAIL_NOT_VERIFIED and leave it at 2, no lock; one more wrong password reaches the threshold and locks.
Red first
  • Cases 1–3 on origin/main 0d0c377 (fix absent): 2 failed / 7 passed. Case 1: attempt 1: expected 1 to be +0; case 2: expected 3 to be 2. Case 3 (controls) green on main, as intended.
  • Cases 4–5 against the first head's after-hook (c38cc4cf, skip set = IP list only; file restored to the tree only, then git checkout HEAD, git diff HEAD empty): 2 failed / 9 passed, both attempt 1: expected 3 to be 2. On main the same two refusals count too (sweep below).
  • The card's sequence, probed on main with the same harness (throwaway file, deleted): [403 IP_NOT_ALLOWED count 1, 403 IP_NOT_ALLOWED count 2, 403 IP_NOT_ALLOWED count 3, then from inside 403 ACCOUNT_LOCKED count 3], matching the card.
Ablation, one leg per code

node scripts/ablation-replace.mjs --delete on auth-manager-after-hook.ts (blob e06b13de2fc4 at the fix commit), one list entry at a time; each leg restored to blob == HEAD with git diff HEAD empty:

  • VENDOR_BANNED_USER_CODE, deleted (→ 46ae4dc78472): case 4 red only, 1 failed / 10 passed.
  • BASE_ERROR_CODES.EMAIL_NOT_VERIFIED.code, deleted (→ 92ed2737beda): case 5 red only, 1 failed / 10 passed.
  • SESSION_SCOPING_REFUSAL.code, deleted (→ 117bb456302b): cases 1 and 2 red, 2 failed / 9 passed.

The first head's whole-skip ablation (c38cc4c, the skip never firing) read cases 1 and 2 red. The test resolves the after-hook from src/ through relative imports inside the package, so no dist/ leg applies.

Acceptance notes

Sweep of the other refusals that can reach this counter on /sign-in/email after the password passed (measured with throwaway probes on the same harness, deleted; better-auth 1.7.12):

  • Ban (403 BANNED_USER), with the admin plugin on (plugins.admin, or forced on by SCIM): thrown by the vendor admin plugin's session.create.before, the same path as the organization list. On main it counted: three correct-password sign-ins of a banned account locked it, and after the ban was lifted the next correct sign-in answered 403 ACCOUNT_LOCKED. Fixed here (claim amendment on plugin-auth: a correct-password sign-in refused by an organization IP list counts toward the account lockout #31251, cases 4).
  • Email not verified (403 EMAIL_NOT_VERIFIED), with emailAndPassword.requireEmailVerification: thrown by signInEmail right after password.verify succeeded. On main it counted: count 2 → 3 and locked on one correct-password attempt. Fixed here (same amendment, case 5).
  • MFA hold (plugins.twoFactor on, the account enrolled): the sign-in answers 200 { twoFactorRedirect: true }; this hook runs before the two-factor plugin's after-hook and sees the session payload, so it clears the count (2 → 0). Not a refusal; the password passed and the second factor has its own lockout projection. Correct as is.
  • Concurrent-session cap: enforceConcurrentCap runs on the success branch and ends the oldest sessions; it refuses no sign-in, so nothing reaches the counter (by reading).
  • Organization idle / absolute limits are judged on session reads and on an organization switch, not at sign-in; an organization's MFA requirement is a hold recorded on the login row with a 200 answer (by reading). Neither reaches the counter as a refusal.
  • Deployment IP list: refused at the auth-route middleware before better-auth, never reaching the counter (by reading). Unchanged.
  • PR feat(plugin-auth, plugin-audit): a sign-in the IP allow-list refuses writes one login_failed row, bounded per address #31249 (in review) touches auth-session-audit.ts and auth-manager-database-hooks.ts; this PR only imports refusalCodeOf / SIGN_IN_EMAIL_PATH from the former and changes neither. The new pins assert no login_failed row content, so they hold whichever lands first.
  • Published answers: the 403 IP_NOT_ALLOWED, 403 BANNED_USER and 403 EMAIL_NOT_VERIFIED answers are unchanged (the after-hook only observes them); the only answer that moves is a later sign-in that main refused 403 ACCOUNT_LOCKED and now admits, which is the defect. Hence Clause-②: no.

Verification

All on head f2bb01d5 (base origin/main 0d0c377; origin/main has since moved, with no commit under packages/plugins/plugin-auth, so no merge was taken; PR #31249 had not landed when this was read).

  • pnpm --filter @objectstack/plugin-auth test under the verification lock: VERDICT command-exit 0, Test Files 237 passed (237), Tests 4209 passed / 10 skipped.
  • pnpm --filter @objectstack/plugin-auth typecheck (tsc, examples, check:test-typecheck) exit 0, after the package's own build.
  • Scoped lint: pnpm exec eslint --no-inline-config on the two touched .ts files, exit 0.
  • Gate union derived by node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack (62 families; the widened diff adds check:dispatcher-error-vocabulary, exit 0), each run with stdin closed and its exit captured before any pipe, reconciled with --ran: "62 derived famil(ies) accounted for — 61 run, 1 NOT-MEASURED". 61 exit 0.
  • NOT MEASURED: pnpm check:dual-build-cjs-loads, reason: exit 3 PREREQUISITE NOT MET (it loads every package's dist/; only the plugin-auth closure was built). CI's Lint & Repo Gates runs it on a full build.
  • node scripts/check-plugin-teardown-shape.mjs --self-test exit 0 after git fetch --depth=1 origin 621a4876 (its pinned fixture, on this shallow clone).
  • CI families are read from this PR's check-runs, not re-run here.

Generated by Claude Code

@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 7, 2026
@github-actions

github-actions Bot commented Nov 7, 2026 •

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

4 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to list — not a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • the SDK route bridge reached 63 of 226 client-bound route-ledger rows — the other 163 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 163: 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; 104 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 455229e7aebae2fe91ada77736377fe145defba3 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from adf4f1f709d86bea6cdf6d7deb1a57317260f2ae — the merge of head f2bb01d5b4e7a7a9ae663165c40c690ffeb02263 into base 455229e7aebae2fe91ada77736377fe145defba3, 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 adf4f1f709d86bea6cdf6d7deb1a57317260f2ae && git checkout adf4f1f709d86bea6cdf6d7deb1a57317260f2ae
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 455229e7aebae2fe91ada77736377fe145defba3 f2bb01d5b4e7a7a9ae663165c40c690ffeb02263 && git checkout -B drift-repro 455229e7aebae2fe91ada77736377fe145defba3 && git merge --no-ff f2bb01d5b4e7a7a9ae663165c40c690ffeb02263

node scripts/docs-audit/affected-docs.mjs --json 455229e7aebae2fe91ada77736377fe145defba3

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

…ssword passed no longer moves the lockout counter

Claude-Session: https://claude.ai/code/session_01FEkQ5mSU6a4NFvNfFHukWv
Co-Authored-By: Claude <noreply@anthropic.com>
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#6 · os-project-manager · session_01FEkQ5p3T5YWhLjurajudnQ

席位核对 · ACCEPT · 2026-11-07T21:19Z · head f2bb01d5b4e7a7a9ae663165c40c690ffeb02263

Seat domain:server#6 reviewed the diff of its own dispatch (claim and amendment 6712743965 on #31251, card thread session_01FEkQ5mSU6a4NFvNfFHukWv, reports 6712725182 and 6713110625). Read against GitHub, not the report.

  1. Shape: 3 files, +195 / -4. The ADR-0069 D2 counter in auth-manager-after-hook.ts leaves the count as it stands (neither counts nor clears) for three /sign-in/email refusals that come after the password verified: EMAIL_NOT_VERIFIED (BASE_ERROR_CODES, lazily imported from @better-auth/core/error), the organization IP list (SESSION_SCOPING_REFUSAL.code), and a ban (VENDOR_BANNED_USER_CODE, one module constant citing ADMIN_ERROR_CODES.BANNED_USER, since the admin plugin exports no public path). Read at call time (import cycle and lazy vendor load, both explained). A wrong password still counts; a success still clears; the 403s and their audit rows are unchanged. Body says Fixes #31251; no other closing keyword.
  2. Premise: on main 0d0c377b three correct-password sign-ins refused by an organization IP list locked the account (ACCOUNT_LOCKED on the fourth, from inside the list); red reproduced by the dev, and the ban / unverified-address cases measured red on the first head.
  3. Gates / check-runs: 52 runs on f2bb01d5, 37 success, 15 skipped, 0 red, including Test Core and Type Check · workspace. mergeable_state clean, and the head merges cleanly with main after PR feat(plugin-auth, plugin-audit): a sign-in the IP allow-list refuses writes one login_failed row, bounded per address #31249 (git merge-tree no conflict). Dev: plugin-auth 237/237 files (4209 tests), typecheck and scoped lint green, 61/62 derived gates exit 0 with check:dual-build-cjs-loads NOT MEASURED (exit 3 prerequisite; CI's build gates are green), one ablation per code turning only its own pin red, restored.
  4. Contract review: Clause-②: no (an internal counter; no published type, route, answer or key changes), no packages/spec/src path: none owed.
  5. Out of scope / Skills-docs: the MFA hold (200 twoFactorRedirect, clears the count) is correct as is; the session cap, organization limits and organization MFA hold never reach the counter as refusals (dev, by reading). Metadata-protocol: none; Skills-docs: not owed. Nothing filed.
  6. Landing: check-governed-merges --pr 31262: not governed, 199 changed lines. Changeset patch for plugin-auth names all three refusals. Landing on the maintainer's standing words 「按照 pm-dispatch 规则复核过的都授权」 (2026-10-25T11:35Z) and 「你应该使用 pm-dispatch 技能审核和落地pr」 (2026-11-07T16:59Z). Relay pr_ready, then automerge_enable.

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review November 7, 2026 21:20
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge November 7, 2026 21:20
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Nov 7, 2026
Merged via the queue into main with commit 2f369cd Nov 7, 2026
53 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/project-thread-t94vm3 branch November 7, 2026 22:05
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.

plugin-auth: a correct-password sign-in refused by an organization IP list counts toward the account lockout

1 participant