From d91d6d18d59f5d30d368012fd85365f596f311be Mon Sep 17 00:00:00 2001 From: Martin Kanis Date: Mon, 22 Jul 2024 15:15:41 +0200 Subject: [PATCH] Can not update organization group error when trying to create organisation from REST API Closes #31144 Signed-off-by: Martin Kanis --- .../jpa/JpaOrganizationProvider.java | 10 +++---- .../organization/jpa/OrganizationAdapter.java | 30 ++++++++++++++----- .../organization/utils/Organizations.java | 8 ++--- .../organization/login/test-org-commons.ftl | 2 +- .../admin/AbstractOrganizationTest.java | 3 ++ .../admin/OrganizationThemeTest.java | 4 ++- 6 files changed, 39 insertions(+), 18 deletions(-) diff --git a/model/jpa/src/main/java/org/keycloak/organization/jpa/JpaOrganizationProvider.java b/model/jpa/src/main/java/org/keycloak/organization/jpa/JpaOrganizationProvider.java index 92fd299abe..60e262deee 100644 --- a/model/jpa/src/main/java/org/keycloak/organization/jpa/JpaOrganizationProvider.java +++ b/model/jpa/src/main/java/org/keycloak/organization/jpa/JpaOrganizationProvider.java @@ -93,7 +93,7 @@ public class JpaOrganizationProvider implements OrganizationProvider { } RealmModel realm = getRealm(); - OrganizationAdapter adapter = new OrganizationAdapter(realm, this); + OrganizationAdapter adapter = new OrganizationAdapter(session, realm, this); try { session.setAttribute(OrganizationModel.class.getName(), adapter); @@ -194,7 +194,7 @@ public class JpaOrganizationProvider implements OrganizationProvider { @Override public OrganizationModel getById(String id) { OrganizationEntity entity = getEntity(id, false); - return entity == null ? null : new OrganizationAdapter(getRealm(), entity, this); + return entity == null ? null : new OrganizationAdapter(session, getRealm(), entity, this); } @Override @@ -205,7 +205,7 @@ public class JpaOrganizationProvider implements OrganizationProvider { query.setParameter("name", domain.toLowerCase()); try { OrganizationEntity entity = query.getSingleResult(); - return new OrganizationAdapter(realm, entity, this); + return new OrganizationAdapter(session, realm, entity, this); } catch (NoResultException nre) { return null; } @@ -227,7 +227,7 @@ public class JpaOrganizationProvider implements OrganizationProvider { query.setParameter("realmId", realm.getId()); return closing(paginateQuery(query, first, max).getResultStream() - .map(entity -> new OrganizationAdapter(realm, entity, this))); + .map(entity -> new OrganizationAdapter(session, realm, entity, this))); } @Override @@ -260,7 +260,7 @@ public class JpaOrganizationProvider implements OrganizationProvider { Predicate finalPredicate = builder.and(predicates.toArray(new Predicate[0])); TypedQuery typedQuery = em.createQuery(query.select(org).where(finalPredicate)); return closing(paginateQuery(typedQuery, first, max).getResultStream()) - .map(entity -> new OrganizationAdapter(realm, entity, this)); + .map(entity -> new OrganizationAdapter(session, realm, entity, this)); } @Override diff --git a/model/jpa/src/main/java/org/keycloak/organization/jpa/OrganizationAdapter.java b/model/jpa/src/main/java/org/keycloak/organization/jpa/OrganizationAdapter.java index 672db67048..2c1b178adf 100644 --- a/model/jpa/src/main/java/org/keycloak/organization/jpa/OrganizationAdapter.java +++ b/model/jpa/src/main/java/org/keycloak/organization/jpa/OrganizationAdapter.java @@ -19,7 +19,6 @@ package org.keycloak.organization.jpa; import static java.util.Optional.ofNullable; -import java.util.HashMap; import java.util.HashSet; import java.util.Map; import java.util.List; @@ -31,6 +30,7 @@ import java.util.stream.Stream; import org.keycloak.models.GroupModel; import org.keycloak.models.IdentityProviderModel; +import org.keycloak.models.KeycloakSession; import org.keycloak.models.ModelValidationException; import org.keycloak.models.OrganizationDomainModel; import org.keycloak.models.OrganizationModel; @@ -46,13 +46,15 @@ import org.keycloak.utils.StringUtil; public final class OrganizationAdapter implements OrganizationModel, JpaModel { + private final KeycloakSession session; private final RealmModel realm; private final OrganizationEntity entity; private final OrganizationProvider provider; private GroupModel group; private Map> attributes; - public OrganizationAdapter(RealmModel realm, OrganizationProvider provider) { + public OrganizationAdapter(KeycloakSession session, RealmModel realm, OrganizationProvider provider) { + this.session = session; entity = new OrganizationEntity(); entity.setId(KeycloakModelUtils.generateId()); entity.setRealmId(realm.getId()); @@ -60,7 +62,8 @@ public final class OrganizationAdapter implements OrganizationModel, JpaModel attrsToRemove = getAttributes().keySet(); - attrsToRemove.removeAll(attributes.keySet()); - attrsToRemove.forEach(group::removeAttribute); - attributes.forEach(group::setAttribute); + + // add organization to the session as the following code updates the underlying group + OrganizationModel current = (OrganizationModel) session.getAttribute(OrganizationModel.class.getName()); + if (current == null) { + session.setAttribute(OrganizationModel.class.getName(), this); + } + + try { + Set attrsToRemove = getAttributes().keySet(); + attrsToRemove.removeAll(attributes.keySet()); + attrsToRemove.forEach(group::removeAttribute); + attributes.forEach(group::setAttribute); + } finally { + if (current == null) { + session.removeAttribute(OrganizationModel.class.getName()); + } + } } @Override diff --git a/services/src/main/java/org/keycloak/organization/utils/Organizations.java b/services/src/main/java/org/keycloak/organization/utils/Organizations.java index 8e420c340a..c6e05b3340 100644 --- a/services/src/main/java/org/keycloak/organization/utils/Organizations.java +++ b/services/src/main/java/org/keycloak/organization/utils/Organizations.java @@ -190,10 +190,10 @@ public class Organizations { model.setDescription(rep.getDescription()); model.setAttributes(rep.getAttributes()); model.setDomains(ofNullable(rep.getDomains()).orElse(Set.of()).stream() - .filter(Objects::nonNull) - .filter(domain -> StringUtil.isNotBlank(domain.getName())) - .map(Organizations::toModel) - .collect(Collectors.toSet())); + .filter(Objects::nonNull) + .filter(domain -> StringUtil.isNotBlank(domain.getName())) + .map(Organizations::toModel) + .collect(Collectors.toSet())); return model; } diff --git a/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/resources/theme/organization/login/test-org-commons.ftl b/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/resources/theme/organization/login/test-org-commons.ftl index 7d7abc02f2..1c0cbb37fb 100644 --- a/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/resources/theme/organization/login/test-org-commons.ftl +++ b/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/resources/theme/organization/login/test-org-commons.ftl @@ -2,7 +2,7 @@ <#if org?has_content> Sign-in to ${org.name} organization <#list org.attributes?keys as key> - The ${key} is ${org.attributes[key]} + The ${key} is ${org.attributes[key]?join(", ")} <#if org.member> User is member of ${org.name} diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/AbstractOrganizationTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/AbstractOrganizationTest.java index 8ccc71c509..fde7f3b188 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/AbstractOrganizationTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/AbstractOrganizationTest.java @@ -26,6 +26,7 @@ import static org.junit.Assert.assertTrue; import static org.keycloak.testsuite.broker.BrokerTestTools.waitForPage; import java.util.List; +import java.util.Map; import java.util.function.Function; import jakarta.ws.rs.core.Response; import jakarta.ws.rs.core.Response.Status; @@ -148,6 +149,8 @@ public abstract class AbstractOrganizationTest extends AbstractAdminTest { org.addDomain(domainRep); } + org.setAttributes(Map.of("key", List.of("value1", "value2"))); + return org; } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/OrganizationThemeTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/OrganizationThemeTest.java index 6f9f7b0408..6bdbb6f93c 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/OrganizationThemeTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/OrganizationThemeTest.java @@ -19,12 +19,14 @@ package org.keycloak.testsuite.organization.admin; +import static org.hamcrest.MatcherAssert.assertThat; import static org.keycloak.testsuite.broker.BrokerTestTools.waitForPage; import java.util.List; import java.util.Map.Entry; import jakarta.ws.rs.core.Response; +import org.hamcrest.Matchers; import org.jboss.arquillian.graphene.page.Page; import org.junit.Assert; import org.junit.Before; @@ -175,7 +177,7 @@ public class OrganizationThemeTest extends AbstractOrganizationTest { loginPage.loginUsername("non-user@myorg.com"); Assert.assertTrue(driver.getPageSource().contains("Sign-in to myorg organization")); for (Entry> attribute : orgRep.getAttributes().entrySet()) { - Assert.assertTrue(driver.getPageSource().contains("The " + attribute.getKey() + " is " + attribute.getValue())); + assertThat(driver.getPageSource(), Matchers.containsString("The " + attribute.getKey() + " is " + String.join(", ", attribute.getValue()))); } Assert.assertFalse(loginPage.isPasswordInputPresent()); }