p2-inc / keycloak-orgs

Single realm, multi-tenancy for SaaS apps
Other
362 stars 65 forks source link

Add 'organization/role' flattened claim array mapper #139

Closed fr6nco closed 8 months ago

fr6nco commented 8 months ago

Adds a new mapper which maps roles to a claim in '{organization}/{role}' format. I would love to map organization roles into the RBAC model of ArgoCD, which cant work tych mapped type of organization / role.

https://argo-cd.readthedocs.io/en/stable/operator-manual/user-management/keycloak/

Im not a java programmer, tried to do my best. Feedback and edits are welcome.

xgp commented 8 months ago

@fr6nco Thanks for the PR. It seems like a reasonable thing to add. Can you write some tests? Since we are changing the abstract class that other mappers use, we need to ensure backwards compatibility.

fr6nco commented 8 months ago

There are no changes in the AbstractOrganizationMapper. Type changed from Map to Object (L69), whereas claim in the setClaim functions expect an Object return value from getOrganizationClaim anyways. UserModel user = .. (L94) removed because it was redundant. upstream keycloak project lacks test on mappers too.

Writing these tests are out of my capabilities. Im more than happy to assist if u can help with it.

xgp commented 8 months ago

Thanks for the context. I've never found "nobody else is doing it" to be a compelling reason not to have tests when things are changed. I don't have time to write them for you, so please reopen when you have found someone to help you.