From a36f41ef9d170c459e5248cd23c3c86cab0f367c Mon Sep 17 00:00:00 2001 From: Valentin Deniaud Date: Thu, 28 Jul 2022 15:50:20 +0200 Subject: [PATCH 4/7] auth_saml: add roles using model and remove useless code (#67025) --- src/authentic2_auth_saml/adapters.py | 133 +++------------------------ tests/test_auth_saml.py | 30 +++--- 2 files changed, 26 insertions(+), 137 deletions(-) diff --git a/src/authentic2_auth_saml/adapters.py b/src/authentic2_auth_saml/adapters.py index 12db59f1..4f44d3d8 100644 --- a/src/authentic2_auth_saml/adapters.py +++ b/src/authentic2_auth_saml/adapters.py @@ -19,14 +19,10 @@ import logging from contextlib import contextmanager from django.contrib import messages -from django.core.exceptions import MultipleObjectsReturned from django.db.transaction import atomic from django.utils.translation import ugettext as _ from mellon.adapters import DefaultAdapter, UserCreationError -from mellon.utils import get_setting -from authentic2.a2_rbac.models import OrganizationalUnit as OU -from authentic2.a2_rbac.models import Role from authentic2.a2_rbac.utils import get_default_ou from authentic2.backends import get_user_queryset from authentic2.models import Lock @@ -126,52 +122,11 @@ class AuthenticAdapter(DefaultAdapter): @atomic def provision_a2_attributes(self, user, idp, saml_attributes): - """Copy incoming SAML attributes to user attributes, A2_ATTRIBUTE_MAPPING must be a list of - dictionaries like: - - { - 'attribute': 'email', - 'saml_attribute': 'email', - # optional: - 'mandatory': False, - } - - If an attribute is not mandatory any error is just logged, if the attribute is - mandatory, login will fail. - """ saml_attributes = saml_attributes.copy() - attribute_mapping = get_setting(idp, 'A2_ATTRIBUTE_MAPPING', []) - if not isinstance(attribute_mapping, list): - raise MappingError(_('invalid A2_ATTRIBUTE_MAPPING')) - - self.apply_attribute_mapping(user, idp, saml_attributes, attribute_mapping) - - def apply_attribute_mapping(self, user, idp, saml_attributes, attribute_mapping): self.rename_attributes(idp, saml_attributes) self.set_attributes(user, idp, saml_attributes) - - for mapping in attribute_mapping: - if not isinstance(mapping, dict): - raise MappingError(_('invalid mapping action "%(mapping)s"'), mapping=mapping) - action = mapping.get('action', 'set-attribute') - mandatory = mapping.get('mandatory', False) is True - method = None - if isinstance(action, str): - try: - method = getattr(self, 'action_' + action.replace('-', '_')) - except AttributeError: - pass - try: - if not method: - raise MappingError(_('invalid action')) - logger.debug('auth_saml: applying provisionning mapping %s', mapping) - method(user, idp, saml_attributes, mapping) - except MappingError as e: - if mandatory: - # it's mandatory, provisionning should fail completely - raise e - logger.warning('auth_saml: mapping action failed: %s', e) + self.action_add_role(user, idp, saml_attributes) def rename_attributes(self, idp, saml_attributes): for action in idp['authenticator'].rename_attribute_actions.all(): @@ -215,67 +170,9 @@ class AuthenticAdapter(DefaultAdapter): return True return False - def get_ou(self, role_desc): - ou_desc = role_desc.get('ou') - if ou_desc is None: - return None - if not isinstance(ou_desc, dict): - raise MappingError(_('invalid ou description')) - slug = ou_desc.get('slug') - name = ou_desc.get('name') - if slug: - if not isinstance(slug, str): - raise MappingError(_('invalid ou.slug in ou description')) - try: - return OU.objects.get(slug=slug) - except OU.DoesNotExist: - raise MappingError(_('unknown ou: "%(slug)s"'), slug=slug) - elif name: - if not isinstance(name, str): - raise MappingError(_('invalid ou.slug in ou description')) - try: - return OU.objects.get(name=name) - except OU.DoesNotExist: - raise MappingError(_('unknown ou: "%(name)s"'), name=name) - else: - raise MappingError(_('invalid ou description')) - - def get_role(self, mapping): - role_desc = mapping.get('role') - if not role_desc or not isinstance(role_desc, dict): - raise MappingError(_('missing or invalid role description')) - slug = role_desc.get('slug') - name = role_desc.get('name') - ou = self.get_ou(role_desc) - - kwargs = {} - if ou: - kwargs['ou'] = ou - - if slug: - if not isinstance(slug, str): - raise MappingError(_('invalid role slug: "%(slug)s"'), slug=slug) - kwargs['slug'] = slug - elif name: - if not isinstance(name, str): - raise MappingError(_('invalid role name: "%(name)s"'), name=name) - kwargs['name'] = name - else: - raise MappingError(_('invalid role description')) - - try: - return Role.objects.get(**kwargs) - except Role.DoesNotExist: - raise MappingError(_('unknown role: %(kwargs)s'), kwargs=kwargs) - except MultipleObjectsReturned: - raise MappingError(_('ambiugous role description: %(kwargs)s'), kwargs=kwargs) - - def evaluate_condition(self, user, saml_attributes, mapping): - condition = mapping.get('condition') - if condition is None: + def evaluate_condition(self, user, saml_attributes, condition): + if not condition: return True - if not isinstance(condition, str): - raise MappingError(_('invalid condition')) try: # use a proxy to simplify condition expressions as subscript is forbidden # you can write "email == 'a@b.com'" but also "'a@b.com' in email__list" @@ -285,19 +182,17 @@ class AuthenticAdapter(DefaultAdapter): except Exception as e: raise MappingError(_('condition evaluation failed: %(message)s'), message=e) - def action_add_role(self, user, idp, saml_attributes, mapping): - role = self.get_role(mapping) - if self.evaluate_condition(user, saml_attributes, mapping): - if role not in user.roles.all(): - logger.info('auth_saml: adding role "%s"', role, extra={'user': user}) - user.roles.add(role) - else: - if role in user.roles.all(): - logger.info('auth_saml: removing role "%s"', role, extra={'user': user}) - user.roles.remove(role) - - def action_toggle_role(self, *args, **kwargs): - return self.action_add_role(*args, **kwargs) + def action_add_role(self, user, idp, saml_attributes): + for action in idp['authenticator'].add_role_actions.all(): + with wrap_mapping_errors(action): + if self.evaluate_condition(user, saml_attributes, action.condition): + if action.role not in user.roles.all(): + logger.info('auth_saml: adding role "%s"', action.role, extra={'user': user}) + user.roles.add(action.role) + else: + if action.role in user.roles.all(): + logger.info('auth_saml: removing role "%s"', action.role, extra={'user': user}) + user.roles.remove(action.role) def auth_login(self, request, user): utils_misc.login(request, user, 'saml') diff --git a/tests/test_auth_saml.py b/tests/test_auth_saml.py index 5e1315a2..c179f397 100644 --- a/tests/test_auth_saml.py +++ b/tests/test_auth_saml.py @@ -28,7 +28,12 @@ from authentic2.apps.authenticators.models import LoginPasswordAuthenticator from authentic2.custom_user.models import DeletedUser from authentic2.models import Attribute from authentic2_auth_saml.adapters import AuthenticAdapter, MappingError -from authentic2_auth_saml.models import RenameAttributeAction, SAMLAuthenticator, SetAttributeAction +from authentic2_auth_saml.models import ( + AddRoleAction, + RenameAttributeAction, + SAMLAuthenticator, + SetAttributeAction, +) from .utils import login @@ -150,7 +155,7 @@ def test_apply_attribute_mapping_missing_attribute_logged( ): caplog.set_level('WARNING') saml_attributes['http://nice/attribute/givenName'] = [] - adapter.apply_attribute_mapping(user, idp, saml_attributes, idp['A2_ATTRIBUTE_MAPPING']) + adapter.provision_a2_attributes(user, idp, saml_attributes) assert re.match('.*no value.*first_name', caplog.records[-1].message) @@ -160,7 +165,7 @@ def test_apply_attribute_mapping_missing_attribute_exception( saml_attributes['http://nice/attribute/givenName'] = [] SetAttributeAction.objects.filter(attribute='first_name').update(mandatory=True) with pytest.raises(MappingError, match='no value'): - adapter.apply_attribute_mapping(user, idp, saml_attributes, idp['A2_ATTRIBUTE_MAPPING']) + adapter.provision_a2_attributes(user, idp, saml_attributes) request = rf.get('/') request._messages = mock.Mock() @@ -180,19 +185,8 @@ class TestAddRole: enabled=True, metadata='meta1.xml', slug='idp1', - a2_attribute_mapping=[ - { - 'action': action_name, - 'role': { - 'name': simple_role.name, - 'ou': { - 'name': simple_role.ou.name, - }, - }, - 'condition': "roles == 'A'", - } - ], ) + AddRoleAction.objects.create(authenticator=authenticator, role=simple_role, condition="roles == 'A'") return authenticator.settings @pytest.fixture @@ -214,17 +208,17 @@ class TestAddRole: def test_lookup_user_mandatory_condition(self, adapter, simple_role, idp, saml_attributes): # if a toggle-role is mandatory, failure to evaluate condition block user creation - idp['A2_ATTRIBUTE_MAPPING'][0]['mandatory'] = True + AddRoleAction.objects.update(mandatory=True) assert adapter.lookup_user(idp, saml_attributes) is None def test_lookup_user_mandatory(self, adapter, simple_role, idp, saml_attributes): - idp['A2_ATTRIBUTE_MAPPING'][0]['mandatory'] = True + AddRoleAction.objects.update(mandatory=True) saml_attributes['roles'] = ['A'] user = adapter.lookup_user(idp, saml_attributes) assert simple_role in user.roles.all() def test_lookup_user_use_list(self, adapter, simple_role, idp, saml_attributes): - idp['A2_ATTRIBUTE_MAPPING'][0]['condition'] = "'A' in roles__list" + AddRoleAction.objects.update(condition="'A' in roles__list") saml_attributes['roles'] = ['A'] user = adapter.lookup_user(idp, saml_attributes) assert simple_role in user.roles.all() -- 2.30.2