Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat: Add keycloak_groups_permissions for admin_fine_grained_authz #617

Merged
merged 5 commits into from
Nov 5, 2021

Conversation

hoeggi
Copy link
Contributor

@hoeggi hoeggi commented Oct 19, 2021

This is a to add support for fine grained permissions on groups, it's somewhat similar to #409

I needed this internally so it is still missing documentation and tests. I marked this PR as WIP to ask if you are interested in this and if yes i will finish up with documentation and add the test cases.

Edit: I already added some documentation and a basic test case. Let me know if you would like anything more.

@hoeggi hoeggi changed the title [WIP] Add keycloak_groups_permissions for admin_fine_grained_authz feat: Add keycloak_groups_permissions for admin_fine_grained_authz Oct 20, 2021
@mrparkers
Copy link
Contributor

This looks great, thanks for the PR! The only thing I'd like to be changed is to rename this resource from keycloak_groups_permissions to keycloak_group_permissions, as this resource operates on a single group.

You may have borrowed this naming convention from keycloak_users_permissions, but that resource deals with permissions for all users in a realm (via /realms/${realmId}/users-permissions), whereas this resource is only looking at a single group.

Other than this change, everything else LGTM.

@hoeggi
Copy link
Contributor Author

hoeggi commented Oct 27, 2021

Yes you are right, i did borrow lot's of existing code :)

I did a full rename from groups to group on all filenames/resources.

Copy link
Contributor

@mrparkers mrparkers left a comment

Choose a reason for hiding this comment

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

Sorry for the late approval. Everything looks great, thanks for the PR!

@mrparkers mrparkers merged commit fa9748f into keycloak:master Nov 5, 2021
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants