From c5ae46c2a53771bf1bfaf39cd69794807f1ad298 Mon Sep 17 00:00:00 2001 From: Lyndon Garvey Date: Wed, 17 Oct 2018 13:29:57 +0100 Subject: [PATCH 01/13] Fix login view parses the authn request using correct binding --- djangosaml2idp/views.py | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/djangosaml2idp/views.py b/djangosaml2idp/views.py index 1f10a0d..11fe409 100644 --- a/djangosaml2idp/views.py +++ b/djangosaml2idp/views.py @@ -15,7 +15,7 @@ from django.views.decorators.cache import never_cache from django.views.decorators.csrf import csrf_exempt from django.views.decorators.http import require_http_methods -from saml2 import BINDING_HTTP_POST +from saml2 import BINDING_HTTP_POST, BINDING_HTTP_REDIRECT from saml2.authn_context import PASSWORD, AuthnBroker, authn_context_class_ref from saml2.config import IdPConfig from saml2.ident import NameID @@ -42,7 +42,15 @@ def sso_entry(request): """ Entrypoint view for SSO. Gathers the parameters from the HTTP request, stores them in the session and redirects the requester to the login_process view. """ - passed_data = request.POST if request.method == 'POST' else request.GET + if request.method == 'POST': + passed_data = request.POST + binding = BINDING_HTTP_POST + else: + passed_data = request.GET + binding = BINDING_HTTP_REDIRECT + + request.session['Binding'] = binding + try: request.session['SAMLRequest'] = passed_data['SAMLRequest'] except (KeyError, MultiValueDictKeyError) as e: @@ -99,9 +107,11 @@ class LoginProcessView(LoginRequiredMixin, IdPHandlerViewMixin, View): """ def get(self, request, *args, **kwargs): + binding = request.session.get('Binding', BINDING_HTTP_POST) + # Parse incoming request try: - req_info = self.IDP.parse_authn_request(request.session['SAMLRequest'], BINDING_HTTP_POST) + req_info = self.IDP.parse_authn_request(request.session['SAMLRequest'], binding) except Exception as excp: return self.handle_error(request, exception=excp) # TODO this is taken from example, but no idea how this works or whats it does. Check SAML2 specification? From 470e8881b56976eefd640ceddf4583f55a66c7aa Mon Sep 17 00:00:00 2001 From: Lyndon Garvey Date: Mon, 3 Dec 2018 14:56:07 +0000 Subject: [PATCH 02/13] Improve processor logic Pass in the SP entity_id BaseProcessor was not being instantiated Pass extra config into the processor.create_identity method - this allows for providing additional params required by AWS IAM Saml2 This enables the staff-sso app to use as model based processor which can reference saml2 data config based on entity_id --- djangosaml2idp/processors.py | 6 +++++- djangosaml2idp/views.py | 14 +++++++------- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/djangosaml2idp/processors.py b/djangosaml2idp/processors.py index cc99834..6c6537b 100644 --- a/djangosaml2idp/processors.py +++ b/djangosaml2idp/processors.py @@ -3,6 +3,9 @@ class BaseProcessor: and to construct the identity dictionary which is sent to the SP """ + def __init__(self, entity_id): + self._entity_id = entity_id + def has_access(self, user): """ Check if this user is allowed to use this IDP """ @@ -13,9 +16,10 @@ def enable_multifactor(self, user): """ return False - def create_identity(self, user, sp_mapping): + def create_identity(self, user, sp_mapping, **extra_config): """ Generate an identity dictionary of the user based on the given mapping of desired user attributes by the SP """ + return { out_attr: getattr(user, user_attr) for user_attr, out_attr in sp_mapping.items() diff --git a/djangosaml2idp/views.py b/djangosaml2idp/views.py index 1f10a0d..7ee1527 100644 --- a/djangosaml2idp/views.py +++ b/djangosaml2idp/views.py @@ -74,22 +74,22 @@ def dispatch(self, request, *args, **kwargs): return self.handle_error(request, exception=e) return super(IdPHandlerViewMixin, self).dispatch(request, *args, **kwargs) - def get_processor(self, sp_config): + def get_processor(self, entity_id, sp_config): """ "Instantiate user-specified processor or fallback to all-access base processor """ processor_string = sp_config.get('processor', None) if processor_string: try: - return import_string(processor_string)() + return import_string(processor_string)(entity_id) except Exception as e: logger.error("Failed to instantiate processor: {} - {}".format(processor_string, e), exc_info=True) - return BaseProcessor + return BaseProcessor(entity_id) def get_identity(self, processor, user, sp_config): """ Create Identity dict (using SP-specific mapping) """ sp_mapping = sp_config.get('attribute_mapping', {'username': 'username'}) - return processor.create_identity(user, sp_mapping) + return processor.create_identity(user, sp_mapping, **sp_config.get('extra_data', {})) @method_decorator(never_cache, name='dispatch') @@ -129,7 +129,7 @@ def get(self, request, *args, **kwargs): except Exception: return self.handle_error(request, exception=ImproperlyConfigured("No config for SP %s defined in SAML_IDP_SPCONFIG" % resp_args['sp_entity_id']), status=400) - processor = self.get_processor(sp_config) + processor = self.get_processor(resp_args['sp_entity_id'], sp_config) # Check if user has access to the service of this SP if not processor.has_access(request.user): @@ -201,8 +201,8 @@ def get(self, request, *args, **kwargs): service="assertion_consumer_service", entity_id=sp_entity_id) - processor = self.get_processor(sp_config) - + processor = self.get_processor(sp_entity_id, sp_config) + import pdb; pdb.set_trace() # Check if user has access to the service of this SP if not processor.has_access(request.user): return self.handle_error(request, exception=PermissionDenied("You do not have access to this resource"), status=403) From d4395fe09390c8dea317ac3621427451a2b8c447 Mon Sep 17 00:00:00 2001 From: Lyndon Garvey Date: Mon, 3 Dec 2018 15:30:03 +0000 Subject: [PATCH 03/13] Remove pdb --- djangosaml2idp/views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/djangosaml2idp/views.py b/djangosaml2idp/views.py index 7ee1527..8b20e38 100644 --- a/djangosaml2idp/views.py +++ b/djangosaml2idp/views.py @@ -202,7 +202,7 @@ def get(self, request, *args, **kwargs): entity_id=sp_entity_id) processor = self.get_processor(sp_entity_id, sp_config) - import pdb; pdb.set_trace() + # Check if user has access to the service of this SP if not processor.has_access(request.user): return self.handle_error(request, exception=PermissionDenied("You do not have access to this resource"), status=403) From 06da0c3514d4f11d060a97dea450273d9d2b1b60 Mon Sep 17 00:00:00 2001 From: Lyndon Garvey Date: Mon, 3 Dec 2018 15:38:56 +0000 Subject: [PATCH 04/13] Honour the CustomUser.USERNAME_FIELD for lookups --- djangosaml2idp/views.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/djangosaml2idp/views.py b/djangosaml2idp/views.py index 8b20e38..7e8301c 100644 --- a/djangosaml2idp/views.py +++ b/djangosaml2idp/views.py @@ -142,11 +142,13 @@ def get(self, request, *args, **kwargs): AUTHN_BROKER = AuthnBroker() AUTHN_BROKER.add(authn_context_class_ref(req_authn_context), "") + username = getattr(request.user, getattr(request.user, 'USERNAME_FIELD', 'username')) + # Construct SamlResponse message try: authn_resp = self.IDP.create_authn_response( - identity=identity, userid=request.user.username, - name_id=NameID(format=resp_args['name_id_policy'].format, sp_name_qualifier=resp_args['destination'], text=request.user.username), + identity=identity, userid=username, + name_id=NameID(format=resp_args['name_id_policy'].format, sp_name_qualifier=resp_args['destination'], text=username), authn=AUTHN_BROKER.get_authn_by_accr(req_authn_context), sign_response=self.IDP.config.getattr("sign_response", "idp") or False, sign_assertion=self.IDP.config.getattr("sign_assertion", "idp") or False, @@ -213,10 +215,12 @@ def get(self, request, *args, **kwargs): AUTHN_BROKER = AuthnBroker() AUTHN_BROKER.add(authn_context_class_ref(req_authn_context), "") + username = getattr(request.user, getattr(request.user, 'USERNAME_FIELD', 'username')) + # Construct SamlResponse messages try: name_id_formats = self.IDP.config.getattr("name_id_format", "idp") or [NAMEID_FORMAT_UNSPECIFIED] - name_id = NameID(format=name_id_formats[0], text=request.user.username) + name_id = NameID(format=name_id_formats[0], text=username) authn = AUTHN_BROKER.get_authn_by_accr(req_authn_context) sign_response = self.IDP.config.getattr("sign_response", "idp") or False sign_assertion = self.IDP.config.getattr("sign_assertion", "idp") or False @@ -225,7 +229,7 @@ def get(self, request, *args, **kwargs): in_response_to="IdP_Initiated_Login", destination=destination, sp_entity_id=sp_entity_id, - userid=request.user.username, + userid=username, name_id=name_id, authn=authn, sign_response=sign_response, From 9c87dfc084c7f3484163574ae6789de7ceb4d597 Mon Sep 17 00:00:00 2001 From: Lyndon Garvey Date: Mon, 3 Dec 2018 16:20:50 +0000 Subject: [PATCH 05/13] Failure to load processor should not pass silently --- djangosaml2idp/views.py | 1 + 1 file changed, 1 insertion(+) diff --git a/djangosaml2idp/views.py b/djangosaml2idp/views.py index 7e8301c..1038bc8 100644 --- a/djangosaml2idp/views.py +++ b/djangosaml2idp/views.py @@ -83,6 +83,7 @@ def get_processor(self, entity_id, sp_config): return import_string(processor_string)(entity_id) except Exception as e: logger.error("Failed to instantiate processor: {} - {}".format(processor_string, e), exc_info=True) + raise return BaseProcessor(entity_id) def get_identity(self, processor, user, sp_config): From cc36000e96c8ad1d4aa95f762c3a664472243a6e Mon Sep 17 00:00:00 2001 From: Lyndon Garvey Date: Tue, 4 Dec 2018 15:40:59 +0000 Subject: [PATCH 06/13] Rename IDP extra_data key to extra_config --- djangosaml2idp/views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/djangosaml2idp/views.py b/djangosaml2idp/views.py index 1038bc8..bde55f9 100644 --- a/djangosaml2idp/views.py +++ b/djangosaml2idp/views.py @@ -90,7 +90,7 @@ def get_identity(self, processor, user, sp_config): """ Create Identity dict (using SP-specific mapping) """ sp_mapping = sp_config.get('attribute_mapping', {'username': 'username'}) - return processor.create_identity(user, sp_mapping, **sp_config.get('extra_data', {})) + return processor.create_identity(user, sp_mapping, **sp_config.get('extra_config', {})) @method_decorator(never_cache, name='dispatch') From ee8986435375447eaea9309f639ae81eaabcf748 Mon Sep 17 00:00:00 2001 From: Lyndon Garvey Date: Wed, 5 Dec 2018 17:30:41 +0000 Subject: [PATCH 07/13] Add logic to allow `settings.SAML_IDP_DJANGO_USER_MAIN_ATTRIBUTE` --- djangosaml2idp/views.py | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/djangosaml2idp/views.py b/djangosaml2idp/views.py index bde55f9..507ade7 100644 --- a/djangosaml2idp/views.py +++ b/djangosaml2idp/views.py @@ -92,6 +92,11 @@ def get_identity(self, processor, user, sp_config): sp_mapping = sp_config.get('attribute_mapping', {'username': 'username'}) return processor.create_identity(user, sp_mapping, **sp_config.get('extra_config', {})) + def extract_user_id(self, user): + user_field = getattr(settings, 'SAML_IDP_DJANGO_USER_MAIN_ATTRIBUTE', None) or \ + getattr(user, 'USERNAME_FIELD', 'username') + return str(getattr(user, user_field)) + @method_decorator(never_cache, name='dispatch') class LoginProcessView(LoginRequiredMixin, IdPHandlerViewMixin, View): @@ -143,13 +148,13 @@ def get(self, request, *args, **kwargs): AUTHN_BROKER = AuthnBroker() AUTHN_BROKER.add(authn_context_class_ref(req_authn_context), "") - username = getattr(request.user, getattr(request.user, 'USERNAME_FIELD', 'username')) + user_id = self.extract_user_id(request.user) # Construct SamlResponse message try: authn_resp = self.IDP.create_authn_response( - identity=identity, userid=username, - name_id=NameID(format=resp_args['name_id_policy'].format, sp_name_qualifier=resp_args['destination'], text=username), + identity=identity, userid=user_id, + name_id=NameID(format=resp_args['name_id_policy'].format, sp_name_qualifier=resp_args['destination'], text=user_id), authn=AUTHN_BROKER.get_authn_by_accr(req_authn_context), sign_response=self.IDP.config.getattr("sign_response", "idp") or False, sign_assertion=self.IDP.config.getattr("sign_assertion", "idp") or False, @@ -216,12 +221,12 @@ def get(self, request, *args, **kwargs): AUTHN_BROKER = AuthnBroker() AUTHN_BROKER.add(authn_context_class_ref(req_authn_context), "") - username = getattr(request.user, getattr(request.user, 'USERNAME_FIELD', 'username')) + user_id = self.extract_user_id(request.user) # Construct SamlResponse messages try: name_id_formats = self.IDP.config.getattr("name_id_format", "idp") or [NAMEID_FORMAT_UNSPECIFIED] - name_id = NameID(format=name_id_formats[0], text=username) + name_id = NameID(format=name_id_formats[0], text=user_id) authn = AUTHN_BROKER.get_authn_by_accr(req_authn_context) sign_response = self.IDP.config.getattr("sign_response", "idp") or False sign_assertion = self.IDP.config.getattr("sign_assertion", "idp") or False @@ -230,7 +235,7 @@ def get(self, request, *args, **kwargs): in_response_to="IdP_Initiated_Login", destination=destination, sp_entity_id=sp_entity_id, - userid=username, + userid=user_id, name_id=name_id, authn=authn, sign_response=sign_response, From a50fd636b0f011af46b0e441aa5e4fdeb7be13b9 Mon Sep 17 00:00:00 2001 From: Lyndon Garvey Date: Mon, 10 Dec 2018 14:50:16 +0000 Subject: [PATCH 08/13] Add processor.is_enabled() to allow for additional checks --- djangosaml2idp/processors.py | 4 ++++ djangosaml2idp/views.py | 6 +++--- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/djangosaml2idp/processors.py b/djangosaml2idp/processors.py index 6c6537b..6d45aa8 100644 --- a/djangosaml2idp/processors.py +++ b/djangosaml2idp/processors.py @@ -11,6 +11,10 @@ def has_access(self, user): """ return True + def is_enabled(self, request): + """Is this saml2 integration enabled""" + return True + def enable_multifactor(self, user): """ Check if this user should use a second authentication system """ diff --git a/djangosaml2idp/views.py b/djangosaml2idp/views.py index 507ade7..ece0783 100644 --- a/djangosaml2idp/views.py +++ b/djangosaml2idp/views.py @@ -93,7 +93,7 @@ def get_identity(self, processor, user, sp_config): return processor.create_identity(user, sp_mapping, **sp_config.get('extra_config', {})) def extract_user_id(self, user): - user_field = getattr(settings, 'SAML_IDP_DJANGO_USER_MAIN_ATTRIBUTE', None) or \ + user_field = getattr(settings, 'SAML_IDP_DJANGO_USERNAME_FIELD', None) or \ getattr(user, 'USERNAME_FIELD', 'username') return str(getattr(user, user_field)) @@ -138,7 +138,7 @@ def get(self, request, *args, **kwargs): processor = self.get_processor(resp_args['sp_entity_id'], sp_config) # Check if user has access to the service of this SP - if not processor.has_access(request.user): + if not processor.has_access(request.user) or not processor.is_enabled(request): return self.handle_error(request, exception=PermissionDenied("You do not have access to this resource"), status=403) identity = self.get_identity(processor, request.user, sp_config) @@ -212,7 +212,7 @@ def get(self, request, *args, **kwargs): processor = self.get_processor(sp_entity_id, sp_config) # Check if user has access to the service of this SP - if not processor.has_access(request.user): + if not processor.has_access(request.user) or not processor.is_enabled(request): return self.handle_error(request, exception=PermissionDenied("You do not have access to this resource"), status=403) identity = self.get_identity(processor, request.user, sp_config) From fdbe0043045dd1cd742f3d34fc4764c28214c8db Mon Sep 17 00:00:00 2001 From: Lyndon Garvey Date: Mon, 10 Dec 2018 14:52:13 +0000 Subject: [PATCH 09/13] .gitignore virtualenv and .idea dirs --- .gitignore | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 8035d84..893ddbe 100644 --- a/.gitignore +++ b/.gitignore @@ -9,6 +9,7 @@ *.db .env +env/ .cache .coverage @@ -18,4 +19,6 @@ build/ dist/ _build/ -tags \ No newline at end of file +tags + +.idea/ From db88da3b9167c0ac253f18d06b0fb80341772d63 Mon Sep 17 00:00:00 2001 From: Lyndon Garvey Date: Mon, 10 Dec 2018 14:57:35 +0000 Subject: [PATCH 10/13] Fix up test and add some basic tests (more to follow) --- README.rst | 11 +++++++ pytest.ini | 5 +++ requirements-dev.txt | 5 +++ runtests.py | 15 --------- tests/settings.py | 17 +++++++---- tests/test_views.py | 72 ++++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 104 insertions(+), 21 deletions(-) delete mode 100644 runtests.py create mode 100644 tests/test_views.py diff --git a/README.rst b/README.rst index ebdd401..e36cd84 100644 --- a/README.rst +++ b/README.rst @@ -47,6 +47,17 @@ Now you can install the djangosaml2idp package using pip. This will also install pip install djangosaml2idp +Running the test suite +====================== +Install the dev dependencies in ``requirements-dev.txt``:: + + pip install -r requirements-dev.txt + +Run ``py.test`` from the project root:: + + py.test + + Configuration & Usage ===================== diff --git a/pytest.ini b/pytest.ini index 7dffe55..9b6f225 100644 --- a/pytest.ini +++ b/pytest.ini @@ -5,3 +5,8 @@ skip = build/*,dist/*,docs/*,*/manage.py [pep8] ignore = C0111,C0301,E122,E127,E128,E131,E501,E502,E722,E731,W605 + +[pytest] +django_find_project = false +norecursedirs = env +DJANGO_SETTINGS_MODULE=tests.settings diff --git a/requirements-dev.txt b/requirements-dev.txt index d29b092..9abf3a9 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -1,2 +1,7 @@ django>2.0,<2.1 pysaml2>=4.4.0 + +pytest==4.0.1 +pytest-django==3.4.4 +pytest-pythonpath==0.7.3 +python-dateutil==2.7.5 diff --git a/runtests.py b/runtests.py deleted file mode 100644 index 0c2ef30..0000000 --- a/runtests.py +++ /dev/null @@ -1,15 +0,0 @@ -#!/usr/bin/env python -import os -import sys - -import django -from django.conf import settings -from django.test.utils import get_runner - -if __name__ == "__main__": - os.environ['DJANGO_SETTINGS_MODULE'] = 'tests.settings' - django.setup() - TestRunner = get_runner(settings) - test_runner = TestRunner() - failures = test_runner.run_tests(["tests"]) - sys.exit(bool(failures)) diff --git a/tests/settings.py b/tests/settings.py index 8d8fe7f..4f6b538 100644 --- a/tests/settings.py +++ b/tests/settings.py @@ -6,12 +6,12 @@ DATABASES = { 'default': { - 'ENGINE': 'django.db.backends.sqlite3', # Add 'postgresql_psycopg2', 'postgresql', 'mysql', 'sqlite3' or 'oracle'. - 'NAME': './idptest.sqlite', # Or path to database file if using sqlite3. - 'USER': '', # Not used with sqlite3. - 'PASSWORD': '', # Not used with sqlite3. - 'HOST': '', # Set to empty string for localhost. Not used with sqlite3. - 'PORT': '', # Set to empty string for default. Not used with sqlite3. + 'ENGINE': 'django.db.backends.sqlite3', + 'NAME': './idptest.sqlite', + 'USER': '', + 'PASSWORD': '', + 'HOST': '', + 'PORT': '', } } @@ -42,3 +42,8 @@ ) ROOT_URLCONF = 'tests.urls' + +# --- + +SAML_IDP_SPCONFIG = { +} diff --git a/tests/test_views.py b/tests/test_views.py new file mode 100644 index 0000000..bb327af --- /dev/null +++ b/tests/test_views.py @@ -0,0 +1,72 @@ +import pytest + +from django.contrib.auth import get_user_model + +from djangosaml2idp.views import IdPHandlerViewMixin + +from djangosaml2idp.processors import BaseProcessor + + +User = get_user_model() + + +class CustomProcessor(BaseProcessor): + pass + + +class TestIdPHandlerViewMixin: + def test_get_identity_provides_extra_config(self): + obj = IdPHandlerViewMixin() + + def test_extract_user_id_configure_by_user_class(self): + + user = User() + user.USERNAME_FIELD = 'email' + user.email = 'test_email' + + assert IdPHandlerViewMixin().extract_user_id(user) == 'test_email' + + def test_extract_user_id_configure_by_settings(self, settings): + """Should use `settings.SAML_IDP_DJANGO_USERNAME_FIELD` to determine the user id field""" + + settings.SAML_IDP_DJANGO_USERNAME_FIELD = 'first_name' + + user = User() + user.first_name = 'test_first_name' + + assert IdPHandlerViewMixin().extract_user_id(user) == 'test_first_name' + + def test_get_processor_errors_if_processor_cannot_be_loaded(self): + sp_config = { + 'processor': 'this.does.not.exist' + } + + with pytest.raises(Exception): + IdPHandlerViewMixin().get_processor('entity_id', sp_config) + + def test_get_processor_defaults_to_base_processor(self): + sp_config = { + } + + assert isinstance(IdPHandlerViewMixin().get_processor('entity_id', sp_config), BaseProcessor) + + def test_get_processor_loads_custom_processor(self): + sp_config = { + 'processor': 'tests.test_views.CustomProcessor' + } + + assert isinstance(IdPHandlerViewMixin().get_processor('entity_id', sp_config), CustomProcessor) + + +class TestIdpInitiatedFlow: + pass + + +class TestMetadata: + pass + + +class LoginFlow: + def test_requires_authentication(self): + """test redriect to settings.LOGIN_VIEW""" + pass From c321a8757308cda718dc3bb4b0a76dd7ba2a62a4 Mon Sep 17 00:00:00 2001 From: Lyndon Garvey Date: Wed, 12 Dec 2018 14:18:42 +0000 Subject: [PATCH 11/13] Move user id retrieval logic to processor --- djangosaml2idp/processors.py | 8 ++++++++ djangosaml2idp/views.py | 9 ++------- tests/settings.py | 3 +++ tests/test_processor.py | 29 +++++++++++++++++++++++++++++ tests/test_views.py | 18 ------------------ 5 files changed, 42 insertions(+), 25 deletions(-) create mode 100644 tests/test_processor.py diff --git a/djangosaml2idp/processors.py b/djangosaml2idp/processors.py index 6d45aa8..49c8ec5 100644 --- a/djangosaml2idp/processors.py +++ b/djangosaml2idp/processors.py @@ -1,3 +1,6 @@ +from django.conf import settings + + class BaseProcessor: """ Processor class is used to determine if a user has access to a client service of this IDP and to construct the identity dictionary which is sent to the SP @@ -20,6 +23,11 @@ def enable_multifactor(self, user): """ return False + def get_user_id(self, user): + user_field = getattr(settings, 'SAML_IDP_DJANGO_USERNAME_FIELD', None) or \ + getattr(user, 'USERNAME_FIELD', 'username') + return str(getattr(user, user_field)) + def create_identity(self, user, sp_mapping, **extra_config): """ Generate an identity dictionary of the user based on the given mapping of desired user attributes by the SP """ diff --git a/djangosaml2idp/views.py b/djangosaml2idp/views.py index ece0783..11e2459 100644 --- a/djangosaml2idp/views.py +++ b/djangosaml2idp/views.py @@ -92,11 +92,6 @@ def get_identity(self, processor, user, sp_config): sp_mapping = sp_config.get('attribute_mapping', {'username': 'username'}) return processor.create_identity(user, sp_mapping, **sp_config.get('extra_config', {})) - def extract_user_id(self, user): - user_field = getattr(settings, 'SAML_IDP_DJANGO_USERNAME_FIELD', None) or \ - getattr(user, 'USERNAME_FIELD', 'username') - return str(getattr(user, user_field)) - @method_decorator(never_cache, name='dispatch') class LoginProcessView(LoginRequiredMixin, IdPHandlerViewMixin, View): @@ -148,7 +143,7 @@ def get(self, request, *args, **kwargs): AUTHN_BROKER = AuthnBroker() AUTHN_BROKER.add(authn_context_class_ref(req_authn_context), "") - user_id = self.extract_user_id(request.user) + user_id = processor.get_user_id(request.user) # Construct SamlResponse message try: @@ -221,7 +216,7 @@ def get(self, request, *args, **kwargs): AUTHN_BROKER = AuthnBroker() AUTHN_BROKER.add(authn_context_class_ref(req_authn_context), "") - user_id = self.extract_user_id(request.user) + user_id = processor.get_user_id(request.user) # Construct SamlResponse messages try: diff --git a/tests/settings.py b/tests/settings.py index 4f6b538..e48bcc3 100644 --- a/tests/settings.py +++ b/tests/settings.py @@ -46,4 +46,7 @@ # --- SAML_IDP_SPCONFIG = { + 'test_idp_1': { + 'attribute_mapping': {} + } } diff --git a/tests/test_processor.py b/tests/test_processor.py new file mode 100644 index 0000000..5b440df --- /dev/null +++ b/tests/test_processor.py @@ -0,0 +1,29 @@ +import pytest + +from django.contrib.auth import get_user_model + +from djangosaml2idp.processors import BaseProcessor + + +User = get_user_model() + + +class TestBaseProcessor: + + def test_extract_user_id_configure_by_user_class(self): + + user = User() + user.USERNAME_FIELD = 'email' + user.email = 'test_email' + + assert BaseProcessor('entity-id').get_user_id(user) == 'test_email' + + def test_extract_user_id_configure_by_settings(self, settings): + """Should use `settings.SAML_IDP_DJANGO_USERNAME_FIELD` to determine the user id field""" + + settings.SAML_IDP_DJANGO_USERNAME_FIELD = 'first_name' + + user = User() + user.first_name = 'test_first_name' + + assert BaseProcessor('entity-id').get_user_id(user) == 'test_first_name' diff --git a/tests/test_views.py b/tests/test_views.py index bb327af..9170495 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -18,24 +18,6 @@ class TestIdPHandlerViewMixin: def test_get_identity_provides_extra_config(self): obj = IdPHandlerViewMixin() - def test_extract_user_id_configure_by_user_class(self): - - user = User() - user.USERNAME_FIELD = 'email' - user.email = 'test_email' - - assert IdPHandlerViewMixin().extract_user_id(user) == 'test_email' - - def test_extract_user_id_configure_by_settings(self, settings): - """Should use `settings.SAML_IDP_DJANGO_USERNAME_FIELD` to determine the user id field""" - - settings.SAML_IDP_DJANGO_USERNAME_FIELD = 'first_name' - - user = User() - user.first_name = 'test_first_name' - - assert IdPHandlerViewMixin().extract_user_id(user) == 'test_first_name' - def test_get_processor_errors_if_processor_cannot_be_loaded(self): sp_config = { 'processor': 'this.does.not.exist' From 135c8a9157957e3306188ccd1f7a905a1a5c7f39 Mon Sep 17 00:00:00 2001 From: Lyndon Garvey Date: Wed, 12 Dec 2018 16:11:54 +0000 Subject: [PATCH 12/13] Built in exeptions don't have __module__ --- djangosaml2idp/error_views.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/djangosaml2idp/error_views.py b/djangosaml2idp/error_views.py index 737418d..a50485b 100644 --- a/djangosaml2idp/error_views.py +++ b/djangosaml2idp/error_views.py @@ -9,8 +9,9 @@ class SamlIDPErrorView(TemplateView): def get_context_data(self, **kwargs): context = super().get_context_data(**kwargs) exception = kwargs.get("exception") + context.update({ - "exception_type": ".".join([exception.__module__, exception.__class__.__name__]) if exception else None, + "exception_type": exception.__class__.__name__ if exception else None, "exception_msg": str(exception) if exception else None, "extra_message": kwargs.get("extra_message"), }) From 2b8ea7c92ed5a8404027667208dfc253afc0e869 Mon Sep 17 00:00:00 2001 From: Lyndon Garvey Date: Fri, 11 Jan 2019 17:43:24 +0000 Subject: [PATCH 13/13] Remove Processor.is_enabled and pass request into has_access --- djangosaml2idp/processors.py | 6 +----- djangosaml2idp/views.py | 4 ++-- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/djangosaml2idp/processors.py b/djangosaml2idp/processors.py index 49c8ec5..b138413 100644 --- a/djangosaml2idp/processors.py +++ b/djangosaml2idp/processors.py @@ -9,15 +9,11 @@ class BaseProcessor: def __init__(self, entity_id): self._entity_id = entity_id - def has_access(self, user): + def has_access(self, request): """ Check if this user is allowed to use this IDP """ return True - def is_enabled(self, request): - """Is this saml2 integration enabled""" - return True - def enable_multifactor(self, user): """ Check if this user should use a second authentication system """ diff --git a/djangosaml2idp/views.py b/djangosaml2idp/views.py index 069eee4..5eb3493 100644 --- a/djangosaml2idp/views.py +++ b/djangosaml2idp/views.py @@ -143,7 +143,7 @@ def get(self, request, *args, **kwargs): processor = self.get_processor(resp_args['sp_entity_id'], sp_config) # Check if user has access to the service of this SP - if not processor.has_access(request.user) or not processor.is_enabled(request): + if not processor.has_access(request): return self.handle_error(request, exception=PermissionDenied("You do not have access to this resource"), status=403) identity = self.get_identity(processor, request.user, sp_config) @@ -217,7 +217,7 @@ def get(self, request, *args, **kwargs): processor = self.get_processor(sp_entity_id, sp_config) # Check if user has access to the service of this SP - if not processor.has_access(request.user) or not processor.is_enabled(request): + if not processor.has_access(request): return self.handle_error(request, exception=PermissionDenied("You do not have access to this resource"), status=403) identity = self.get_identity(processor, request.user, sp_config)