fix(ux/login): field-specific error copy for empty + malformed inputs - #485
Conversation
Surface actionable, field-targeted messages on the login modal instead of the generic backend strings users were seeing on empty / malformed input. Issue #455: empty email previously showed "Invalid email format" and empty password showed "Invalid email or password", neither of which told the user which field was actually blank. Issue #456 (case 3.3): a malformed email previously came back from the server as "authentication failed", which read like a credential problem rather than a typo. Changes (frontend/src/auth.ts): * `validateLoginInputs` pre-flight runs before the rate-limit check so an accidental click on a blank form does not burn the cooldown. Maps empty / both-empty / malformed to "Enter email address", "Enter password", "Enter email and password", "Incorrect email format". * `mapServerLoginError` translates the known generic backend strings to clearer copy. "authentication failed" and "invalid email or password" both collapse to "Incorrect email or password" so the account-enumeration mitigation called out in #456 is preserved (the backend stays generic on the credential check). Unknown server messages pass through unchanged so MFA / rate-limit / server signals are not suppressed. * Extracted `showLoginError` to remove three near-duplicate error-div update blocks. Tests: 8 new component tests in frontend/src/__tests__/auth.test.ts cover each branch; full suite (1750 tests) green. Closes #455, #456.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughClient-side pre-flight checks and a server-error mapper were added to the login flow; the handler integrates these and defensively stringifies non-Error rejections. Tests cover empty/malformed inputs, mapped server errors, pass-throughs, and non-Error rejections. ChangesLogin form validation and error handling
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/auth.ts`:
- Around line 524-526: The code casts `err` to `Error` unsafely in
mapServerLoginError; change `mapServerLoginError` to accept an `unknown` error,
then defensively extract a message: if `err instanceof Error` use `err.message`,
else use `String(err)` (or a default like `''`) before calling `.toLowerCase()`
so `mapServerLoginError` and any callers (e.g., where `api.login()` rejections
are handled) won't throw when a non-Error rejection arrives.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bf101751-b558-4283-9589-1866b338b98f
📒 Files selected for processing (2)
frontend/src/__tests__/auth.test.tsfrontend/src/auth.ts
CodeRabbit flagged that `error as Error` in the catch block can produce `undefined.toLowerCase()` inside `mapServerLoginError` if `api.login` rejects with a non-Error value (string, plain object, etc.). Replace the unsafe cast with an `instanceof Error` guard that falls back to `String(error)`. Adds two regression tests covering string and plain-object rejections.
|
Addressed CodeRabbit pass 1: replaced the @coderabbitai review |
|
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Summary
Replace the generic / wrong-field error copy on the login modal with field-specific, user-actionable messages, while preserving the account-enumeration mitigation already in the backend.
validateLoginInputsruns as a client-side pre-flight before the rate-limit check, so an accidental click on a blank form does not burn the cooldown the user needs for their real attempt. Empty / both-empty / malformed inputs are caught locally and produce "Enter email address", "Enter password", "Enter email and password", or "Incorrect email format".mapServerLoginErrortranslates the known generic backend strings into clearer copy. The backend'sauthentication failed(user-not-found) andinvalid email or password(wrong-password) BOTH collapse to a single client-side message ("Incorrect email or password"), so the enumeration mitigation called out in ux(login): wrong-data error messages are generic ("authentication failed") and hide which field is wrong #456 is preserved. Unknown server messages pass through unchanged so MFA / rate-limit / server signals are not suppressed.Test plan
frontend/src/__tests__/auth.test.tscover empty email, empty password, both-empty, malformed email, backendauthentication failed, backendinvalid email or password, backendinvalid email format, and the unknown-error pass-through.tsc --noEmitclean.Security note
The QA suggestion in #456 for cases 3.2 / 3.5 (distinguish "user does not exist" from "wrong password") is intentionally NOT applied. Doing so would leak account-existence information and weaken the brute-force / enumeration mitigation that the backend already enforces by returning the generic
invalid email or passwordfrom both paths. The user-visible improvement here comes from catching empty / malformed inputs locally before the request goes out and from rewording the generic credential message; the security-critical merge of "wrong-user" and "wrong-password" stays.Closes #455, #456.
Summary by CodeRabbit
New Features
Tests