Skip to content
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

organization groups api prototype #3

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

a8t3r
Copy link
Contributor

@a8t3r a8t3r commented Mar 1, 2023

This is an api prototype for organization groups, related to p2-inc/keycloak-orgs#25
Currently PR is intended for discussion and agreement on the scheme.

@a8t3r
Copy link
Contributor Author

a8t3r commented Mar 1, 2023

Of course, an implementation PR will be created after the schema is accepted.

@xgp
Copy link
Member

xgp commented Mar 2, 2023

@a8t3r This is an interesting proposal. Let me fully review it this weekend, but I think this is a great idea.

@xgp
Copy link
Member

xgp commented Mar 11, 2023

@a8t3r Thanks again for sending this over. Sorry it took me so long to review it. The idea, API and implementation look solid. However, I wanted to introduce an idea for a change that we had been thinking about making to the Organizations model.

Currently, an OrganizationRole is associated with a single Organization. This means that if you have an OrganizationRole that is meaningful in your application that can be given to users in each Organization, you have to create an OrganizationRole for each Organization. This creates lots of duplication, and requires the developer to manage these roles in order to make sure that each Organization has the same OrganizationRoles. Despite our initial implementation following this model, we aren't aware of a single customer using it in a way that requires the OrganizationRole to be associated with an Organization.

We're considering de-coupling OrganizationRoles from Organizations. The API for CRUD OrganizationRoles would change to

/:realm/orgs/roles/:name

but the API for granting OrganizationRole for Users would not change, as the OrganizationRole would be associated with a User for a single Organization

/:realm/orgs/:orgId/roles/:name/users/:userId

The upside is that this would have little effect on how people are using OrganizationRoles today (very few customers us the APIs for CRUD OrganizationRoles), and your idea for adding an OrganizationGroups model would not need to change much. What do you think about that suggestion?

@xgp
Copy link
Member

xgp commented Mar 22, 2023

Hi @a8t3r just wanted to check to see if you had any comments on the proposal. I'm getting ready to implement that, and will come back to the groups PR after that.

@a8t3r
Copy link
Contributor Author

a8t3r commented Mar 22, 2023

@xgp I agree decoupling roles from organizations make sense. But, as I understand, your implementation details don't contains any role to organization associations. This means that different organizations (customers, tenants) could see roles created by others - which is unacceptable for b2b applications. I think some relation between organizations and allowed organization roles is required.

PS. Recently you mentioned :

  • the organization-roles hierarchy is independent of the Keycloak roles

Frankly, I would like to see an implementation in which OrganizationRole entity is replaced with a regular org.keycloak.models.RoleModel. I understand that this is too big a change, but some progress towards a more general solution is needed.

@xgp
Copy link
Member

xgp commented Mar 22, 2023

@a8t3r Regarding your comment about role to organization association, I didn't think it was necessary. From my perspective, Roles are defined by the application, because that is where Roles have meaning. E.g. a photo sharing application defines view-photos and manage-photos Roles. Those roles control functionality within the application. Each Organization administrator can grant those Roles to members of his organization. I couldn't think of a use case where the Organization administrator would define a new role, specific to that Organization, simply because it would have no meaning in the application. Can you think of an example use case where it would?

The problem with implementing this as a standard org.keycloak.models.RoleModel is that hooking into the various places in Keycloak where you'd need to give the user those "effective" roles isn't possible as an extension. I agree that it is desirable to have it be unified, but we could never figure out a way to do it without forking keycloak.

That said, it looks like there is some momentum behind adding Organizations to Keycloak directly. It looks like the Keycloak leader will be designing the implementation. Initial "discussions" are here:

I'm not really sure where this leaves this extension, other than the time between "discussions" and implementations at Keycloak has been years in my observation.

@a8t3r
Copy link
Contributor Author

a8t3r commented Mar 22, 2023

@xgp Thanks for the clarification, I agree with your arguments.

@p2-inc p2-inc deleted a comment from mnsuccess Mar 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants