Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Support unmanaged attributes for service accounts and make sure they are only managed through the admin api #29571

Merged
merged 1 commit into from
May 21, 2024
Merged

Support unmanaged attributes for service accounts and make sure they are only managed through the admin api #29571

merged 1 commit into from
May 21, 2024

Conversation

pedroigor
Copy link
Contributor

@pedroigor pedroigor commented May 15, 2024 •
edited

Closes #29362

  • Also removes customizations in the UI Admin REST API related to users as they are no longer needed by moving the endpoint to return unmanaged attributes to the User API
  • Makes sure that attributes for service accounts can only be managed through the User API. In practice, this change makes impossible to manage service accounts through any user-facing context such as account, update profile, etc.

@pedroigor pedroigor requested review from a team as code owners May 15, 2024 15:50
pedroigor
pedroigor commented May 15, 2024
View reviewed changes
js/apps/admin-ui/cypress/e2e/users_test.spec.ts
@@ -203,7 +203,7 @@ describe("User creation", () => {

cy.wait("@save-user").should(({ request, response }) => {
expect(response?.statusCode).to.equal(204);
expect(request.body.attributes, "response body").deep.equal({
expect(request.body.attributes, "response body").deep.contains({
Copy link
Contributor Author

@pedroigor pedroigor May 15, 2024 •
edited

Choose a reason for hiding this comment

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

Using equals might end up in false negatives because you can have built-in/internal attributes returned as a user attribute.

I watched the video and this assertion was failing because the locale attribute is also available in the attributes map.

I'm not sure why it is failing for this specific PR though because the changes here are unrelated to this failure.

pedroigor
pedroigor commented May 15, 2024
View reviewed changes
js/apps/admin-ui/src/components/users/resource.ts Outdated
Copy link
Contributor Author

Choose a reason for hiding this comment

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

No longer needed because the endpoint was moved to the User API.

pedroigor
pedroigor commented May 15, 2024
View reviewed changes
rest/admin-ui-ext/src/main/java/org/keycloak/admin/ui/rest/UserResource.java Outdated
Copy link
Contributor Author

Choose a reason for hiding this comment

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

No longer needed because the endpoint was moved to the User API.

pedroigor
pedroigor commented May 15, 2024
View reviewed changes
rest/admin-ui-ext/src/main/java/org/keycloak/admin/ui/rest/UsersResource.java Outdated
Copy link
Contributor Author

Choose a reason for hiding this comment

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

No longer needed because the endpoint was moved to the User API.

pedroigor
pedroigor commented May 15, 2024
View reviewed changes
services/src/main/java/org/keycloak/userprofile/ServiceAccountAttributes.java Outdated
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The LegacyAttributes is only about representing attributes for service accounts. This renamed/new version makes simpler how such attributes are managed os that we don't need checks for service accounts in the DefaultAttributes implementation. The latter is targeted to regular users.

This was referenced May 15, 2024
Open
Open
Open
@keycloak-github-bot keycloak-github-bot bot mentioned this pull request May 16, 2024
Open
@pedroigor
Support unmanaged attributes for service accounts and make sure they …
9b92fad 
…are only managed through the admin api

Closes keycloak#29362

Signed-off-by: Pedro Igor <pigor.craveiro@gmail.com>
This was referenced May 21, 2024
Open
Open
keycloak-github-bot[bot]
keycloak-github-bot bot reviewed May 21, 2024
View reviewed changes
Copy link

@keycloak-github-bot keycloak-github-bot bot left a comment

Choose a reason for hiding this comment

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

Unreported flaky test detected, please review

@keycloak-github-bot
Copy link

Unreported flaky test detected

If the flaky tests below are affected by the changes, please review and update the changes accordingly. Otherwise, a maintainer should report the flaky tests prior to merging the PR.

org.keycloak.testsuite.x509.X509BrowserCRLTest#loginFailedWithIntermediateRevocationListFromHttp

Keycloak CI - FIPS IT (non-strict)

java.lang.RuntimeException: Could not create statement
	at org.jboss.arquillian.junit.Arquillian.methodBlock(Arquillian.java:307)
	at org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103)
...

Report flaky test

org.keycloak.testsuite.x509.X509BrowserCRLTest#loginFailedWithInvalidSignatureCRL

Keycloak CI - FIPS IT (non-strict)

java.lang.RuntimeException: Could not create statement
	at org.jboss.arquillian.junit.Arquillian.methodBlock(Arquillian.java:307)
	at org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103)
...

Report flaky test

org.keycloak.testsuite.x509.X509BrowserCRLTest#loginWithMultipleRevocationLists

Keycloak CI - FIPS IT (non-strict)

java.lang.RuntimeException: Could not create statement
	at org.jboss.arquillian.junit.Arquillian.methodBlock(Arquillian.java:307)
	at org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103)
...

Report flaky test

@pedroigor pedroigor merged commit b019cf6 into keycloak:main May 21, 2024
71 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@keycloak-github-bot keycloak-github-bot[bot] keycloak-github-bot left review comments

@edewit edewit edewit approved these changes

@sguilhen sguilhen sguilhen approved these changes

@mposolda mposolda mposolda approved these changes

Assignees
No one assigned
Labels
flaky-test team/cloud-native team/core-clients team/core-iam team/ui
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Custom user attributes are not shown for service account users in the Admin UI
4 participants
@pedroigor @edewit @sguilhen @mposolda