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

feat: improved user creation validation #524

Merged
merged 1 commit into from
Oct 7, 2022
Merged

Conversation

jmorel
Copy link
Contributor

@jmorel jmorel commented Oct 7, 2022

Signed-off-by: Jérémy Morel jeremy.morel@owkin.com

Description

  • Adds validation for username so that the actual creation doesn't fail with an IntegrityError
  • Adds validation on empty password before using the configured validators. This is necessary because the zxcvbn validator will raise a list index out of range exception on an empty password string. See IndexError: list index out of range, on empty password dwolfhub/zxcvbn-python#64
  • Decodes the url encoded kwargs.get('username') so that the username is properly handled (John Doe is a valid username).

How has this been tested?

Manually.

Checklist

  • changelog was updated with notable changes
  • documentation was updated

sergebouchut2
sergebouchut2 previously approved these changes Oct 7, 2022
@jmorel jmorel dismissed sergebouchut2’s stale review October 7, 2022 09:43

I added to this PR decoding of the "username" part of the URL when editing users.

Copy link
Contributor

@sergebouchut2 sergebouchut2 left a comment

Choose a reason for hiding this comment

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

It would be really nice to add some unit tests and prevent from future regression.

@cboillet-dev
Copy link
Contributor

looking good to me but it would be worse adding a few validation tests in backend/users/tests/test_user.py

Signed-off-by: Jérémy Morel <jeremy.morel@owkin.com>
@jmorel
Copy link
Contributor Author

jmorel commented Oct 7, 2022

By popular demand (wink wink @cboillet-dev @sergebouchut2), let there be tests!

@jmorel jmorel merged commit 233c112 into main Oct 7, 2022
@jmorel jmorel deleted the feat/password_validation branch October 7, 2022 12:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants