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: modify filtergroup(s) behavior to allow (multi-)group filtering #13550

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

ImmanuelVonNeumann
Copy link

Details

This PR aims to change add group filtering to the SCIMProvider by incorporating the following changes:

  • Rename and modify filter_group to filter_groups (including django migrations to keep a previously selected filter_group present)
  • Multiple groups can be selected as filter_groups
  • Only groups within filter_groups are synced to the SCIM Server
  • Only users which are members of at least one group within filter_groups are synced to the SCIM Server

Reason:
Most SCIM Clients allow groups to be filtered, however authentik currently does not.
This issue has been mentioned in #6065 which has been given the enhancement/confirmed-label.

Among the many use-cases one would be a multi-tenant setup where groups of one tenant should not be visible to another tenant.

Note

Since this does change group-syncing behavior (i.e. not syncing previously synced groups anymore) I suggest this change to be implemented in a major release.
Additionally, blueprints for scim_providers need to adapt to the new name filter_groups.

Fixes #6065


Checklist

  • Local tests pass (ak test authentik/)
  • The code has been formatted (make lint-fix)

If an API change has been made

  • The API schema has been updated (make gen-build)

If changes to the frontend have been made

  • The code has been formatted (make web)

If applicable

  • The documentation has been updated
  • The documentation has been formatted (make website)

Sorry, something went wrong.

@ImmanuelVonNeumann ImmanuelVonNeumann requested review from a team as code owners March 17, 2025 15:51
Copy link

netlify bot commented Mar 17, 2025

Deploy Preview for authentik-storybook ready!

Name Link
🔨 Latest commit 6ee9884
🔍 Latest deploy log https://app.netlify.com/sites/authentik-storybook/deploys/67dd3089777187000883c1eb
😎 Deploy Preview https://deploy-preview-13550--authentik-storybook.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Mar 17, 2025

Deploy Preview for authentik-docs ready!

Name Link
🔨 Latest commit 6ee9884
🔍 Latest deploy log https://app.netlify.com/sites/authentik-docs/deploys/67dd3089cb47270009a661f3
😎 Deploy Preview https://deploy-preview-13550--authentik-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

codecov bot commented Mar 17, 2025

Codecov Report

Attention: Patch coverage is 75.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 92.74%. Comparing base (ad27f26) to head (8ee78fc).
Report is 17 commits behind head on main.

Files with missing lines Patch % Lines
authentik/providers/scim/models.py 71.42% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main   #13550   +/-   ##
=======================================
  Coverage   92.73%   92.74%           
=======================================
  Files         794      794           
  Lines       40425    40428    +3     
=======================================
+ Hits        37487    37493    +6     
+ Misses       2938     2935    -3     
Flag Coverage Δ
e2e 48.03% <25.00%> (+0.04%) ⬆️
integration 24.31% <12.50%> (-0.01%) ⬇️
unit 90.50% <75.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@BeryJu
Copy link
Member

BeryJu commented Mar 19, 2025

Hi, thanks for the PR!

When I added the filter_group it was actually more so thought of as a "temporary" solution with the intended solution being that the SCIM provider would use the policies bound to the application the provider is a backchannel provider of to figure out which users to sync. However this would still leave room for either always syncing all groups or syncing all groups the synced users in.

There is also an already existing option for doing this more dynamically which is having a custom user/group mapping that dynamically raises SkipObject, which allows for more fine-grained rules for skipping certain objects

@ImmanuelVonNeumann
Copy link
Author

Hi, thanks for the PR!

When I added the filter_group it was actually more so thought of as a "temporary" solution with the intended solution being that the SCIM provider would use the policies bound to the application the provider is a backchannel provider of to figure out which users to sync. However this would still leave room for either always syncing all groups or syncing all groups the synced users in.

There is also an already existing option for doing this more dynamically which is having a custom user/group mapping that dynamically raises SkipObject, which allows for more fine-grained rules for skipping certain objects

Thanks for your fast response, @BeryJu!

From my understanding the final solution could be that:

  • Users who have access to the connected application are being synced
  • Groups who are in filter_groups are being synced
  • Users gain memberships for groups they are in if the group is also within filter_groups (since others are not synced)

Is this correct?


One further question: I could not find documentation on the implications of the field "filterset_fields" within the model.
Is there a documentation for it?

@BeryJu
Copy link
Member

BeryJu commented Mar 20, 2025

Hi, thanks for the PR!
When I added the filter_group it was actually more so thought of as a "temporary" solution with the intended solution being that the SCIM provider would use the policies bound to the application the provider is a backchannel provider of to figure out which users to sync. However this would still leave room for either always syncing all groups or syncing all groups the synced users in.
There is also an already existing option for doing this more dynamically which is having a custom user/group mapping that dynamically raises SkipObject, which allows for more fine-grained rules for skipping certain objects

Thanks for your fast response, @BeryJu!

From my understanding the final solution could be that:

  • Users who have access to the connected application are being synced
  • Groups who are in filter_groups are being synced
  • Users gain memberships for groups they are in if the group is also within filter_groups (since others are not synced)

Correct, I think having SkipObject in a property mapping as an advanced option makes sense but having a static list of groups to sync is a more accessible way. We can add the policy-based user-filtering at a later stage.

One potential issue that could arise when this is merged is what happens with groups that were previously provisioned and are now outside of the group filter, they would still exist in authentik and the remote system but would not get further updates (and currently would not be correctly deletable iirc)

Is this correct?

One further question: I could not find documentation on the implications of the field "filterset_fields" within the model. Is there a documentation for it?

filterset_fields comes from django-filters, see https://django-filter.readthedocs.io/en/latest/guide/rest_framework.html

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
…iltering

Replaces the scim provider's argument filter_group with filter_groups.
Allows multiple filter_groups to be selected.
Only syncs groups within filter_groups and users which are members of at least one filter_group for scim.
@ImmanuelVonNeumann
Copy link
Author

One potential issue that could arise when this is merged is what happens with groups that were previously provisioned and are now outside of the group filter, they would still exist in authentik and the remote system but would not get further updates (and currently would not be correctly deletable iirc)

I believe so, if the previously sent state is not currently being tracked.
Also group-membership would only be synced for filtered groups.

To solve this, the sent state would need to be tracked so that differences to it can be identified and remediated on the next sync.

filterset_fields comes from django-filters, see https://django-filter.readthedocs.io/en/latest/guide/rest_framework.html

Thanks!

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 Group Filtering
2 participants