8000 Service account user ID available on client representation instead of exposing separate endpoint returning whole user representation by vramik · Pull Request #54056 · keycloak/keycloak · GitHub
[go: up one dir, main page]
Skip to content

Service account user ID available on client representation instead of exposing separate endpoint returning whole user representation - #54056

Merged
pedroigor merged 1 commit into
keycloak:mainfrom
vramik:54047-ClientResource-getServiceAccountUser
Oct 20, 2026
Merged

Service account user ID available on client representation instead of exposing separate endpoint returning whole user representation#54056
pedroigor merged 1 commit into
keycloak:mainfrom
vramik:54047-ClientResource-getServiceAccountUser

Conversation

@vramik
@vramik vramik commented Oct 16, 2026
Copy link
Copy Markdown
Contributor

Closes #54047

@vramik
vramik requested a review from a team as a code owner October 16, 2026 09:14
Copilot AI balanced review requested due to automatic review settings October 16, 2026 09:14
Copilot AI reviewed Oct 16, 2026
Copilot AI left a comment
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@vramik
vramik requested a balanced review from Copilot October 16, 2026 09:14
Copilot AI reviewed Oct 16, 2026
View reviewed changes
Copilot AI left a comment
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The focused authorization change addresses the disclosure and includes regression coverage for both access outcomes; tests were not executed.

0 open findings

🧠 Review effort: Balanced

Copilot AI balanced review requested due to automatic review settings October 19, 2026 17:43
@vramik
vramik force-pushed the 54047-ClientResource-getServiceAccountUser branch from e813c0d to da9edcf Compare October 19, 2026 17:43
@vramik
vramik requested review from a team as code owners October 19, 2026 17:43
@vramik vramik changed the title Add per-user auth check when getting UserRep from service account Service account user ID available on client representation instead of exposing separate endpoint returning whole user representation Oct 19, 2026
Copilot AI reviewed Oct 19, 2026
View reviewed changes
Copilot AI left a comment
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Enabling or re-enabling service accounts leaves the Admin UI using missing or stale user IDs, breaking links and role mappings.

1 open issue

Review effort: Balanced

Comment thread js/apps/admin-ui/src/clients/service-account/ServiceAccount.tsx Resolved
@vramik
vramik marked this pull request as draft October 19, 2026 18:27
@vramik
vramik force-pushed the 54047-ClientResource-getServiceAccountUser branch from da9edcf to 1c5379e Compare October 19, 2026 18:37
@vramik
vramik requested a balanced review from Copilot October 19, 2026 18:38
Copilot AI reviewed Oct 19, 2026
View reviewed changes
Copilot AI left a comment
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Disabling and re-enabling service accounts leaves the console using a deleted user ID, breaking profile links and role mappings.

2 open issues

Review effort: Balanced

Comment thread js/apps/admin-ui/src/clients/service-account/ServiceAccount.tsx Resolved
@vramik
vramik force-pushed the 54047-ClientResource-getServiceAccountUser branch from 1c5379e to 521ad8e Compare October 19, 2026 18:50
@vramik
vramik requested a balanced review from Copilot October 19, 2026 18:52
Copilot AI reviewed Oct 19, 2026
View reviewed changes
Copilot AI left a comment
Copy link
Copy Markdown
Contributor
8000

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Partial-import account initialization regresses, client listing gains per-account queries, and the reported authorization scenario lacks regression coverage.

3 open issues
2 resolved since last review

Review effort: Balanced

Comment thread server-spi-private/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java Resolved
Comment thread server-spi-private/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java Resolved
Comment thread tests/base/src/test/java/org/keycloak/tests/admin/ClientTest.java Resolved
@vramik
vramik force-pushed the 54047-ClientResource-getServiceAccountUser branch from 521ad8e to 5071d9e Compare October 19, 2026 19:35
@vramik
vramik requested a balanced review from Copilot October 19, 2026 19:40
Copilot AI reviewed Oct 19, 2026
View reviewed changes
Copilot AI left a comment
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Eager service-account creation breaks partial imports that also include the corresponding user.

1 open issue
3 resolved since last review

Review effort: Balanced

Comment thread services/src/main/java/org/keycloak/partialimport/ClientsPartialImport.java Resolved
@vramik
vramik force-pushed the 54047-ClientResource-getServiceAccountUser branch from 5071d9e to baba68b Compare October 19, 2026 20:41
@vramik
vramik requested a balanced review from Copilot October 19, 2026 20:42
Copilot AI reviewed Oct 19, 2026
View reviewed changes
Copilot AI left a comment
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Post-import initialization can enable service accounts on clients that the import policy explicitly skips.

1 open issue
1 resolved since last review

Review effort: Balanced

Comment thread services/src/main/java/org/keycloak/partialimport/PartialImportManager.java Resolved
@vramik
vramik force-pushed the 54047-ClientResource-getServiceAccountUser branch from baba68b to 2c421da Compare October 19, 2026 20:55
@vramik
vramik requested a balanced review from Copilot October 19, 2026 20:56
Copilot AI reviewed Oct 19, 2026
View reviewed changes
Copilot AI left a comment
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The security-sensitive API removal and import lifecycle changes warrant human validation, with an unresolved Admin UI error-handling issue.

1 open issue
1 resolved since last review

Review effort: Balanced

Comment thread js/apps/admin-ui/src/clients/service-account/ServiceAccount.tsx Resolved
@vramik
vramik marked this pull request as ready for review October 19, 2026 21:04
@vramik
vramik requested review from pedroigor and a balanced review from Copilot October 19, 2026 21:05
@vramik
vramik force-pushed the 54047-ClientResource-getServiceAccountUser branch from 2c421da to 68fc3f1 Compare October 19, 2026 21:56
Copilot AI reviewed Oct 19, 2026
View reviewed changes
Copilot AI left a comment
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Security-sensitive endpoint removal, public API compatibility changes, and service-account lifecycle changes warrant final maintainer review.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

@vramik
Service account user ID available on client representation instead of…
568fba0 
… exposing separate endpoint returning whole user representation

Closes keycloak#54047

Signed-off-by: vramik <vramik@redhat.com>
Copilot AI balanced review requested due to automatic review settings October 20, 2026 12:00
@vramik
vramik force-pushed the 54047-ClientResource-getServiceAccountUser branch from 68fc3f1 to 568fba0 Compare October 20, 2026 12:00
Copilot AI reviewed Oct 20, 2026
View reviewed changes
Copilot AI left a comment
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Typed service-account clients can remain uninitialized, leaving the replacement account ID unavailable.

1 open issue

Review effort: Balanced

Comment thread server-spi-private/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java
Comment on lines +1592 to +1594
UserModel serviceAccount = session.users().getServiceAccount(client);
if (serviceAccount != null) {
rep.setServiceAccountUserId(serviceAccount.getId());
@pedroigor
pedroigor commented Oct 20, 2026
Copy link
Copy Markdown
Contributor

Thanks, @vramik. LGTM now.

@pedroigor
pedroigor merged commit e203767 into keycloak:main Oct 20, 2026
98 checks passed
@michalvavrik michalvavrik mentioned this pull request Oct 22, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@pedroigor pedroigor pedroigor approved these changes

Assignees

No one assigned

Labels

team/core-iam

Projects

None yet

Milestone

No milestone

3 participants

@vramik @pedroigor
0