-
Notifications
You must be signed in to change notification settings - Fork 10k
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
feat: Add LDAP group validation strategy setting to channels and roles sync #32436
base: develop
Are you sure you want to change the base?
Conversation
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
🦋 Changeset detectedLatest commit: 7da85e4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 34 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #32436 +/- ##
===========================================
- Coverage 55.84% 55.84% -0.01%
===========================================
Files 2432 2432
Lines 53480 53480
Branches 10993 10993
===========================================
- Hits 29868 29864 -4
- Misses 20973 20979 +6
+ Partials 2639 2637 -2
Flags with carried forward coverage won't be shown. Click here to find out more. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR should still be flagged as a breaking change since it'll no longer be possible to use both strategies at the same time.
There should also be a migration to select the right strategy automatically if only one of them is currently being used.
Ignore this review, I see now that the PR is making a different change than the one we had discussed last time.
Co-authored-by: Pierre Lehnen <55164754+pierre-lehnen-rc@users.noreply.github.com>
Proposed changes (including videos or screenshots)
Available strategies:
#{groupName}
replacement tag to define membership (e.g. when filtering by thememberOf
field in groups);#{groupName}
replacement tag is not used by the filter (e.g. when filtering by themember
field in groups).Issue(s)
Steps to test or reproduce
The new "Group membership validation strategy" setting is available under both "Sync Channels" and "Sync Roles" sections in LDAP Premium settings. Both features should work just the same as in previous version when using the default "Apply filter for each group" search strategy or the new and faster "Apply filter once to get all memberships" strategy -- the only difference here is the amount of LDAP search requests triggered by RC.
Sample configuration
Sample configuration for using the "Apply filter once to get all memberships" search strategy:
Sample configuration for using the "Apply filter for each group" search strategy:
Caution
Switching to the new and faster "Apply filter once to get all memberships" search strategy may not work with the currently configured LDAP search filter!
When switching to the new strategy, make sure to update the user group filter so as to get all groups at once in a single query with it (be sure not to use the
#{groupName}
replacement tag since it is not supported by this new strategy)Further comments
CORE-402