From 9fe8aaf0bed71b6d142a8dc20f25019694268a84 Mon Sep 17 00:00:00 2001 From: Benjamin Dauvergne Date: Fri, 26 Feb 2016 13:23:12 +0100 Subject: [PATCH] adapters: improve logging during provisionning - user creation is logged - attributes are only changed if different from the provisionning value, and changes are logged. --- mellon/adapters.py | 26 +++++++--- tests/conftest.py | 10 ++++ tests/metadata.xml | 17 +++++++ tests/test_default_adapter.py | 90 ++++++++++++++++++++++++----------- 4 files changed, 109 insertions(+), 34 deletions(-) create mode 100644 tests/metadata.xml diff --git a/mellon/adapters.py b/mellon/adapters.py index d6cb13b..6d2a552 100644 --- a/mellon/adapters.py +++ b/mellon/adapters.py @@ -69,6 +69,8 @@ class DefaultAdapter(object): if created: user.username = username user.save() + self.logger.info('created new user %s with name_id %s from issuer %s', + user, name_id, issuer) else: user.delete() user = saml_id.user @@ -92,11 +94,15 @@ class DefaultAdapter(object): self.logger.warning( u'invalid reference in attribute mapping template %r: %s', tpl, e) else: - attribute_set = True model_field = user._meta.get_field(field) if hasattr(model_field, 'max_length'): value = value[:model_field.max_length] - setattr(user, field, value) + if getattr(user, field) != value: + old_value = getattr(user, field) + setattr(user, field, value) + attribute_set = True + self.logger.info(u'set field %s of user %s to value %r (old value %r)', field, + user, value, old_value) if attribute_set: user.save() @@ -104,6 +110,7 @@ class DefaultAdapter(object): superuser_mapping = utils.get_setting(idp, 'SUPERUSER_MAPPING') if not superuser_mapping: return + attribute_set = False for key, values in superuser_mapping.iteritems(): if key in saml_attributes: if not isinstance(values, (tuple, list)): @@ -114,15 +121,20 @@ class DefaultAdapter(object): attribute_values = [attribute_values] attribute_values = set(attribute_values) if attribute_values & values: - user.is_staff = True - user.is_superuser = True - user.save() - break + if not (user.is_staff and user.is_superuser): + user.is_staff = True + user.is_superuser = True + attribute_set = True + self.logger.info('flag is_staff and is_superuser added to user %s', user) + break else: if user.is_superuser or user.is_staff: user.is_staff = False user.is_superuser = False - user.save() + self.logger.info('flag is_staff and is_superuser removed from user %s', user) + attribute_set = True + if attribute_set: + user.save() def provision_groups(self, user, idp, saml_attributes): User = user.__class__ diff --git a/tests/conftest.py b/tests/conftest.py index 9bdb738..c973315 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,3 +1,4 @@ +import logging import pytest import django_webtest @@ -32,3 +33,12 @@ def private_settings(request): django.conf.settings._wrapped = old request.addfinalizer(finalizer) return django.conf.settings + + +@pytest.fixture +def caplog(caplog): + import py.io + caplog.setLevel(logging.INFO) + caplog.handler.stream = py.io.TextIO() + caplog.handler.records = [] + return caplog diff --git a/tests/metadata.xml b/tests/metadata.xml new file mode 100644 index 0000000..ca736bd --- /dev/null +++ b/tests/metadata.xml @@ -0,0 +1,17 @@ + +MIIC+TCCAeGgAwIBAgIJAJqAKDUDlSinMA0GCSqGSIb3DQEBBQUAMBMxETAPBgNV +BAMMCHdob2NhcmVzMB4XDTE0MDUyNzE0MzE0OVoXDTI0MDUyNDE0MzE0OVowEzER +MA8GA1UEAwwId2hvY2FyZXMwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIB +AQDrUFQGviUE+unV4afJQiRUPp4/D+Ltvuw59BuJwdNEWHA2vchhnwDLlp3RWKaf +SWBJift55C4ybQKn5AEe6FHlIapJPvNqYnVP+0IgUFJmrxTWG9IT/5ZvJS0yer/O +093I5HTqthgcByIAj2L4R3oW21HNCojT4WZDYjG6RAxRFU/10BYY1ILe1SPAMXqc +99QC5fy2sZEJ/Cyd2Vlt1kAQ1+BZSZCL3vvdLfVRKjKZn2yYp8XbSplAZxB+b/iM +duSQHtLaRsV5tizPCdftXECaDn1FKqK0JmcolHFBsfOH2x7I8XEljO/DR/Oy4kzv +/cLdZB5fft4+nCqwLzI7fcRFAgMBAAGjUDBOMB0GA1UdDgQWBBSFV52hDdxJAdbM +Nht32j7+PyFbKTAfBgNVHSMEGDAWgBSFV52hDdxJAdbMNht32j7+PyFbKTAMBgNV +HRMEBTADAQH/MA0GCSqGSIb3DQEBBQUAA4IBAQCoNxpm99qip4nROCedBIbZnqWj +EkqHRLvIsm+oxf4Ctc6x/N1d2ngEygfT1xf5N5V221XTOgLCkuqi5r0/T6EB7U9y +6ACfVJQmvNaPbFmn2J9rNIAPYPj2cengSZyL3mWyrkPFLj5TsgT98GASX9iThhds +Nq6btZUL9ZUq8v3O7Y1uruMHJAACim4eYBjsCXaF7diKYaftFiwZWy1+3IQzUhmg +Ov4KR9P9bb+W/43i7zAYmdUrBr31/amEvGHoco7cO2bp43/1H8fFOcnkX0wRdN/k +r/hRVIsfeC6ss1NPDu/KzbRVVn5p9qKK6YVqqT3QapnQELgajEfhxpgY7AQx \ No newline at end of file diff --git a/tests/test_default_adapter.py b/tests/test_default_adapter.py index 7d22bba..37e6eb1 100644 --- a/tests/test_default_adapter.py +++ b/tests/test_default_adapter.py @@ -1,17 +1,21 @@ import threading import pytest +import re from django.contrib import auth from django.db import connection from mellon.adapters import DefaultAdapter +from mellon.backends import SAMLBackend pytestmark = pytest.mark.django_db -idp = {} +idp = { + 'METADATA': file('tests/metadata.xml').read(), +} saml_attributes = { 'name_id_content': 'x' * 32, - 'issuer': 'https://idp.example.net/saml/metadata', + 'issuer': 'https://cresson.entrouvert.org/idp/saml2/metadata', 'username': ['foobar'], 'email': ['test@example.net'], 'first_name': ['Foo'], @@ -69,57 +73,89 @@ def test_lookup_user_transaction(transactional_db, concurrency): assert len(set(user.pk for user in users)) == 1 -def test_provision(settings): - settings.MELLON_GROUP_ATTRIBUTE = 'group' - User = auth.get_user_model() - adapter = DefaultAdapter() +def test_provision_user_attributes(settings, django_user_model, caplog): + settings.MELLON_IDENTITY_PROVIDERS = [idp] settings.MELLON_ATTRIBUTE_MAPPING = { 'email': '{attributes[email][0]}', 'first_name': '{attributes[first_name][0]}', 'last_name': '{attributes[last_name][0]}', } - user = User(username='xx') - user.save() - adapter.provision(user, idp, saml_attributes) + user = SAMLBackend().authenticate(saml_attributes=saml_attributes) + assert user.username == 'x' * 30 assert user.first_name == 'Foo' assert user.last_name == 'Bar' assert user.email == 'test@example.net' assert user.is_superuser is False + assert user.is_staff is False + assert len(caplog.records()) == 4 + assert 'created new user' in caplog.text() + assert 'set field first_name' in caplog.text() + assert 'set field last_name' in caplog.text() + assert 'set field email' in caplog.text() + + +def test_provision_user_groups(settings, django_user_model, caplog): + settings.MELLON_IDENTITY_PROVIDERS = [idp] + settings.MELLON_GROUP_ATTRIBUTE = 'group' + user = SAMLBackend().authenticate(saml_attributes=saml_attributes) assert user.groups.count() == 3 assert set(user.groups.values_list('name', flat=True)) == set(saml_attributes['group']) + assert len(caplog.records()) == 4 + assert 'created new user' in caplog.text() + assert 'adding group GroupA' in caplog.text() + assert 'adding group GroupB' in caplog.text() + assert 'adding group GroupC' in caplog.text() saml_attributes2 = saml_attributes.copy() saml_attributes2['group'] = ['GroupB', 'GroupC'] - adapter.provision(user, idp, saml_attributes2) + user = SAMLBackend().authenticate(saml_attributes=saml_attributes2) assert user.groups.count() == 2 assert set(user.groups.values_list('name', flat=True)) == set(saml_attributes2['group']) - User.objects.all().delete() + assert len(caplog.records()) == 5 + assert 'removing group GroupA' in caplog.records()[-1].message + +def test_provision_is_superuser(settings, django_user_model, caplog): + settings.MELLON_IDENTITY_PROVIDERS = [idp] settings.MELLON_SUPERUSER_MAPPING = { 'is_superuser': 'true', } - user = User(username='xx') - user.save() - adapter.provision(user, idp, saml_attributes) + user = SAMLBackend().authenticate(saml_attributes=saml_attributes) assert user.is_superuser is True - User.objects.all().delete() + assert 'flag is_staff and is_superuser added' in caplog.text() - local_saml_attributes = saml_attributes.copy() - del local_saml_attributes['email'] - user = User(username='xx') - user.save() - adapter.provision(user, idp, local_saml_attributes) - assert not user.email - User.objects.all().delete() - local_saml_attributes = saml_attributes.copy() +def test_provision_absent_attribute(settings, django_user_model, caplog): + settings.MELLON_IDENTITY_PROVIDERS = [idp] settings.MELLON_ATTRIBUTE_MAPPING = { 'email': '{attributes[email][0]}', 'first_name': '{attributes[first_name][0]}', 'last_name': '{attributes[last_name][0]}', } + local_saml_attributes = saml_attributes.copy() + del local_saml_attributes['email'] + user = SAMLBackend().authenticate(saml_attributes=local_saml_attributes) + assert not user.email + assert len(caplog.records()) == 4 + assert 'created new user' in caplog.text() + assert re.search(r'invalid reference.*email', caplog.text()) + assert 'set field first_name' in caplog.text() + assert 'set field last_name' in caplog.text() + + +def test_provision_long_attribute(settings, django_user_model, caplog): + settings.MELLON_IDENTITY_PROVIDERS = [idp] + settings.MELLON_ATTRIBUTE_MAPPING = { + 'email': '{attributes[email][0]}', + 'first_name': '{attributes[first_name][0]}', + 'last_name': '{attributes[last_name][0]}', + } + local_saml_attributes = saml_attributes.copy() local_saml_attributes['first_name'] = [('y' * 32)] - user = User(username='xx') - user.save() - adapter.provision(user, idp, local_saml_attributes) + user = SAMLBackend().authenticate(saml_attributes=local_saml_attributes) assert user.first_name == 'y' * 30 - User.objects.all().delete() + assert len(caplog.records()) == 4 + assert 'created new user' in caplog.text() + assert 'set field first_name' in caplog.text() + assert 'to value %r ' % (u'y' * 30) in caplog.text() + assert 'set field last_name' in caplog.text() + assert 'set field email' in caplog.text()