Question

when using a custom User model, extending AbstractBaseUser, I've run into a problem when the USERNAME_FIELD points to a nullable field

what I'm trying to accomplish is to allow multiple methods of login (facebook, username) and for that I have two unique identifier fields (facebook_id, username) but both have to be nullable because an account doesn't have to have both (and most accounts won't have both)

so for logging in with username, I rely on Django's built in login auth backend

where it falls apart is when trying to log in with an alternative login method (facebook) and the Django auth is attempted first

class ModelBackend(object):
    """
    Authenticates against settings.AUTH_USER_MODEL.
    """

    def authenticate(self, username=None, password=None, **kwargs):
        UserModel = get_user_model()
        if username is None:
            username = kwargs.get(UserModel.USERNAME_FIELD)
        try:
            user = UserModel._default_manager.get_by_natural_key(username)
            if user.check_password(password):
                return user
        except UserModel.DoesNotExist:
            # Run the default password hasher once to reduce the timing
            # difference between an existing and a non-existing user (#20760).
            UserModel().set_password(password)

the if username is None hack is meant to fix the situation where the username field isn't actually called "username" so the username parameter is empty but the actual provided username can be found in kwargs

the issue is that it ends up with None value anyway (because kwargs contains facebook_id=12345) and ends up passing that to get_by_natural_key

def get_by_natural_key(self, username):
    return self.get(**{self.model.USERNAME_FIELD: username})

which translates roughly to User.objects.get(username=None)

since my username field is nullable, this returns multiple results, the whole thing throws an models.MultipleObjectsReturned exception ("get() returned more than one User -- it returned 417!") and prevents any other auth backends from running

from my perspective there's several issues:

  1. ModelBackend shouldn't be just blindly attempting to retrieve a user if its attempts at figuring out the provided username result with None
  2. get_by_natural_key should catch the MultipleObjectsReturned and gracefully fail, or authenticate() should handle that scenario and fail to log the user in, instead of bombing the entire auth mechanism

known workarounds:

I know I can move the ModelBackend to be the last method attempted but that just makes the dumb query run sometimes instead of always, I'd rather fix it so it never does. I'm also slightly concerned with its performance as number of username=None users grows

I also know I can extend ModelBackend, patch the holes and just not use the stock ModelBackend, but I don't know if there are any gotchas to that approach... are existing sessions using ModelBackend lost?

For simplification purposes, I skipped the part where I actually use a nullable email field as the username :D and I know that for that I could just easily use a custom auth backend but once I got nullable-email-field-login working I started thinking I can probably save myself some headache in generating usernames for users coming from facebook (and letting them choose an unused username later on in the process) by making username field nullable as well :D

I'm concerned that having a nullable USERNAME_FIELD is going to make Django fail in various different ways, and I saw no warning for this or explanation for why. It's silly that a system that supports multiple login schemes cannot survive it's stock auth identifier being empty.

Was it helpful?

Solution

Since you're replacing the default User model with your own, I'd go with patching its manager's get_by_natural_key method to raise DoesNotExist when called with a None natural key, as that situation clearly indicates your user is being authenticated using some other scheme. Your model has no situation where a record can have a legitimate None natural key, so in any case it should raise a DoesNotExist for such a key.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top