-
Notifications
You must be signed in to change notification settings - Fork 384
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: check password max length in checkPasswordStrength #1659
Conversation
3a680df
to
57a90ab
Compare
Pull Request Test Coverage Report for Build 9986796684Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think one concern in the past was that adminCreateUser doesn't do a password check
I think it relies on models.NewUser
to call GenerateFromPassword
which then performs the check.
Let's ensure the validation check on password length for adminUserCreate
is preserved before moving forward.
Otherwise looks good to me
Ah my bad - tested and I realize bcrypt will still throw an error on this line |
@J0 that's a good point, the admin create user error will return an error if the password length exceeds the max length but the error won't be obvious and is returned as a 500 i've updated the PR to wrap the error and return a bad request error instead |
🤖 I have created a release *beep* *boop* --- ## [2.155.5](v2.155.4...v2.155.5) (2024-07-19) ### Bug Fixes * check password max length in checkPasswordStrength ([#1659](#1659)) ([1858c93](1858c93)) * don't update attribute mapping if nil ([#1665](#1665)) ([7e67f3e](7e67f3e)) * refactor mfa models and add observability to loadFactor ([#1669](#1669)) ([822fb93](822fb93)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
## What kind of change does this PR introduce? * Check if password exceeds max length in `checkPasswordStrength` ## What is the current behavior? Please link any relevant issues here. ## What is the new behavior? Feel free to include screenshots if it includes visual changes. ## Additional context Add any other context or screenshots.
🤖 I have created a release *beep* *boop* --- ## [2.155.5](supabase/auth@v2.155.4...v2.155.5) (2024-07-19) ### Bug Fixes * check password max length in checkPasswordStrength ([supabase#1659](supabase#1659)) ([1858c93](supabase@1858c93)) * don't update attribute mapping if nil ([supabase#1665](supabase#1665)) ([7e67f3e](supabase@7e67f3e)) * refactor mfa models and add observability to loadFactor ([supabase#1669](supabase#1669)) ([822fb93](supabase@822fb93)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
## What kind of change does this PR introduce? * Check if password exceeds max length in `checkPasswordStrength` ## What is the current behavior? Please link any relevant issues here. ## What is the new behavior? Feel free to include screenshots if it includes visual changes. ## Additional context Add any other context or screenshots.
🤖 I have created a release *beep* *boop* --- ## [2.155.5](supabase/auth@v2.155.4...v2.155.5) (2024-07-19) ### Bug Fixes * check password max length in checkPasswordStrength ([supabase#1659](supabase#1659)) ([1858c93](supabase@1858c93)) * don't update attribute mapping if nil ([supabase#1665](supabase#1665)) ([7e67f3e](supabase@7e67f3e)) * refactor mfa models and add observability to loadFactor ([supabase#1669](supabase#1669)) ([822fb93](supabase@822fb93)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
## What kind of change does this PR introduce? * Check if password exceeds max length in `checkPasswordStrength` ## What is the current behavior? Please link any relevant issues here. ## What is the new behavior? Feel free to include screenshots if it includes visual changes. ## Additional context Add any other context or screenshots.
🤖 I have created a release *beep* *boop* --- ## [2.155.5](supabase/auth@v2.155.4...v2.155.5) (2024-07-19) ### Bug Fixes * check password max length in checkPasswordStrength ([supabase#1659](supabase#1659)) ([1858c93](supabase@1858c93)) * don't update attribute mapping if nil ([supabase#1665](supabase#1665)) ([7e67f3e](supabase@7e67f3e)) * refactor mfa models and add observability to loadFactor ([supabase#1669](supabase#1669)) ([822fb93](supabase@822fb93)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
What kind of change does this PR introduce?
checkPasswordStrength
What is the current behavior?
Please link any relevant issues here.
What is the new behavior?
Feel free to include screenshots if it includes visual changes.
Additional context
Add any other context or screenshots.