ux(auth): soften login error copy (closes #550)

[cristim]

Summary

• Replaces the terse "invalid email or password" with the friendlier "Check your email address and password and try again" across all 5 authentication failure paths in internal/auth/service.go (user-not-found, inactive, locked, empty-password-hash, wrong-password).
• Frontend mapServerLoginError in auth.ts updated to match and surface the new copy; the function still collapses both "authentication failed" and the new backend string into the same display message, preserving the OWASP non-enumeration invariant (closes ux(login): wrong-data error messages are generic ("authentication failed") and hide which field is wrong #456 regression risk noted in ux(auth): soften 'Incorrect email or password' copy without leaking enumeration signal #550).
• All existing backend and frontend tests updated to assert on the new string.

OWASP rationale (closing note on #550 suggestions)

QA suggestions #1, #3, and #4 in issue #550 (distinct per-scenario messages) were explicitly rejected because they would turn the login endpoint into an email-existence oracle (OWASP ASVS V3.3.4 / V14.2). This PR ships only the copy softening: semantics unchanged, no info leak, friendlier tone.

OWASP invariant regression guard

Added TestLogin_OWASPEnumerationInvariant in internal/auth/service_lockout_test.go -- a table-driven test that exercises all 5 failure modes and asserts each returns the same error string. Any future PR that accidentally re-introduces a path-specific message will fail this test.

Test plan

• go test ./internal/auth/... ./internal/api/... -- all pass (497 + 1151 tests)
• go vet ./... -- clean
• Frontend jest (auth, api-mfa suites) -- 76 tests pass
• npx tsc --noEmit -- no type errors
• git grep "invalid email or password" internal/ pkg/ frontend/src/ -- no hits

Summary by CodeRabbit

• Bug Fixes

  • Login error messages now show a single, consistent generic message for all authentication failures (including MFA and non-MFA cases), reducing account-enumeration risk.

• Tests

  • Updated tests to assert the standardized user-facing error text across multiple failure scenarios.
  • Added a comprehensive table-driven test covering various login failure modes to ensure invariant behavior.

[coderabbitai]

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 1eaa1917-5eea-4bf7-872d-01225688ac07

📥 Commits

Reviewing files that changed from the base of the PR and between ea6afe2 and dff8616.

📒 Files selected for processing (1)

• internal/auth/service_lockout_test.go

---

📝 Walkthrough

Walkthrough

Backend and frontend login failures now return the single message "Check your email address and password and try again" for all credential validation failures; tests updated and a new table-driven OWASP enumeration-invariant test ensures all failure paths produce the identical message.

Changes

Unified authentication error messaging

Layer / File(s)	Summary
Backend error message unification internal/auth/service.go	getUserAndValidateStatus and verifyPasswordAndMFA now return the unified user-facing message for all authentication failures (user not found, inactive, locked, missing password hash, wrong password).
Backend test expectations and enumeration invariant internal/auth/service_test.go, internal/auth/service_lockout_test.go	Updated tests expect the unified message; added TestLogin_OWASPEnumerationInvariant, a table-driven test asserting identical messages across user-not-found, inactive, locked, missing-password, and wrong-password scenarios.
Frontend error message mapping frontend/src/auth.ts	mapServerLoginError now maps backend strings like authentication failed and existing check your email address and password to the unified UI message; comment updated accordingly.
Frontend test assertions for error mapping frontend/src/__tests__/auth.test.ts, frontend/src/__tests__/api-mfa.test.ts	Frontend tests updated to assert the softened unified message mapping and to align MFA/non-MFA error expectations with the new text.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related issues

• ux(auth): soften 'Incorrect email or password' copy without leaking enumeration signal #550 — The issue implements the same UX change (softening the login message) across backend and frontend; strongly related to these edits.
• refactor(auth/mfa): promote substring-matched error messages in mapMFAServiceError to sentinel-typed errors #512 — Changes backend error strings and frontend substring matching, which may overlap with proposals to replace string checks with sentinel errors.

Possibly related PRs

• LeanerCloud/CUDly#551: Implements the same end-to-end login-error softening and tests, matching many of these code-level changes.
• LeanerCloud/CUDly#485: Also modifies frontend/src/auth.ts mapping and related frontend test assertions for login error messages.

Poem

🐰 I hopped through code both near and far,
One message now — no guessing who you are.
Tiny feet, one unified say,
"Check your email" — and hop away! 🥕

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	The PR addresses issue #550 but the objectives reference issue #456. While both concern login error messaging, the PR partially misaligns with #456's expectations: rather than field-specific messages (rejected for security), it softens generic copy, which does not solve the core problem of distinguishing wrong-email vs wrong-password scenarios.	Clarify alignment: confirm whether #550 reframes the security-tradeoff decision (e.g., accepting generic copy for OWASP compliance) or whether field-specific messaging on client-side is still expected per #456.
Docstring Coverage	⚠️ Warning	Docstring coverage is 62.50% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'ux(auth): soften login error copy (closes #550)' clearly and concisely describes the main change: softening UX error messages in authentication, with a direct reference to the issue.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to unifying login error messaging across five failure paths and updating corresponding tests; no unrelated changes detected.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/issue-550-login-copy-softening

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[cristim]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (2)

frontend/src/auth.ts (1)

653-657: ⚡ Quick win

Keep legacy backend error mapping for rollout compatibility.

If any older node still returns invalid email or password, users will see inconsistent copy. Map it to the same unified message.

♻️ Suggested tweak

   if (
     lower.includes('authentication failed') ||
+    lower.includes('invalid email or password') ||
     lower.includes('check your email address and password')
   ) {
     return 'Check your email address and password and try again';
   }

🤖 Prompt for 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.

In `@frontend/src/auth.ts` around lines 653 - 657, The current error-mapping block
checks for "authentication failed" and "check your email address and password"
on the lowercase error text (variable lower) and returns the unified message;
add an additional branch to treat the legacy backend string "invalid email or
password" the same way by checking lower.includes('invalid email or password')
and returning the identical unified message so older nodes produce the same
copy; update the same conditional (the if that references lower) to include this
legacy phrase.

internal/auth/service.go (1)

143-190: ⚡ Quick win

Centralize the generic login-failure copy into one constant.

This string is duplicated across multiple auth-failure branches. A future single-branch edit can accidentally break consistency.

♻️ Suggested refactor

 const (
+	GenericLoginFailureMessage = "Check your email address and password and try again"
 	// PasswordResetExpiry is how long password reset tokens are valid
 	PasswordResetExpiry = 1 * time.Hour
@@
 	if user == nil {
-		return nil, fmt.Errorf("Check your email address and password and try again")
+		return nil, fmt.Errorf(GenericLoginFailureMessage)
 	}
@@
 	if !user.Active {
-		return nil, fmt.Errorf("Check your email address and password and try again")
+		return nil, fmt.Errorf(GenericLoginFailureMessage)
 	}
@@
-		return nil, fmt.Errorf("Check your email address and password and try again")
+		return nil, fmt.Errorf(GenericLoginFailureMessage)
 	}
@@
 	if user.PasswordHash == "" {
-		return fmt.Errorf("Check your email address and password and try again")
+		return fmt.Errorf(GenericLoginFailureMessage)
 	}
@@
-		return fmt.Errorf("Check your email address and password and try again")
+		return fmt.Errorf(GenericLoginFailureMessage)
 	}

🤖 Prompt for 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.

In `@internal/auth/service.go` around lines 143 - 190, Centralize the repeated
generic login-failure string by adding a package-level constant (e.g.
GenericLoginFailureMsg = "Check your email address and password and try again")
and replace all literal occurrences that return fmt.Errorf(...) with
fmt.Errorf(GenericLoginFailureMsg); update uses in verifyPasswordAndMFA (the
PasswordHash and failed password branches) and the earlier auth checks that
return the same message so every auth-failure branch (including the
account-inactive and locked-account returns) uses the single named constant.

🤖 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 `@internal/auth/service_lockout_test.go`:
- Around line 418-437: Each subtest in the scenarios loop sets expectations on
the mocks (MockStore, MockEmailSender) but never asserts them; add assertion(s)
inside the t.Run closure after invoking service.Login to validate the expected
mock interactions. Specifically, call mockStore.AssertExpectations(t) and
mockEmail.AssertExpectations(t) (or a combined AssertExpectations on each mock)
immediately after the require/assert checks in the anonymous test function that
uses createTestService and service.Login so failures in expected calls are
reported per-subtest.

---

Nitpick comments:
In `@frontend/src/auth.ts`:
- Around line 653-657: The current error-mapping block checks for
"authentication failed" and "check your email address and password" on the
lowercase error text (variable lower) and returns the unified message; add an
additional branch to treat the legacy backend string "invalid email or password"
the same way by checking lower.includes('invalid email or password') and
returning the identical unified message so older nodes produce the same copy;
update the same conditional (the if that references lower) to include this
legacy phrase.

In `@internal/auth/service.go`:
- Around line 143-190: Centralize the repeated generic login-failure string by
adding a package-level constant (e.g. GenericLoginFailureMsg = "Check your email
address and password and try again") and replace all literal occurrences that
return fmt.Errorf(...) with fmt.Errorf(GenericLoginFailureMsg); update uses in
verifyPasswordAndMFA (the PasswordHash and failed password branches) and the
earlier auth checks that return the same message so every auth-failure branch
(including the account-inactive and locked-account returns) uses the single
named constant.

🪄 Autofix (Beta)

✅ Autofix completed

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4f570af8-072b-4b30-aa85-dbd24f85e7f3

📥 Commits

Reviewing files that changed from the base of the PR and between b876d7f and ea6afe2.

📒 Files selected for processing (6)

• frontend/src/__tests__/api-mfa.test.ts
• frontend/src/__tests__/auth.test.ts
• frontend/src/auth.ts
• internal/auth/service.go
• internal/auth/service_lockout_test.go
• internal/auth/service_test.go

[coderabbitai]

Note

Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it.

Fixes Applied Successfully

Fixed 1 file(s) based on 1 unresolved review comment.

Files modified:

• internal/auth/service_lockout_test.go

Commit: 789d8ff22ea06d0c956548cc0f5f218250267ad6

The changes have been pushed to the feat/issue-550-login-copy-softening branch.

Time taken: 4m 35s

[cristim]

Fixed gofmt indentation drift on the OWASP invariant test (commit pushed). Pre-commit hooks should now pass on the new run.

[cristim]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.