From d4758333611681f87e4d453e0057bca6f483dcdf Mon Sep 17 00:00:00 2001 From: Pedro Igor Date: Tue, 9 Jul 2024 13:54:30 -0300 Subject: [PATCH] Do not expose kc.org attribute in user representations Closes #31143 Signed-off-by: Pedro Igor --- .../models/utils/ModelToRepresentation.java | 1 + .../services/resources/admin/UserResource.java | 2 ++ .../DeclarativeUserProfileProviderFactory.java | 9 ++------- .../member/OrganizationMemberTest.java | 17 ++++++++++------- 4 files changed, 15 insertions(+), 14 deletions(-) diff --git a/server-spi-private/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java b/server-spi-private/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java index b8a0d3c2b0..9c27ed77c6 100755 --- a/server-spi-private/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java +++ b/server-spi-private/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java @@ -248,6 +248,7 @@ public class ModelToRepresentation { } if (attributes != null && !copy.isEmpty()) { Map> attrs = new HashMap<>(copy); + attrs.remove(OrganizationModel.ORGANIZATION_ATTRIBUTE); rep.setAttributes(attrs); } diff --git a/services/src/main/java/org/keycloak/services/resources/admin/UserResource.java b/services/src/main/java/org/keycloak/services/resources/admin/UserResource.java index 429f90e106..4050e202cc 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/UserResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/UserResource.java @@ -49,6 +49,7 @@ import org.keycloak.models.KeycloakSession; import org.keycloak.models.ModelDuplicateException; import org.keycloak.models.ModelException; import org.keycloak.models.ModelIllegalStateException; +import org.keycloak.models.OrganizationModel; import org.keycloak.models.RealmModel; import org.keycloak.models.UserConsentModel; import org.keycloak.models.UserCredentialModel; @@ -1075,6 +1076,7 @@ public class UserResource { attributes.remove(UserModel.USERNAME); attributes.remove(UserModel.EMAIL); + attributes.remove(OrganizationModel.ORGANIZATION_ATTRIBUTE); return attributes.entrySet().stream() .filter(entry -> ofNullable(entry.getValue()).orElse(emptyList()).stream().anyMatch(StringUtil::isNotBlank)) diff --git a/services/src/main/java/org/keycloak/userprofile/DeclarativeUserProfileProviderFactory.java b/services/src/main/java/org/keycloak/userprofile/DeclarativeUserProfileProviderFactory.java index e8a4e66cf5..c68ee2a87a 100644 --- a/services/src/main/java/org/keycloak/userprofile/DeclarativeUserProfileProviderFactory.java +++ b/services/src/main/java/org/keycloak/userprofile/DeclarativeUserProfileProviderFactory.java @@ -358,13 +358,8 @@ public class DeclarativeUserProfileProviderFactory implements UserProfileProvide metadata.addAttribute(OrganizationModel.ORGANIZATION_ATTRIBUTE, -1, new AttributeValidatorMetadata(OrganizationMemberValidator.ID), new AttributeValidatorMetadata(ImmutableAttributeValidator.ID)) - .addReadCondition(c -> USER_API.equals(c.getContext())) - .addWriteCondition(context -> { - // the attribute can only be managed within the scope of the Organization API - // we assume, for now, that if the organization is set as a session attribute, we are operating within the scope if the Organization API - KeycloakSession session = context.getSession(); - return session.getAttribute(OrganizationModel.class.getName()) != null; - }); + .addReadCondition((c) -> false) + .addWriteCondition((c) -> false); } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/member/OrganizationMemberTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/member/OrganizationMemberTest.java index 5c3015fdf9..048772c7e5 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/member/OrganizationMemberTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/member/OrganizationMemberTest.java @@ -40,12 +40,13 @@ import jakarta.ws.rs.NotFoundException; import jakarta.ws.rs.core.Response; import jakarta.ws.rs.core.Response.Status; import java.io.IOException; -import org.hamcrest.Matchers; + import org.junit.Test; import org.keycloak.admin.client.resource.OrganizationMemberResource; import org.keycloak.admin.client.resource.OrganizationResource; import org.keycloak.admin.client.resource.UserResource; import org.keycloak.common.Profile.Feature; +import org.keycloak.models.KeycloakSession; import org.keycloak.models.OrganizationModel; import org.keycloak.models.RealmModel; import org.keycloak.models.UserModel; @@ -90,7 +91,6 @@ public class OrganizationMemberTest extends AbstractOrganizationTest { testRealm().users().userProfile().update(upConfig); OrganizationResource organization = testRealm().organizations().get(createOrganization().getId()); UserRepresentation expected = addMember(organization); - List expectedOrganizations = expected.getAttributes().get(ORGANIZATION_ATTRIBUTE); expected.singleAttribute(ORGANIZATION_ATTRIBUTE, "invalid"); @@ -109,11 +109,14 @@ public class OrganizationMemberTest extends AbstractOrganizationTest { expected.getAttributes().remove(ORGANIZATION_ATTRIBUTE); userResource.update(expected); expected = userResource.toRepresentation(); - assertThat(expected.getAttributes().get(ORGANIZATION_ATTRIBUTE), Matchers.containsInAnyOrder(expectedOrganizations.toArray())); + assertNull(expected.getAttributes()); + getTestingClient().server(TEST_REALM_NAME).run(OrganizationMemberTest::assertMembersHaveOrgAttribute); + } - userResource.update(expected); - expected = userResource.toRepresentation(); - assertThat(expected.getAttributes().get(ORGANIZATION_ATTRIBUTE), Matchers.containsInAnyOrder(expectedOrganizations.toArray())); + private static void assertMembersHaveOrgAttribute(KeycloakSession session) { + OrganizationModel organization = session.getProvider(OrganizationProvider.class).getByDomainName("neworg.org"); + assertTrue(session.getProvider(OrganizationProvider.class).getMembersStream(organization, null, false, -1, -1). + anyMatch(userModel -> userModel.getAttributes().getOrDefault(ORGANIZATION_ATTRIBUTE, List.of()).contains(organization.getId()))); } @Test @@ -281,7 +284,7 @@ public class OrganizationMemberTest extends AbstractOrganizationTest { OrganizationResource organization = testRealm().organizations().get(createOrganization().getId()); UserRepresentation expected = addMember(organization); assertNotNull(expected.getAttributes()); - assertTrue(expected.getAttributes().get(ORGANIZATION_ATTRIBUTE).contains(organization.toRepresentation().getId())); + assertNull(expected.getAttributes().get(ORGANIZATION_ATTRIBUTE)); OrganizationMemberResource member = organization.members().member(expected.getId()); try (Response response = member.delete()) {