Opened 2 years ago

Last modified 19 months ago

#25617 assigned Cleanup/optimization

Disallow usernames that differ only in case in UserCreationForm

Reported by: Tim Graham Owned by: Neven Munđar
Component: contrib.auth Version: master
Severity: Normal Keywords:
Cc: zachborboa@…, nmundar@…, berker.peksag@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

Most applications don't treat user names as case sensitive. While we can't treat usernames as case-insensitive everywhere in Django due to backwards compatibility (#2273), by using username__iexact when checking for uniqueness of new usernames in UserCreationForm, we can at least prevent the creation of new usernames that differ only in case from an existing one. This protection won't cover creating a user in the shell or through the createsuperuser management command, but I don't think this is critical.

This wouldn't affect any usernames that already exist, and users will still need to login with the same case that they register with.

Attachments (1)

ticket_25617.patch (2.1 KB) - added by Neven Munđar 2 years ago.

Download all attachments as: .zip

Change History (12)

comment:1 Changed 2 years ago by Claude Paroz

Triage Stage: UnreviewedAccepted

comment:2 Changed 2 years ago by Zach Borboa

Cc: zachborboa@… added

comment:3 Changed 2 years ago by Neven Munđar

Cc: nmundar@… added
Owner: changed from nobody to Neven Munđar
Status: newassigned

Changed 2 years ago by Neven Munđar

Attachment: ticket_25617.patch added

comment:4 Changed 2 years ago by Neven Munđar

It's possible to achieve the effect described in this ticket by raising ValidationError in UserCreationForm.clean_username. However, this introduces one additional side-effect in tests of password validation logic. UserAttributeSimilarityValidator will not be able to check if username is similar to password because previously raised ValidationError will make username attribute None in password validator and "The password is too similar to the username." message will be missing from error list. Since the username in this case has to be changed anyway, omitting this message may not be relevant because password similarity check makes sense only on valid usernames. That's the explanation why auth_tests.test_forms.UserCreationFormTest.test_validates_password has to be tweaked in the patch.

comment:5 Changed 2 years ago by Neven Munđar

Has patch: set

comment:6 Changed 2 years ago by Tim Graham

Are you able to convert the patch into a pull request?

comment:8 Changed 2 years ago by Tim Graham

Patch needs improvement: set

There's a test failure.

comment:9 Changed 2 years ago by Neven Munđar

Patch needs improvement: unset

It's fixed now, stupid mistake. Previous form data that I've used didn't trigger password length validation.

comment:10 Changed 23 months ago by Tim Graham

Patch needs improvement: set

comment:11 Changed 19 months ago by Berker Peksag

Cc: berker.peksag@… added

Looking at https://github.com/django/django/pull/5572 again, I'd suggest the following API:

  • Adding a clean_username method would make UserCreationForm less subclass friendly. I'd suggest rename UserCreationForm to BaseUserCreationForm and document it as a preferred way to extend user creation form
  • Add a clean_username method to UserCreationForm (it will be a subclass of BaseUserCreationForm)

Thoughts?

Note: See TracTickets for help on using tickets.
Back to Top