fix(scim source): Make SCIM group and user handling fully SCIM 2.0 compliant#15046
fix(scim source): Make SCIM group and user handling fully SCIM 2.0 compliant#15046amritbrartech wants to merge 4 commits intogoauthentik:mainfrom
Conversation
✅ Deploy Preview for authentik-docs canceled.
|
✅ Deploy Preview for authentik-storybook canceled.
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #15046 +/- ##
==========================================
- Coverage 92.76% 92.61% -0.15%
==========================================
Files 815 815
Lines 42117 42231 +114
==========================================
+ Hits 39068 39111 +43
- Misses 3049 3120 +71
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| query |= Q(uuid=member.value) | ||
| if query: | ||
| group.users.set(User.objects.filter(query)) | ||
| group.users.set(User.objects.filter(query)) |
There was a problem hiding this comment.
This now always runs a filter(query) even if query is an empty Q() (which matches everything). It could unintentionally set all users if members are empty. I'd add back if query:
| if not isinstance(value, list): | ||
| value = [value] | ||
| for member in value: | ||
| member_id = member.get("value") |
There was a problem hiding this comment.
This member is always a dict, but malformed input might send strings or nested formats. I'd suggest adding validation such as if not isinstance(member, dict) and raise a SCIMValidationError.
| path = op.get("path", "") | ||
| value = op.get("value") | ||
|
|
||
| if path.lower() == "displayname": |
There was a problem hiding this comment.
Only lowercase path, but SCIM allows json path-like notation (members[value eq "abc"]) or nested fields.
There was a problem hiding this comment.
For this filter parsing we should really be using something like https://github.com/15five/scim2-filter-parser as it does a correct token -> AST parsing (that library seems to not have been updated in quite a while so we might want to vendor it)
| "totalResults": page.paginator.count, | ||
| "itemsPerPage": page.paginator.per_page, | ||
| "startIndex": page.start_index(), | ||
| "startIndex": start_index, |
There was a problem hiding this comment.
uses user-supplied startIndex, not page.start_index() which reflects actual pagination result. I'd use page.start_index() to indicate the real start.
| path = op.get("path", "") | ||
| value = op.get("value") | ||
|
|
||
| if path.lower() == "username" or path.lower() == "userName": |
There was a problem hiding this comment.
only userName is handled. SCIM clients may send emails, name.familyName, etc.
| data["userName"] = value | ||
| elif op_type == "remove": | ||
| data.pop("userName", None) | ||
| # Add more SCIM attribute handling as needed |
Details
This PR enhances SCIM source functionality in Authentik to ensure full compliance with the SCIM 2.0 specification for both users and groups. Key changes include:
SCIM 2.0 Compliance Enhancements:
• Updated PATCH handling for groups and users to support SCIM 2.0 operations (add, replace, remove).
• Corrected schemas usage in SCIM payloads for both users and groups.
• Added support for PUT operations on group and user endpoints.
• Fixed error handling to return proper SCIM error responses with appropriate scimType.
Group Handling Improvements:
• Replaced incorrect schema in group SCIM response.
• Ensured members field is handled correctly during PATCH operations.
• Guarded against invalid UUID inputs for group IDs.
User Handling Improvements:
• Added PATCH endpoint with proper SCIM operation support (e.g., update userName).
• Normalized startIndex parsing and improved pagination response compliance.
General Enhancements:
• Introduced SCIMValidationError exception for standardized error messages.
• Included JSONParser alongside SCIMParser to support broader content types.
• Improved robustness of group_to_scim and user_to_scim by validating existence of internal objects.
Checklist
ak test authentik/)make lint-fix)If an API change has been made
make gen-build)If changes to the frontend have been made
make web)If applicable
make website)