Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

providers/scim: fix scim sync (#11165) #11827

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kpodp0ra
Copy link

Issue

Closes #11165

During SCIM synchronization, server can throw ObjectExistsSyncException to mark that one of the N objects are already synchronized. Authentik doesn't handle this error properly and crashes instead of displaying a simple warning.

Details

SCIM can throw these errors:

if response.status_code == HttpResponseNotFound.status_code:
raise NotFoundSyncException(response)
if response.status_code in [HTTP_TOO_MANY_REQUESTS, HTTP_SERVICE_UNAVAILABLE]:
raise TransientSyncException()
if response.status_code == HTTP_CONFLICT:
raise ObjectExistsSyncException(response)

But only BadRequestSyncException and TransientSyncException are caught and handled properly:

except BadRequestSyncException as exc:

except TransientSyncException as exc:

NotFoundSyncException and ObjectExistsSyncException are unhandled.

Solution

I've come up with two solutions:

  1. Add two another except cases: 'except NotFoundSyncException as exc: ' and 'except ObjectExistsSyncException as exc: ' to authentik/lib/sync/outgoing/tasks.py
  2. Subclass these two unhandled exceptions to already-handled BadRequestSyncException

In this PR I've chosen the second option and looked up the code to make sure that this will create no conflicts.

@kpodp0ra kpodp0ra requested a review from a team as a code owner October 27, 2024 01:00
Copy link

netlify bot commented Oct 27, 2024

Deploy Preview for authentik-storybook canceled.

Name Link
🔨 Latest commit 48a8364
🔍 Latest deploy log https://app.netlify.com/sites/authentik-storybook/deploys/671d90a73c99940008472813

Copy link

netlify bot commented Oct 27, 2024

Deploy Preview for authentik-docs canceled.

Name Link
🔨 Latest commit 48a8364
🔍 Latest deploy log https://app.netlify.com/sites/authentik-docs/deploys/671d90a752fcc700081e6e20

@BeryJu BeryJu self-assigned this Oct 31, 2024
Copy link
Member

@BeryJu BeryJu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While this does fix the error, ObjectExistsSyncException should be handled separately as it should fetch the data from SCIM to get the remote ID and establish the connection

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SCIM sync fails with exception
2 participants