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

[BUG] login confuses usernames that differ in terms of upper/lower case #595

Closed
mishaschwartz opened this issue Nov 8, 2023 · 4 comments · Fixed by #597
Closed

[BUG] login confuses usernames that differ in terms of upper/lower case #595

mishaschwartz opened this issue Nov 8, 2023 · 4 comments · Fixed by #597
Assignees
Labels
bug Problem, error, or invalid behaviour

Comments

@mishaschwartz
Copy link
Collaborator

mishaschwartz commented Nov 8, 2023

Describe the bug

Ziggurat foundation converts usernames to lowercase before looking them up in the database: https://github.com/ergo/ziggurat_foundations/blob/0.9.1/ziggurat_foundations/models/services/user.py#L326-L328

This means:

  • if a user named "Test" exists, logging in with any of "test", "TEST", "tEst", etc. will all log in as the "Test" user
  • if users named "test" and "Test" exist, both users will be logged in as the "test" user ("Test" will never be able to log in)

To Reproduce
Steps to reproduce the behavior:

  1. Create a user "Test"
  2. Log in with the username "test" (result: login success as the user "Test")
  3. Create a user "test" with a different password than "Test"
  4. Log in with the username "Test" (result: login failure)

Expected behavior

Users whose usernames differ in terms of capitalization only, should be treated as distinct users in all parts of the application, including logins.

@fmigneault
Copy link
Collaborator

Is the wrong UserService used somewhere?
This implementation that overrides and extends by_user_name should be used instead:

class UserSearchService(UserService):

@fmigneault
Copy link
Collaborator

Oh, never mind. I just saw the super(UserSearchService, cls).by_user_name(...) call.

@mishaschwartz
Copy link
Collaborator Author

@fmigneault I've added a PR to fix this here: #596

I don't know if you saw it buy I'm not a member or this github organization so I can't request a reviewer

@tlvu
Copy link
Contributor

tlvu commented Nov 9, 2023

@fmigneault I've added a PR to fix this here: #596

I don't know if you saw it buy I'm not a member or this github organization so I can't request a reviewer

@mishaschwartz You have write access to this repo now. For future PR, you can push directly to this repo, no need to use your fork anymore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Problem, error, or invalid behaviour
Projects
None yet
3 participants