fix(frontend): tell users to sign in again or wait after too many MFA codes (#872) - #880
Merged
Merged
Conversation
The challenge is authenticated by the short-lived MFA token, not by the session. When that token was revoked after five wrong codes, or had expired, the response interceptor treated the 401 as a lost session: it tried to refresh an unrelated session, then reloaded /login. The user landed on an empty password form with no word about what happened. Requests to /auth/mfa/challenge now pass their errors straight to the caller. Every other request keeps the refresh and redirect behaviour. Signed-off-by: alex-dembele <alexandredembele16@gmail.com>
… codes (#872) Since #689 the challenge refuses a code in four ways, and the screen said "incorrect code" for all of them. Someone whose sign-in had used its five codes kept typing new ones into a token the server had closed. A spent, revoked or expired challenge now returns to the password form, with the address and password still filled in and a banner saying why. A locked account is told how many minutes to wait; the code field and the button stay disabled until then, and focus comes back to the field when the lock ends. The per-address limit, which gives no duration, reads as "try again in a few minutes". A wrong code is unchanged. The copy lives in authStrings.ts with the rest of the auth screens, in French and English. Signed-off-by: alex-dembele <alexandredembele16@gmail.com>
…st (#872) axe flagged two serious colour-contrast failures on the sign-in and MFA screens. The error banner's 12.5px text measured 4.46:1 on the light theme, and this change shows that banner in more situations. Its tint goes from 10% to 7%, which measures 4.68:1 light and 5.20:1 dark. The footer year was dimmed with opacity-70 to 3.35:1; it now takes the colour of the rest of the line, which passes. Signed-off-by: alex-dembele <alexandredembele16@gmail.com>
3 tasks
3 tasks
…h a reason (#872) Mandated enrolment runs on a 15-minute MFA_ENROLLMENT token. Once it expired, /auth/mfa/verify answered 401, the response interceptor took it for a lost session, and /login reloaded with no word of explanation. The exemption added for the challenge was keyed on its URL, which could not cover enrolment: /auth/mfa/setup and /verify also run from Settings on a real session, where an expired access token must still refresh. So the exemption now follows the credential instead. authService marks a request ownCredential when it sends an MFA token, and the interceptor leaves only those alone. On the enrolment screen, a refusal no code can fix returns to the password form with "this sign-in step has expired" instead of "incorrect code", whether it happens at setup or at verify. Signed-off-by: alex-dembele <alexandredembele16@gmail.com>
Open
4 tasks
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.
Closes #872
Since #689 (PR #875) the MFA challenge can refuse a code in ways a new code cannot fix, but the sign-in screen said "incorrect code" every time. Worse, after five wrong codes the next attempt came back
401 TOKEN_REVOKED, and the axios interceptor treated it as a lost session: it tried to refresh an unrelated session, then reloaded/login. The user landed on an empty password form with no explanation.What changes
Interceptor (
lib/api.ts). A request that carries its own MFA token keeps its 401s:authServicemarks itownCredential, a typed axios config flag that is never sent to the server. That covers the challenge and mandated enrolment, whose 401s are the screen's to explain./auth/mfa/setupand/verifycalled from Settings run on the session, so they keep the refresh-and-redirect behaviour, as does every other request.Classifying the refusal (
features/auth/challengeRefusal.ts, pure and typed):MFA_CHALLENGE_EXHAUSTEDandTOKEN_REVOKEDmean the sign-in used its codes;TOKEN_EXPIRED,TOKEN_INVALID,UNAUTHORIZEDor any other 401 mean the step expired;429 MFA_LOCKEDmeans wait. The duration comes fromretry_after, thenRetry-After, rounded up to whole minutes;Screen (
AuthScreen.tsx):The copy is FR/EN in
authStrings.ts, typed byAuthCopy, where the rest of the auth copy lives. That differs from criterion 4 of the issue, which namedfr.json/en.json: I wrote that criterion without checking how this module handles its copy.Mandated enrolment (criterion 6, added to the issue). An expired 15-minute enrolment token, at setup or at verify, returns to the password form with "this sign-in step has expired" instead of "incorrect code", or instead of the old silent reload of
/login.Contrast: axe flagged two serious failures on these screens.
opacity-70, which I removed.Verification
npx tsc -b --noEmit: exit 0.npx vite build: built.npx vitest run: 103 files, 965 tests passed.npx eslint --max-warnings=0on every touched file: clean.challengeRefusal.test.ts(7),mfaChallengeRefusals.test.tsx(9), plus 5 interceptor cases inapi.test.ts;AuthScreen.tsx, 8 of the 9 screen tests fail. The one that passes covers "incorrect code", whose behaviour is unchanged;Live: this branch's Vite build against the #689 backend (local Postgres and Redis), signing in as an admin enrolled in TOTP:
Between the EN lock and the EN exhausted check, I cleared the lock keys in the throwaway Redis instead of waiting fifteen minutes.
Criteria 6 and 7, live against master plus #888 (master does not compile without it, see #886). In both cases a valid code was entered, so only the expiry explains the refusal:
Signing in again after the expired enrolment then hit a separate backend defect: setup answered 400 forever. That is #889, fixed in #892, and verified live to end on a session.
Tests added for this part:
authServiceOwnCredential.test.ts(3), 3 enrolment cases inmfaChallengeRefusals.test.tsx, and the interceptor tests rewritten around the flag, including a Settings-on-session case that must still refresh. Against the previous screen, the 2 expiry tests fail.Merge order
#689 is already on master (via #871).
npm run lint:ceilingfails on this branch exactly as it does on master:no-irregular-whitespace,no-raw-colorsandreact-hooks/refs, all in files this PR does not touch. #891 fixes them. Merge #891, then bring master into this branch.Not done
eslint .failures on master are handled by chore(lint): make the ESLint ratchet pass on master again, and tighten it to 99 (#627) #891 (chore(frontend): lint:ceiling fails on a clean master — stale no-irregular-whitespace count #627). My earlier note on debt(frontend): lint errors on master: any in EditRiskModal, setState in effects, dead GlobalShortcuts import #864, which listed three files, understated the problem.