From 942019d31f1670204cc6454befac4baaf71d7308 Mon Sep 17 00:00:00 2001 From: Jens L Date: Thu, 20 Jun 2024 16:16:24 +0900 Subject: [PATCH] core: rework base for SkipObject exception to better support control flow exceptions (#10186) Signed-off-by: Jens Langhammer --- authentik/core/expression/exceptions.py | 7 ++----- authentik/core/models.py | 3 +++ authentik/core/tests/test_property_mapping.py | 16 +++++++++++++++- authentik/lib/expression/evaluator.py | 4 +++- authentik/lib/expression/exceptions.py | 6 ++++++ authentik/lib/sync/mapper.py | 8 ++------ authentik/lib/sync/outgoing/base.py | 4 ++-- 7 files changed, 33 insertions(+), 15 deletions(-) create mode 100644 authentik/lib/expression/exceptions.py diff --git a/authentik/core/expression/exceptions.py b/authentik/core/expression/exceptions.py index bf0de9bd42..b45987175c 100644 --- a/authentik/core/expression/exceptions.py +++ b/authentik/core/expression/exceptions.py @@ -1,5 +1,6 @@ """authentik core exceptions""" +from authentik.lib.expression.exceptions import ControlFlowException from authentik.lib.sentry import SentryIgnoredException @@ -12,11 +13,7 @@ class PropertyMappingExpressionException(SentryIgnoredException): self.mapping = mapping -class SkipObjectException(PropertyMappingExpressionException): +class SkipObjectException(ControlFlowException): """Exception which can be raised in a property mapping to skip syncing an object. Only applies to Property mappings which sync objects, and not on mappings which transitively apply to a single user""" - - def __init__(self) -> None: - # For this class only, both of these are set by the function evaluating the property mapping - super().__init__(exc=None, mapping=None) diff --git a/authentik/core/models.py b/authentik/core/models.py index 0a991ad709..d3ab0095b3 100644 --- a/authentik/core/models.py +++ b/authentik/core/models.py @@ -26,6 +26,7 @@ from authentik.blueprints.models import ManagedModel from authentik.core.expression.exceptions import PropertyMappingExpressionException from authentik.core.types import UILoginButton, UserSettingSerializer from authentik.lib.avatars import get_avatar +from authentik.lib.expression.exceptions import ControlFlowException from authentik.lib.generators import generate_id from authentik.lib.models import ( CreatedUpdatedModel, @@ -783,6 +784,8 @@ class PropertyMapping(SerializerModel, ManagedModel): evaluator = PropertyMappingEvaluator(self, user, request, **kwargs) try: return evaluator.evaluate(self.expression) + except ControlFlowException as exc: + raise exc except Exception as exc: raise PropertyMappingExpressionException(self, exc) from exc diff --git a/authentik/core/tests/test_property_mapping.py b/authentik/core/tests/test_property_mapping.py index d235e41daf..080facaa1f 100644 --- a/authentik/core/tests/test_property_mapping.py +++ b/authentik/core/tests/test_property_mapping.py @@ -3,7 +3,10 @@ from django.test import RequestFactory, TestCase from guardian.shortcuts import get_anonymous_user -from authentik.core.expression.exceptions import PropertyMappingExpressionException +from authentik.core.expression.exceptions import ( + PropertyMappingExpressionException, + SkipObjectException, +) from authentik.core.models import PropertyMapping from authentik.core.tests.utils import create_test_admin_user from authentik.events.models import Event, EventAction @@ -42,6 +45,17 @@ class TestPropertyMappings(TestCase): self.assertTrue(events.exists()) self.assertEqual(len(events), 1) + def test_expression_skip(self): + """Test expression error""" + expr = "raise SkipObject" + mapping = PropertyMapping.objects.create(name=generate_id(), expression=expr) + with self.assertRaises(SkipObjectException): + mapping.evaluate(None, None) + events = Event.objects.filter( + action=EventAction.PROPERTY_MAPPING_EXCEPTION, context__expression=expr + ) + self.assertFalse(events.exists()) + def test_expression_error_extended(self): """Test expression error (with user and http request""" expr = "return aaa" diff --git a/authentik/lib/expression/evaluator.py b/authentik/lib/expression/evaluator.py index f17b346d90..80be9e9b39 100644 --- a/authentik/lib/expression/evaluator.py +++ b/authentik/lib/expression/evaluator.py @@ -19,6 +19,7 @@ from structlog.stdlib import get_logger from authentik.core.models import User from authentik.events.models import Event +from authentik.lib.expression.exceptions import ControlFlowException from authentik.lib.utils.http import get_http_session from authentik.policies.models import Policy, PolicyBinding from authentik.policies.process import PolicyProcess @@ -216,7 +217,8 @@ class BaseEvaluator: # so the user only sees information relevant to them # and none of our surrounding error handling exc.__traceback__ = exc.__traceback__.tb_next - self.handle_error(exc, expression_source) + if not isinstance(exc, ControlFlowException): + self.handle_error(exc, expression_source) raise exc return result diff --git a/authentik/lib/expression/exceptions.py b/authentik/lib/expression/exceptions.py new file mode 100644 index 0000000000..dea2001e1c --- /dev/null +++ b/authentik/lib/expression/exceptions.py @@ -0,0 +1,6 @@ +from authentik.lib.sentry import SentryIgnoredException + + +class ControlFlowException(SentryIgnoredException): + """Exceptions used to control the flow from exceptions, not reported as a warning/ + error in logs""" diff --git a/authentik/lib/sync/mapper.py b/authentik/lib/sync/mapper.py index 21374807ba..1a287ee3c4 100644 --- a/authentik/lib/sync/mapper.py +++ b/authentik/lib/sync/mapper.py @@ -6,9 +6,9 @@ from django.http import HttpRequest from authentik.core.expression.evaluator import PropertyMappingEvaluator from authentik.core.expression.exceptions import ( PropertyMappingExpressionException, - SkipObjectException, ) from authentik.core.models import PropertyMapping, User +from authentik.lib.expression.exceptions import ControlFlowException class PropertyMappingManager: @@ -60,11 +60,7 @@ class PropertyMappingManager: mapping.set_context(user, request, **kwargs) try: value = mapping.evaluate(mapping.model.expression) - except SkipObjectException as exc: - exc.exc = exc - exc.mapping = mapping - raise exc from exc - except PropertyMappingExpressionException as exc: + except (PropertyMappingExpressionException, ControlFlowException) as exc: raise exc from exc except Exception as exc: raise PropertyMappingExpressionException(exc, mapping.model) from exc diff --git a/authentik/lib/sync/outgoing/base.py b/authentik/lib/sync/outgoing/base.py index 47c2accf9f..da34b3d65b 100644 --- a/authentik/lib/sync/outgoing/base.py +++ b/authentik/lib/sync/outgoing/base.py @@ -9,9 +9,9 @@ from structlog.stdlib import get_logger from authentik.core.expression.exceptions import ( PropertyMappingExpressionException, - SkipObjectException, ) from authentik.events.models import Event, EventAction +from authentik.lib.expression.exceptions import ControlFlowException from authentik.lib.sync.mapper import PropertyMappingManager from authentik.lib.sync.outgoing.exceptions import NotFoundSyncException, StopSync from authentik.lib.utils.errors import exception_to_string @@ -92,7 +92,7 @@ class BaseOutgoingSyncClient[ eval_kwargs.setdefault("user", None) for value in self.mapper.iter_eval(**eval_kwargs): always_merger.merge(raw_final_object, value) - except SkipObjectException as exc: + except ControlFlowException as exc: raise exc from exc except PropertyMappingExpressionException as exc: # Value error can be raised when assigning invalid data to an attribute