Django

Code

Ticket #7263 (closed: fixed)

Opened 7 months ago

Last modified 6 months ago

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

Reported by: Valera Grishin Assigned to: nobody
Milestone: 1.0 alpha Component: django.contrib.admin
Version: SVN Keywords: nfa-blocker
Cc: Triage Stage: Accepted
Has patch: 1 Needs documentation: 0
Needs tests: 0 Patch needs improvement: 0

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

secure_decorator.patch (1.0 kB) - added by Valera Grishin on 05/19/08 08:55:47.
patch

Change History

05/19/08 08:55:47 changed by Valera Grishin

  • attachment secure_decorator.patch added.

patch

05/19/08 09:32:40 changed by Valera Grishin

  • needs_better_patch changed.
  • needs_tests changed.
  • needs_docs changed.

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

05/19/08 09:44:28 changed by leotr

Yes i think that this would be the best solution

06/15/08 01:34:24 changed by russellm

  • keywords set to nfa-blocker.
  • 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.

06/16/08 10:51:25 changed by garcia_marc

  • milestone set to 1.0 alpha.

06/18/08 14:05:16 changed by brosner

  • status changed from new to closed.
  • resolution set to fixed.

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


Add/Change #7263 (Don't check that user has entered e-mail instead of username)




Change Properties
Action