Enforce password validation rules on 'Forgot your password?' form#9090
Merged
Enforce password validation rules on 'Forgot your password?' form#9090
Conversation
ahukkanen
requested changes
Apr 1, 2022
Contributor
ahukkanen
left a comment
There was a problem hiding this comment.
Few small things I noticed. Otherwise this is great and works fine!
Member
Author
|
Failing related specs, I need to check them |
Member
Author
|
This is ready to be reviewed again. Just a comment, the only place that I know for sure that we're not enforcing the password validator is in system. My idea is to handle that in other PR after this is merged. |
ahukkanen
reviewed
Apr 8, 2022
ahukkanen
approved these changes
Apr 8, 2022
This was referenced Apr 19, 2022
andreslucena
added a commit
that referenced
this pull request
May 6, 2022
) * Enforce password validation rules on 'Forgot your password?' form * Improve guard clause for organization * Use a not common password for user factory * Remove unecessary Devise password_length setting * Change default common password in seeds * Change default common password in specs * Change default common password in specs * Change domain in specs to example.org * Change default common password in specs * Fix organization guard clause as suggested in code review
ahukkanen
pushed a commit
that referenced
this pull request
May 10, 2022
) (#9245) * Enforce password validation rules on 'Forgot your password?' form * Improve guard clause for organization * Use a not common password for user factory * Remove unecessary Devise password_length setting * Change default common password in seeds * Change default common password in specs * Change default common password in specs * Change domain in specs to example.org * Change default common password in specs * Fix organization guard clause as suggested in code review
Merged
12 tasks
12 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🎩 What? Why?
Password validation rules didn't get enforced when a registered user reset their password through the "Forgot your password?" form (from Devise).
This PR fixes that.
I guess we could remove the validation from the forms:decidim/decidim-core/app/forms/decidim/registration_form.rb
Line 21 in 05b9883
decidim/decidim-core/app/forms/decidim/account_form.rb
Line 28 in 05b9883
But before changing that, I want to see the specs if it fails some flows that don't need passwords (like Invitations)(Never mind, we need that so the Forms validations work correctly. If not it gives an error without any detail on what exactly is the error)
📌 Related Issues
Testing