Replace django-scim2 filtering code #2049
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cp-at-mit
approved these changes
Feb 18, 2025
rhysyngsun
force-pushed
the
nl/scim-search
branch
from
6d85009 to
43f051d
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What are the relevant tickets?
Closes https://github.com/mitodl/hq/issues/6730
Description (What does it do?)
The
django-scim2library has a bug where it can't handle more than some arbitrary number of filters in the same search request. It errors in their parser because it ends up deeply recursing until it hits aRecursionError. This PR implements replacement parsing code that avoids the recursion. It includes code that maps these filters to a django query, however I have not done a full implementation of this but for now just implemented what we need toscim-for-keycloakto be functional with large (1000) batches.How can this be tested?
You can checkout this branch and perform a sync from Keycloak. Be sure to select "email" for the user attribute and "search/bulk" as the strategy. It should succeed with no errors.