Opened 16 years ago

Closed 13 years ago

Last modified 12 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: dev
Severity: Keywords: nfa-blocker
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

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 16 years ago.
patch

Download all attachments as: .zip

Change History (9)

by Valera Grishin, 16 years ago

Attachment: secure_decorator.patch added

patch

comment:1 by Valera Grishin, 16 years ago

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

comment:2 by leotr, 16 years ago

Yes i think that this would be the best solution

comment:3 by Russell Keith-Magee, 16 years ago

Keywords: nfa-blocker added
Triage Stage: UnreviewedAccepted

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 by Marc Garcia, 16 years ago

milestone: 1.0 alpha

comment:5 by Brian Rosner, 16 years ago

Resolution: fixed
Status: newclosed

(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 by sirex, 13 years ago

Resolution: fixed
Status: closedreopened

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.

in reply to:  6 comment:7 by Ramiro Morales, 13 years ago

Resolution: fixed
Status: reopenedclosed

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 by Jacob, 12 years ago

milestone: 1.0 alpha

Milestone 1.0 alpha deleted

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