support Keycloak roles

[mrparkers]

Closes #102

cc @MatteoJoliveau, @xiaoyang-connyun, @dhardiker, @camelpunch

• roles and client roles
• role mapper
• role data source
• composite roles
• assign roles to groups
• docs

Please let me know if I'm missing anything from the list above. The work I have done so far fits my use case, but I don't want to merge this until it works for everyone who chimed in on #102.

The included roles.tf file is an example of what I have so far. I'll try to keep working on this throughout the week.

[mrparkers]

refactoring - these aren't needed

[mrparkers]

these tests were failing for me locally, probably due to #132. I am not sure why they pass in CI

[camelpunch]

Yeah I nearly raised an Issue about the local test failures, but assumed I'd missed some setup. I can report back on whether the functionality is sufficient, but don't think I have sufficient context to investigate the existing test failures.

[mrparkers]

Don't worry too much about the test failures, that's something I can look in to.

The changes I made will at least allow these tests to pass locally for me. I'd be curious to see if they pass locally for you as well if you pull my branch down.

[mrparkers]

FYI - these failed locally and passed in CI because my local setup uses postgres, which uses a varchar(255) column for user attributes, hence the failure. CI uses in-memory h2 which does not appear to have that limitation.

[fharding1]

I've been using the client roles portion of this and it seems to work well!

[mrparkers]

Thanks for the feedback @fharding1. Do you use composite roles? I'll be finished with that within the next couple of days and there are a few things that I would like some feedback on relating to that.

[fharding1]

No problem, thank you for doing this work 🙏. We don't use composite roles, however, I can definitely mess around with that a bit and give some feedback.

[camelpunch]

We use composite roles, so if that support gets added we'd love to try it out.

[mrparkers]

Just pushed up support for composite roles (no tests yet, sorry).

It works fine with the roles.tf example file. The feedback I'm looking for is in regards to the diff that's shown during tf plan, which looks like this when updating a role's composites:

Terraform will perform the following actions:

  # keycloak_role.pet_api_admin will be updated in-place
  ~ resource "keycloak_role" "pet_api_admin" {
        client_id       = "9cddfee4-e00f-4628-a170-59e56c5e8918"
      ~ composite_roles = [
            "02e03e74-4bfa-443a-8b51-4de640e52b1c",
            "8f7fe776-d225-471b-86d9-820211776337",
          - "a1ad0fb9-0fd6-4d46-844c-268db8dfde84",
        ]
        id              = "16648524-8e01-4fb1-895c-11654fac749a"
        name            = "admin"
        realm_id        = "roles-example"
    }

Plan: 0 to add, 1 to change, 0 to destroy.

To me, this diff kinda sucks because you don't really know which role is being removed here, just its ID, and you obviously wouldn't know or care about that.

I can change the composite_roles attribute to accept strings that you see in the GUI, which follow the pattern of client_name.role_name for client roles, or role_name for realm roles. Perhaps I can do this by exporting a role_id computed attribute that follows this pattern, so variable interpolation can still be used. The downside of this approach is that a lot more API calls need to be made in order to determine this extra information. I'm not sure if that is worth the friendlier diff.

I'd be interested to hear someone else's thoughts on this.

[camelpunch]

While it sucks, I wouldn't worry about it for a first version. We're likely to double check through the GUI and automated tests anyway, so trusting that the IDs line up is sufficient for now.

[fharding1]

I'll +1 what @camelpunch said. I think while it's not perfect that the IDs aren't very friendly, making more API calls isn't ideal either, and people should be checking results via the GUI anyways. I agree that this is totally fine for now.

@mrparkers would you like some assistance with the last portion "assign roles to groups"? We would like to use this soon (and I definitely don't want to put any pressure on you to feel the need to rush this), so I would be happy to work on it if you're busy with the other portions.

[fharding1]

What were you thinking about doing for the HCL for client roles, since they're a map of strings to lists of strings and terraform doesn't support that?

I was thinking it could be either a list of lists where you specify the client as the first value and the role as the second, or a map of strings to strings with the value being a comma separated list of roles. We could even do something like a list of strings where each string is "${client}.${role}". Another (probably cleaner, but quite different from the API and GUI) option could be to specify a list of IDs, that are looked up to get the client and role name, however this would likely be very expensive since you would have to look up all the clients and their roles. Not sure I like any of these. Thoughts?

[mrparkers]

Actually, I just have client_id as an optional attribute on the keycloak_role resource, so if that attribute is provided then it will be a client role, otherwise it will be a realm role. The example file I've included in this PR has some client roles within it.

I liked that approach because it works well with Keycloak's API, which doesn't treat client roles as an attribute on the client itself, but rather an independent resource.

Let me know if this approach doesn't make sense and we can come up with a different way to handle it.

[mrparkers]

would you like some assistance with the last portion "assign roles to groups"? We would like to use this soon (and I definitely don't want to put any pressure on you to feel the need to rush this), so I would be happy to work on it if you're busy with the other portions.

I appreciate the offer! I have some time set aside today to finish this though, so I should be all good.

[fharding1]

I should have clarified that I was referring specifically to groups, and I was just looking at the GroupRepresentation in the docs that has realmRoles as a array and clientRoles as a map<string, array>, however I didn't even realize that there are endpoints like POST /{realm}/groups/{id}/role-mappings/realm and POST /{realm}/groups/{id}/role-mappings/clients/{client} so nevermind 🤦‍♂️

And awesome 🎉 figured I would ask.

[fharding1]

Thanks for adding the missing navbar link!

[mrparkers]

I just pushed up the keycloak_group_roles resource. I'd love to hear any feedback anyone in this thread might have.

One thing I noticed when messing with it was that adding a role to a group would "soft fail" if that role was already an "effective role", meaning that some other role with that role as a composite is already mapped to that group. I could use /{realm}/groups/{id}/role-mappings/clients/{client}/composite for validation to see if a new role is already an effective role, or I could just document it and figure that out later since this PR has been open for long enough.

Depending on what everyone things of my progress here, all I have left to do is write docs, merge this, and cut a release.

[camelpunch]

We've used the keycloak_group_roles resource and it was fairly intuitive. There was a slight hurdle to get over to realise that the ID was the same as the keycloak_group resource. Worth adding that to the docs perhaps.

Otherwise we're chomping at the bit for a release. Thank you so much for rescuing us from monster-ball-of-JSON hell!

[fharding1]

I've been testing out keycloak_group_roles this morning. Seems to work great! Excited for this to be merged, thanks for working on it 💯

[rlewan]

We'd like to be able to attach client roles to users. Looking into the code this doesn't seem to be possible at the moment? Do you plan on supporting that use case?

[mrparkers]

@rlewan I don't have a problem with adding support for that. Most of the hard work for that has already been taken care of in the keycloak_group_roles resource.

Could you help me understand your case for assigning roles directly to users as opposed to groups of users? I'm just curious since it is not something I would personally use.

[camelpunch]

@mrparkers I think this use case @rlewan was talking about was for service accounts attached to clients.

[rlewan]

@mrparkers exactly what @camelpunch said.

We use service accounts for service-to-service communication, so far Keycloak UI allowed us to assign necessary client roles to those (Service Account Roles tab under Clients, the client has to be confidential and have service accounts enabled).

[mrparkers]

Gotcha, that use case definitely makes sense. I forgot to consider that.

I can definitely support that, but I'd like to double check with you first on the HCL you'd expect to use. I opened #152 to keep track of this and discuss the HCL you'd expect.

I'm probably going to merge this PR and cut a release without this support just to keep things moving, but I can work on what you're asking for pretty soon afterwards.

[fharding1]

🎉 thanks again for adding this!