Add Management Permissions, Sync Mode and generic client role mapper.

[m-v-k]

• keycloak_generic_client_role_mapper
• Partial support for Management Permissions
• Identity Brokering Sync Mode (Keycloak 10)

Extension of existing keycloak_generic_client_role_mapper resource used to generically associate roles to client/client-scope with ability to include realm-level roles association.

usage example:

resource "keycloak_role" "realm_admin" {
  realm_id    = "[example_realm]"
  name        = "realm_admin"
  description = "Admin realm role"
}

resource "keycloak_openid_client_scope" "petstore_api_access_scope" {
  realm_id        = "[example_realm]"
  name            = "petstore-api-access"
  description     = "scope offering additional information for petstore api access"
}

resource "keycloak_generic_client_role_mapper" "petstore_api_access_scope_admin" {
    realm_id           = "[example_realm]"
    client_scope_id    = keycloak_openid_client_scope.petstore_api_access_scope.id
    role_id            = keycloak_role.realm_admin.id
}

Partial implementation of the issue Support Management Permissions Reference #285

Implemented is the management permissions reference for clients (/{realm}/clients/{id}/management/permissions).
Import example for a client which id is 72e58412-1137-407b-af8b-805e40aa60be:

usage example:

resource "keycloak_openid_client_management_permissions_reference" "permissions_reference" {
}

terraform import keycloak_openid_client_management_permissions_reference.permissions_reference test_realm/clients/72e58412-1137-407b-af8b-805e40aa60be/management/permissions

Setting on the management permissions reference for a client:

resource "keycloak_openid_client_management_permissions_reference" "management_permissions_reference" {
  realm_id        = [realm_id]
  client_id       = [client_id]
}

KC10 introduced Identity Brokering Sync Mode with mandatory 'Sync Mode Override' field in all IDP mappers.

This is the extension of the provider to support this in the form of an added "extra_config" optional field which can take any values and maps them in IDP Mappers Configuration.

usage example (against KC10):

resource keycloak_oidc_identity_provider oidc {
    realm                = "my-realm"
    alias                = "my-idp"
    authorization_url    = "https://authorizationurl.com"
    client_id            = "clientID"
    client_secret        = "clientSecret" # or "$${vault.ID}"
    token_url            = "https://tokenurl.com"
    extra_config         = {
      "clientAuthMethod"    = "client_secret_post"
    }
}

resource keycloak_hardcoded_role_identity_provider_mapper oidc {
    realm                   = "[realm]"
    name                    = "[hardcodedRole]"
    identity_provider_alias = keycloak_oidc_identity_provider.oidc.alias
    role                    = "[role]"
    extra_config            = {
        syncMode = "FORCE"
    }
}

also includes examples and tests.

[m-v-k]

(@mrparkers , please rerun that timed out test)

[mrparkers]

Nice changes here, thanks @m-v-k, @slavko-vega, and @branislav-vega!

This is a pretty large PR so I'll take some time over the weekend to look it over.

[m-v-k]

hey @mrparkers , any update on this pr?
just fyi, we currently use this version in (10.0.2) production environment without pain. but i would prefer upstreaming and consuming the community version. just wondering when to expect a merge or feedback.
we are happy to help if necessary.
thanks

[mrparkers]

@m-v-k I'm sorry, I've neglected this PR. I know I said this last time, but I've legitimately set aside time this weekend to take care of the open PRs on this project (including this one) as well as a few of the issues.

I appreciate your patience and the contribution you've made here. Hopefully we'll be able to include these changes in a release some time next week.

[mrparkers]

The code mostly LGTM, thanks again for your work here. Besides the two comments I added for the code itself, I want to try and summarize these changes for my understanding.

1. fix for the keycloak_generic_client_role_mapper resource to support realm roles. the documentation already claims that this is supported, but it clearly wasn't working without your change to the API url the provider uses based on the type of role.
2. adding the extra_config attribute to identity provider mappers, similar to the existing extra_config attribute for the identity provider resources.

both of these changes above are great and could be merged as-is, nice work!

however, I'm going to need a little bit of help to better understand the keycloak_openid_client_management_permissions_reference resource. I did some reading on the docs for this, which helped a little, but I'm having a hard time really "understanding" this, and translating this to what the HCL should look like.

for starters, could you help me understand why this was implemented as a separate resource, as opposed to an attribute on the existing keycloak_openid_client resource? for example, it could look like this:

resource "keycloak_openid_client" "test_client" {
  client_id   = "test-openid-client"
  name        = "test-openid-client"
  realm_id    = "master"
  description = "a test openid client"

  permissions {
    enabled = true // or perhaps the existence of the `permissions` block implies that it's enabled
  }
}

I'm not trying to say that one approach is correct or better than the other, I just want to understand why you decided to use a separate resource for this.

second, could you help me understand how to use the resource and scope_permissions attributes here? it looks like simply enabling permissions for a client will create an authorization resource and several authorization policies within the built-in realm-management client, so how would a user set these attributes themselves instead of omitting them? if a user wanted to set these themselves, do the resource and permissions have to exist within the realm-management client, or could they exist in any client with authorization enabled? if they can exist in any client with authorization enabled, what would an example of this look like?

lastly, I think the current implementation carries a risk that terraform state and actual state could potentially get out of sync. when you create a keycloak_openid_client_management_permissions_reference resource, it enables permissions for that client, and if you navigate to the client permissions tab in the GUI and disable them, terraform doesn't understand that it needs to re-enable them to keep the state consistent. perhaps a dedicated enabled attribute for this resource would help with this.

thanks for sticking through this. I'm looking forward to your response to some of my questions about the client permissions. if you'd like, you can open a new PR with only the first two changes and I'd be happy to merge that while we figure out what to do with the permissions resource in the meantime.

[mrparkers]

nitpick: can you update all TF references using the old 0.11 format to use the new 0.12 format?

example change:

Suggested change

	realm_id = "${keycloak_realm.roles_example.id}"
	realm_id = keycloak_realm.roles_example.id

[mrparkers]

any particular reason the import string needs to include /management/permissions?

[m-v-k]

Thanks @mrparkers ,
I agree, i think it's best to drop that keycloak_openid_client_management_permissions_reference resource for now, look at the 2 comments and merge the rest. We'll have a look this week.

[tomrutsaert]

My 2 cents concerning the permission part.

Idp configuration also has a "Permission" tab. I created support for it in #318. (it is still a preview feature, but it is the same is this)
This biggest difference is that I only needed take into account the token exchange scope based permission. For the openid client, enabling the permissions, automatically creates 7 scope based permissions (including token exchange).

Generally once an administrator enables the permissions on a client, he only needs to a specific policy on 1 or more permissions and he is done.
The point is, enabling permissions on a openid client creates a whole bunch of things under the hood on the fine grained authorization services of the 'realm-management' client. It is not just a simple switch that is on or off.

So imho just like I did in #318, I think it is good idea to handle it as a separate resource.
This is also handled on a different endpoint than where the other client settings are handled.
PUT /auth/admin/realms/myrealm/clientsmyclientid vs PUT /auth/admin/realms//myrealm/clientsmyclientid/management/permissions

In #318 I automatically created a policy, I do not know if that is a good idea in this case.
I do not even know if it was good idea within #318.

I would separate this feature from the 2 other features in this PR.
Approve the 2 other features and start a separate discussion about this feature in a separate PR.

(We could reuse/refactor/rename keycloak/identity_provider_permissions.go to handle also the permissions created by enabling permissions on a client)

Also take a look at the documentation of #318
https://github.com/mrparkers/terraform-provider-keycloak/blob/master/docs/resources/keycloak_identity_provider_token_exchange_scope_permission.md
It might give some insight

[slavko-vega]

Thank you @mrparkers and @tomrutsaert. As @m-v-k wrote, we're dropping keycloak_openid_client_management_permissions_reference from this PR. We also made changes on code accordingly to @mrparkers comments and the upstream branch is updated. @mrparkers, could you review the PR now?

[mrparkers]

Looks good! thanks for the contribution!