Repository navigation
Commit 2f369cd
fix(plugin-auth): a correct-password sign-in refused by an organization IP list no longer counts toward the account lockout (#31262)
<!-- claude-projects-attribution: {"github_login":"os-project-manager"}
-->
_Requested by **Leehom** · [project
thread](https://claude.ai/code/project/chan_01TenzsRopZj4jMbvjX8uz1X?thread=cmsg_01TenzsRopZj4jMbvjX8uz1X2yt1mWbT9wcdVqxqkNbGab)_
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
#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):
4. 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.
5. 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.
<details><summary>Red first</summary>
- 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.
</details>
<details><summary>Ablation, one leg per code</summary>
`node scripts/ablation-replace.mjs --delete` on
`auth-manager-after-hook.ts` (blob e06b13d 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.
</details>
## 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
#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 #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 621a487` (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](https://claude.ai/code/session_01FEkQ5mSU6a4NFvNfFHukWv)_
<!-- signed: domain:server#6 · os-project-manager ·
session_01FEkQ5p3T5YWhLjurajudnQ -->
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 07559ce commit 2f369cd
3 files changed
Lines changed: 195 additions & 4 deletions
File tree
- .changeset
- packages/plugins/plugin-auth/src
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
21 | 21 | | |
22 | 22 | | |
23 | 23 | | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
24 | 29 | | |
25 | 30 | | |
26 | 31 | | |
| |||
56 | 61 | | |
57 | 62 | | |
58 | 63 | | |
59 | | - | |
| 64 | + | |
| 65 | + | |
60 | 66 | | |
61 | 67 | | |
62 | 68 | | |
| |||
65 | 71 | | |
66 | 72 | | |
67 | 73 | | |
| 74 | + | |
68 | 75 | | |
69 | 76 | | |
70 | 77 | | |
| |||
74 | 81 | | |
75 | 82 | | |
76 | 83 | | |
77 | | - | |
| 84 | + | |
78 | 85 | | |
79 | 86 | | |
80 | 87 | | |
81 | 88 | | |
82 | 89 | | |
83 | 90 | | |
| 91 | + | |
84 | 92 | | |
85 | 93 | | |
86 | 94 | | |
| |||
226 | 234 | | |
227 | 235 | | |
228 | 236 | | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
| 245 | + | |
| 246 | + | |
| 247 | + | |
| 248 | + | |
| 249 | + | |
| 250 | + | |
| 251 | + | |
| 252 | + | |
| 253 | + | |
| 254 | + | |
| 255 | + | |
| 256 | + | |
| 257 | + | |
| 258 | + | |
| 259 | + | |
| 260 | + | |
| 261 | + | |
| 262 | + | |
| 263 | + | |
| 264 | + | |
| 265 | + | |
| 266 | + | |
| 267 | + | |
| 268 | + | |
| 269 | + | |
| 270 | + | |
| 271 | + | |
| 272 | + | |
| 273 | + | |
| 274 | + | |
| 275 | + | |
| 276 | + | |
| 277 | + | |
| 278 | + | |
| 279 | + | |
| 280 | + | |
| 281 | + | |
| 282 | + | |
| 283 | + | |
| 284 | + | |
| 285 | + | |
| 286 | + | |
| 287 | + | |
| 288 | + | |
| 289 | + | |
| 290 | + | |
| 291 | + | |
| 292 | + | |
| 293 | + | |
| 294 | + | |
| 295 | + | |
| 296 | + | |
| 297 | + | |
| 298 | + | |
| 299 | + | |
| 300 | + | |
| 301 | + | |
| 302 | + | |
| 303 | + | |
| 304 | + | |
| 305 | + | |
| 306 | + | |
| 307 | + | |
| 308 | + | |
| 309 | + | |
| 310 | + | |
| 311 | + | |
| 312 | + | |
| 313 | + | |
| 314 | + | |
| 315 | + | |
| 316 | + | |
| 317 | + | |
| 318 | + | |
| 319 | + | |
| 320 | + | |
| 321 | + | |
| 322 | + | |
| 323 | + | |
| 324 | + | |
| 325 | + | |
| 326 | + | |
| 327 | + | |
| 328 | + | |
| 329 | + | |
| 330 | + | |
| 331 | + | |
| 332 | + | |
| 333 | + | |
| 334 | + | |
| 335 | + | |
| 336 | + | |
| 337 | + | |
| 338 | + | |
| 339 | + | |
| 340 | + | |
| 341 | + | |
| 342 | + | |
| 343 | + | |
| 344 | + | |
| 345 | + | |
| 346 | + | |
| 347 | + | |
| 348 | + | |
| 349 | + | |
| 350 | + | |
| 351 | + | |
| 352 | + | |
| 353 | + | |
| 354 | + | |
| 355 | + | |
| 356 | + | |
| 357 | + | |
| 358 | + | |
| 359 | + | |
| 360 | + | |
| 361 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
42 | 42 | | |
43 | 43 | | |
44 | 44 | | |
45 | | - | |
| 45 | + | |
46 | 46 | | |
| 47 | + | |
47 | 48 | | |
48 | 49 | | |
49 | 50 | | |
| |||
89 | 90 | | |
90 | 91 | | |
91 | 92 | | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
92 | 135 | | |
93 | 136 | | |
94 | 137 | | |
| |||
149 | 192 | | |
150 | 193 | | |
151 | 194 | | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
152 | 198 | | |
153 | 199 | | |
154 | 200 | | |
| |||
159 | 205 | | |
160 | 206 | | |
161 | 207 | | |
162 | | - | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
163 | 211 | | |
164 | 212 | | |
165 | 213 | | |
| |||
0 commit comments