Skip to content

Commit

Permalink
Merge pull request #279 from snok/fix_azure_ad
Browse files Browse the repository at this point in the history
fix: do not hardcode scopes for azure AD v2
  • Loading branch information
JonasKs authored Mar 30, 2023
2 parents 8b7cf83 + 1f2dc70 commit ed486dc
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 7 deletions.
2 changes: 1 addition & 1 deletion django_auth_adfs/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,4 @@
Adding imports here will break setup.py
"""

__version__ = '1.11.4'
__version__ = '1.11.5'
10 changes: 9 additions & 1 deletion django_auth_adfs/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ def __init__(self):
self.PROXIES = None

self.VERSION = 'v1.0'
self.SCOPES = []

required_settings = [
"AUDIENCE",
Expand Down Expand Up @@ -138,6 +139,10 @@ def __init__(self):
elif "VERSION" in _settings:
raise ImproperlyConfigured("The VERSION cannot be set when TENANT_ID is not set.")

if self.VERSION == "v2.0" and not self.SCOPES and self.RELYING_PARTY_ID:
warnings.warn('Use `SCOPES` for AzureAD instead of RELYING_PARTY_ID', DeprecationWarning)
if not isinstance(self.SCOPES, list):
raise ImproperlyConfigured("Scopes must be a list")
# Overwrite defaults with user settings
for setting, value in _settings.items():
if hasattr(self, setting):
Expand Down Expand Up @@ -346,7 +351,10 @@ def build_authorization_endpoint(self, request, disable_sso=None, force_mfa=Fals
})
if self._mode == "openid_connect":
if settings.VERSION == 'v2.0':
query["scope"] = f"openid api://{settings.RELYING_PARTY_ID}/.default"
if settings.SCOPES:
query['scope'] = " ".join(settings.SCOPES)
else:
query["scope"] = f"openid api://{settings.RELYING_PARTY_ID}/.default"
query.pop("resource")
else:
query["scope"] = "openid"
Expand Down
18 changes: 14 additions & 4 deletions docs/settings_ref.rst
Original file line number Diff line number Diff line change
Expand Up @@ -117,17 +117,17 @@ The dictionary can also map extra details to the Django user account using an
`Extension of the User model <https://docs.djangoproject.com/en/stable/topics/auth/customizing/#extending-the-existing-user-model>`_
Set a dictionary as value in the CLAIM_MAPPING setting with as key the name User model.
You will need to make sure the related field exists before the user authenticates.
This can be done by creating a receiver on the
This can be done by creating a receiver on the
`post_save <https://docs.djangoproject.com/en/4.0/ref/signals/#post-save>`_ signal that
creates the related instance when the ``User`` instance is created.

example

.. code-block:: python
'CLAIM_MAPPING': {'first_name': 'given_name',
'last_name': 'family_name',
'email': 'upn',
'CLAIM_MAPPING': {'first_name': 'given_name',
'last_name': 'family_name',
'email': 'upn',
'userprofile': {
'employee_id': 'employeeid'
}}
Expand Down Expand Up @@ -369,6 +369,16 @@ RETRIES
The number of time a request to the ADFS server is retried. It allows, in combination with :ref:`timeout_setting`
to fine tune the behaviour of the connection to ADFS.


SCOPES
------
* **Default**: ``[]``
* **Type**: ``list``

**Only used when you have v2 AzureAD config**



SERVER
------
* **Default**:
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[tool.poetry]
name = 'django-auth-adfs'
version = "1.11.4" # Remember to also change __init__.py version
version = "1.11.5" # 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]>']
Expand Down
27 changes: 27 additions & 0 deletions tests/test_authentication.py
Original file line number Diff line number Diff line change
Expand Up @@ -476,6 +476,33 @@ def test_oauth_redir_azure_version_two(self):
self.assertEqual(redir.path.rstrip("/"), '/01234567-89ab-cdef-0123-456789abcdef/oauth2/authorize')
self.assertEqual(qs, sq_expected)

@mock_adfs("azure")
def test_scopes_generated_correctly(self):
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["VERSION"] = 'v2.0'
settings.AUTH_ADFS["SCOPES"] = ['openid', 'api://your-configured-client-id/user_impersonation']
with patch("django_auth_adfs.config.django_settings", settings), \
patch("django_auth_adfs.config.settings", Settings()), \
patch("django_auth_adfs.views.provider_config", ProviderConfig()):
response = self.client.get("/oauth2/login?next=/test/")
self.assertEqual(response.status_code, 302)
redir = urlparse(response["Location"])
qs = parse_qs(redir.query)
sq_expected = {
'scope': ['openid api://your-configured-client-id/user_impersonation'],
'client_id': ['your-configured-client-id'],
'state': ['L3Rlc3Qv'],
'response_type': ['code'],
'redirect_uri': ['http://testserver/oauth2/callback']
}
self.assertEqual(redir.scheme, 'https')
self.assertEqual(redir.hostname, 'login.microsoftonline.com')
self.assertEqual(redir.path.rstrip("/"), '/01234567-89ab-cdef-0123-456789abcdef/oauth2/authorize')
self.assertEqual(qs, sq_expected)

@mock_adfs("2016")
def test_inactive_user(self):
user = User.objects.create(**{
Expand Down

0 comments on commit ed486dc

Please sign in to comment.