8000 Support for managing members via group resource type by pedroigor · Pull Request #47636 · keycloak/keycloak · GitHub
[go: up one dir, main page]
Skip to content

Support for managing members via group resource type - #47636

Merged
ahus1 merged 1 commit into
keycloak:mainfrom
pedroigor:issue-46216
Apr 2, 2026
Merged

Support for managing members via group resource type#47636
ahus1 merged 1 commit into
keycloak:mainfrom
pedroigor:issue-46216

Conversation

@pedroigor
@pedroigor pedroigor commented Mar 31, 2026
Copy link
Copy Markdown
Contributor

Closes #46216

  • Allow managing group members via group resource type and its members attribute
  • Make sure Group#members attribute is only returned if requested to avoid fetching all memberships
  • Make sure User#groups attribute is only returned if requested to avoid fetching all memberships

@pedroigor
pedroigor requested a review from a team as a code owner March 31, 2026 13:34
vramik
vramik suggested changes Mar 31, 2026
View reviewed changes
@vramik vramik left a comment
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @pedroigor for the PR, I noticed we will have to also add permission checks before we proceed to joining or leaving groups for users.

I have a #47616 addressing exactly this from the user side of the relationship.

@pedroigor
pedroigor commented Mar 31, 2026
Copy link
Copy Markdown
Contributor Author

@vramik I'll wait for your PR.

sguilhen
sguilhen suggested changes Mar 31, 2026
View reviewed changes
@sguilhen sguilhen left a comment
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If I got this right, nothing is calling the ScimJsonNodeFilterEvaluator anymore. Even Path.getValue() doesn't seem to be used anymore. If that's the case, can't we remove these things?

Comment thread scim/tests/base/src/test/java/org/keycloak/tests/scim/tck/SchemaTest.java Resolved
@pedroigor
pedroigor force-pushed the issue-46216 branch 3 times, most recently from 04d7bd7 to a1478f7 Compare April 1, 2026 13:17
@pedroigor
pedroigor commented Apr 1, 2026
Copy link
Copy Markdown
Contributor Author

ScimJsonNodeFilterEvaluator

@sguilhen Yeah, the getValue is still a valid name, and I renamed the toJsonNode method. Also removed unnecessary classes.

@pedroigor
pedroigor commented Apr 1, 2026
Copy link
Copy Markdown
Contributor Author

@vramik Access checks updated to Group#members.

vramik
vramik reviewed Apr 2, 2026
View reviewed changes
@vramik vramik left a comment •
edited
Loading
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@pedroigor thanks, I see the permissions are correctly applied.

I have two questions:

  • how do we aim to handle organization groups in SCIM endpoints?
    • I purposely blocked the access in my PR, see UserCoreModelSchema.checkGroupMembershipPermission, when looking at GroupResourceTypeProvider.getGroupPredicates it does filter out org groups -> we should then add a check for org group into GroupCoreModelSchema
  • optionally we can introduce small shared utility class, something like GroupMembershipPermissions where checkUserMembershipPermission and checkGroupMembershipPermission could live.

@pedroigor
Support for managing members via group resource type
97c2be1 
    Closes keycloak#46216

Signed-off-by: Pedro Igor <pigor.craveiro@gmail.com>
@ahus1
ahus1 requested a review from sguilhen April 2, 2026 16:07
@ahus1
ahus1 enabled auto-merge (squash) April 2, 2026 18:06
@ahus1
ahus1 merged commit 6a4f865 into keycloak:main Apr 2, 2026
85 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ahus1 ahus1 ahus1 approved these changes
+2 more reviewers
@sguilhen sguilhen sguilhen approved these changes
@vramik vramik vramik approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Support for managing group members via group resource type

4 participants

@pedroigor @sguilhen @vramik @ahus1
0