-
Notifications
You must be signed in to change notification settings - Fork 425
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(auth): add password strength validation to admin.createUser #1964
base: master
Are you sure you want to change the base?
fix(auth): add password strength validation to admin.createUser #1964
Conversation
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.
@bharabhi01 thanks for contributing! i think we would also want this check to be present in adminUserUpdate
and generateLink
with the option to bypass the password strength check requirement by passing in an argument of some sort
some folks use the admin create endpoint to create users with arbitrary random passwords, then inviting them and asking them to update their password on first login
Got it! I'll make the required changes to bypass the password strength using an argument wherever necessary. |
The admin.createUser endpoint wasn't checking password strength against configured rules, while admin.updateUser was doing this validation. This creates a security gap where users created via the admin API could have weak passwords that don't meet the configured requirements. This commit adds the missing password strength validation to ensure consistent security across all user creation paths. Fixes supabase#1959
This commit adds the ability for admin users to bypass password strength validation when creating users, updating users, or generating signup links. This feature is useful for temporary account creation where passwords will be changed on first login. - Add BypassPasswordCheck field to AdminUserParams and GenerateLinkParams - Modify adminUserCreate, adminUserUpdate, and adminGenerateLink to respect this flag - Update validateSignupParams to accept bypass parameter - Add comprehensive tests for password bypass functionality This gives administrators more flexibility while maintaining security by restricting the bypass capability to authenticated admin endpoints only.
74f3c9b
to
62934ab
Compare
I think that we should add a parameter to include rather than exclude as this would be a BC break. CC @kangmingtay @hf |
Thanks for your comment, @cstockton. Are you suggesting that the default behavior should be to bypass password validation, not enforce it? In my current implementation:
Please confirm if this is not the expected behaviour, and I'll update the implementation to skip the password check by default and only do the check when some parameter (maybe like enforce_password_check) is set to true. |
My main concern is changing any behavior that we have today. Some users may rely on the check being skipped, for better or worst. Having an additional flag that is false by default to enforce it is a backwards compatible change, having that setting true by default is not. |
Thanks for the clarification. I will make the required changes. |
Add EnforcePasswordCheck parameter to Admin APIs to enforce password strength validation when: - Creating users via adminUserCreate - Updating users via adminUserUpdate - Generating signup links via adminGenerateLink - Validating signup params via validateSignupParams The bool EnforcePasswordCheck is by default false. Setting the bool to true will ensure the password strength validation
The admin.createUser endpoint wasn't checking password strength against configured rules, while admin.updateUser was doing this validation. This creates a security gap where users created via the admin API could have weak passwords that don't meet the configured requirements.
This commit adds the missing password strength validation to ensure consistent security across all user creation paths.
What kind of change does this PR introduce?
This is a security-related bug fix that closes a gap in password validation. It maintains backward compatibility for all valid use cases while enforcing proper security standards for passwords created through the admin API.
What is the current behavior?
Currently, there's an inconsistency in password validation within the Supabase Auth API:
This inconsistency creates a security vulnerability where:
As noted in issue #1959, this means that even if an application has strict password rules configured, an administrator can inadvertently create users with weak passwords that would otherwise be rejected if created through normal registration or updated later.
What is the new behavior?
Checklist for Submitting Pull Requests
Is there a corresponding issue created for it? If so, please include it in the PR description so we can track/refer to it.:
Fixes #1959
Does your PR follow the semantic-release commit guidelines?:
Yes
Are the existing tests passing?:
The full test suite (
make test
) shows failures, but these failures existed before my changes and are unrelated to the password validation functionality I've fixed. I've verified that my specific changes work by running the relevant tests in isolation.To verify my changes, I ran:
go test ./internal/api -run TestAdmin -v
which confirms the password validation is now working correctly.Have you written some tests for your PR?:
Yes