sec(auth): collapse 'MFA not configured' into generic auth failure (closes #391)#830
Conversation
|
Warning Review limit reached
More reviews will be available in 3 minutes and 31 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
#391) When MFA is enabled but the secret is missing (data-integrity anomaly), returning a distinct "MFA is enabled but not configured" error reveals that the supplied password was correct. Collapse the branch to the same generic "Check your email address and password and try again" message used for wrong passwords, and log the anomaly internally (by user ID, no PII) so operators can detect it without relying on the API response. Remove the dead substring match from mapMFAServiceError in handler_auth.go. Add a sub-test that asserts both paths return an identical response body.
|
Rebased on
Verified post-rebase: @coderabbitai review |
|
🧠 Learnings used✅ Action performedReview finished.
|
Summary
"MFA is enabled but not configured"error into the same generic"Check your email address and password and try again"response used for wrong passwords, so a credential-stuffing attacker cannot distinguish "password correct, MFA data broken" from "wrong password"logging.Errorf(user ID only, no email/PII) so operators can detect the data-integrity anomaly without relying on the API responsemapMFAServiceErrorinhandler_auth.goTestLogin_WithMFA_NoSecretwith a sub-test that asserts both paths return the identical generic error bodyTest plan
go test github.com/LeanerCloud/CUDly/internal/auth/... -run TestLogin_WithMFA_NoSecret-- all 3 sub-tests passgo build ./...cleancurl/ integration testCloses #391