Ouranosinc / Magpie

AuthN/AuthZ services
https://pavics-magpie.readthedocs.io
Apache License 2.0
1 stars 5 forks source link

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

Closed mishaschwartz closed 9 months ago

mishaschwartz commented 10 months ago

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:

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 commented 10 months ago

Is the wrong UserService used somewhere? This implementation that overrides and extends by_user_name should be used instead: https://github.com/Ouranosinc/Magpie/blob/c6909c18aa1bd8c6ea548fead484edc404eb5b19/magpie/models.py#L434

fmigneault commented 10 months ago

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

mishaschwartz commented 10 months ago

@fmigneault I've added a PR to fix this here: https://github.com/Ouranosinc/Magpie/pull/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 commented 10 months ago

@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.