Skip to content

fix(auth): make email verification recoverable - #1504

Merged
lilyshen0722 merged 1 commit into
mainfrom
fix/task-103-signup-wall
Sep 2, 2026
Merged

lilyshen0722 merged 1 commit into
mainfrom
fix/task-103-signup-wall

Conversation

@lilyshen0722

Copy link
Copy Markdown
Contributor

Summary

  • add a rate-limited, enumeration-safe verification-resend endpoint
  • make the unverified login state accessible, name the submitted address, and provide a resend action
  • give v2 auth controls a 44px / 16px mobile floor

Verification

  • backend: npm test -- --runInBand __tests__/unit/controllers/authController.test.js (20 passed)
  • frontend: npx jest --watchAll=false --runInBand --runTestsByPath src/v2/__tests__/V2Login.test.tsx src/v2/__tests__/v2-layout-invariants.test.ts (129 passed)
  • frontend: npm run typecheck
  • frontend: npm run build
  • backend: npm run build

The broader backend auth-route suite is currently loader-blocked on Node 26 by the existing buffer-equal-constant-time / jsonwebtoken incompatibility before it reaches the route.

@lilyshen0722

Copy link
Copy Markdown
Contributor Author

UX gate (TASK-103) @ 9bb229d — APPROVED, two nits.

Read against the TASK-101 doc §P1-A. What the walk hit is fixed: login now returns code: EMAIL_UNVERIFIED after the password check (so the state is only revealed to a correct credential), the login screen shows the address in a role=alert, and there is a working, rate-limited, always-200 resend with a role=status notice. Register and login share .v2-login__input / .v2-login__submit, so the 44 px / 16 px phone floor covers both forms; the invariant test pins it.

This PR keeps Sam's pending fork open (banner-after-signin vs wall) — nothing here forecloses letting unverified users in later. Good.

Nits (not blocking):

  1. .v2-login__resend is 32 px; at ≤480 px it sits in the same thumb flow as the 44 px inputs — give it the same floor.
  2. The "ADR numbers are unique" check is red on this PR only; the log shows no ADR output, so it looks like a base-time artefact rather than anything in this diff. @sprint-review's guard, their call.

Live check owed after deploy: fresh unverified account → sign in → alert names the address, resend returns the generic notice.

@lilyshen0722

Copy link
Copy Markdown
Contributor Author

UX gate (TASK-103) @ 9bb229d — APPROVED, two nits.

Read against the TASK-101 doc §P1-A.

  • Login: unverified sign-in now returns code: EMAIL_UNVERIFIED only after the password matches — correct ordering, no account-state oracle. The UI renders it in a role="alert", names the address, and offers "Resend verification email" with a role="status" outcome. That is the measured gap closed.
  • Resend: dedicated route, own rate bucket, generic 200, shares sendVerificationEmail with registration so both links match.
  • 390: .v2-login__input and .v2-login__submit get min-height: 44px; font-size: 16px under 480px, and V2Register uses the same classes, so signup is covered too; presence test added.
  • Does not foreclose Sam's open card (sign in unverified + banner) — that stays a separate change if he picks it.

Nits (non-blocking):

  1. .v2-login__resend is min-height: 32px; at ≤480px it sits inside the alert where every other control is 44px — bump it in the same media block.
  2. "ADR numbers are unique" is red on this PR alone and the job log ends at exit code 1 with no ADR output; nothing in this diff touches docs/adr. Looks like a base artefact — flagging for @sprint-review rather than treating it as a gate failure here.

Live check owed after deploy: fresh unverified account → alert + resend at 1440 and 390.

lilyshen0722 added a commit that referenced this pull request Sep 2, 2026
…#1508)

The PR arm checks out the PR HEAD by design (the merge ref is stale),
but GitHub takes the WORKFLOW from the merge ref. So a PR branched
before #1481 gets the workflow file and not scripts/verify-adr-numbering.js,
and the step dies with MODULE_NOT_FOUND before printing a single ADR line —
red, with nothing an author can act on. Hit #1504 first; it would have hit
most open PRs on their next event.

Resolve the checker the same way the ADRs are resolved: the PR's version if
it changed the script, main's otherwise.

Verified against #1504's head (script absent): green, 29 ADRs. Same head
with a synthetic second ADR-025 added: red, naming both files. Against main
(script present): resolves from HEAD, green.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
@samxu01
samxu01 force-pushed the fix/task-103-signup-wall branch from 9bb229d to 29e7ccc Compare September 2, 2026 16:16
@lilyshen0722

Copy link
Copy Markdown
Contributor Author

Rebased onto current main at 29e7ccc so the #1508 workflow fix is in the actual merge tree. Behavior is unchanged from UX-approved 9bb229d. Revalidated backend auth-controller 20/20, full frontend suite/build, and both builds locally; fresh CI is running.

@lilyshen0722

Copy link
Copy Markdown
Contributor Author

UX gate update @ 29e7ccc — HOLD (product ruling changed the shape, not the code).

  • Head moved 9bb229d29e7ccc by rebase only (main picked up ci(adr): make a duplicate ADR number go red #1481/ci: close two version/CI gaps a green PR cannot show you #1500/ci(adr): resolve the ADR checker from the effective tree, not the PR head #1508; the PR's own diff is unchanged). My APPROVED read carries to this head as a read of the wall shape.
  • Sam ruled the unverified sign-in card (Sprint HQ 62583): "Sign in unverified, banner until verified." So the login-time EMAIL_UNVERIFIED wall after the password check is no longer the target: a correct password signs the user in, and the shell shows a persistent banner until the address is verified — "Verify your email — we sent a link to . Resend" (sentence case, one accent, role=status, 16 px text and ≤ 44 px tall at 390; collapsible for the session, back on next load).
  • Keeps: the rate-limited enumeration-safe resend endpoint, the named address, the 44 px / 16 px floors, the i18n keys.
  • Either amend here or press the resend + 390 half and ship the banner as a follow-up — your call, @sprint-impl; say which in the pod so the press is deliberate. I gate the banner at 1440 + 390 with a fresh unverified account once it exists.

@lilyshen0722
lilyshen0722 merged commit c597696 into main Sep 2, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant