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

Custom user models (#3011) #370

Closed
wants to merge 41 commits into from
Closed
Show file tree
Hide file tree
Changes from 34 commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
7cc0baf
Added model Meta option for swappable models, and made auth.User a sw…
freakboy3742 Jun 4, 2012
dabe362
Modified auth management commands to handle custom user definitions.
freakboy3742 Jun 4, 2012
507bb50
Modified auth app so that login with alternate auth app is possible.
freakboy3742 Jun 4, 2012
8e3fd70
Merged recent changes from trunk.
freakboy3742 Aug 20, 2012
e29c010
Merge remote-tracking branch 'django/master' into t3011
freakboy3742 Aug 20, 2012
7e82e83
Merged master changes.
freakboy3742 Sep 7, 2012
d088b3a
Admin app login form should use swapped user model
thsutton Aug 20, 2012
75118bd
Admin app should not allow username discovery
thsutton Aug 20, 2012
e6aaf65
Added first draft of custom User docs.
freakboy3742 Sep 8, 2012
40ea8b8
Added documentation for REQUIRED_FIELDS in custom auth.
freakboy3742 Sep 8, 2012
20d1892
Added conditional skips for all tests dependent on the default User m…
freakboy3742 Sep 8, 2012
2c5e833
Corrected admin_views tests following removal of the email fallback o…
freakboy3742 Sep 8, 2012
1952656
Merge recent changes from master.
freakboy3742 Sep 9, 2012
f2ec915
Merge remote-tracking branch 'django/master' into t3011
freakboy3742 Sep 9, 2012
57ac6e3
Merge remote-tracking branch 'django/master' into t3011
freakboy3742 Sep 15, 2012
5d7bb22
Ensure swapped models can't be queried.
freakboy3742 Sep 15, 2012
334cdfc
Added release notes for new swappable User feature.
freakboy3742 Sep 15, 2012
9184972
Deprecate AUTH_PROFILE_MODULE and get_profile().
freakboy3742 Sep 15, 2012
579f152
Merge remote-tracking branch 'django/master' into t3011
freakboy3742 Sep 15, 2012
d9f5e5a
Reworked REQUIRED_FIELDS + create_user() interaction
akaariai Sep 15, 2012
08bcb4a
Splitted User to AbstractUser and User
akaariai Sep 15, 2012
b441a6b
Added note about backwards incompatible change to admin login messages.
freakboy3742 Sep 16, 2012
52a02f1
Refactored common 'get' pattern into manager method.
freakboy3742 Sep 16, 2012
b550a6d
Refactored skipIfCustomUser into the contrib.auth tests.
freakboy3742 Sep 16, 2012
fd8bb4e
Documentation improvements coming from community review.
freakboy3742 Sep 16, 2012
a9491a8
Merge commit '08bcb4aec1ed154cefc631b8510ee13e9af0c19d' into t3011
freakboy3742 Sep 16, 2012
abcb027
Cleanup and documentation of AbstractUser base class.
freakboy3742 Sep 16, 2012
dfd7213
Added protection against proxying swapped models.
freakboy3742 Sep 16, 2012
dbb3900
Fixes for Python 3 compatibility.
freakboy3742 Sep 16, 2012
280bf19
Merge remote-tracking branch 'django/master' into t3011
freakboy3742 Sep 16, 2012
913e1ac
Added test for proxy model safeguards on swappable models.
freakboy3742 Sep 16, 2012
ffd535e
Corrected attribute access on for get_by_natural_key
freakboy3742 Sep 16, 2012
5a04cde
Removed some unused imports.
freakboy3742 Sep 16, 2012
6494bf9
Improved validation of swappable model settings.
freakboy3742 Sep 17, 2012
0229209
Merged recent Django trunk changes.
freakboy3742 Sep 23, 2012
98aba85
Improved error handling and docs for get_user_model()
freakboy3742 Sep 23, 2012
e2b6e22
Modifications to the handling and docs for auth forms.
freakboy3742 Sep 23, 2012
8a527dd
Ensure sequences are reset correctly in the presence of swapped models.
freakboy3742 Sep 23, 2012
29d1abb
Merge remote-tracking branch 'django/master' into t3011
freakboy3742 Sep 23, 2012
531e771
Merged recent trunk changes.
freakboy3742 Sep 25, 2012
d84749a
Merge remote-tracking branch 'django/master' into t3011
freakboy3742 Sep 26, 2012
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions django/conf/global_settings.py
Expand Up @@ -488,6 +488,8 @@
# AUTHENTICATION #
##################

AUTH_USER_MODEL = 'auth.User'

AUTHENTICATION_BACKENDS = ('django.contrib.auth.backends.ModelBackend',)

LOGIN_URL = '/accounts/login/'
Expand Down
15 changes: 2 additions & 13 deletions django/contrib/admin/forms.py
Expand Up @@ -4,12 +4,12 @@

from django.contrib.auth import authenticate
from django.contrib.auth.forms import AuthenticationForm
from django.contrib.auth.models import User
from django.utils.translation import ugettext_lazy, ugettext as _
from django.utils.translation import ugettext_lazy

ERROR_MESSAGE = ugettext_lazy("Please enter the correct username and password "
"for a staff account. Note that both fields are case-sensitive.")


class AdminAuthenticationForm(AuthenticationForm):
"""
A custom authentication form used in the admin app.
Expand All @@ -26,17 +26,6 @@ def clean(self):
if username and password:
self.user_cache = authenticate(username=username, password=password)
if self.user_cache is None:
if '@' in username:
# Mistakenly entered e-mail address instead of username? Look it up.
try:
user = User.objects.get(email=username)
except (User.DoesNotExist, User.MultipleObjectsReturned):
# Nothing to do here, moving along.
pass
else:
if user.check_password(password):
message = _("Your e-mail address is not your username."
" Try '%s' instead.") % user.username
raise forms.ValidationError(message)
elif not self.user_cache.is_active or not self.user_cache.is_staff:
raise forms.ValidationError(message)
Expand Down
6 changes: 4 additions & 2 deletions django/contrib/admin/models.py
@@ -1,8 +1,8 @@
from __future__ import unicode_literals

from django.db import models
from django.conf import settings
from django.contrib.contenttypes.models import ContentType
from django.contrib.auth.models import User
from django.contrib.admin.util import quote
from django.utils.translation import ugettext_lazy as _
from django.utils.encoding import smart_text
Expand All @@ -12,15 +12,17 @@
CHANGE = 2
DELETION = 3


class LogEntryManager(models.Manager):
def log_action(self, user_id, content_type_id, object_id, object_repr, action_flag, change_message=''):
e = self.model(None, None, user_id, content_type_id, smart_text(object_id), object_repr[:200], action_flag, change_message)
e.save()


@python_2_unicode_compatible
class LogEntry(models.Model):
action_time = models.DateTimeField(_('action time'), auto_now=True)
user = models.ForeignKey(User)
user = models.ForeignKey(settings.AUTH_USER_MODEL)
Copy link
Member

Choose a reason for hiding this comment

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

Would it not be more future proof to have the FK set to get_user_model() ?

The disadvantage is it is somewhat less direct - but would allow more flexibility in future refactors of how the swapped model might be specified (hint hint, wink wink app-loading ;-)

Copy link
Member

Choose a reason for hiding this comment

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

This is deliberate -- otherwise we have a 'chicken and egg' situation. In order for get_user_model() to run, you need to have the app cache parsed; but the app cache can't be fully populated until all the models are loaded.

Copy link
Member

Choose a reason for hiding this comment

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

ah yes - I forgot just how much I had changed to actually cook that particular chicken and scramble that particular egg in the app-loading branch. Looks like promoting the convention of explicitly referring to the setting is unavoidable for now then.

content_type = models.ForeignKey(ContentType, blank=True, null=True)
object_id = models.TextField(_('object id'), blank=True, null=True)
object_repr = models.CharField(_('object repr'), max_length=200)
Expand Down
34 changes: 20 additions & 14 deletions django/contrib/admin/sites.py
Expand Up @@ -9,7 +9,6 @@
from django.core.exceptions import ImproperlyConfigured
from django.core.urlresolvers import reverse, NoReverseMatch
from django.template.response import TemplateResponse
from django.utils.safestring import mark_safe
from django.utils import six
from django.utils.text import capfirst
from django.utils.translation import ugettext as _
Expand All @@ -18,12 +17,15 @@

LOGIN_FORM_KEY = 'this_is_the_login_form'


class AlreadyRegistered(Exception):
pass


class NotRegistered(Exception):
pass


class AdminSite(object):
"""
An AdminSite object encapsulates an instance of the Django admin application, ready
Expand All @@ -41,7 +43,7 @@ class AdminSite(object):
password_change_done_template = None

def __init__(self, name='admin', app_name='admin'):
self._registry = {} # model_class class -> admin_class instance
self._registry = {} # model_class class -> admin_class instance
self.name = name
self.app_name = app_name
self._actions = {'delete_selected': actions.delete_selected}
Expand Down Expand Up @@ -80,20 +82,23 @@ def register(self, model_or_iterable, admin_class=None, **options):
if model in self._registry:
raise AlreadyRegistered('The model %s is already registered' % model.__name__)

# If we got **options then dynamically construct a subclass of
# admin_class with those **options.
if options:
# For reasons I don't quite understand, without a __module__
# the created class appears to "live" in the wrong place,
# which causes issues later on.
options['__module__'] = __name__
admin_class = type("%sAdmin" % model.__name__, (admin_class,), options)
# Ignore the registration if the model has been
# swapped out.
if not model._meta.swapped:
# If we got **options then dynamically construct a subclass of
# admin_class with those **options.
if options:
# For reasons I don't quite understand, without a __module__
# the created class appears to "live" in the wrong place,
# which causes issues later on.
options['__module__'] = __name__
admin_class = type("%sAdmin" % model.__name__, (admin_class,), options)

# Validate (which might be a no-op)
validate(admin_class, model)
# Validate (which might be a no-op)
validate(admin_class, model)

# Instantiate the admin class to save in the registry
self._registry[model] = admin_class(model, self)
# Instantiate the admin class to save in the registry
self._registry[model] = admin_class(model, self)

def unregister(self, model_or_iterable):
"""
Expand Down Expand Up @@ -319,6 +324,7 @@ def login(self, request, extra_context=None):
REDIRECT_FIELD_NAME: request.get_full_path(),
}
context.update(extra_context or {})

defaults = {
'extra_context': context,
'current_app': self.name,
Expand Down
2 changes: 1 addition & 1 deletion django/contrib/admin/templates/admin/base.html
Expand Up @@ -26,7 +26,7 @@
{% if user.is_active and user.is_staff %}
<div id="user-tools">
{% trans 'Welcome,' %}
<strong>{% filter force_escape %}{% firstof user.first_name user.username %}{% endfilter %}</strong>.
<strong>{% filter force_escape %}{% firstof user.get_short_name user.username %}{% endfilter %}</strong>.
{% block userlinks %}
{% url 'django-admindocs-docroot' as docsroot %}
{% if docsroot %}
Expand Down
1 change: 1 addition & 0 deletions django/contrib/admin/views/decorators.py
Expand Up @@ -4,6 +4,7 @@
from django.contrib.auth.views import login
from django.contrib.auth import REDIRECT_FIELD_NAME


def staff_member_required(view_func):
"""
Decorator for views that checks that the user is logged in and is a staff
Expand Down
20 changes: 19 additions & 1 deletion django/contrib/auth/__init__.py
Expand Up @@ -6,9 +6,10 @@
BACKEND_SESSION_KEY = '_auth_user_backend'
REDIRECT_FIELD_NAME = 'next'


def load_backend(path):
i = path.rfind('.')
module, attr = path[:i], path[i+1:]
module, attr = path[:i], path[i + 1:]
try:
mod = import_module(module)
except ImportError as e:
Expand All @@ -21,6 +22,7 @@ def load_backend(path):
raise ImproperlyConfigured('Module "%s" does not define a "%s" authentication backend' % (module, attr))
return cls()


def get_backends():
from django.conf import settings
backends = []
Expand All @@ -30,6 +32,7 @@ def get_backends():
raise ImproperlyConfigured('No authentication backends have been defined. Does AUTHENTICATION_BACKENDS contain anything?')
return backends


def authenticate(**credentials):
"""
If the given credentials are valid, return a User object.
Expand All @@ -46,6 +49,7 @@ def authenticate(**credentials):
user.backend = "%s.%s" % (backend.__module__, backend.__class__.__name__)
return user


def login(request, user):
"""
Persist a user id and a backend in the request. This way a user doesn't
Expand All @@ -69,6 +73,7 @@ def login(request, user):
request.user = user
user_logged_in.send(sender=user.__class__, request=request, user=user)


def logout(request):
"""
Removes the authenticated user's ID from the request and flushes their
Expand All @@ -86,6 +91,19 @@ def logout(request):
from django.contrib.auth.models import AnonymousUser
request.user = AnonymousUser()


def get_user_model():
"Return the User model that is active in this project"
from django.conf import settings
from django.db.models import get_model

try:
app_label, model_name = settings.AUTH_USER_MODEL.split('.')
except ValueError:
raise ImproperlyConfigured("AUTH_USER_MODEL must be of the form 'app_label.model_name'")
return get_model(app_label, model_name)
Copy link
Member

Choose a reason for hiding this comment

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

if settings.AUTH_USER_MODEL is set to an invalid value that conforms to the pattern ie: "typodapp.nonmodel" then get_model and hence get_user_model will return None.

I'd propose that get_user_model test for None and raise ImproperlyConfigured

Copy link
Member

Choose a reason for hiding this comment

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

Good point - I've just implemented this.



def get_user(request):
from django.contrib.auth.models import AnonymousUser
try:
Expand Down
24 changes: 15 additions & 9 deletions django/contrib/auth/backends.py
@@ -1,6 +1,6 @@
from __future__ import unicode_literals

from django.contrib.auth.models import User, Permission
from django.contrib.auth import get_user_model
from django.contrib.auth.models import Permission


class ModelBackend(object):
Expand All @@ -12,10 +12,11 @@ class ModelBackend(object):
# configurable.
def authenticate(self, username=None, password=None):
try:
user = User.objects.get(username=username)
UserModel = get_user_model()
user = UserModel.objects.get_by_natural_key(username)
if user.check_password(password):
return user
except User.DoesNotExist:
except UserModel.DoesNotExist:
return None

def get_group_permissions(self, user_obj, obj=None):
Expand Down Expand Up @@ -60,8 +61,9 @@ def has_module_perms(self, user_obj, app_label):

def get_user(self, user_id):
try:
return User.objects.get(pk=user_id)
except User.DoesNotExist:
UserModel = get_user_model()
return UserModel.objects.get(pk=user_id)
except UserModel.DoesNotExist:
return None


Expand Down Expand Up @@ -94,17 +96,21 @@ def authenticate(self, remote_user):
user = None
username = self.clean_username(remote_user)

UserModel = get_user_model()

# Note that this could be accomplished in one try-except clause, but
# instead we use get_or_create when creating unknown users since it has
# built-in safeguards for multiple threads.
if self.create_unknown_user:
user, created = User.objects.get_or_create(username=username)
user, created = UserModel.objects.get_or_create(**{
getattr(UserModel, 'USERNAME_FIELD', 'username'): username
})
if created:
user = self.configure_user(user)
else:
try:
user = User.objects.get(username=username)
except User.DoesNotExist:
user = UserModel.objects.get_by_natural_key(username)
except UserModel.DoesNotExist:
pass
return user

Expand Down
32 changes: 21 additions & 11 deletions django/contrib/auth/management/__init__.py
Expand Up @@ -6,9 +6,10 @@
import getpass
import locale
import unicodedata
from django.contrib.auth import models as auth_app

from django.contrib.auth import models as auth_app, get_user_model
from django.core import exceptions
from django.db.models import get_models, signals
from django.contrib.auth.models import User
from django.utils import six
from django.utils.six.moves import input

Expand Down Expand Up @@ -64,7 +65,9 @@ def create_permissions(app, created_models, verbosity, **kwargs):
def create_superuser(app, created_models, verbosity, db, **kwargs):
from django.core.management import call_command

if auth_app.User in created_models and kwargs.get('interactive', True):
UserModel = get_user_model()

if UserModel in created_models and kwargs.get('interactive', True):
msg = ("\nYou just installed Django's auth system, which means you "
"don't have any superusers defined.\nWould you like to create one "
"now? (yes/no): ")
Expand Down Expand Up @@ -113,28 +116,35 @@ def get_default_username(check_db=True):
:returns: The username, or an empty string if no username can be
determined.
"""
from django.contrib.auth.management.commands.createsuperuser import (
RE_VALID_USERNAME)
# If the User model has been swapped out, we can't make any assumptions
# about the default user name.
if auth_app.User._meta.swapped:
return ''

default_username = get_system_username()
try:
default_username = unicodedata.normalize('NFKD', default_username)\
.encode('ascii', 'ignore').decode('ascii').replace(' ', '').lower()
except UnicodeDecodeError:
return ''
if not RE_VALID_USERNAME.match(default_username):

# Run the username validator
try:
auth_app.User._meta.get_field('username').run_validators(default_username)
Copy link
Member

Choose a reason for hiding this comment

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

Use get_field_by_name('username')[0], get_field is O(n) whereas get_field_by_name is O(1). No joke.

Copy link
Member

Choose a reason for hiding this comment

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

Rofl, shouldn't we just change get_field to get_field_by_name('bla')[0] then? Using ugly API to be more performent smells bad if you ask me.

Copy link
Member

Choose a reason for hiding this comment

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

Hell yeah - fix the cause, not the symptom.

Copy link
Member

Choose a reason for hiding this comment

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

Turns out it's not quite as simple as I thought...

Firstly - Yes, it's O(n), but, N is very small, and the way get_field_by_name() is O(1) is by pre-caching all possible lookups, so there's a first-call cache warming expense. In this usage, we're on a management command, and get_field will be invoked exactly once, so we're going to be in territory where the expense of cache warming to get the O(1) lookup will exceed the O(N) cost we're trying to avoid.

Secondly, it's not as simple as replacing the internals of get_field() with get_field_by_name(), because the cache priming in get_field_by_name() assumes all the models have been loaded (because it includes all reverse FK and M2M relations). get_field() only depends on self. This should get a little cleaner once app loading lands, but as it currently stands, it's difficult to guarantee that get_field_by_name() is actually callable.

So - I'm going to call this a no-fix; or, at least, a "not my bailiwick" fix. Meta certainly could be cleaned up, but that's a much bigger fish that needs frying.

except exceptions.ValidationError:
return ''

# Don't return the default username if it is already taken.
if check_db and default_username:
try:
User.objects.get(username=default_username)
except User.DoesNotExist:
auth_app.User.objects.get(username=default_username)
except auth_app.User.DoesNotExist:
pass
else:
return ''
return default_username


signals.post_syncdb.connect(create_permissions,
dispatch_uid = "django.contrib.auth.management.create_permissions")
dispatch_uid="django.contrib.auth.management.create_permissions")
signals.post_syncdb.connect(create_superuser,
sender=auth_app, dispatch_uid = "django.contrib.auth.management.create_superuser")
sender=auth_app, dispatch_uid="django.contrib.auth.management.create_superuser")
16 changes: 10 additions & 6 deletions django/contrib/auth/management/commands/changepassword.py
@@ -1,8 +1,8 @@
import getpass
from optparse import make_option

from django.contrib.auth import get_user_model
from django.core.management.base import BaseCommand, CommandError
from django.contrib.auth.models import User
from django.db import DEFAULT_DB_ALIAS


Expand Down Expand Up @@ -30,12 +30,16 @@ def handle(self, *args, **options):
else:
username = getpass.getuser()

UserModel = get_user_model()

try:
u = User.objects.using(options.get('database')).get(username=username)
except User.DoesNotExist:
u = UserModel.objects.using(options.get('database')).get(**{
getattr(UserModel, 'USERNAME_FIELD', 'username'): username
})
except UserModel.DoesNotExist:
raise CommandError("user '%s' does not exist" % username)

self.stdout.write("Changing password for user '%s'\n" % u.username)
self.stdout.write("Changing password for user '%s'\n" % u)

MAX_TRIES = 3
count = 0
Expand All @@ -48,9 +52,9 @@ def handle(self, *args, **options):
count = count + 1

if count == MAX_TRIES:
raise CommandError("Aborting password change for user '%s' after %s attempts" % (username, count))
raise CommandError("Aborting password change for user '%s' after %s attempts" % (u, count))

u.set_password(p1)
u.save()

return "Password changed successfully for user '%s'" % u.username
return "Password changed successfully for user '%s'" % u