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

Fix case sensitivity issue in username handling #1146

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

tdrivas
Copy link
Contributor

@tdrivas tdrivas commented Mar 10, 2025

This PR resolves an issue where Django Allauth was treating all usernames as lowercase during authentication, preventing users with capital letters in their usernames from logging in.

The problem
Usernames were correctly stored in the database with their original case (e.g., Bob), but authentication failed if users entered their names with uppercase letters.

Solution
Updated ACCOUNT_PRESERVE_USERNAME_CASING to True so to ensure usernames are not altered during authentication and are matched exactly as stored in the database.

Impact

  • Users can now login using the their, exact case, username as stored in the db.
  • No impact on existing stored usernames as only the authentication behavior has been adjusted.

Test new functionality (Manual tests)

  • Tested logging in with usernames having uppercase letters
  • Ensured that new users can sign up and log in as expected.
  • Verified password reset still works with case-sensitive usernames

@duke-nyuki
Copy link
Collaborator

@tdrivas tdrivas self-assigned this Mar 10, 2025
@tdrivas tdrivas requested review from suricactus and gounux March 10, 2025 05:07
Copy link
Collaborator

@suricactus suricactus left a comment

Choose a reason for hiding this comment

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

I am only afraid what will be the implications of such change. Let us discuss in the next meeting.

Btw, would you mind adding the docs in settings file since you are on it?

@@ -394,7 +394,7 @@ def before_send(event, hint):
# Choose one of "mandatory", "optional", or "none".
# For local development and test use "optional" or "none"
ACCOUNT_EMAIL_VERIFICATION = os.environ.get("ACCOUNT_EMAIL_VERIFICATION")
ACCOUNT_PRESERVE_USERNAME_CASING = False
ACCOUNT_PRESERVE_USERNAME_CASING = True
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
ACCOUNT_PRESERVE_USERNAME_CASING = True
# This setting determines whether the username is stored in lowercase (False) or whether its casing is to be preserved (True).
# Note that when casing is preserved, potentially expensive __iexact lookups are performed when filter on username.
# For now, the default is set to True to maintain backwards compatibility.
# See https://docs.allauth.org/en/dev/account/configuration.html
ACCOUNT_PRESERVE_USERNAME_CASING = True

@gounux
Copy link
Member

gounux commented Mar 11, 2025

Could test locally, auth with uppercases in the login username seems to work fine !

I am only afraid what will be the implications of such change

Feeling the same.

@suricactus
Copy link
Collaborator

@tdrivas let us know what tests you have performed to ensure no regressions are there.

@tdrivas
Copy link
Contributor Author

tdrivas commented Mar 11, 2025

@tdrivas let us know what tests you have performed to ensure no regressions are there.

Check updated description please!

@suricactus
Copy link
Collaborator

suricactus commented Mar 11, 2025

Test new functionality (Manual tests)
Tested logging in with usernames having uppercase letters
Ensured that new users can sign up and log in as expected.
Verified password reset still works with case-sensitive usernames

A few other manual tests on top of the ones you performed:

  • ensure usernames with lowercase letters can login
  • ensure that username "SURICACTUS" or similar can access their profile no matter if they are written in lowercase or uppercase in the login form for:
  • a) Django admin
  • b) app.qfiled.cloud login
  • ensure that after login, one can load host.com/a/SURICACTUS and host.com/a/suricactus with the same result.

I believe we can write a new test function similar to

with username with capitals.

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.

4 participants