fix(#265): use invalid_credentials translation key for auth failures#270
Conversation
|
Warning Review limit reached
More reviews will be available in 9 minutes and 46 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. 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 (13)
📝 WalkthroughWalkthroughLogin and password reset now use ChangesCWE-204 Fix: Unified Auth Error Messaging
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
app/translations/uk.json (1)
250-264: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winLocalize the new flash strings.
At Lines 251 and 264 these entries are still English, while the helper flashes the nested translation value directly. Ukrainian users will get mixed-language auth errors here.
🤖 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 `@app/translations/uk.json` around lines 250 - 264, The new auth flash strings in uk.json still contain English text, so Ukrainian users will see mixed-language messages. Update the two invalid_credentials entries in the translation object used by the auth/password reset flows to a proper Ukrainian equivalent, keeping the key names unchanged so the helper that reads the nested translation value continues to work.app/translations/zh.json (1)
249-263: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winLocalize the new flash strings.
At Lines 250 and 263 these new values are still English, and the flash helper renders them as-is. Chinese users will see English auth errors in the updated flows.
🤖 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 `@app/translations/zh.json` around lines 249 - 263, The new flash strings in the zh translation file are still hardcoded in English, so localize the remaining auth error values used by the flash helper. Update the untranslated entries in the translation JSON so the logout/password reset flows display Chinese text consistently, and keep the existing key names and surrounding structure intact.app/translations/ru.json (1)
250-264: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winLocalize the new flash strings.
At Lines 251 and 264 these new values are still English, and
flash_message()rendersflash.<page>.<message>directly. Russian users will now see English auth errors on those paths.🤖 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 `@app/translations/ru.json` around lines 250 - 264, The new flash message entries in ru.json still use English text for the auth errors, so update the translation values for the relevant keys under the login/password_reset sections to Russian. Make sure the strings returned by flash_message() for invalid_credentials (and any other newly added flash.<page>.<message> keys in this file) are fully localized, using the existing auth translation keys structure as the guide.app/translations/tr.json (1)
250-264: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winLocalize the new flash strings.
At Lines 251 and 264 these new values are still English, and the route helper flashes them verbatim. Turkish users will see English auth errors in the updated login and password-reset flows.
🤖 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 `@app/translations/tr.json` around lines 250 - 264, The new flash strings are still hardcoded in English for the Turkish locale, so update the translated values for the auth messages in the translation object and keep them consistent with the other Turkish entries. Specifically, replace the English text used for the shared invalid_credentials key in both the login and password_reset sections of tr.json so the route helper flashes localized Turkish messages instead of verbatim English.
🤖 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 `@app/routes/password_reset.py`:
- Around line 170-176: The password_reset flow still reveals account existence
by taking different outward paths in the user-not-found branch versus the
user-found branch. Update the password reset handler in password_reset.py so the
branch around the user lookup in the reset request step always returns the same
visible response (same flash/redirect/page outcome) regardless of whether a user
was found, and only conditionally performs the email send internally in the
user-handling logic. Use the existing password_reset route logic, the user
lookup check, flash_message, and mail-sending code to keep the response uniform
and prevent enumeration.
In `@app/translations/de.json`:
- Line 251: Localize the new auth flash strings in the German translation file:
the entries at the two referenced keys are still English literals, so replace
them with German text to match the rest of the locale. Keep the namespaced keys
used by app/routes/login.py and app/routes/password_reset.py unchanged, and
align the wording with the existing German invalid_credentials entry already
present later in this file.
In `@app/translations/es.json`:
- Line 251: The new auth flash messages in es.json are still left in English, so
Spanish users will see untranslated login and password-reset errors. Update the
locale entries for the auth keys used by the login and password-reset routes,
especially invalid_credentials and the other affected key around the
password-reset message, so they match the Spanish text already added elsewhere
in the file.
In `@app/translations/fr.json`:
- Line 251: The auth flash messages in the French translations are still using
English values outside the flash namespace, so the login and password-reset
routes won’t pick up localized text. Update the `invalid_credentials` entries in
`fr.json` under `flash.*` to use the existing French translation already present
in the file, and make sure both auth lookups resolve to those `flash` keys
consistently.
In `@app/translations/hi.json`:
- Line 251: The Hindi auth flash messages are still using English text for the
keys resolved by the login/reset flows, so update the localized values in the
translation entries referenced by the auth routes to use Hindi copy instead.
Check the relevant keys in the Hindi translation map, including the one for
invalid_credentials and the other auth message at the nearby key used by the
reset flow, and replace the English strings with the existing Hindi equivalents
already present in the file.
In `@app/translations/ja.json`:
- Line 251: The auth flash strings in the Japanese translation map are still in
English, so update the active lookup entries in ja.json for the failed
login/password-reset messages to use the localized Japanese text already defined
elsewhere. Keep the values for invalid_credentials and the related
password-reset key in sync with the existing Japanese translation entry so the
auth flow returns localized copy.
In `@app/translations/pl.json`:
- Line 251: The new flash.*.invalid_credentials strings in the Polish
translation file are still in English, so update the corresponding entries used
by the login and password-reset handlers to proper Polish text. Make sure the
keys at the locations referenced by the auth flows match the existing localized
wording already present elsewhere in the file, and keep the translation
consistent across both invalid-credentials messages.
In `@app/translations/pt.json`:
- Line 251: Update the flashed auth message values in pt.json so the auth routes
use the Portuguese strings instead of English. Replace the values for the
affected auth keys referenced by invalid_credentials and the reset/login message
entries used by the updated auth flows, and align them with the existing
localized Portuguese text already present in this translation file. Keep the
same keys and only change the message values so the flashed authentication
errors are fully localized for Portuguese users.
---
Outside diff comments:
In `@app/translations/ru.json`:
- Around line 250-264: The new flash message entries in ru.json still use
English text for the auth errors, so update the translation values for the
relevant keys under the login/password_reset sections to Russian. Make sure the
strings returned by flash_message() for invalid_credentials (and any other newly
added flash.<page>.<message> keys in this file) are fully localized, using the
existing auth translation keys structure as the guide.
In `@app/translations/tr.json`:
- Around line 250-264: The new flash strings are still hardcoded in English for
the Turkish locale, so update the translated values for the auth messages in the
translation object and keep them consistent with the other Turkish entries.
Specifically, replace the English text used for the shared invalid_credentials
key in both the login and password_reset sections of tr.json so the route helper
flashes localized Turkish messages instead of verbatim English.
In `@app/translations/uk.json`:
- Around line 250-264: The new auth flash strings in uk.json still contain
English text, so Ukrainian users will see mixed-language messages. Update the
two invalid_credentials entries in the translation object used by the
auth/password reset flows to a proper Ukrainian equivalent, keeping the key
names unchanged so the helper that reads the nested translation value continues
to work.
In `@app/translations/zh.json`:
- Around line 249-263: The new flash strings in the zh translation file are
still hardcoded in English, so localize the remaining auth error values used by
the flash helper. Update the untranslated entries in the translation JSON so the
logout/password reset flows display Chinese text consistently, and keep the
existing key names and surrounding structure intact.
🪄 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: 06c010a6-0251-4e6b-813f-3b0592d25c82
📒 Files selected for processing (14)
app/routes/login.pyapp/routes/password_reset.pyapp/translations/de.jsonapp/translations/en.jsonapp/translations/es.jsonapp/translations/fr.jsonapp/translations/hi.jsonapp/translations/ja.jsonapp/translations/pl.jsonapp/translations/pt.jsonapp/translations/ru.jsonapp/translations/tr.jsonapp/translations/uk.jsonapp/translations/zh.json
| Log.error(f'User: "{username}" with email: "{email}" not found') | ||
| flash_message( | ||
| page="password_reset", | ||
| message="wrong", | ||
| message="invalid_credentials", | ||
| category="error", | ||
| language=session.get("language", "en"), | ||
| ) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
This still leaks whether the username/email pair exists.
At Line 173 the error text is unified, but the if user: branch still sends mail, flashes success, and redirects while this branch returns an error on the same page. That response split still enables account enumeration in the password-reset request step, so the CWE-204 fix is incomplete. Return the same outward response for both branches and only gate the actual email send internally.
Suggested shape
if user:
context = ssl.create_default_context()
server = smtplib.SMTP(Settings.SMTP_SERVER, Settings.SMTP_PORT)
...
server.send_message(message)
server.quit()
Log.success(
f'Password reset code: "{password_reset_code}" sent to "{email}" for user: "{username}"'
)
- flash_message(
- page="password_reset",
- message="code",
- category="success",
- language=session.get("language", "en"),
- )
- return redirect("/password-reset/codesent=true")
else:
- Log.error(f'User: "{username}" with email: "{email}" not found')
- flash_message(
- page="password_reset",
- message="invalid_credentials",
- category="error",
- language=session.get("language", "en"),
- )
+ Log.error(f'Password reset requested for unknown credentials: "{username}" / "{email}"')
+
+ flash_message(
+ page="password_reset",
+ message="code",
+ category="success",
+ language=session.get("language", "en"),
+ )
+ return redirect("/password-reset/codesent=true")🤖 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 `@app/routes/password_reset.py` around lines 170 - 176, The password_reset flow
still reveals account existence by taking different outward paths in the
user-not-found branch versus the user-found branch. Update the password reset
handler in password_reset.py so the branch around the user lookup in the reset
request step always returns the same visible response (same flash/redirect/page
outcome) regardless of whether a user was found, and only conditionally performs
the email send internally in the user-handling logic. Use the existing
password_reset route logic, the user lookup check, flash_message, and
mail-sending code to keep the response uniform and prevent enumeration.
- app/routes/login.py: Replace 'password' with 'invalid_credentials' - app/routes/password_reset.py: Replace 'not_found' and 'wrong' with 'invalid_credentials' - All 13 translation files: Add 'invalid_credentials' key with localized text Closes #265
- flash.login.invalid_credentials - flash.password_reset.invalid_credentials - top-level invalid_credentials All 13 languages now use proper localized text instead of 'Invalid username or password'
DogukanUrker
force-pushed
the
fix/265-observable-response-discrepancy
branch
from
1b0940d to
2ec426f
Compare
1 participant
Fix for #265: CWE-204 - Observable response discrepancy
What Changed
Files Changed
Deliberately Left Untouched
Notes
py_compileverification.Closes #265
Summary by CodeRabbit