Merge pull request #166 from Seykotron/master

Setting a second USERNAME_CLAIM to allow guest users to log in

Author: JonasKs

CHANGELOG.rst

@@ -1,6 +1,14 @@
 Changelog
 =========
 
+`1.9.0`_ - 2021-08-27
+---------------------
+
+**Features**
+
+* Add ``GUEST_USERNAME_CLAIM``, a setting that allow you to use a different username claim for guest users. @JonasKs and @Seykotron #166
+
+
 `1.8.1`_ - 2021-08-27
 ---------------------
 
@@ -261,6 +269,7 @@ Changelog
 
 * Initial release
 
+.. _1.9.0: https://github.com/snok/django-auth-adfs/compare/1.8.1...1.9.0
 .. _1.8.1: https://github.com/snok/django-auth-adfs/compare/1.8.0...1.8.1
 .. _1.8.0: https://github.com/snok/django-auth-adfs/compare/1.7.0...1.8.0
 .. _1.7.0: https://github.com/snok/django-auth-adfs/compare/1.6.1...1.7.0

django_auth_adfs/__init__.py

@@ -4,4 +4,4 @@
 Adding imports here will break setup.py
 """
 
-__version__ = '1.8.1'
+__version__ = '1.9.0'

django_auth_adfs/backend.py

@@ -128,11 +128,22 @@ def create_user(self, claims):
         """
         # Create the user
         username_claim = settings.USERNAME_CLAIM
+        guest_username_claim = settings.GUEST_USERNAME_CLAIM
         usermodel = get_user_model()
 
+        if (
+            guest_username_claim
+            and not claims.get(username_claim)
+            and not settings.BLOCK_GUEST_USERS
+            and claims.get('tid') != settings.TENANT_ID
+        ):
+            username_claim = guest_username_claim
+
         if not claims.get(username_claim):
-            logger.error("User claim's doesn't have the claim '%s' in his claims: %s" % (username_claim, claims))
+            logger.error("User claim's doesn't have the claim '%s' in his claims: %s" %
+                         (username_claim, claims))
             raise PermissionDenied
+
         userdata = {usermodel.USERNAME_FIELD: claims[username_claim]}
 
         try:

django_auth_adfs/config.py

@@ -70,6 +70,7 @@ def __init__(self):
         self.TENANT_ID = None  # Required
         self.TIMEOUT = 5
         self.USERNAME_CLAIM = "winaccountname"
+        self.GUEST_USERNAME_CLAIM = None
         self.JWT_LEEWAY = 0
         self.CUSTOM_FAILED_RESPONSE_VIEW = lambda request, error_message, status: render(
             request, 'django_auth_adfs/login_failed.html', {'error_message': error_message}, status=status

docs/settings_ref.rst

@@ -262,6 +262,25 @@ example
    The group doesn't need to exist in Django for this to work. This will work as long as it's in the groups claim
    in the access token.
 
+GUEST_USERNAME_CLAIM
+--------------------
+* **Default**: ``None``
+* **Type**: ``string``
+
+When these criteria are met:
+
+1. A ``guest_username_claim`` is configured
+2. Token claims do not have the configured ``settings.USERNAME_CLAIM`` in it
+3. The ``settings.BLOCK_GUEST_USERS`` is set to ``False``
+4. The claims ``tid`` does not match ``settings.TENANT_ID``
+
+Then, the ``GUEST_USERNAME_CLAIM`` can be used to populate a username, when the ``USERNAME_CLAIM`` cannot be found in
+the claims.
+
+This can be useful when you want to use ``upn`` as a username claim for your own users,
+but some guest users (such as normal outlook users) don't have that claim.
+
+
 LOGIN_EXEMPT_URLS
 -----------------
 * **Default**: ``None``
@@ -423,13 +442,13 @@ The value of the claim must be a unique value. No 2 users should ever have the s
 .. NOTE::
    You can find the short name for the claims you configure in the ADFS management console underneath
    **ADFS** ➜ **Service** ➜ **Claim Descriptions**
-   
-   
+
+
 .. _version_setting:
 
 VERSION
 --------------
-* **Default**: ``v1.0`` 
+* **Default**: ``v1.0``
 * **Type**: ``string``
 
 Version of the Azure Active Directory endpoint version. By default it is set to ``v1.0``. At the time of writing this documentation, it can also be set to ``v2.0``. For new projects, ``v2.0`` is recommended. ``v1.0`` is kept as a default for backwards compatibility.

poetry.lock

@@ -1,6 +1,6 @@
 [tool.poetry]
 name = 'django-auth-adfs'
-version = '1.8.1'  # Remember to also change __init__.py version
+version = '1.9.0'  # Remember to also change __init__.py version
 description = 'A Django authentication backend for Microsoft ADFS and AzureAD'
 authors = ['Joris Beckers <[email protected]>']
 maintainers = ['Jonas Krüger Svensson <[email protected]>', 'Sondre Lillebø Gundersen <[email protected]>']
@@ -46,6 +46,7 @@ responses = '*'
 mock = '*'
 coverage = '*'
 djangorestframework = '*'
+django-filter = "^2.4.0"
 
 [build-system]
 requires = ["poetry-core>=1.0.0"]

pyproject.toml

@@ -9,7 +9,7 @@
 from django_auth_adfs.config import ProviderConfig, Settings
 from django_auth_adfs.rest_framework import AdfsAccessTokenAuthentication
 from .utils import build_access_token_adfs, build_access_token_azure, build_access_token_azure_guest, \
-    build_access_token_azure_not_guest, mock_adfs
+    build_access_token_azure_guest_no_upn, build_access_token_azure_not_guest, mock_adfs
 
 
 class RestFrameworkIntegrationTests(TestCase):
@@ -28,6 +28,9 @@ def setUp(self):
         azure_response_no_guest = build_access_token_azure_not_guest(RequestFactory().get('/'))[2]
         self.access_token_azure_no_guest = json.loads(azure_response_no_guest)['access_token']
 
+        azure_response_guest = build_access_token_azure_guest_no_upn(RequestFactory().get('/'))[2]
+        self.access_token_azure_guest_no_upn = json.loads(azure_response_guest)['access_token']
+
     @mock_adfs("2012")
     def test_access_token_2012(self):
         access_token_header = "Bearer {}".format(self.access_token_adfs)
@@ -95,6 +98,40 @@ def test_access_token_azure_no_guest(self):
                         user, token = self.drf_auth_class.authenticate(request)
                         self.assertEqual(user.username, "testuser")
 
+    @mock_adfs("azure")
+    def test_access_token_azure_guest_but_no_upn(self):
+        access_token_header = "Bearer {}".format(self.access_token_azure_guest_no_upn)
+        request = RequestFactory().get('/api', HTTP_AUTHORIZATION=access_token_header)
+        from django_auth_adfs.config import django_settings
+        settings = deepcopy(django_settings)
+        del settings.AUTH_ADFS["SERVER"]
+        settings.AUTH_ADFS["TENANT_ID"] = "dummy_tenant_id"
+        settings.AUTH_ADFS["GUEST_USERNAME_CLAIM"] = "email"
+        settings.AUTH_ADFS["BLOCK_GUEST_USERS"] = False
+        with patch("django_auth_adfs.config.django_settings", settings):
+            with patch('django_auth_adfs.backend.settings', Settings()):
+                with patch("django_auth_adfs.config.settings", Settings()):
+                    with patch("django_auth_adfs.backend.provider_config", ProviderConfig()):
+                        user, token = self.drf_auth_class.authenticate(request)
+                        self.assertEqual(user.username, "[email protected]")
+
+    @mock_adfs("azure")
+    def test_access_token_azure_guest_but_no_upn_but_no_guest_username_claim(self):
+        access_token_header = "Bearer {}".format(self.access_token_azure_guest_no_upn)
+        request = RequestFactory().get('/api', HTTP_AUTHORIZATION=access_token_header)
+        from django_auth_adfs.config import django_settings
+        settings = deepcopy(django_settings)
+        del settings.AUTH_ADFS["SERVER"]
+        settings.AUTH_ADFS["TENANT_ID"] = "dummy_tenant_id"
+        settings.AUTH_ADFS["GUEST_USERNAME_CLAIM"] = None  # <--- Set to None, should not be validated as OK
+        settings.AUTH_ADFS["BLOCK_GUEST_USERS"] = False
+        with patch("django_auth_adfs.config.django_settings", settings):
+            with patch('django_auth_adfs.backend.settings', Settings()):
+                with patch("django_auth_adfs.config.settings", Settings()):
+                    with patch("django_auth_adfs.backend.provider_config", ProviderConfig()):
+                        with self.assertRaises(exceptions.AuthenticationFailed):
+                            self.drf_auth_class.authenticate(request)
+
     @mock_adfs("2012")
     def test_access_token_exceptions(self):
         access_token_header = "Bearer non-existing-token"

tests/test_drf_integration.py

@@ -83,12 +83,17 @@ def build_access_token_azure_guest(request):
     return do_build_access_token(request, issuer, schema='guest_tenant_id')
 
 
+def build_access_token_azure_guest_no_upn(request):
+    issuer = "https://sts.windows.net/01234567-89ab-cdef-0123-456789abcdef/"
+    return do_build_access_token(request, issuer, schema='guest_tenant_id', no_upn=True)
+
+
 def do_build_mfa_error(request):
     response = {'error_description': 'AADSTS50076'}
     return 400, [], json.dumps(response)
 
 
-def do_build_access_token(request, issuer, schema=None):
+def do_build_access_token(request, issuer, schema=None, no_upn=False):
     issued_at = int(time.time())
     expires = issued_at + 3600
     auth_time = datetime.utcnow()
@@ -116,6 +121,8 @@ def do_build_access_token(request, issuer, schema=None):
     if issuer.startswith('https://sts.windows.net'):
         claims['upn'] = 'testuser'
         claims['groups'] = claims['group']
+    if no_upn:
+        del claims['upn']
     token = jwt.encode(claims, signing_key_b, algorithm="RS256")
     response = {
         'resource': 'django_website.adfs.relying_party_id',