Can not update organization group error when trying to create organis…

…ation from REST API Closes keycloak#31144 Signed-off-by: Martin Kanis <[email protected]>
Hitachi · Jul 29, 2024 · d91d6d1 · d91d6d1
1 parent 00d8e06
commit d91d6d1
Show file tree

Hide file tree

Showing 6 changed files with 39 additions and 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
@@ -93,7 +93,7 @@ public OrganizationModel create(String name, String alias) {
         }
 
         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 @@ private boolean addMember(OrganizationModel organization, UserModel user, Member
     @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 OrganizationModel getByDomainName(String domain) {
         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 Stream<OrganizationModel> getAllStream(String search, Boolean exact, Inte
         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 Stream<OrganizationModel> getAllStream(Map<String, String> attributes, In
         Predicate finalPredicate = builder.and(predicates.toArray(new Predicate[0]));
         TypedQuery<OrganizationEntity> 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
@@ -19,7 +19,6 @@
 
 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 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,21 +46,24 @@
 
 public final class OrganizationAdapter implements OrganizationModel, JpaModel<OrganizationEntity> {
 
+    private final KeycloakSession session;
     private final RealmModel realm;
     private final OrganizationEntity entity;
     private final OrganizationProvider provider;
     private GroupModel group;
     private Map<String, List<String>> 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());
         this.realm = realm;
         this.provider = provider;
     }
 
-    public OrganizationAdapter(RealmModel realm, OrganizationEntity entity, OrganizationProvider provider) {
+    public OrganizationAdapter(KeycloakSession session, RealmModel realm, OrganizationEntity entity, OrganizationProvider provider) {
+        this.session = session;
         this.realm = realm;
         this.entity = entity;
         this.provider = provider;
@@ -137,10 +140,23 @@ public void setAttributes(Map<String, List<String>> attributes) {
         if (attributes == null) {
             return;
         }
-        Set<String> 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<String> 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
@@ -190,10 +190,10 @@ public static OrganizationModel toModel(OrganizationRepresentation rep, Organiza
         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/...ices/testsuite-providers/src/main/resources/theme/organization/login/test-org-commons.ftl b/...ices/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(", ")}
         </#list>
         <#if org.member>
             User is member of ${org.name}

diff --git a/...ase/src/test/java/org/keycloak/testsuite/organization/admin/AbstractOrganizationTest.java b/...ase/src/test/java/org/keycloak/testsuite/organization/admin/AbstractOrganizationTest.java
@@ -26,6 +26,7 @@
 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 @@ protected OrganizationRepresentation createRepresentation(String name, String...
             org.addDomain(domainRep);
         }
 
+        org.setAttributes(Map.of("key", List.of("value1", "value2")));
+
         return org;
     }
 

diff --git a/...s/base/src/test/java/org/keycloak/testsuite/organization/admin/OrganizationThemeTest.java b/...s/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 void testOrganizationAttributes() {
         loginPage.loginUsername("[email protected]");
         Assert.assertTrue(driver.getPageSource().contains("Sign-in to myorg organization"));
         for (Entry<String, List<String>> 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());
     }