Repository navigation
fix(auth): TestLogin_WithMFA_NoSecret fails on base -- #830/#887 message collision leaks MFA-not-configured vs wrong-password #939
Description
Summary
TestLogin_WithMFA_NoSecret/wrong_password_returns_same_generic_error fails on feat/multicloud-web-frontend (the shared base branch), independent of any feature PR. Reproduced on a pristine detached checkout of origin/feat/multicloud-web-frontend:
[FAIL] TestLogin_WithMFA_NoSecret/wrong_password_returns_same_generic_error
internal/auth/service_test.go:508:
Error: "invalid email or password" does not contain "Check your email address and password and try again"
[FAIL] TestLogin_WithMFA_NoSecret
Root cause
This is a collision between two merged PRs on the base:
- sec(auth): collapse 'MFA not configured' into generic auth failure (closes #391) #830 (
sec(auth): collapse MFA-not-configured into generic auth failure, commit878046eae) addedTestLogin_WithMFA_NoSecret. Itswrong password returns same generic errorsubtest asserts that a wrong password against an MFA-enabled-but-secret-missing account returns the same generic message as the missing-secret path:"Check your email address and password and try again", so the two cases are indistinguishable to the caller. - sec(auth): unify login error response for unknown vs wrong-password (closes #416) #887 (
sec(auth): unify login error response for unknown vs wrong-password, commite98822480) made the wrong-password branch inverifyPasswordAndMFAreturn"invalid email or password"instead.
As a result the wrong-password path now returns "invalid email or password" while the missing-secret path returns "Check your email address and password and try again". The two messages differ, so:
- The sec(auth): collapse 'MFA not configured' into generic auth failure (closes #391) #830 test now fails on base.
- More importantly, the original anti-enumeration intent of sec(auth): collapse 'MFA not configured' into generic auth failure (closes #391) #830 is partially regressed for this specific account state: a caller can distinguish "MFA enabled but secret missing" (correct password) from "wrong password" by the differing message text.
Decision needed
Pick one consistent message for both branches of verifyPasswordAndMFA (and align the test):
- Option 1: make the missing-secret branch also return
"invalid email or password"(align with sec(auth): unify login error response for unknown vs wrong-password (closes #416) #887's unified message). Simplest; the missing-secret data anomaly is still logged internally vialogging.Errorf. - Option 2: make the wrong-password branch return
"Check your email address and password and try again"(align with sec(auth): collapse 'MFA not configured' into generic auth failure (closes #391) #830's message), keeping sec(auth): collapse 'MFA not configured' into generic auth failure (closes #391) #830's test as-is. This would touch sec(auth): unify login error response for unknown vs wrong-password (closes #416) #887's unification choice, so confirm with sec(auth): unify login error response for unknown vs wrong-password (closes #416) #887's owner.
Either way, update TestLogin_WithMFA_NoSecret so both subtests assert the single chosen message.
Scope note
Discovered while rebasing PR #883 (refactor(auth/mfa): use sentinel errors in mapMFAServiceError) onto the advanced base. #883 does not touch verifyPasswordAndMFA's wrong-password or missing-secret messages (its Option-A reconciliation deliberately keeps #830's behavior verbatim), so this base failure is out of scope for #883 and is filed separately rather than fixed there. The 2 failures will appear on #883's CI and on any other PR built on the current base until this is resolved.
Activity
cristim commentedon Jul 27, 2026
Verification sweep against
main(101f099fb) on 2026-07-27 finds this already resolved.The wrong-password and missing-MFA-secret paths now return the identical generic error.
Evidence: commit e31ed68 (#1107). Same root fix as #388.
Recommending close.
cristim commentedon Sep 2, 2026
Verified resolved at 3c0f8ac: the wrong-password and MFA-secret-missing branches of verifyPasswordAndMFA return the identical generic message, and TestLogin_WithMFA_NoSecret passes (go test ./internal/auth -run TestLogin_WithMFA_NoSecret: ok). Evidence: internal/auth/service.go:233 and :246 (#1107). Residual axes checked: missing-password-hash branch (:229, same message), handler-layer collapse to 401 'invalid credentials' (internal/api/handler_auth.go:58); none found. Closing as completed; reopen if the behaviour recurs.