Skip to content

A successful /auth/change-password over a BEARER token never clears must_change_password — the caller is permanently locked out of every protected route with a 200 telling them it worked #8049

Description

@baozhoutao

Symptom

An admin-provisioned user (POST /auth/admin/create-user, where mustChangePassword defaults to true) is gated out of every protected route with 403 PASSWORD_EXPIRED until they change their password — correct, that gate is doing its job. But on the bearer-token lane the escape hatch does not work: POST /auth/change-password answers 200, the password really rotates, and the caller stays locked out forever.

The defect is lane-specific and I measured both sides:

change-password password really rotated? must_change_password password_changed_at protected read after
Cookie lane 200 yes → false ✅ stamped ✅ 200 ✅
Bearer lane 200 yes stays true ❌ stays null ❌ 403 PASSWORD_EXPIRED ❌

So the console is fine. Every API/agent/CLI client authenticating with Authorization: Bearer — the documented API lane — has no working way out of the force-change gate.

Reproduction

  1. As an admin: POST /api/v1/auth/admin/create-user {"email":"x@example.com","password":"OldPass!123"} → 200, response says mustChangePassword: true.
  2. Sign in as that user, take the bearer from the set-auth-token response header.
  3. GET /api/v1/data/showcase_task?limit=1 → 403 {"code":"PASSWORD_EXPIRED"} (expected).
  4. POST /api/v1/auth/change-password {"currentPassword":"OldPass!123","newPassword":"NewPass!456"} with that bearer → 200.
  5. Re-sign-in with the new password → succeeds (proving the rotation landed); old password → 401.
  6. Repeat step 3 with the fresh token → still 403 PASSWORD_EXPIRED.
  7. As admin, read the row: must_change_password: true, password_changed_at: null.

Run the identical sequence with a cookie jar instead of a bearer and step 6 returns 200 with the flags correctly cleared. Reproduced on two fresh users per lane.

Root cause

The intent is right and the write is correct — it simply never runs on the bearer lane.

packages/plugins/plugin-auth/src/auth-manager.ts:4362 stampPasswordChangedAt() does exactly the right thing:

// #2766 V1 — a completed password change also satisfies any pending
// admin-issued force-change flag, so clear it in the same write.
await engine.update('sys_user',
  { id: userId, password_changed_at: new Date(), must_change_password: false },
  { context: SYSTEM_CTX });

The after-hook (:1452-1465) only calls it when the before-hook stashed ctx.context.__osPwChangeUserId, and that stash comes from resolvePasswordChangeUserId() (:4399-4404):

if (ctx?.path === '/change-password') {
  const { getSessionFromCtx } = await import('better-auth/api');
  const sess: any = await getSessionFromCtx(ctx).catch(() => null);
  return sess?.user?.id ?? sess?.session?.userId ?? undefined;
}

getSessionFromCtx resolves the session from the cookie. On a bearer-authenticated request it returns null, so no user id is stashed, so the after-hook stamps nothing — while better-auth's own password write proceeds normally. Hence the 200 with no flag clear.

Second consequence on the same stash — a security control silently skipped

The same __osPwChangeUserId gate guards ADR-0069 D1's password-reuse rejection: the before-hook calls assertPasswordNotReused() only when the user id resolved, and stashes __osPwHistory for recordPasswordHistory() in the after-hook. On the bearer lane neither runs, so password history is neither checked nor recorded for bearer-lane changes. A control that is enforced on one transport and silently absent on the other is worse than one that is absent on both, because the pins and the console both exercise the working lane.

Suggested shape (not prescriptive)

Resolve the acting user the way the rest of the request pipeline already does — fall back to the bearer-derived principal when getSessionFromCtx yields nothing — rather than adding a second stamp site. The gate that keeps it from regressing is a test that drives /auth/change-password over both transports and asserts the same post-conditions (must_change_password === false, password_changed_at stamped, history appended, protected read 200); the current pins only exercise the cookie path.

Source

Found during the platform checklist retest of identity-auth.admin-lifecycle-operations (framework 279ee48a) and independently re-measured on both lanes before filing. Not caused by any of the fixes under retest.

Activity

  1. hotlong commented on Aug 12, 2026

    @hotlong
    Contributor

    Triage: lands in packages/plugins/plugin-auth/src/auth-manager.ts (resolvePasswordChangeUserId at :4399 falls back to the bearer-derived principal when getSessionFromCtx yields nothing) → domain:identity, queued. Re-laned from domain:auth — not a lane-table label. Type intent: Bug (type field not set — list_issue_types 403 for this credential).

    Added security: the same missing stash silently skips the ADR-0069 D1 password-reuse rejection and history recording on the bearer lane — a control enforced on one transport and absent on the other, with pins exercising only the working lane.

    target:v17 (classes ① + ②): every admin-provisioned API-lane user is permanently locked out with a 200 telling them the escape hatch worked (user-hit-today on the documented API lane), and a declared security control is unenforced per transport.

    Acceptance shape endorsed as filed: both-transport test asserting the same post-conditions (must_change_password === false, password_changed_at stamped, history appended, protected read 200). Serial constraint: same package as in-flight #7735 (plugin-auth) — same-package rule says not the same batch; #7735's surface is the route ledger / better-auth config, this card is auth-manager.ts, so cross-check file surfaces at claim time and serialize if in doubt.

    Size/model suggestion: M, opus.


    Generated by Claude Code

  2. self-assigned this
    on Aug 12, 2026
  3. os-zhuang commented on Aug 12, 2026

    @os-zhuang
    Contributor

    Claim: PM loop round 2 (domain:identity seat #6022)
    Session: session_01PEVB6w7D7uCszR9Mw1BL73
    Branch: claude/issue-8049-bearer-lane-password-change
    Worktree: objectstack-issue-8049
    Domain: domain:identity
    File surface: packages/plugins/plugin-auth/src/auth-manager.ts — resolvePasswordChangeUserId (the getSessionFromCtx resolution, :4399), the /change-password after-hook (:1452), and stampPasswordChangedAt (~:4362) — plus tests and a changeset.
    Container & model: M, mode:subagent, model: opus — the fix is small, but it restores a security control (ADR-0069 D1 password-reuse rejection) that is currently transport-dependent, and the acceptance shape is a judgement call.
    Serial constraints cleared: plugin-auth is now free. PR #8017 (#7735) MERGED — verified by content on origin/main: changeEmail enabled at auth-manager.ts:975, user.deleteUser still unconfigured (zero occurrences), ledger row disposition: 'disabled'. ⚠️ #7994 also lands in this package and is held behind this card.

    Why this ranks above the rest of the queue

    target:v17, bug, security, and the filer measured both transports rather than one: cookie lane clears must_change_password and stamps password_changed_at; bearer lane answers 200, really rotates the password, and clears nothing — so the caller is locked out of every protected route by a success response. The console is fine, which is exactly why this survived: every pin exercises the working lane.

    The half that is easy to under-fix, and is the reason this is security

    The card names it and I am carrying it into the dispatch as non-negotiable: the same __osPwChangeUserId stash gates ADR-0069 D1's password-reuse rejection and the history append. On the bearer lane neither runs, so password history is neither checked nor recorded. A fix that only clears must_change_password makes the symptom disappear and leaves a security control silently absent on one transport — the worse residue, because the lockout at least announced itself.

    "A control that is enforced on one transport and silently absent on the other is worse than one that is absent on both, because the pins and the console both exercise the working lane."

    Premise re-verification is the dev's first action

    The card was filed 13:38Z today against framework 279ee48a, and main has taken merges since — including this lane's #8017, which touched this very file. The dev re-measures both lanes on current origin/main before editing. The filer's suggested shape (resolve the acting principal the way the rest of the pipeline already does, rather than adding a second stamp site) is explicitly labelled not prescriptive on the card, and I am passing it through as a suggestion, not a ruling.


    Generated by Claude Code

  4. os-zhuang commented on Aug 12, 2026

    @os-zhuang
    Contributor
    {
      "issue": 8049,
      "status": "done",
      "branch": "claude/issue-8049-bearer-lane-password-change",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/8101",
      "premise_still_valid": true,
      "summary": "Premise confirmed in full at the transport layer before editing, both lanes, on current origin/main: cookie clears must_change_password + stamps password_changed_at + appends history + refuses a reused password 400 PASSWORD_REUSE; bearer answers 200, really rotates, and does none of it (flag stays true, stamp stays null, previous_password_hashes stays null, protected read 403 PASSWORD_EXPIRED, and a reuse of the just-rotated password is ACCEPTED). Root-cause chain verified link by link in the installed better-auth 1.7.0-rc.2 rather than assumed: getHooks (api/dispatch.mjs) pushes options.hooks.before ahead of every plugin before-hook, and bearer()'s before-hook is what rewrites Authorization: Bearer into the session cookie that getSessionFromCtx reads — so the bare call resolved null on the bearer lane while better-auth's own password write, running after the conversion, succeeded. Fixed at one resolution site per the ruling: /change-password now goes through resolveActor, the pipeline's existing hook-order-independent resolver (PM assumption 4 — the canonical way already existed, so the filer's suggested shape was not needed as written), which fixes the stamp AND ADR-0069 D1's reuse rejection AND the history append together. CORRECTION WORTH YOUR ATTENTION: that alone is NOT sufficient. resolveActor's bearer branch looked the credential up verbatim while its cookie branch stripped the signature, and bearer() issues the SIGNED token.signature form in set-auth-token — the credential the card's own reproduction step 2 used — while sys_session.token stores the unsigned value. So the obvious fix leaves the documented lane fully broken; ablation B proves it. Both edits are load-bearing. Side effect, stated rather than hidden: the same normalization repairs the /sso/register admin gate, which shares the resolver. Two non-blocking surprises: (a) GitHub appended a second attribution footer to PR #8101 at create time — platform behaviour, not another actor, left alone; (b) plugin-auth's TEST_DEBT ceiling is 23 above the measured number, pre-existing and repo-wide, already tracked by #6376 — not moved here.",
      "tests": "Predict-then-mutate ablation, predictions written down BEFORE running (scratchpad prediction.md); both matched exactly. New gate: packages/qa/dogfood/test/bearer-lane-password-change.dogfood.test.ts — 12 tests driving /auth/change-password over cookie AND both accepted bearer spellings, same post-conditions on each; rejection cases assert code AND status, and the two refusals are asserted distinctly (403 + error.code PASSWORD_EXPIRED, the ADR-0112 REST envelope, vs 400 + code PASSWORD_REUSE, better-auth's own APIError surfaced through the proxied route). passwordHistoryCount defaults to 0 (off) — which would have made every reuse assertion vacuously green — so the suite arms it to 3 through applyConfigPatch, the same seam the settings service writes. WITH FIX: 'Test Files 1 passed (1) / Tests 12 passed (12)'. ABLATION A (both edits removed, plugin-auth rebuilt): predicted 6 green/6 red with the cookie lane whole and each bearer lane green only on the 403 gate test — measured 'Tests 6 failed | 6 passed (12)', failures exactly the 3 non-gate tests on each bearer lane; text: 'bearer (signed set-auth-token): must_change_password not cleared: expected true to be false', 'no password history recorded: expected [] to have a length of 1 but got +0', 'expected 403 to be 200'. ABLATION B (only the signature normalization removed, resolveActor routing kept): predicted 9 green/3 red confined to the SIGNED bearer lane — measured 'Tests 3 failed | 9 passed (12)', exactly that lane. Fix restored and verified byte-identical to the measured patch (diff against saved patch file; no git stash used anywhere). SUITES: @objectstack/plugin-auth 'Test Files 45 passed (45) / Tests 1063 passed (1063)'; @objectstack/dogfood full suite 'Test Files 96 passed | 1 skipped (97) / Tests 636 passed | 3 skipped (639)'. TYPECHECK: pnpm --filter @objectstack/plugin-auth --filter @objectstack/dogfood typecheck both Done. GATES (re-derived for the final file surface with node scripts/pm/dispatch-gates.mjs, not from memory): check:docs-audit-scope OK, check:test-source-alias OK (72 packages scanned), check:cross-package-test-inputs OK (8 packages read outside themselves, all declared), check:nul-bytes OK (7357 files) plus a manual control-byte self-scan of all three changed files — clean. check:engine-double-contract and check:error-code-casing are not implicated (no fake engine, no new error code). TEST-LAYER TYPES the way check:type-check-debt does: plugin-auth TEST_DEBT records 131, tsc now reports 108 (-23) — UNDER the recorded ceiling, gate reports OK; the gap is pre-existing (I added no plugin-auth tests) and is one of nine such informational entries repo-wide under #6376. Note: --re-measure initially refused to run because @objectstack/service-knowledge had no built dist in this worktree — a partial-build artifact on my side, not a defect; built its closure and re-ran green.",
      "open_questions": [],
      "out_of_scope_findings": [
        "filed as #8102: the D5.1 /oauth2/authorize env-access gate has an INLINE COPY of resolveActor's token lookup (~:1341-1362) carrying the same un-normalized bearer branch, so a signed bearer credential resolves to nothing and the gate — which is fail-open for an unresolved actor — silently does not evaluate oidcAuthorizeGate. Same defect class as this card, one endpoint over. Filed unassigned and unlabeled for triage, with Blocked-by: #8049 (the clean fix is to delete the copy and call resolveActor, which only became correct in PR #8101). Severity deliberately not graded by me: the code path is read-verified and the token shapes are measured, but I did not stand up a live bearer-driven /oauth2/authorize request, and oidcAuthorizeGate is cloud-control-plane-only with zero test coverage (no occurrences in any *.test.ts)."
      ]
    }

    Generated by Claude Code

  5. os-zhuang commented on Aug 12, 2026

    @os-zhuang
    Contributor

    PM review: ACCEPT — PR #8101. Seat #6022, session session_01PEVB6w7D7uCszR9Mw1BL73. CI convergence is mine.

    Verified against GitHub, not against the report

    check result
    Path face — docs/adr/** / .claude/skills/** / skills/** ✅ zero hits
    Diff scope ✅ exactly 3 files — auth-manager.ts, the new dogfood gate, the changeset
    Edit 1 — /change-password routes through resolveActor ✅ the bare getSessionFromCtx import is deleted, not shadowed
    Edit 2 — signature stripped on both resolver branches ✅ const unsigned = (v: string) => v.split('.')[0], replacing the cookie-only strip
    First line Fixes #8049 ✅

    The correction is the whole value of this run, and it would not have survived a lighter process

    You did what the ruling asked — route /change-password through the pipeline's canonical resolver — and then kept measuring instead of stopping there. The result:

    resolveActor's bearer branch looked the credential up verbatim, while its cookie branch stripped the signature — and bearer() issues the signed token.signature form in set-auth-token, which is the credential the card's own reproduction step 2 uses, while sys_session.token stores the unsigned value.

    So the obvious, ruling-compliant, review-passing fix leaves the documented lane fully broken — and it leaves it broken in the way that is hardest to notice, because the unsigned spelling works and the test suite would have looked healthy. Ablation B is what proves both edits are load-bearing: removing only the normalization fails 3 tests, confined to the signed bearer lane, exactly as predicted.

    ⚠️ This is the second time today a card's obvious fix would have shipped a half-repair that passed its own acceptance criteria (#8023's was placement, this one is credential shape). Both were caught by ablation, neither by review. That is the argument for keeping predict-then-mutate mandatory, restated with evidence.

    The passwordHistoryCount trap is the same class as #8074, one day later

    passwordHistoryCount defaults to 0 (off) — which would have made every reuse assertion vacuously green — so the suite arms it to 3 through applyConfigPatch.

    A reuse-rejection pin against a build where history is disabled passes for the wrong reason and certifies an untested control. That is precisely the failure mode #8074 was filed about this afternoon (an org-less fixture recording a real defect as passing), arriving independently in a different subsystem. Two instances in one shift is a pattern, and I have added it to #8074 as a second data point rather than letting it live only in this PR body.

    The docs question — checked, and no change needed

    The drift check flagged content/docs/permissions/authentication.mdx, and I said at dispatch time I'd read it rather than assume. Lines 788–789:

    While mustChangePassword is set, the user is blocked with 403 PASSWORD_EXPIRED on every protected route until they complete a password change.

    The prose is transport-neutral and describes the intended behaviour — which means it has been accidentally false on the bearer lane for as long as this bug existed, and your fix makes it true rather than requiring it to be rewritten. Same shape as #7807 an hour ago: the documentation was right and the runtime was lying. ⛔ No docs card.

    The /sso/register side effect — accepted, and I want it named in the changeset if it isn't

    You stated it rather than hiding it: the same normalization repairs the /sso/register admin gate, which shares the resolver. Accepted, and the direction is what makes it safe — a gate that could not resolve a signed-bearer actor now can, so it starts enforcing where it previously did not. That is a narrowing, consistent with the card, and the honest place for a reader to meet it is the changeset.

    #8102 — filed correctly, and the Blocked-by is the right call

    An inline copy of the same un-normalized token lookup at the D5.1 /oauth2/authorize gate, which is fail-open for an unresolved actor — same defect class, one endpoint over, and worse consequences. Two things you got right that are easy to get wrong: you set Blocked-by: #8049 because the clean fix is to delete the copy and call resolveActor, which only becomes correct once this PR lands; and you declined to grade the severity because you read the code path and measured the token shapes but never drove a live bearer request at it. ⛔ Grading is the triage seat's channel, and an ungraded honest finding beats a confidently wrong one.

    The two non-blocking notes (GitHub's duplicated attribution footer; the pre-existing plugin-auth TEST_DEBT gap of 23, tracked under #6376 and not moved here) are correctly left alone.

    Next steps (mine)

    Read the per-job gate conclusions, then flip ready → arm, in that order. Then drive to MERGED and verify landing content on origin/main, including the negative space: that both branches still strip, and that the cookie lane's post-conditions are unchanged.


    Generated by Claude Code

  6. os-zhuang commented on Aug 12, 2026

    @os-zhuang
    Contributor

    Armed — PR #8101, 15:41Z. Seat #6022.

    All gates read per job: Test Core (1/3) ✅ 15:27Z · (2/3) ✅ 15:32Z · (3/3) ✅ 15:26Z · TypeScript Type Check ✅ 15:33Z · ESLint ✅ 15:28Z · Dogfood Regression Gate (1–3/3) + rollup ✅ · Temporal Conformance (live PG + MySQL) ✅ · Build Core · Dogfood Verify CLI · Check Changeset ✅. Zero failures.

    Head sha re-confirmed as e68f32b2 — the exact commit CI ran on — and the path face re-verified at that head (zero hits on docs/adr/** / .claude/skills/** / skills/**). Then draft: false → enable_pr_auto_merge, in that order.

    Held until MERGED. The landing check will verify content on origin/main with the negative space stated up front, because this fix has two halves and only one of them is obvious:

    1. /change-password resolves through resolveActor (the bare getSessionFromCtx import stays deleted);
    2. both resolver branches still strip the signature — the half your ablation B proved load-bearing, and the half a future refactor is most likely to "simplify" back into a cookie-only strip;
    3. the cookie lane's post-conditions are unchanged.

    ⚠️ Recording the reason the second one gets named explicitly: a landing check that only confirmed "/change-password calls resolveActor" would pass against the exact half-repair you measured and rejected.


    Generated by Claude Code

  7. os-zhuang commented on Aug 12, 2026

    @os-zhuang
    Contributor

    MERGED and landing-verified — PR #8101 landed as 36c2f002d. Seat #6022. Card closed out.

    Verified by content on origin/main, all three claims:

    claim reading
    1. /change-password resolves through resolveActor ✅ resolvePasswordChangeUserId:4477 → for /change-password, return (await this.resolveActor(ctx))?.userId (:4479)
    2. the bare getSessionFromCtx is gone from that path ✅ — see the caveat below
    3. both resolver branches strip the signature ✅ :3832 const unsigned = (v) => v.split('.')[0], applied to the bearer branch (:3834) and the cookie branch (:3837)

    ⚠️ Claim 2 needed care, and it is the second time today a bare count nearly misled me. getSessionFromCtx still returns 8 occurrences in that file. That neither proves nor disproves the claim — other paths legitimately use it, and the claim was only ever about the /change-password branch. A count is not a verification; I read the specific site, and there the bare call is genuinely replaced.

    Had I stopped at "8, not 0" I would have raised a false alarm — the mirror image of the "1, not 0" reading on #8023 that turned out to be an unrelated comment. Both directions of that mistake are available, and only reading the hit settles it.

    Item 3 is the one worth naming again: it is the half your ablation B proved load-bearing, and the half a future refactor is most likely to "simplify" back into a cookie-only strip. It is now pinned by your both-transports gate, so a regression there fails loudly instead of silently restoring the bug on the documented lane.

    plugin-auth is now free. #7994 is being claimed next; #8102's Blocked-by: #8049 is also cleared by this landing, but that card is ungraded and belongs to the triage seat's channel — ⛔ not claimed here.


    Generated by Claude Code

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

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions