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.

Sign up for GitHub

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

WIP: Multiple idps #201

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

WIP: Multiple idps #201

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
82bb97f
Updated contribution to include how to run test suite
peterfarrell Nov 19, 2021
b107e60
Added basic support and test for IDPS dict in Settings
peterfarrell Nov 19, 2021
c45c685
Fixed formatting
peterfarrell Nov 19, 2021
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
4 changes: 3 additions & 1 deletion CONTRIBUTING.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,4 +60,6 @@ Set up your environment

5. Push the topic branch to your personal fork

6. Create a pull request to the django-auth-adfs repository with a detailed explanation
6. Test your code by running the test suite ``poetry run coverage run manage.py test -v 2``

7. Create a pull request to the django-auth-adfs repository with a detailed explanation
147 changes: 83 additions & 64 deletions django_auth_adfs/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,13 @@
AZURE_AD_SERVER = "login.microsoftonline.com"
DEFAULT_SETTINGS_CLASS = 'django_auth_adfs.config.Settings'

REQUIRED_SETTINGS = [
"AUDIENCE",
"CLIENT_ID",
"RELYING_PARTY_ID",
"USERNAME_CLAIM",
]


class ConfigLoadError(Exception):
pass
Expand Down Expand Up @@ -78,13 +85,6 @@ def __init__(self):

self.VERSION = 'v1.0'

required_settings = [
"AUDIENCE",
"CLIENT_ID",
"RELYING_PARTY_ID",
"USERNAME_CLAIM",
]

deprecated_settings = {
"AUTHORIZE_PATH": "This setting is automatically loaded from ADFS.",
"ISSUER": "This setting is automatically loaded from ADFS.",
Expand All @@ -102,72 +102,91 @@ def __init__(self):
if "SETTINGS_CLASS" in _settings:
del _settings["SETTINGS_CLASS"]

# Handle deprecated settings
for setting, message in deprecated_settings.items():
if setting in _settings:
warnings.warn("Setting {} is deprecated and it's value was ignored. {}".format(setting, message),
if "IDPS" in _settings:
if any(item in REQUIRED_SETTINGS for item in _settings):
raise ImproperlyConfigured(
"The IDPS configuration cannot be set when any of these {} are set".format(REQUIRED_SETTINGS))

if not len(_settings['IDPS']):
raise ImproperlyConfigured(
"The IDPS configuration must have at least one configuration defined.")

# Deprecated settings are not supported in IDPS

for idp_name, idp_settings in _settings['IDPS'].items():
for setting in REQUIRED_SETTINGS:
if setting not in idp_settings:
raise ImproperlyConfigured(
"django_auth_adfs setting '{}' has not been set for IDP key '{}'".format(setting, idp_name)
)

else:
# Handle deprecated settings
for setting, message in deprecated_settings.items():
if setting in _settings:
warnings.warn("Setting {} is deprecated and it's value was ignored. {}".format(setting, message),
DeprecationWarning)
del _settings[setting]

if "CERT_MAX_AGE" in _settings:
_settings["CONFIG_RELOAD_INTERVAL"] = _settings["CERT_MAX_AGE"]
warnings.warn('Setting CERT_MAX_AGE has been renamed to CONFIG_RELOAD_INTERVAL. The value was copied.',
DeprecationWarning)
del _settings[setting]

if "CERT_MAX_AGE" in _settings:
_settings["CONFIG_RELOAD_INTERVAL"] = _settings["CERT_MAX_AGE"]
warnings.warn('Setting CERT_MAX_AGE has been renamed to CONFIG_RELOAD_INTERVAL. The value was copied.',
DeprecationWarning)
del _settings["CERT_MAX_AGE"]

if "GROUP_CLAIM" in _settings:
_settings["GROUPS_CLAIM"] = _settings["GROUP_CLAIM"]
warnings.warn('Setting GROUP_CLAIM has been renamed to GROUPS_CLAIM. The value was copied.',
DeprecationWarning)
del _settings["GROUP_CLAIM"]

if "RESOURCE" in _settings:
_settings["RELYING_PARTY_ID"] = _settings["RESOURCE"]
del _settings["RESOURCE"]
if "TENANT_ID" in _settings:
# If a tenant ID was set, switch to Azure AD mode
if "SERVER" in _settings:
raise ImproperlyConfigured("The SERVER cannot be set when TENANT_ID is set.")
self.SERVER = AZURE_AD_SERVER
self.USERNAME_CLAIM = "upn"
self.GROUPS_CLAIM = "groups"
self.CLAIM_MAPPING = {"first_name": "given_name",
"last_name": "family_name",
"email": "email"}
elif "VERSION" in _settings:
raise ImproperlyConfigured("The VERSION cannot be set when TENANT_ID is set.")

# Overwrite defaults with user settings
for setting, value in _settings.items():
if hasattr(self, setting):
setattr(self, setting, value)
else:
msg = "'{0}' is not a valid configuration directive for django_auth_adfs."
raise ImproperlyConfigured(msg.format(setting))
del _settings["CERT_MAX_AGE"]

if "GROUP_CLAIM" in _settings:
_settings["GROUPS_CLAIM"] = _settings["GROUP_CLAIM"]
warnings.warn('Setting GROUP_CLAIM has been renamed to GROUPS_CLAIM. The value was copied.',
DeprecationWarning)
del _settings["GROUP_CLAIM"]

if "RESOURCE" in _settings:
_settings["RELYING_PARTY_ID"] = _settings["RESOURCE"]
del _settings["RESOURCE"]

if "TENANT_ID" in _settings:
# If a tenant ID was set, switch to Azure AD mode
if "SERVER" in _settings:
raise ImproperlyConfigured("The SERVER cannot be set when TENANT_ID is set.")
self.SERVER = AZURE_AD_SERVER
self.USERNAME_CLAIM = "upn"
self.GROUPS_CLAIM = "groups"
self.CLAIM_MAPPING = {"first_name": "given_name",
"last_name": "family_name",
"email": "email"}
elif "VERSION" in _settings:
raise ImproperlyConfigured("The VERSION cannot be set when TENANT_ID is set.")

# Overwrite defaults with user settings
for setting, value in _settings.items():
if hasattr(self, setting):
setattr(self, setting, value)
else:
msg = "'{0}' is not a valid configuration directive for django_auth_adfs."
raise ImproperlyConfigured(msg.format(setting))

if self.SERVER != AZURE_AD_SERVER and self.BLOCK_GUEST_USERS:
raise ImproperlyConfigured("You can only set BLOCK_GUEST_USERS when self.TENANT_ID is set")

if self.SERVER != AZURE_AD_SERVER and self.BLOCK_GUEST_USERS:
raise ImproperlyConfigured("You can only set BLOCK_GUEST_USERS when self.TENANT_ID is set")
if self.TENANT_ID is None:
# For on premises ADFS, the tenant ID is set to adfs
# On AzureAD the adfs part in the URL happens to be replace by the tenant ID.
self.TENANT_ID = "adfs"

if self.TENANT_ID is None:
# For on premises ADFS, the tenant ID is set to adfs
# On AzureAD the adfs part in the URL happens to be replace by the tenant ID.
self.TENANT_ID = "adfs"
for setting in REQUIRED_SETTINGS:
if not getattr(self, setting):
raise ImproperlyConfigured("django_auth_adfs setting '{0}' has not been set".format(setting))

for setting in required_settings:
if not getattr(self, setting):
msg = "django_auth_adfs setting '{0}' has not been set".format(setting)
raise ImproperlyConfigured(msg)
# Validate setting conflicts
usermodel = get_user_model()
if usermodel.USERNAME_FIELD in self.CLAIM_MAPPING:
raise ImproperlyConfigured("You cannot set the username field of the user model from "
"the CLAIM_MAPPING setting. Instead use the USERNAME_CLAIM setting.")

# Setup dynamic settings
if not callable(self.CUSTOM_FAILED_RESPONSE_VIEW):
self.CUSTOM_FAILED_RESPONSE_VIEW = import_string(self.CUSTOM_FAILED_RESPONSE_VIEW)

# Validate setting conflicts
usermodel = get_user_model()
if usermodel.USERNAME_FIELD in self.CLAIM_MAPPING:
raise ImproperlyConfigured("You cannot set the username field of the user model from "
"the CLAIM_MAPPING setting. Instead use the USERNAME_CLAIM setting.")


class ProviderConfig(object):
def __init__(self):
Expand Down
43 changes: 42 additions & 1 deletion tests/test_settings.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from django.test import TestCase, SimpleTestCase, override_settings
from mock import patch
from django_auth_adfs.config import django_settings
from django_auth_adfs.config import Settings
from django_auth_adfs.config import Settings, REQUIRED_SETTINGS
from .custom_config import Settings as CustomSettings


Expand All @@ -24,6 +24,47 @@ def test_claim_mapping_overlapping_username_field(self):
with self.assertRaises(ImproperlyConfigured):
Settings()

def test_idps_as_mutually_exclusive(self):
settings = deepcopy(django_settings)
settings.AUTH_ADFS["IDPS"] = {}
with patch("django_auth_adfs.config.django_settings", settings):
with self.assertRaises(ImproperlyConfigured):
Settings()

def test_idps_empty_entries(self):
settings = deepcopy(django_settings)
for setting in REQUIRED_SETTINGS:
if setting in settings.AUTH_ADFS:
del settings.AUTH_ADFS[setting]
settings.AUTH_ADFS["IDPS"] = {}
with patch("django_auth_adfs.config.django_settings", settings):
with self.assertRaises(ImproperlyConfigured) as cm:
Settings()

self.assertEqual(
str(cm.exception),
"The IDPS configuration must have at least one configuration defined."
)

def test_idps_missing_required_settings_in_entry(self):
settings = deepcopy(django_settings)
for setting in REQUIRED_SETTINGS:
if setting in settings.AUTH_ADFS:
del settings.AUTH_ADFS[setting]
settings.AUTH_ADFS["IDPS"] = {
"adfs": {
"CLIENT_ID": "abc"
}
}
with patch("django_auth_adfs.config.django_settings", settings):
with self.assertRaises(ImproperlyConfigured) as cm:
Settings()

self.assertEqual(
str(cm.exception),
"django_auth_adfs setting 'AUDIENCE' has not been set for IDP key 'adfs'"
)

def test_tenant_and_server(self):
settings = deepcopy(django_settings)
settings.AUTH_ADFS["TENANT_ID"] = "abc"
Expand Down