List service accounts in the organization members API #244

kedare · 2024-06-10T15:37:36Z

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 · 2024-06-10T15:40:09Z

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 · 2024-06-10T16:28:23Z

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 · 2024-06-11T09:19:23Z

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

xgp · 2024-06-11T18:20:40Z

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

kedare · 2024-06-27T14:43:25Z

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 · 2024-06-27T15:57:35Z

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 · 2024-07-01T10:37:48Z

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.

kedare · 2024-07-08T09:37:49Z

@xgp Let me know if you have any hints about anything else I would need to update.

For the current API part, does that looks ok to you ?

xgp · 2024-07-09T17:58:36Z

For the frontend part I guess I should update this ?

Yes. Current branch for 25 to be updated is here: https://github.com/p2-inc/keycloak/blob/25.0.0_orgs_admin_ui/js/apps/admin-ui/src/phaseII/orgs/useOrgFetcher.ts#L171-L183

We also need to update the openapi spec

https://github.com/p2-inc/phasetwo-docs/blob/main/openapi.yaml#L613

For the current API part, does that looks ok to you ?

Yes. As long as all tests are passing.

kedare · 2024-07-10T09:09:31Z

For the frontend part, what should be the behaviour ? Display the service accounts by default ?

xgp · 2024-07-10T09:25:12Z

Backwards compatible. The same behavior as before the change.

rtufisi · 2024-07-17T15:01:45Z

src/main/java/io/phasetwo/service/model/jpa/OrganizationAdapter.java

@@ -198,8 +198,7 @@ public Long getMembersCount() {
  public Stream<UserModel> getMembersStream() {


@xpg do we need to update getMembersCount?

I couldn't find the usage of /members/count endpoint

Yes, this should be updated and tested also.

@kedare Please update the /members/count method also

Do you know if by default it already contains the service account ? Because it's a named query that counds entries on OrganizationMemberEntity so I guess yes ? (if not that would mean the service accounts are on another table?)

By default the /members/count endpoint contains both users + service account users.

I guess I have to check if getServiceAccountClientLink() is null ?
Also I see there are no relation defined between OrganizationMemberEntity and UserEntity (just the ID field buy no FK), is there any reason for this ? Should this be added? It's not 100% clear how to join them via HQL, it looks like the usual SQL join syntax would not work on this case ?

You can join them on the id

What would be the best way to test HQL named queries ? Or I need to compile and run the whole test suite every time ?

Maybe someone could write the HQL query directly ? I guess it would take less than a few minutes for someone that knows Hibernate well.

Please let me know when you are able to find someone to complete this feature for you. I am closing the PR until that time.

src/main/java/io/phasetwo/service/resource/MembersResource.java

pierre-tranchard · 2024-07-31T13:59:03Z

@rtufisi @xgp is there anything remaining to do before you can merge it? Thank you in advance 🙏

xgp · 2024-07-31T16:04:05Z

Closing until it is possible to submit a completed PR.

List service accounts in the organization members API

12f1543

kedare added 2 commits July 1, 2024 12:03

Update org members api endpoint to allow to list service accounts

c11bdbe

Fix members endpoint

a712c91

kedare mentioned this pull request Jul 10, 2024

Update openapi for org members endpoint p2-inc/phasetwo-docs#177

Closed

kedare mentioned this pull request Jul 10, 2024

Support for service accounts as organization members p2-inc/keycloak#246

Open

pierre-tranchard approved these changes Jul 15, 2024

View reviewed changes

xgp requested a review from rtufisi July 15, 2024 17:55

rtufisi reviewed Jul 17, 2024

View reviewed changes

kedare and others added 2 commits July 29, 2024 12:06

Add API endpoint tests

a0d97f7

Merge branch 'main' into kedare/m2m-client-api-orgs

af48a64

xgp closed this Jul 31, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

List service accounts in the organization members API #244

List service accounts in the organization members API #244

kedare commented Jun 10, 2024 •

edited

Loading

kedare commented Jun 10, 2024

xgp commented Jun 10, 2024

kedare commented Jun 11, 2024

xgp commented Jun 11, 2024

kedare commented Jun 27, 2024 •

edited

Loading

xgp commented Jun 27, 2024

kedare commented Jul 1, 2024 •

edited

Loading

kedare commented Jul 8, 2024

xgp commented Jul 9, 2024

kedare commented Jul 10, 2024

xgp commented Jul 10, 2024 via email

rtufisi Jul 17, 2024

xgp Jul 18, 2024

xgp Jul 19, 2024

kedare Jul 22, 2024

rtufisi Jul 28, 2024

kedare Jul 30, 2024

xgp Jul 30, 2024

kedare Jul 31, 2024

kedare Jul 31, 2024

xgp Jul 31, 2024

pierre-tranchard commented Jul 31, 2024

xgp commented Jul 31, 2024

		@@ -198,8 +198,7 @@ public Long getMembersCount() {
		public Stream<UserModel> getMembersStream() {

List service accounts in the organization members API #244

List service accounts in the organization members API #244

Conversation

kedare commented Jun 10, 2024 • edited Loading

kedare commented Jun 10, 2024

xgp commented Jun 10, 2024

kedare commented Jun 11, 2024

xgp commented Jun 11, 2024

kedare commented Jun 27, 2024 • edited Loading

xgp commented Jun 27, 2024

kedare commented Jul 1, 2024 • edited Loading

kedare commented Jul 8, 2024

xgp commented Jul 9, 2024

kedare commented Jul 10, 2024

xgp commented Jul 10, 2024 via email

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pierre-tranchard commented Jul 31, 2024

xgp commented Jul 31, 2024

kedare commented Jun 10, 2024 •

edited

Loading

kedare commented Jun 27, 2024 •

edited

Loading

kedare commented Jul 1, 2024 •

edited

Loading