Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix blocked users being able to sign in using forgot password #10787

Merged
merged 2 commits into from
Aug 24, 2021

Conversation

derrickmehaffy
Copy link
Member

Signed-off-by: Derrick Mehaffy derrickmehaffy@gmail.com

What does it do?

Adds a check in the forgot password controller to see if a user is blocked and return the valid error message if they are

Why is it needed?

Blocked users should not be able to perform any request at all

How to test it?

Create a user, set them as blocked, and attempt to send a password reset request

Related issue(s)/PR(s)

fixes #10776

Signed-off-by: Derrick Mehaffy <derrickmehaffy@gmail.com>
@derrickmehaffy derrickmehaffy added the issue: bug Issue reporting a bug label Aug 23, 2021
@codecov
Copy link

codecov bot commented Aug 23, 2021

Codecov Report

Merging #10787 (1729565) into master (5202c4e) will not change coverage.
The diff coverage is n/a.

❗ Current head 1729565 differs from pull request most recent head d598cfa. Consider uploading reports for the commit d598cfa to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##           master   #10787   +/-   ##
=======================================
  Coverage   58.12%   58.12%           
=======================================
  Files         185      185           
  Lines        6429     6429           
  Branches     1395     1395           
=======================================
  Hits         3737     3737           
  Misses       2230     2230           
  Partials      462      462           
Flag Coverage Δ
front ∅ <ø> (∅)
unit 58.12% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5202c4e...d598cfa. Read the comment docs.

@@ -310,6 +310,11 @@ module.exports = {
);
}

// User blocked
if (user.blocked) {
return ctx.badRequest('blocked.user');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you use the same format as line 304? Thus the message can be directly displayed in the front!

(and if you can fix the missing "a" line 286 too :p)

Copy link
Contributor

@petersg83 petersg83 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM :)

@petersg83 petersg83 added this to the 3.6.8 milestone Aug 24, 2021
@petersg83 petersg83 added source: plugin:users-permissions Source is plugin/users-permissions package issue: security Issue reporting a security problem labels Aug 24, 2021
@alexandrebodin alexandrebodin merged commit 9648ac1 into master Aug 24, 2021
@derrickmehaffy derrickmehaffy deleted the fix/10776 branch August 25, 2021 18:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
issue: bug Issue reporting a bug issue: security Issue reporting a security problem source: plugin:users-permissions Source is plugin/users-permissions package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Blocked user can still login by using the forgot password feature
3 participants