p2-inc / keycloak-orgs

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

List service accounts in the organization members API #244

Open kedare opened 3 weeks ago

kedare commented 3 weeks ago

Fixes #163

Update the https://{instance}/realms/{realm}/orgs/{realm_id}/members/ endpoint to allow listing service-account users (removing the filter condition that would hide them)

Before:

[
    {
        "id": "2f8ba25c-db51-4525-87b2-b63bd9d58cc7",
        "createdTimestamp": 1718032779879,
        "username": "org-admin-1c074437-7429-435c-b769-d830ac7dcad2",
        "enabled": true,
        "totp": false,
        "emailVerified": true,
        "email": "org-admin-1c074437-7429-435c-b769-d830ac7dcad2@noreply.phasetwo.io",
        "disableableCredentialTypes": [],
        "requiredActions": [],
        "notBefore": 0
    },
    {
        "id": "87bdcee7-cd52-4662-b217-b205def4257b",
        "createdTimestamp": 1709729483380,
        "username": "mathieu.poussin@xxx.com",
        "enabled": true,
        "totp": false,
        "emailVerified": true,
        "firstName": "Mathieu",
        "lastName": "Poussin",
        "email": "mathieu.poussin@xxx.com",
        "disableableCredentialTypes": [],
        "requiredActions": [],
        "notBefore": 0
    }
]

After:

[
    {
        "id": "2f8ba25c-db51-4525-87b2-b63bd9d58cc7",
        "username": "org-admin-1c074437-7429-435c-b769-d830ac7dcad2",
        "email": "org-admin-1c074437-7429-435c-b769-d830ac7dcad2@noreply.phasetwo.io",
        "emailVerified": true,
        "createdTimestamp": 1718032779879,
        "enabled": true,
        "totp": false,
        "disableableCredentialTypes": [],
        "requiredActions": [],
        "notBefore": 0
    },
    {
        "id": "87bdcee7-cd52-4662-b217-b205def4257b",
        "username": "mathieu.poussin@xxx.com",
        "firstName": "Mathieu",
        "lastName": "Poussin",
        "email": "mathieu.poussin@xxx.com",
        "emailVerified": true,
        "createdTimestamp": 1709729483380,
        "enabled": true,
        "totp": false,
        "disableableCredentialTypes": [],
        "requiredActions": [],
        "notBefore": 0
    },
    {
        "id": "f9a3fba5-85d5-4ffa-97b7-ff2d5b1364e0",
        "username": "service-account-insomnia-m2m",
        "emailVerified": false,
        "createdTimestamp": 1709729428736,
        "enabled": true,
        "totp": false,
        "disableableCredentialTypes": [],
        "requiredActions": [],
        "notBefore": 0
    }
]

The service user will also appear in the admin UI on the members list

This did not impact the tests, it looks like this part is not covered by unit testing ?

kedare commented 3 weeks ago

It may be interesting to do also a second PR to allow the service accounts to be listed in the "Add members" part, but as it's managed by the Keycloak API I guess it's more a frontend modification ? (Not really something I know well)

xgp commented 3 weeks ago

We'd need to make sure any upstream users of this method aren't expecting it to be filtered. I know that the APIs assume that service accounts will not be returned/counted.

kedare commented 3 weeks ago

Should it then be a new API endpoint or a parameter on the endpoint ?

xgp commented 2 weeks ago

It should be a parameter that defaults to not showing/including them so that calling them as now produces the same result.

kedare commented 5 days ago

Any preference for the name of the parameters ? Something like includesServiceAccounts ? I will put the same filter on the API endpoint directly based on this conditional.

How should we do for the keycloak API endpoint used to display the list of users that can be added ? Likely it would be a frontend change instead ?

xgp commented 5 days ago

Any preference for the name of the parameters ? Something like includesServiceAccounts ? I will put the same filter on the API endpoint directly based on this conditional.

includeServiceAccounts

How should we do for the keycloak API endpoint used to display the list of users that can be added ? Likely it would be a frontend change instead?

You would need to extend Keycloak to do that.

kedare commented 1 day ago

This should be ok with this updated endpoint ?

For the frontend part I guess I should update this ? https://github.com/p2-inc/keycloak/blob/23.0.1_orgs_admin_ui/js/apps/admin-ui/src/phaseII/orgs/useOrgFetcher.ts#L169-L181 Should I get the service accounts by default on the frontend part ?

I took a look at the keycloak source code also to add the same parameter to the users endpoints but no luck so far finding where to add is, the code structure is much more complex.