Fix the auth-gate lockouts found in review - #24
Merged
Conversation
Twelve confirmed findings against the auth-gate rework, four of them reachable lockouts on the sign-in path. The central one: "Your session ended" was wired only into boot()'s /me 401, but boot() runs exactly twice and the portal 302s unauthenticated document requests before the SPA ever mounts. The path that actually produces an expired session is an api() call from the mounted shell, which had no 401 handling at all — so the gate was near-unreachable in the deployment it was written for, and an expired session still rendered the literal string "sign in" as a turn error. api() now reports every 401 through a handler the shell registers, so any call from anywhere reaches the gate; the two raw uploader fetches report through the same hook. Other lockouts: - The dev form's data-pending latch was never cleared and lit reuses the <form> node across an error re-render, so one failed attempt left the form permanently inert. Pending is now render state, and the typed value survives the error. - type="email" blocked the bare principal ids the repo's own tests and docs use. Back to text with inputmode=email and required. - An allowlist rejection rendered as "Your session ended" and bounced off /auth/login forever. authenticate() now distinguishes unauthenticated from not_allowed, and the client has a distinct "You don't have access" state. - The portal gate's Sign in looped when the portal wasn't the front door. A one-shot attempt marker breaks it after a single try and explains that the surface is reached through the portal. Also: boot() rejections route through bootSafely() keyed on whether the shell actually mounted, so a throw between /me and mountShell() no longer leaves a blank page, and Try again gets the same handler; authMode is refreshed from every /me rather than only ever pushed toward dev; sign-out checks the logout response instead of navigating as if it worked, and skips the dead /signout round-trip in portal mode; chassis treats a blank signing secret as unset, so an empty CORE_SIGNING_SECRET can no longer silently select cookie auth (admin carried the same latent falsy check); and the Google OAuth smoke script uses a minted portal identity instead of the now-404ing dev endpoint. Adds the auth-mode server tests the contract shipped without — 10 cases across portal and dev mode, covering the /signin gate, the 400/403 split, the mode and reason fields, and that a bare principal id can still sign in.
16francej
force-pushed
the
web-ui-auth-gate-fixes
branch
from
July 30, 2026 20:01
fe99577 to
219fbd7
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-on to #23. An xhigh code review returned 15 findings, 12 confirmed — four of them reachable lockouts on the sign-in path. This fixes all of them.
The central one
"Your session ended" was wired only into
boot()'s/me401. Butboot()runs exactly twice, and the portal 302s unauthenticated document requests to/auth/loginbefore the SPA ever mounts. The path that actually produces an expired session is anapi()call from the mounted shell — which had no 401 handling at all.So the gate was near-unreachable in the deployment it was written for, and an expired session still rendered the literal string
sign inas a turn error. That is the same defect #23 set out to fix, relocated rather than removed.api()now reports every 401 through a handler the shell registers, so a 401 from anywhere reaches the gate. The two raw uploaderfetches report through the same hook.The other lockouts
data-pendingwas never cleared and lit reuses the<form>node across an error re-render, so a corrected retry hit the guard forever. Pending is render state now, and the typed value survives the error.type="email"blocked bare principal ids likealice— the form the repo's own tests and docs use. Back totextwithinputmode="email"andrequired./auth/loginforever.authenticate()distinguishesunauthenticatedfromnot_allowed, and the client has a distinct "You don't have access" state.:8096, the smoke image, a misconfigured deploy). A one-shot attempt marker breaks it after a single try and explains the real cause.Everything else the review found
boot()rejections route throughbootSafely()keyed on whether the shell actually mounted — a throw between/meandmountShell()no longer leaves a blank page. "Try again" gets the same handler instead of a barevoid boot().authModeis refreshed from every/merather than only ever pushed towarddev, so an open tab can't keep a mode the server no longer serves./signoutround-trip in portal mode.CORE_SIGNING_SECRETcan no longer silently select cookie auth. Fixed at the shared layer because admin carries the identical falsy check.Tests
The auth-mode contract shipped in #23 with no coverage at all. Adds 10 cases across two files (mode is fixed at module load, so one process each): the
/signingate, the 400/403 split, themodeandreasonfields, that a cookie confers nothing in portal mode, and that a bare principal id can still sign in.440 web-ui tests pass. Repo typecheck, all three web-ui tsconfigs, and eslint clean.
Verification
Exercised the actions this time, not just the renders — that gap is what let the original bugs through:
alice), no@.api()401 → gate. Confirms the central hook.Screenshots of the two new states: https://claude.ai/code/artifact/d81b8955-230d-40db-996f-40edabc1a8fe
🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.