2

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.

3
  • Why don't you simply use something like github.com/omab/python-social-auth? Apr 7, 2014 at 9:07
  • @BerislavLopac I knew I forgot the "please don't suggest auth packages as solution" line...
    – allixsenos
    Apr 7, 2014 at 9:30
  • I thought you might have, that's why I put a comment instead of an answer. Say hi to Penny! ;-) Apr 7, 2014 at 17:49

1 Answer 1

2

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.

7
  • this indeed fixes the issue... I wonder if it's worth submitting as a Django bug? leaving open for a bit more to see if any other responses come in
    – allixsenos
    Apr 7, 2014 at 9:24
  • I don't see this as a Django bug. This is a peculiarity of your specific replacement for the User model, and as such it is your responsibility to support Django's authentication model, not vice-versa. Granted, Django's authentication model is not the most flexible API in the world, but in this specific case the fix clearly belongs in your model, not theirs.
    – lanzz
    Apr 7, 2014 at 9:29
  • I think it should either be documented ("username field has to be unique and cannot be null") or handled :)
    – allixsenos
    Apr 7, 2014 at 9:32
  • 1
    Go ahead and submit a bug report then. You don't need me to agree whether this is a bug or not.
    – lanzz
    Apr 7, 2014 at 9:46
  • 1
    @BerislavLopac: unique=True implies database uniqueness, NULL != NULL in database world, so the records are unique
    – allixsenos
    Apr 8, 2014 at 6:35

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.