Skip to content

security: normalize login error messages to prevent user enumeration (#6)#583

Merged
lakhansamani merged 2 commits intomainfrom
security/login-error-normalization
Apr 7, 2026
Merged

security: normalize login error messages to prevent user enumeration (#6)#583
lakhansamani merged 2 commits intomainfrom
security/login-error-normalization

Conversation

@lakhansamani
Copy link
Copy Markdown
Contributor

Summary

Distinct login error messages let attackers enumerate registered users (user not found vs wrong password vs email not verified vs wrong auth method). All authentication failures now return the same generic invalid credentials message; the specific reason is recorded at debug level.

Changes

  • internal/graphql/login.go — collapse all distinct failure paths to invalid credentials. Add a precomputed dummy bcrypt comparison on the user-not-found and other early-exit paths so request latency matches the real password-verification path.
  • internal/graphql/forgot_password.go — return generic success for unknown email and revoked accounts.
  • internal/graphql/resend_verify_email.go — return generic success for unknown user / missing verification request.
  • internal/graphql/magic_link_login.go — return generic success for revoked accounts.
  • Updated tests: login_test.go, forgot_password_test.go, metrics_test.go now assert on the new generic message and on the silent-success behaviour (the failure metric is still recorded internally).

Test plan

  • TEST_DBS=sqlite go test -run "TestLogin|TestForgotPassword|TestMagicLink|TestResendVerifyEmail|TestForgotPasswordMetrics" ./internal/integration_tests/

)

Distinct login error messages let attackers enumerate registered users:
- "user has not signed up email & password"
- "email not verified"
- "email verification pending"
- "user has not signed up with phone number & password"
- "phone number is not verified"
- "bad user credentials"
- "user not found" (forgot password, resend verify, magic link)
- "user access has been revoked"

All authentication failures now return the same generic "invalid credentials"
message to the client. The real reason is recorded in the debug log for ops
visibility via structured fields (reason="user_not_found", "bad_password",
"account_revoked", etc.).

Forgot password, resend verify email, and magic link login no longer reveal
whether the email exists — they return the same generic success response
in all cases.

Timing-attack defense: a precomputed dummy bcrypt hash is compared in the
user-not-found and other early-exit paths so total request time matches the
real authentication path. Without this, an attacker can distinguish "no
such user" from "wrong password" by latency alone.

Tests updated to assert on the new generic error message.
…typos (#6)

The previous generic-success messages ("Please check your inbox") were
identical between the success and silent-failure paths — good for
preventing enumeration — but gave the user no recourse when the email
never arrived because they had typed the address wrong.

Updated the message in all three handlers to a single sentence that:
1. Explicitly says the email was sent IF the account exists (so the
   user knows the response is conditional)
2. Tells them to check their inbox
3. Hints that they should double-check the address for typos if nothing
   arrives within a few minutes

The message is identical across the success path AND the silent-failure
paths in each handler, so enumeration is still impossible:

- forgot_password.go: user_not_found, account_revoked, and the success
  branch all return the same string
- resend_verify_email.go: user_not_found, verification_request_not_found,
  and the success branch all return the shared genericResponse
- magic_link_login.go: account_revoked branch and the trailing success
  branch share the same string
@lakhansamani lakhansamani merged commit c5c51db into main Apr 7, 2026
@lakhansamani lakhansamani deleted the security/login-error-normalization branch April 7, 2026 04:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant