Authentication fails using custom user based on AbstractUser with email as username

vanderzielj commented 5 months ago

First of all - thank you for providing, maintaining and supporting this excellent library. I have tried about 3 libraries and the level of integration of django-auth-adfs and the support of group claims, as well as its reputation is what made me select this one over the others.

I have a few years of Python and about 5 months of Django under my belt (about 6+ years each of C#/.Net and then Java with some Perl and a year or so of front-end JavaScript/Angular work thrown in for good measure) so if I make some unexpected assumptions about Django or Python please ask for clarification.

I have an issue/question getting the django-auth-adfs package to work with my custom usermodel (explained more below).

I have configured the adfs demo provided by in your GitHub source to point at my Azure AD instance (administered by another department) as shown below. I was able to login with my Entra corporate account and access the Admin section with the created user. I then logged out and logged in again with no issues. Everything seemed to work.

AUTH_ADFS = {
    "AUDIENCE": client_id,
    "CLIENT_ID": client_id,
    "CLIENT_SECRET": client_secret,
    "CLAIM_MAPPING": {
        "first_name": "given_name",
        "last_name": "family_name",
        "email": "upn",
    },
    "GROUPS_CLAIM": "groups",
    "MIRROR_GROUPS": False,
    "GROUP_TO_FLAG_MAPPING": {
        "is_superuser": ["1XXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX"],
        "is_staff": ["1XXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX"],
    },
    "USERNAME_CLAIM": "upn",
    "CREATE_NEW_USERS": True,
    "TENANT_ID": tenant_id,
    "RELYING_PARTY_ID": client_id,
    "LOGIN_EXEMPT_URLS": ["", "home/"],
}

Today I added a custom email-as-username user using the Django example (without the birthday field) to the demo above. I deleted the sqlite3 database, all migrations then re-created and migrated the app. When everything checked out I started it up. I received an error

raise ImproperlyConfigured("You cannot set the username field of the user model from "
django.core.exceptions.ImproperlyConfigured: You cannot set the username field of the user model from the CLAIM_MAPPING setting. Instead use the USERNAME_CLAIM setting.

Notice that I was setting the email field and not the username. I commented out the "email": "upn", line in the CLAIM_MAPPING setting and left the "USERNAME_CLAIM": "upn" setting that was already in place, restarted and I seemed to be in business: I was able to login and a new user was created. However, as soon as I logout and try to login again - or click on any protected page like Admin - I get a Validation error:

DEBUG 2024-01-09 23:53:52,132 django_auth_adfs Attribute 'first_name' for instance 'Justin.VanderZiel@Donaldson.com' was set to 'Justin'.
DEBUG 2024-01-09 23:53:52,132 django_auth_adfs Attribute 'last_name' for instance 'Justin.VanderZiel@Donaldson.com' was set to 'Vander Ziel'.
DEBUG 2024-01-09 23:53:52,139 django_auth_adfs Attribute 'is_superuser' for user 'Justin.VanderZiel@Donaldson.com' was set to 'True'.
DEBUG 2024-01-09 23:53:52,139 django_auth_adfs Attribute 'is_staff' for user 'Justin.VanderZiel@Donaldson.com' was set to 'True'.
Internal Server Error: /oauth2/callback
Traceback (most recent call last):
File "/home/jvanderz/src/learn-django/django-auth-adfs/demo/adfs/.venv/lib/python3.11/site-packages/django/core/handlers/exception.py", line 55, in inner
response = get_response(request)
           ^^^^^^^^^^^^^^^^^^^^^
File "/home/jvanderz/src/learn-django/django-auth-adfs/demo/adfs/.venv/lib/python3.11/site-packages/django/core/handlers/base.py", line 197, in _get_response
response = wrapped_callback(request, *callback_args, **callback_kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/jvanderz/src/learn-django/django-auth-adfs/demo/adfs/.venv/lib/python3.11/site-packages/django/views/generic/base.py", line 104, in view
return self.dispatch(request, *args, **kwargs)
       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/jvanderz/src/learn-django/django-auth-adfs/demo/adfs/.venv/lib/python3.11/site-packages/django/views/generic/base.py", line 143, in dispatch
return handler(request, *args, **kwargs)
       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/jvanderz/src/learn-django/django-auth-adfs/demo/adfs/.venv/lib/python3.11/site-packages/django_auth_adfs/views.py", line 40, in get
user = authenticate(request=request, authorization_code=code)
       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/jvanderz/src/learn-django/django-auth-adfs/demo/adfs/.venv/lib/python3.11/site-packages/django/views/decorators/debug.py", line 73, in sensitive_variables_wrapper
return func(*func_args, **func_kwargs)
       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/jvanderz/src/learn-django/django-auth-adfs/demo/adfs/.venv/lib/python3.11/site-packages/django/contrib/auth/__init__.py", line 79, in authenticate
user = backend.authenticate(request, **credentials)
       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/jvanderz/src/learn-django/django-auth-adfs/demo/adfs/.venv/lib/python3.11/site-packages/django_auth_adfs/backend.py", line 402, in authenticate
user = self.process_access_token(access_token, adfs_response)
       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/jvanderz/src/learn-django/django-auth-adfs/demo/adfs/.venv/lib/python3.11/site-packages/django_auth_adfs/backend.py", line 192, in process_access_token
user.full_clean()
File "/home/jvanderz/src/learn-django/django-auth-adfs/demo/adfs/.venv/lib/python3.11/site-packages/django/db/models/base.py", line 1544, in full_clean
raise ValidationError(errors)
django.core.exceptions.ValidationError: {'email': ['User with this Email address already exists.']}

Notice that this is a validation error - after the user was authenticated. And from looking at the values I can also say that is after the user was already retrieved. Why is it testing if a user exists after all of this has already taken place?

In trying to determine where in the code the decision is made to retrieve an existing user vs creating a new one and my best guess was that it happens in the create_user method:

        try:
            user = usermodel.objects.get(**userdata)
        except usermodel.DoesNotExist:
            if settings.CREATE_NEW_USERS:
                user = usermodel.objects.create(**userdata)
                logger.debug("User '%s' has been created.", claims[username_claim])

this method is called by the AdfsBaseBackend.process_access_token from the same class on the line that reads user = self.create_user(claims) and expects in return, a user instance - preexisting or created. What I don't understand is why after the user has been authenticated and the code has an instance of the user class the process_access_token then calls full_clean on the user model - which throws the exception. I haven't written any validation code, yet, so perhaps this is the standard way it is done.

When I walked through a copy of the back-end code that I placed into my own backend.py that I substituted for the project code (same code just different file), I could see that the user was populated and the exception was never thrown - which seems to indicate that the created user was found.

I appreciate any and all help that can be provided.

Regards, Justin

Upvote & Fund

We're using Polar.sh so you can upvote and help fund this issue.
We receive the funding once the issue is completed & confirmed by you.
Thank you in advance for helping prioritize & fund our backlog.

vanderzielj commented 5 months ago

commenting out the full_clean resulted in a nearly duplicate user being created. Notice the change of case. It seems that the UPN returned is not faithful with respect to character case. That makes sense because I can login using an all lowercase email address without any problems. (Can we re-visit the lowercase email as username issue now? :-)

vanderzielj commented 5 months ago

I apparently missed the fact that the user was not found in the return authentication; hence the creation of the new user. So it would appear that I have a couple choices: either modify the model to always use lowercase (same case) email addresses or modify the lookup to perform a case-insensitive search.

I have resolved this issue by modifying my custom user model to use a custom lower case email field:

class LowercaseEmailField(models.EmailField):
    def get_prep_value(self, value):
        return str(value).lower()

tim-schilling commented 5 months ago

Thank you for following up with your solution!

snok / django-auth-adfs

Authentication fails using custom user based on AbstractUser with email as username #323

Upvote & Fund