Opened 7 years ago

Closed 5 years ago

Last modified 4 years ago

#7263 closed (fixed)

Don't check that user has entered e-mail instead of username

Reported by: Valera Grishin Owned by: nobody
Component: contrib.admin Version: master
Severity: Keywords: nfa-blocker
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

Current django/trunk/contrib/admin/views/decorators.py has function staff_member_required which checks whether user supplied an e-mail address instead of username in the login form.

If so and e-mail is found in the user database it will suggest (via error message) the user to use username instead. The error message will also show the username found by given e-mail.

Here is the code in the staff_member_required function:

        if user is None:
            message = ERROR_MESSAGE
            if '@' in username:
                # Mistakenly entered e-mail address instead of username? Look it up.
                users = list(User.objects.filter(email=username))
                if len(users) == 1:
                    message = _("Your e-mail address is not your username. Try '%s' instead.") % users[0].username
                else:
                    # Either we cannot find the user, or if more than 1 
                    # we cannot guess which user is the correct one.
                    message = _("Usernames cannot contain the '@' character.")
            return _display_login_form(request, message)

This feature actually works as lookup and partially uncovers sensitive information. It can be used to:

  • check whether certain e-mail exists (at all)
  • check whether certain e-mail is registered (on certain site)
  • find username by e-mail

In order to solve this issue the logic can be simplified as follows:

        if user is None:
            message = ERROR_MESSAGE
            if '@' in username:
                message = _("Usernames cannot contain the '@' character.")
            return _display_login_form(request, message)

Corresponding patch is attached.

Attachments (1)

secure_decorator.patch (1.0 KB) - added by Valera Grishin 7 years ago.
patch

Download all attachments as: .zip

Change History (9)

Changed 7 years ago by Valera Grishin

patch

comment:1 Changed 7 years ago by Valera Grishin

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

There is also related #7259 which discusses another aspect of the same code.

comment:2 Changed 7 years ago by leotr

Yes i think that this would be the best solution

comment:3 Changed 7 years ago by russellm

  • Keywords nfa-blocker added
  • Triage Stage changed from Unreviewed to Accepted

This is a good idea; however, we have halted work on admin in favor of newforms-admin. Marking ticket for inclusion in newforms-admin.

comment:4 Changed 7 years ago by garcia_marc

  • milestone set to 1.0 alpha

comment:5 Changed 7 years ago by brosner

  • Resolution set to fixed
  • Status changed from new to closed

(In [7694]) newforms-admin: Fixed #6943 and #7263 -- Handle multiple e-mail addresses when checking if it was mistakenly entered. Also prevent e-mail guessing by checking password before throwing an error. Thanks Michael Newman and Valera Grishin.

comment:6 follow-up: Changed 5 years ago by sirex

  • Resolution fixed deleted
  • Status changed from closed to reopened

Still this security issue is not fixed.

I have my custom authentication backend, which stores email-like username in different table, and in auth.models.User table, username is something like: <id>@<authmethod>.

Every thing works ok, my custom authentication backend prevents users from logging in through auth.models.User username field, requiring user's email address as username. But when I try to log-in through Django admin login form, it shows my generated username, which should not be visible to anyone:

Your e-mail address is not your username. Try '1@email' instead.

If there is possibility to customize authentication, so why admin login form hardcodes that all usernames should not be emails? I suggest that this checking should be removed at all.

comment:7 in reply to: ↑ 6 Changed 5 years ago by ramiro

  • Resolution set to fixed
  • Status changed from reopened to closed

Replying to sirex:

If there is possibility to customize authentication, so why admin login form hardcodes that all usernames should not be emails? I suggest that this checking should be removed at all.

Work in this front is being done now in #8342 that is targeted for Django 1.3.

comment:8 Changed 4 years ago by jacob

  • milestone 1.0 alpha deleted

Milestone 1.0 alpha deleted

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