From 47f09ac28594af7d943ce8da2f03e13ccc324117 Mon Sep 17 00:00:00 2001 From: Jens L Date: Mon, 15 May 2023 14:39:27 +0200 Subject: [PATCH] providers/scim: improve SCIM error messages (#5600) Signed-off-by: Jens Langhammer --- authentik/providers/scim/clients/base.py | 2 +- authentik/providers/scim/clients/exceptions.py | 16 ++++++++++------ authentik/providers/scim/clients/schema.py | 7 +++++++ authentik/providers/scim/tasks.py | 16 ++++++++-------- 4 files changed, 26 insertions(+), 15 deletions(-) diff --git a/authentik/providers/scim/clients/base.py b/authentik/providers/scim/clients/base.py index c841ecfa0..ed99f6070 100644 --- a/authentik/providers/scim/clients/base.py +++ b/authentik/providers/scim/clients/base.py @@ -51,7 +51,7 @@ class SCIMClient(Generic[T, SchemaType]): }, ) except RequestException as exc: - raise SCIMRequestException(None) from exc + raise SCIMRequestException(message="Failed to send request") from exc self.logger.debug("scim request", path=path, method=method, **kwargs) if response.status_code >= 400: if response.status_code == 404: diff --git a/authentik/providers/scim/clients/exceptions.py b/authentik/providers/scim/clients/exceptions.py index 926cd95fb..62c7e78c5 100644 --- a/authentik/providers/scim/clients/exceptions.py +++ b/authentik/providers/scim/clients/exceptions.py @@ -2,10 +2,10 @@ from typing import Optional from pydantic import ValidationError -from pydanticscim.responses import SCIMError from requests import Response from authentik.lib.sentry import SentryIgnoredException +from authentik.providers.scim.clients.schema import SCIMError class StopSync(SentryIgnoredException): @@ -16,7 +16,8 @@ class StopSync(SentryIgnoredException): self.obj = obj self.mapping = mapping - def __str__(self) -> str: + def detail(self) -> str: + """Get human readable details of this error""" msg = f"Error {str(self.exc)}, caused by {self.obj}" if self.mapping: @@ -28,19 +29,22 @@ class SCIMRequestException(SentryIgnoredException): """Exception raised when an SCIM request fails""" _response: Optional[Response] + _message: Optional[str] - def __init__(self, response: Optional[Response] = None) -> None: + def __init__(self, response: Optional[Response] = None, message: Optional[str] = None) -> None: self._response = response + self._message = message - def __str__(self) -> str: + def detail(self) -> str: + """Get human readable details of this error""" if not self._response: - return super().__str__() + return self._message try: error = SCIMError.parse_raw(self._response.text) return error.detail except ValidationError: pass - return super().__str__() + return self._message class ResourceMissing(SCIMRequestException): diff --git a/authentik/providers/scim/clients/schema.py b/authentik/providers/scim/clients/schema.py index 4236e7524..3f2868d96 100644 --- a/authentik/providers/scim/clients/schema.py +++ b/authentik/providers/scim/clients/schema.py @@ -3,6 +3,7 @@ from typing import Optional from pydanticscim.group import Group as BaseGroup from pydanticscim.responses import PatchRequest as BasePatchRequest +from pydanticscim.responses import SCIMError as BaseSCIMError from pydanticscim.service_provider import Bulk, ChangePassword, Filter, Patch from pydanticscim.service_provider import ( ServiceProviderConfiguration as BaseServiceProviderConfiguration, @@ -52,3 +53,9 @@ class PatchRequest(BasePatchRequest): """PatchRequest which correctly sets schemas""" schemas: tuple[str] = ["urn:ietf:params:scim:api:messages:2.0:PatchOp"] + + +class SCIMError(BaseSCIMError): + """SCIM error with optional status code""" + + status: Optional[int] diff --git a/authentik/providers/scim/tasks.py b/authentik/providers/scim/tasks.py index 2b05af550..fe6d664b5 100644 --- a/authentik/providers/scim/tasks.py +++ b/authentik/providers/scim/tasks.py @@ -87,10 +87,10 @@ def scim_sync_users(page: int, provider_pk: int): LOGGER.warning("failed to sync user", exc=exc, user=user) messages.append( _( - "Failed to sync user due to remote error %(name)s: %(error)s" + "Failed to sync user %(user_name)s due to remote error: %(error)s" % { - "name": user.username, - "error": str(exc), + "user_name": user.username, + "error": exc.detail(), } ) ) @@ -100,7 +100,7 @@ def scim_sync_users(page: int, provider_pk: int): _( "Stopping sync due to error: %(error)s" % { - "error": str(exc), + "error": exc.detail(), } ) ) @@ -128,10 +128,10 @@ def scim_sync_group(page: int, provider_pk: int): LOGGER.warning("failed to sync group", exc=exc, group=group) messages.append( _( - "Failed to sync group due to remote error %(name)s: %(error)s" + "Failed to sync group %(group_name)s due to remote error: %(error)s" % { - "name": group.name, - "error": str(exc), + "group_name": group.name, + "error": exc.detail(), } ) ) @@ -141,7 +141,7 @@ def scim_sync_group(page: int, provider_pk: int): _( "Stopping sync due to error: %(error)s" % { - "error": str(exc), + "error": exc.detail(), } ) )