search

crossplane-contrib / provider-keycloak

Apache License 2.0
21 stars 11 forks source link

feature: possibility to import builtin roles and clients #83

Closed Breee closed 2 months ago

Breee commented 5 months ago

Context:

for example: 

>>> Client ID: account, uuid b8727bea-884d-455b-9cb4-b6b39632b0a0
* Role: view-groups, uuid: 53eafb8f-54c1-46f2-9f09-8e6161454ad3
* Role: delete-account, uuid: c95a528f-f775-44a4-bebc-a2f4b4c84c33
* Role: manage-account, uuid: 661d53da-fe9c-4ca8-8e0b-3892630a4849
* Role: view-applications, uuid: a9ac874b-0b6d-4396-b34f-861f4a016641
* Role: view-consent, uuid: e54f4078-3e2a-4977-b273-ffe51312dbbd
* Role: manage-account-links, uuid: 8ea47366-59ce-483b-b822-78d6fcdb44ad
* Role: manage-consent, uuid: ada280ef-2745-4807-aac4-bf9b32c64159
* Role: view-profile, uuid: c6024c59-3489-433f-acf3-5a1486834fcd
>>> Client ID: account-console, uuid c99abc67-e149-4b1d-bf17-6aeaab619892
>>> Client ID: admin-cli, uuid 0c9f326f-9dc9-43fe-9f3d-90069acb1f7f
>>> Client ID: broker, uuid a585bdd7-a02a-43e2-be85-01424f830dcc
* Role: read-token, uuid: 0513fb7b-aa3a-4b52-ab89-3b1d77ce73d5
>>> Client ID: master-realm, uuid 0c3adbf3-8bc6-4c68-a928-6cb806052192
* Role: manage-realm, uuid: 289501ad-0c24-473d-86e7-1e28c977c42b
* Role: query-realms, uuid: 338aa1b7-1592-4b8d-a728-b319641e28f4
* Role: query-groups, uuid: a64bd861-9db8-4c42-9b84-9f06a804d9a0
* Role: query-users, uuid: 25919055-8eb2-4978-a042-45be1d3bb92f
* Role: manage-identity-providers, uuid: 924576f6-7c4a-42f7-98af-f76154a05a79
* Role: manage-authorization, uuid: 09d6eb49-aff1-438d-94d4-42e38591210c
* Role: manage-clients, uuid: 9a0729a4-51ea-4e85-ae71-77818ec3c8ba
* Role: query-clients, uuid: 043fe1f9-db8a-4a6b-b271-cd688ceccc4a
* Role: manage-users, uuid: 9173c5e2-0b55-4302-90b6-04e66193a7fa
* Role: view-identity-providers, uuid: 11110128-4ebb-4d11-af0d-da671b4c7ccb
* Role: view-events, uuid: f42d50d0-b184-4084-8e5d-49c65bc0ac46
* Role: view-clients, uuid: 5662c5d0-7711-4abc-943a-baeec27dffa1
* Role: view-realm, uuid: a6e5bd8c-69ac-441b-844f-3180da5bdfed
* Role: manage-events, uuid: 99af28e7-eeac-4449-be70-1e39aa0e6b62
* Role: impersonation, uuid: c6fbb337-f676-4750-9c47-e834b2222920
* Role: view-authorization, uuid: 1134e38e-ab3c-43b6-8e12-cd3bc23bd5c3
* Role: create-client, uuid: 9c4d6176-e988-4b38-a157-7bf4540e4b62
* Role: view-users, uuid: 7af5df7f-8a32-47db-81fa-de4e71500d90
>>> Client ID: security-admin-console, uuid c9ac072e-4326-4c56-a148-6f15c2a0d8ad
>>> Realm Roles:
* Realm Role: admin, uuid: 9d733d17-53aa-4532-a9de-984d25f2df08
* Realm Role: uma_authorization, uuid: 087dd972-0134-49f3-8831-0ea96b4f4a43
* Realm Role: offline_access, uuid: e4adb92c-3a0b-44e8-b673-05d69a5f093d
* Realm Role: default-roles-master, uuid: a0cd08ee-75f9-42e5-a26f-0ab664881721
* Realm Role: create-realm, uuid: 5e006946-0ab4-4b88-a9af-3a6714dc6314

Problem Statement:

The keycloak API/backend is designed in a way, such that it is required to fetch client and role objects by their UUID. These UUIDs are hard to query / impossible to query without script. (see issues #74 #80)

Possible Solutions

Proposed Solution 1 (simple, provider external): Composition function

Proposed Solution 2 (hard, provider internal): Fetch roles by name in the provider

Breee commented 5 months ago

Current state of the POC can be found here:

https://gitlab.com/corewire/images/crossplane/function-keycloak-builtin-clients

The manifests are here: https://gitlab.com/corewire/images/crossplane/function-keycloak-builtin-clients/-/tree/main/example?ref_type=heads function lives here: https://gitlab.com/corewire/images/crossplane/function-keycloak-builtin-clients/-/blob/main/function/fn.py?ref_type=heads

current plan is that we have a XR that looks like this: 

# Easy to use default
---
apiVersion: crossplane.corewire.io/v1alpha1
kind: XBuiltinObjects
metadata:
  name: keycloak-builtin-clients-myrealm
spec:
  providerConfigName: keycloak-provider-config
  providerSecretName: keycloak-credentials
  realm: my-realm
---
apiVersion: crossplane.corewire.io/v1alpha1
kind: XBuiltinObjects
metadata:
  name: keycloak-builtin-clients-master
spec:
  providerConfigName: keycloak-provider-config
  providerSecretName: keycloak-credentials
  realm: master
  builtinClients: 
  - xxx
  - xxx
  - xxx
  builtinRealmRoles:
  - xxx
  - xxx
  - xxx
  importDefaultRoles: true

which would per default import all builtin clients and their client roles as well as all default realm roles.

The default roles are a bit tricky, because we have to import them and then take them over (That might require another function)

QuadmanSWE commented 5 months ago

Just commenting to keep you motivated. You are doing terrific work and I can't wait to try it out. Let me know if you want someone to rubber duck debug with you.

Breee commented 5 months ago

Don't worry, i'm always motivated - just a bit short of time currently.

It's almost done - i just have to configure automated builds, then you can test.

Of course you can already test right now if you want (only works for amd64 environments in this state): https://gitlab.com/corewire/images/crossplane/function-keycloak-builtin-objects The readme explains everything.

This so far covers the importing of clients and builtin realmroles.

In addition we need to cover the management of the realm default-roles object. This requires more tests tho since the defaultroles are a real pain to manage

Breee commented 5 months ago
vladimirblahoz commented 4 months ago

First of all I really appreciate the effort and how quickly the PoC was made. I have given it a try and for some reason it is not working for me. The XBuildinObjects resource states: Warning ComposeResources 8s (x22 over 28m) defined/compositeresourcedefinition.apiextensions.crossplane.io cannot compose resources: cannot run Composition pipeline step "keycloak-builtin-objects": cannot run Function "function-keycloak-builtin-objects": rpc │ │ error: code = Unknown desc = Unexpected <class 'ValueError'>: Value not set

and the function repeats all over two logs:

"Processing Realm {realm_name}"
"* Realm: example-realm"

Attempt to run the function locally to debug with hatch run development ends with:

Cannot run function: Task <Task pending name='Task-3' coro=<Server.stop() running at /Users/.../git/function-keycloak-builtin-objects/.venv-default/lib/python3.12/site-packages/grpc/aio/_server.py:150> cb=[_run_until_complete_cb() at /Users/.../Library/Caches/pyapp/distributions/_12301412261140716571/python/lib/python3.12/asyncio/base_events.py:182]> got Future <Task pending name='Task-2' coro=<AioServer._server_main_loop()> cb=[AioServer._serving_task_crash_handler()]> attached to a different loop

Sorry, I'd love to help, but I am no python developer

Breee commented 4 months ago

Weird, i tested alot. I also cannot reproduce your issue when you run hatch run development

Can you Post your Manifests? Then i'll test and fix that later

On Tue, May 7, 2024, 14:29 vladimirblahoz @.***> wrote:

First of all I really appreciate the effort and how quickly the PoC was made. I have given it a try and for some reason it is not working for me. The XBuildinObjects resource states: Warning ComposeResources 8s (x22 over 28m) defined/ compositeresourcedefinition.apiextensions.crossplane.io cannot compose resources: cannot run Composition pipeline step "keycloak-builtin-objects": cannot run Function "function-keycloak-builtin-objects": rpc │ │ error: code = Unknown desc = Unexpected <class 'ValueError'>: Value not set

and the function repeats all over two logs:

"Processing Realm {realm_name}" "* Realm: example-realm"

Attempt to run the function locally to debug with hatch run development ends with:

Cannot run function: Task <Task pending name='Task-3' coro=<Server.stop() running at /Users/.../git/function-keycloak-builtin-objects/.venv-default/lib/python3.12/site-packages/grpc/aio/_server.py:150> cb=[_run_until_complete_cb() at /Users/.../Library/Caches/pyapp/distributions/_12301412261140716571/python/lib/python3.12/asyncio/base_events.py:182]> got Future <Task pending name='Task-2' coro=<AioServer._server_main_loop()> cb=[AioServer._serving_task_crash_handler()]> attached to a different loop

Sorry, I'd love to help, but I am no python developer

— Reply to this email directly, view it on GitHub https://github.com/crossplane-contrib/provider-keycloak/issues/83#issuecomment-2098293281, or unsubscribe https://github.com/notifications/unsubscribe-auth/AC3JPMPGPVVMFVOQQYAIQRTZBDCKXAVCNFSM6AAAAABGVGGLUKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAOJYGI4TGMRYGE . You are receiving this because you were assigned.Message ID: @.***>

vladimirblahoz commented 4 months ago

Here are my manifests

The run development thing has got to be sth with my environment, because the example from crossplane documentation does the exact same thing on my machine...

Breee commented 4 months ago

Sorry, was a lot of holiday / longer weekends in germany

I fixed the issue, it was a silly issue. There was a statement credentials = {"username": username, "password": password} after setting up auth credentials that i overlooked when i built the authentication. That made everything fall apart as soon as someone used client_id + client_secret and always tried to use the username + password. Sadly the Keycloak python client threw this obscure exception

Please test with registry.gitlab.com/corewire/images/crossplane/function-keycloak-builtin-objects:v0.6.0

vladimirblahoz commented 4 months ago

Breee, thank you very much for the fix! It's working like a charm. Just one side note - in case I did not provide the builtinRealmRoles property it still failed on the exact same error. Once I added the property, the composition got created properly.

Now is there any chance the default realm roles will be available any time in the future? I really appreciate your work!

Breee commented 4 months ago

I'm on it, i already looked at how to create a custom Controller for that.

Until then you can use the DefaultGroups I implemented, these are working and basically do the same thing.

https://github.com/crossplane-contrib/provider-keycloak/issues/80#issuecomment-2090105050

On Mon, May 20, 2024, 10:58 vladimirblahoz @.***> wrote:

Breee, thank you very much for the fix! It's working like a charm. Just one side note - in case I did not provide the builtinRealmRoles property it still failed on the exact same error. Once I added the property, the composition got created properly.

Now is there any chance the default realm roles will be available any time in the future? I really appreciate your work!

— Reply to this email directly, view it on GitHub https://github.com/crossplane-contrib/provider-keycloak/issues/83#issuecomment-2119996770, or unsubscribe https://github.com/notifications/unsubscribe-auth/AC3JPMMUHTH3R7AUBVY343TZDG3MVAVCNFSM6AAAAABGVGGLUKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMJZHE4TMNZXGA . You are receiving this because you were assigned.Message ID: @.***>

vladimirblahoz commented 4 months ago

That's great. I must have missed that comment completely! Thanks again

olav-st commented 4 months ago

Thanks for creating the POC function! I'm trying it out, but I have an issue with the external-name of the imported clients. The imported clients have the realm name in their external-name while other clients created using the provider do not.

For example, if I create a realm and a client like this:

apiVersion: realm.keycloak.crossplane.io/v1alpha1
kind: Realm
metadata:
  name: testrealm
spec:
  forProvider:
    realm: testrealm
---
apiVersion: openidclient.keycloak.crossplane.io/v1alpha1
kind: Client
metadata:
  name: testclient
spec:
  forProvider:
    name: testclient
    accessType: CONFIDENTIAL
    clientId: testclient
    realmIdRef:
      name: testrealm

and then import the builtin clients from the realm:

apiVersion: keycloak.crossplane.io/v1alpha1
kind: XBuiltinObjects
metadata:
  name: builtin-objects-testrealm
spec:
  providerConfigName: default
  providerSecretName: crossplane-keycloak-credentials
  realm: testrealm
  builtinClients:
    - account
    - account-console
    - admin-cli
    - broker
    - realm-management
    - security-admin-console
  builtinRealmRoles:
    - offline_access
    - uma_authorization

then the builtin clients have the realm name (testrealm/) in their external-name but the testclient does not:

NAME                                       READY   SYNCED   EXTERNAL-NAME                                    AGE
builtin-testrealm-account                  True    True     testrealm/b0dfb3da-2fca-47e2-8b35-bbb961e96048   2m15s
builtin-testrealm-account-console          True    True     testrealm/f9531a8a-eae8-49ae-a8fa-6454fbeef4d0   2m15s
builtin-testrealm-admin-cli                True    True     testrealm/875a954b-4581-4b7e-bba7-5eccade99044   2m17s
builtin-testrealm-broker                   True    True     testrealm/fc8ead7e-4275-4057-9e6d-aade128e7992   2m15s
builtin-testrealm-realm-management         True    True     testrealm/78ee6852-d2ee-44c9-b534-3c7ebf855327   2m16s
builtin-testrealm-security-admin-console   True    True     testrealm/27591ff2-5c44-43fb-8b0e-d21ce343506f   2m16s
testclient                                 True    True     b9e117c7-07f7-475c-8bf7-a683b511bd7b             4m18s

This causes problems when trying to reference the builtin clients in other resources, for example ClientServiceAccountRole.

mustafaStakater commented 4 months ago

I get following error while deploying the function @Breee 

      message: 'cannot unpack package: failed to fetch package digest from remote: failed to fetch package descriptor with a GET request after a previous HEAD request failure: HEAD https://registry.gitlab.com/v2/corewire/images/crossplane/function-keycloak-builtin-objects/manifests/v0.0.5: unexpected status code 404 Not Found (HEAD responses have no body, use GET for details): GET https://registry.gitlab.com/v2/corewire/images/crossplane/function-keycloak-builtin-objects/manifests/v0.0.5: MANIFEST_UNKNOWN: manifest unknown; map[Tag:v0.0.5]'
vladimirblahoz commented 4 months ago

In the examples there is a typo in the version "v0.0.5" -> "v0.5.0", however the current latest is already "v0.6.0", so I'd go for that one

mustafaStakater commented 4 months ago

Refs to builtin roles not working Im having the similar issue as @olav-st

I have a roles.group.keycloak.crossplane.io resource that adds refs to these builtin roles imported by the function. The role ids added are of format realm-name/role-id, These roles arent added to the group because the roleIds have incorrect format. 


apiVersion: group.keycloak.crossplane.io/v1alpha1
kind: Roles
metadata:
  name: my-demo-realm-default-group-roles
spec:
  deletionPolicy: Delete
  forProvider:
    groupIdRef:
      name: my-demo-realm-default-group
    realmId: my-demo-realm
    roleIds:
      - my-demo-realm/ff94abf3-fa01-4e5f-86c9-0f4d0cd17f41
      - my-demo-realm/0829a24e-4fc6-45d3-af52-9b9672a04d59
      - my-demo-realm/32cb1bba-acc6-4926-b1cd-7125b941bc62
    roleIdsRefs:
      - name: builtin-my-demo-realm-realm-management-view-users
      - name: builtin-my-demo-realm-realm-management-view-clients
      - name: builtin-my-demo-realm-realm-management-view-realm
    roleIdsSelector:
      matchLabels:
        defaultRole: 'true'
        realmName: my-demo-realm
  initProvider: {}
  managementPolicies:
    - '*'
  providerConfigRef:
    name: keycloak-config

If i update the CR to remove realm name from roles ids i.e. change my-demo-realm/ff94abf3-fa01-4e5f-86c9-0f4d0cd17f41 to ff94abf3-fa01-4e5f-86c9-0f4d0cd17f41. Roles are added

roleIds from both roleIdsRefs and roleIdsSelector should be used If I specify both roleIdsRefs and roleIdsSelector in custom resource, all roles satisfying the criteria of roleIdsRefs and roleIdsSelector should be added and there results should be merged. Currently, roleIdsRefs takes precedence over roleIdsSelector. Considering the above CR, the roles matching labels specified in roleIdsSelector are not added.

Breee commented 4 months ago

I see the issue. Let's analyze this: Importing e.g. a role usually works like this:

Breee commented 4 months ago

i tried something different:

Please test: xpkg.upbound.io/crossplane-contrib/provider-keycloak:v0.21.0-rc.2

that release will use a UUIDextractor 

func UUIDExtractor() reference.ExtractValueFn {
    return func(mg xpresource.Managed) string {
        paved, err := fieldpath.PaveObject(mg)
        if err != nil {
            // todo(hasan): should we log this error?
            return ""
        }
        r, err := paved.GetString("status.atProvider.id")
        // split at / and return the last element of there are two parts
        // this is to handle the case where the id is a path realm/uuid
        if err != nil {
            // todo(hasan): should we log this error?
            return ""
        }
        split := strings.Split(r, "/")
        if len(split) == 2 {
            return split[1]
        }
        return r
    }
}

which will scrape the correct uuid and strip away "/" if necessary. its probably worth looking into why objects created sometimes have / and sometimes only uuid

Also wrt to @mustafaStakater comment:

"roleIds from both roleIdsRefs and roleIdsSelector should be used If I specify both roleIdsRefs and roleIdsSelector in custom resource, all roles satisfying the criteria of roleIdsRefs and roleIdsSelector should be added and there results should be merged. Currently, roleIdsRefs takes precedence over roleIdsSelector. Considering the above CR, the roles matching labels specified in roleIdsSelector are not added."

I agree that that is what should happen technically, i would say that is an issue in upjet and i have to investigate. But the workaround is simple as you can just import the existing roles you need

olav-st commented 3 months ago

Thanks for looking into it!

I have just tried the new version and it is working! I can now use the built-in clients in ClientServiceAccountRole.

Breee commented 3 months ago

Thanks for looking into it!

I have just tried the new version and it is working! I can now use the built-in clients in ClientServiceAccountRole.

Great, you probably saw it but i released v0.21.0: https://marketplace.upbound.io/providers/crossplane-contrib/provider-keycloak/v0.21.0

Now we just need to write docs for all of this

phac008 commented 3 months ago

I'm on v0.22.0 - but have troubles using build in roles for my client.

what I'm missing here to get view-users role assigned to my service account? if I need additional roles, do I have to create one manifest each?


apiVersion: openidclient.keycloak.crossplane.io/v1alpha1
kind: ClientServiceAccountRole
metadata:
  name: myrole
spec:
  forProvider:
    clientIdRef:
      name: myclient
    realmIdRef: 
      name: myrealm
    role: builtin-myrealm-realm-management-view-users
    serviceAccountUserClientIdRef:
      name: myclient
  providerConfigRef:
    name: "myconfig"
    Last Transition Time:  2024-06-14T13:16:29Z
    Message:               create failed: apply failed: error sending POST request to /admin/realms/myrealm/users/35ee7e22-a70a-488e-93e8-6134a70bd78d/role-mappings/clients/ab6afde7-ec3f-43c3-b861-3c360353c3bc: 404 Not Found. Response body: {"error":"Role not found","error_description":"For more on this error consult the server log at the debug level."}:
    Reason:                ReconcileError
    Status:                False
    Type:                  Synced

Update: for better understanding I added role name I use and add error message I receive. service account is active in myclient clientId and serviceAccountId are correct I'm able to add role to "normal" user in realm

TomBillietKlarrio commented 3 months ago

This is a cool improvement, but it would be nice it this could be taken along more generic. It would be really nice if something like this could work:

apiVersion: role.keycloak.crossplane.io/v1alpha1
kind: Role
metadata:
  name: my-role
  annotations:
    crossplane.io/external-name: "my-role"
spec:
  deletionPolicy: Orphan
  providerConfigRef:
    name: keycloak
  forProvider:
    name: "my-role"

why? I don't want the roles to get deleted if the cluster goes down (or the role objects would happen to get deleted by some fluke). Those roles are granted to a lot of users/groups manually, so if this role would get deleted/recreated all that would be lost. Any chance this could be done? Importing any resource by its name (not the uuid) would be really really useful. The fix in 0.21.0 is nice, but I don't think you can achieve the behavior with this?

Breee commented 3 months ago

Hm, that's how the underlying terraform Part in the upjet generated resources do importing sadly. I'm not sure if we can change that. I'd also prefer something like "realm/rolename" which some of the Ressources do. It's not consistent across the Board tho. I'll have to dig into that and ask the people that wrote upjet.

vladimirblahoz commented 3 months ago

As mentioned in the Speed: Performance issue, this feature seems to be really broken in the v0.24.0-rc.2.

Neither of the external clients nor roles gets synced properly and remains stuck in ReconcileError state with observe failed: external resource does not exist.

What is strange is that the resources obtain proper crossplane.io/external-name annotation with correct external ID.

Is anyone experiencing the same thing?

TomBillietKlarrio commented 3 months ago

I was wondering, what would be the behavior if in external_name.go the config is changed to config.NameAsIdentifier https://github.com/crossplane-contrib/provider-keycloak/blob/9be1e151a09440e0bbdd02c6c4f1e1294a41c11a/config/external_name.go#L26 Would it try to reconcile based on the name instead of the UUID? I was trying it myself, but did not figure out how to run a custom built version on my kind cluster

Breee commented 2 months ago

As mentioned in the Speed: Performance issue, this feature seems to be really broken in the v0.24.0-rc.2.

Neither of the external clients nor roles gets synced properly and remains stuck in ReconcileError state with observe failed: external resource does not exist.

What is strange is that the resources obtain proper crossplane.io/external-name annotation with correct external ID.

Is anyone experiencing the same thing?

This is fixed in v1.0.0, see https://github.com/crossplane-contrib/provider-keycloak/releases/tag/v1.0.0 for details. I would also want to close this issue as we now have the possibility to import the builtin clients and roles.

I will open up a new feature issue to track importing by name instead of uuid, it's not trivial tho.