Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: do not hardcode scopes for azure AD v2 #279

Merged
merged 3 commits into from
Mar 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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