From 5ea54e8f7e5b2b658a26bf19beb1da7c989078fb Mon Sep 17 00:00:00 2001 From: Jens L Date: Mon, 8 May 2023 15:34:43 +0200 Subject: [PATCH] *: improve configuration error events (#5523) * *: improve configuration error events Signed-off-by: Jens Langhammer * delete test-db when resetting Signed-off-by: Jens Langhammer --------- Signed-off-by: Jens Langhammer --- Makefile | 2 ++ authentik/providers/oauth2/tests/test_userinfo.py | 2 +- authentik/providers/oauth2/views/userinfo.py | 4 +++- authentik/providers/saml/processors/assertion.py | 11 +++++++++-- authentik/providers/saml/tests/test_auth_n_request.py | 2 +- authentik/sources/ldap/sync/base.py | 3 ++- authentik/sources/ldap/tests/test_sync.py | 2 +- 7 files changed, 19 insertions(+), 7 deletions(-) diff --git a/Makefile b/Makefile index 84ee1de90..7790c4170 100644 --- a/Makefile +++ b/Makefile @@ -206,6 +206,8 @@ install: web-install website-install dev-reset: dropdb -U postgres -h localhost authentik + # Also remove the test-db if it exists + dropdb -U postgres -h localhost test_authentik || true createdb -U postgres -h localhost authentik redis-cli -n 0 flushall make migrate diff --git a/authentik/providers/oauth2/tests/test_userinfo.py b/authentik/providers/oauth2/tests/test_userinfo.py index 451c86fa3..431b2f18f 100644 --- a/authentik/providers/oauth2/tests/test_userinfo.py +++ b/authentik/providers/oauth2/tests/test_userinfo.py @@ -92,5 +92,5 @@ class TestUserinfo(OAuthTestCase): self.assertTrue(events.exists()) self.assertEqual( events.first().context["message"], - "Failed to evaluate property-mapping: name 'q' is not defined", + "Failed to evaluate property-mapping: 'test'", ) diff --git a/authentik/providers/oauth2/views/userinfo.py b/authentik/providers/oauth2/views/userinfo.py index 2f0578189..2f9b118a4 100644 --- a/authentik/providers/oauth2/views/userinfo.py +++ b/authentik/providers/oauth2/views/userinfo.py @@ -82,9 +82,11 @@ class UserInfoView(View): except PropertyMappingExpressionException as exc: Event.new( EventAction.CONFIGURATION_ERROR, - message=f"Failed to evaluate property-mapping: {str(exc)}", + message=f"Failed to evaluate property-mapping: '{scope.name}'", + provider=provider, mapping=scope, ).from_http(self.request) + LOGGER.warning("Failed to evaluate property mapping", exc=exc) if value is None: continue if not isinstance(value, dict): diff --git a/authentik/providers/saml/processors/assertion.py b/authentik/providers/saml/processors/assertion.py index 773408910..15940dec6 100644 --- a/authentik/providers/saml/processors/assertion.py +++ b/authentik/providers/saml/processors/assertion.py @@ -108,9 +108,11 @@ class AssertionProcessor: # Value error can be raised when assigning invalid data to an attribute Event.new( EventAction.CONFIGURATION_ERROR, - message=f"Failed to evaluate property-mapping: {str(exc)}", + message=f"Failed to evaluate property-mapping: '{mapping.name}'", + provider=self.provider, mapping=mapping, ).from_http(self.http_request) + LOGGER.warning("Failed to evaluate property mapping", exc=exc) continue return attribute_statement @@ -185,9 +187,14 @@ class AssertionProcessor: except PropertyMappingExpressionException as exc: Event.new( EventAction.CONFIGURATION_ERROR, - message=f"Failed to evaluate property-mapping: {str(exc)}", + message=( + "Failed to evaluate property-mapping: " + f"'{self.provider.name_id_mapping.name}'", + ), + provider=self.provider, mapping=self.provider.name_id_mapping, ).from_http(self.http_request) + LOGGER.warning("Failed to evaluate property mapping", exc=exc) return name_id if name_id.attrib["Format"] == SAML_NAME_ID_FORMAT_EMAIL: name_id.text = self.http_request.user.email diff --git a/authentik/providers/saml/tests/test_auth_n_request.py b/authentik/providers/saml/tests/test_auth_n_request.py index 86429b9cc..6acd103c8 100644 --- a/authentik/providers/saml/tests/test_auth_n_request.py +++ b/authentik/providers/saml/tests/test_auth_n_request.py @@ -261,5 +261,5 @@ class TestAuthNRequest(TestCase): self.assertTrue(events.exists()) self.assertEqual( events.first().context["message"], - "Failed to evaluate property-mapping: name 'q' is not defined", + "Failed to evaluate property-mapping: 'test'", ) diff --git a/authentik/sources/ldap/sync/base.py b/authentik/sources/ldap/sync/base.py index a2c562964..5bf412bd5 100644 --- a/authentik/sources/ldap/sync/base.py +++ b/authentik/sources/ldap/sync/base.py @@ -108,7 +108,8 @@ class BaseLDAPSynchronizer: except PropertyMappingExpressionException as exc: Event.new( EventAction.CONFIGURATION_ERROR, - message=f"Failed to evaluate property-mapping: {str(exc)}", + message=f"Failed to evaluate property-mapping: '{mapping.name}'", + source=self._source, mapping=mapping, ).save() self._logger.warning("Mapping failed to evaluate", exc=exc, mapping=mapping) diff --git a/authentik/sources/ldap/tests/test_sync.py b/authentik/sources/ldap/tests/test_sync.py index d9e03d0b5..c5fd09fc4 100644 --- a/authentik/sources/ldap/tests/test_sync.py +++ b/authentik/sources/ldap/tests/test_sync.py @@ -56,7 +56,7 @@ class LDAPSyncTests(TestCase): self.assertFalse(User.objects.filter(username="user1_sn").exists()) events = Event.objects.filter( action=EventAction.CONFIGURATION_ERROR, - context__message="Failed to evaluate property-mapping: name 'q' is not defined", + context__message="Failed to evaluate property-mapping: 'name'", ) self.assertTrue(events.exists())