Add resource keycloak_user_roles

[elmarx]

Hey,

I know there are already two open PRs to add user-role-support, but they both look stale and not ready…

This one is done, it's based heavily upon the keycloak_group_roles, although in some details the implementation differs, since the group-endpoint returns mapped roles, but the user-endpoint not.

On the other hand, explicitly querying the user's role-mapping endpoints returns full roles (opposed to role-names), thus the implementation is simpler, once the role-mapping endpoint has been queried.

I also tested the resource manually, so as far as I know it's done/it works, but I'm looking forward to your feedback.

[n0rig]

This looks great; this is something I was trying to solve. Can we get more eyes on this?

[tomrutsaert]

I see you use "realm" as a keyword in the map of roles. But what if there is a client called "realm"?
Is there a reason for not using the UserRoleMapping struct instead of this map of roles?
I see you moved this method from resource_keycloak_group_roles, so probably that is the way it used there as well...., But the same remark still applies

[elmarx]

Correct, I moved this function to reuse it. And I reused this "pattern" with 'realm' as a special-keywork for clients without challenging it.
But looking at the code, using a dedicated struct might simplify the handling/code, and would avoid the edge-case with clients named "realm".

I will look into this and see if I can change this.

[mrparkers]

Yeah feel free to blame me for this one. Using a dedicated struct is probably better, but I won't force you to fix this since it was my decision to do this in the first place.

[tomrutsaert]

Do I understand correctly that "role_ids" contains both realm role ids and client role ids?
(I guess as they are uuids they can never a duplicate id between client role ids and realm role ids?)

[elmarx]

Yes. That's once again adopted from keycloak_group_roles.

[mrparkers]

Yup, Keycloak doesn't actually care if its a realm or client role, so in this sense, the provider doesn't either. So if you have multiple keycloak_role resources, you can add each keycloak_role.*.id to this set without having to worry about whether the roles are attached to realms or clients.

[mrparkers]

I think this can be merged as-is, but I'll give you the opportunity to make the improvement that @tomrutsaert suggested if you're interested. If not, we can merge this as-is.

Thanks for the PR!

[elmarx]

Thanks for your feedback!

Well I played around with the code, and yes, using a dedicated struct is much cleaner and should be simple. I will update this PR in the next few days :)

[elmarx]

OK, PR has been updated.

Short summary of changes:

The map is gone and I replaced it with a dedicated structure roleMapping, which is similar to Keycloak's UserRoleMapping, but not the same (the map for client-roles uses the uuid instead of the clientid which keycloak uses).
Thus keycloak UserRoleMapping needs to be transformed into the internal roleMapping.

This of course means the previous "diff-function" removeDuplicationRoles from resource_keycloak_group_roles can't be reused, so there's calculateRoleMappingUpdates to figure out the updates.

[mrparkers]

your new implementation for this is great! thanks for your work on this.

I'm going to update the group roles implementation to match yours.

thank you for the PR!

[elmarx]

Awesome, thanks for merging and thanks for adding documentation ;)

[ryanmac8]

Any idea when the documentation will be updated? Would like to try this out but don't know what variables the resource requires.

[elmarx]

mrparkers already updated the documentation: https://github.com/mrparkers/terraform-provider-keycloak/blob/master/docs/resources/keycloak_user_roles.md

But please be aware that since the merge of this PR no version has been released, yet. So you need to compile the provider on your own.

[ryanmac8]

Ah ok. Thanks! I was reading the documentation from https://mrparkers.github.io/terraform-provider-keycloak and couldn't find it. I did end up compelling and testing it out. Works perfectly! :D