Add Toast Messages for Bad Logins #1874
Conversation
rosenjcb
requested review from
dessalines,
SleeplessOne1917,
alectrocute and
jsit
as code owners
src/shared/components/home/login.tsx
Outdated
| if (loginRes.msg === "couldnt_find_that_username_or_email") { | ||
| toast( | ||
| I18NextService.i18n.t("couldnt_find_that_username_or_email"), | ||
| "danger" | ||
| ); | ||
| } | ||
| if (loginRes.msg === "password_incorrect") { | ||
| toast(I18NextService.i18n.t("password_incorrect"), "danger"); | ||
| } |
There was a problem hiding this comment.
Should we be displaying different error messages for these? I would think letting potential attackers know which credential they got wrong is ill advised.
There was a problem hiding this comment.
It could be an attack vector - especially if there's no retry limit on failed password attempts. I can't seem to find a good generic error message that's already provided in lemmy-translations. Do you have a suggestion?
There was a problem hiding this comment.
Not off the top of my head. However, worth pointing out is that even if we change the toast on the frontend, the response from the server still has distinct message for each case, so there will need to be a backend change as well.
There was a problem hiding this comment.
If the backend response login_res is already spilling the beans, then it doesn’t matter much whether we use unique error toasters or not. The attack vector will still be present.
There was a problem hiding this comment.
Ah, I see you already mentioned that @SleeplessOne1917. I need to read better before commenting.
There was a problem hiding this comment.
@SleeplessOne1917 This is what I can do.
1.) Make a PR on lemmy-translations to add a new entry {"incorrect_login": "Incorrect login credentials"}
2.) Make a PR on lemmy to swap out the API response with this new generic entry instead of couldnt_find_that_username_or_email and password_correct.
3.) Update this PR to reflect the changes once they've been merged.
Would that work?
There was a problem hiding this comment.
Great I have LemmyNet/lemmy#3549 and LemmyNet/lemmy-translations#83 set up. The code change isn't covered by tests though, so it's probably safe for someone to check out the change and run it locally before merging.
There was a problem hiding this comment.
Thank you for the attention to security on this
There was a problem hiding this comment.
The two PRs above were approved and merged. I went ahead and swapped the condition check + toast response to use the generic incorrect_login string. Thanks for the feedback and support!
Handle errors from sharp Revert "Handle errors from sharp" This reverts commit 70f6268. Handle any sharp errors gracefully Add Toast Messages for Bad Logins (LemmyNet#1874) * Added toast prompts * Using generic message instead. --------- Co-authored-by: Jacob Rosenzweig <rosenjcb@Jacobs-MacBook-Pro.local> Co-authored-by: SleeplessOne1917 <abias1122@gmail.com> Fix CSP in dev mode (LemmyNet#1918) Remove invalid default option from language list (LemmyNet#1919) Set person_id to myId in handleLeaveModTeam (LemmyNet#1929)
* Added toast prompts * Using generic message instead. --------- Co-authored-by: Jacob Rosenzweig <rosenjcb@Jacobs-MacBook-Pro.local> Co-authored-by: SleeplessOne1917 <abias1122@gmail.com>
Handle errors from sharp Revert "Handle errors from sharp" This reverts commit 70f6268. Handle any sharp errors gracefully Add Toast Messages for Bad Logins (LemmyNet#1874) * Added toast prompts * Using generic message instead. --------- Co-authored-by: Jacob Rosenzweig <rosenjcb@Jacobs-MacBook-Pro.local> Co-authored-by: SleeplessOne1917 <abias1122@gmail.com> Fix CSP in dev mode (LemmyNet#1918) Remove invalid default option from language list (LemmyNet#1919) Set person_id to myId in handleLeaveModTeam (LemmyNet#1929)
Handle errors from sharp Revert "Handle errors from sharp" This reverts commit 70f6268. Handle any sharp errors gracefully Add Toast Messages for Bad Logins (LemmyNet#1874) * Added toast prompts * Using generic message instead. --------- Co-authored-by: Jacob Rosenzweig <rosenjcb@Jacobs-MacBook-Pro.local> Co-authored-by: SleeplessOne1917 <abias1122@gmail.com> Fix CSP in dev mode (LemmyNet#1918) Remove invalid default option from language list (LemmyNet#1919) Set person_id to myId in handleLeaveModTeam (LemmyNet#1929)
Description
Right now if you try to login and you provide an unknown username/email or a wrong password, you will get no toast message. This PR adds toast messages for those scenarios and uses the messages already found in
lemmy-translations.Screenshots