Make sure organization are only manageable by the admin users with the manage-realm role

Closes #28733

Signed-off-by: vramik <vramik@redhat.com>

model/jpa/src/main/java/org/keycloak/models/jpa/entities/OrganizationEntity.java

@@ -31,9 +31,6 @@ import jakarta.persistence.NamedQueries;
 import jakarta.persistence.NamedQuery;
 import jakarta.persistence.OneToMany;
 import jakarta.persistence.Table;
-import org.hibernate.annotations.BatchSize;
-import org.hibernate.annotations.Fetch;
-import org.hibernate.annotations.FetchMode;
 
 @Table(name="ORG")
 @Entity
@@ -60,8 +57,6 @@ public class OrganizationEntity {
     private String idpAlias;
 
     @OneToMany(cascade = CascadeType.ALL, orphanRemoval = true, mappedBy="organization")
-    @Fetch(FetchMode.SELECT)
-    @BatchSize(size = 20)
     protected Set<OrganizationDomainEntity> domains = new HashSet<>();
 
     public String getId() {

model/jpa/src/main/java/org/keycloak/organization/jpa/OrganizationAdapter.java

@@ -17,16 +17,16 @@
 
 package org.keycloak.organization.jpa;
 
-import org.keycloak.models.GroupModel;
-
 import java.util.HashSet;
 import java.util.Map;
+import java.util.List;
 import java.util.Set;
 import java.util.Objects;
 import java.util.function.Function;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
+import org.keycloak.models.GroupModel;
 import org.keycloak.models.IdentityProviderModel;
 import org.keycloak.models.ModelValidationException;
 import org.keycloak.models.OrganizationDomainModel;
@@ -37,8 +37,7 @@ import org.keycloak.models.jpa.entities.OrganizationDomainEntity;
 import org.keycloak.models.jpa.entities.OrganizationEntity;
 import org.keycloak.organization.OrganizationProvider;
 import org.keycloak.utils.EmailValidationUtil;
-
+import org.keycloak.utils.StringUtil;
-import java.util.List;
 
 public final class OrganizationAdapter implements OrganizationModel, JpaModel<OrganizationEntity> {
 
@@ -104,7 +103,7 @@ public final class OrganizationAdapter implements OrganizationModel, JpaModel<Or
         }
 
         Map<String, OrganizationDomainModel> modelMap = domains.stream()
-                .peek(this::validateDomainRepresentation)
+                .map(this::validateDomain)
                 .collect(Collectors.toMap(OrganizationDomainModel::getName, Function.identity()));
 
         for (OrganizationDomainEntity domainEntity : new HashSet<>(this.entity.getDomains())) {
@@ -122,7 +121,7 @@ public final class OrganizationAdapter implements OrganizationModel, JpaModel<Or
         // create the remaining domains.
         for (OrganizationDomainModel model : modelMap.values()) {
             OrganizationDomainEntity domainEntity = new OrganizationDomainEntity();
-            domainEntity.setName(model.getName().toLowerCase());
+            domainEntity.setName(model.getName());
             domainEntity.setVerified(model.getVerified());
             domainEntity.setOrganization(this.entity);
             this.entity.addDomain(domainEntity);
@@ -160,23 +159,24 @@ public final class OrganizationAdapter implements OrganizationModel, JpaModel<Or
     }
 
     /**
-     * Validates the domain representation. Specifically, the method first checks if the specified domain is valid,
+     * Validates the domain. Specifically, the method first checks if the specified domain is valid,
      * and then checks if the domain is not already linked to a different organization.
      *
      * @param domainModel the {@link OrganizationDomainModel} representing the domain being added.
      * @throws {@link ModelValidationException} if the domain is invalid or is already linked to a different organization.
      */
-    private void validateDomainRepresentation(OrganizationDomainModel domainModel) {
+    private OrganizationDomainModel validateDomain(OrganizationDomainModel domainModel) {
         String domainName = domainModel.getName();
 
         // we rely on the same validation util used by the EmailValidator to ensure the domain part is consistently validated.
-        if(domainName == null || domainName.isEmpty() || !EmailValidationUtil.isValidEmail("nouser@" + domainName)) {
+        if (StringUtil.isBlank(domainName) || !EmailValidationUtil.isValidEmail("nouser@" + domainName)) {
             throw new ModelValidationException("The specified domain is invalid: " + domainName);
         }
         OrganizationModel orgModel = provider.getByDomainName(domainName);
         if (orgModel != null && !Objects.equals(getId(), orgModel.getId())) {
             throw new ModelValidationException("Domain " + domainName + " is already linked to another organization");
         }
+        return domainModel;
     }
 
     private GroupModel getGroup() {

server-spi/src/main/java/org/keycloak/models/OrganizationDomainModel.java

@@ -26,15 +26,15 @@ import java.io.Serializable;
  */
 public class OrganizationDomainModel implements Serializable {
 
-    private String name;
+    private final String name;
-    private boolean verified;
+    private final boolean verified;
 
     public OrganizationDomainModel(String name) {
         this(name, false);
     }
 
     public OrganizationDomainModel(String name, boolean verified) {
-        this.name = name;
+        this.name = name == null ? null : name.trim().toLowerCase();
         this.verified = verified;
     }
 
@@ -42,18 +42,10 @@ public class OrganizationDomainModel implements Serializable {
         return this.name;
     }
 
-    public void setName(String name) {
-        this.name = name != null ? name.trim() : null;
-    }
-
     public boolean getVerified() {
         return this.verified;
     }
 
-    public void setVerified(boolean verified) {
-        this.verified = verified;
-    }
-
     @Override
     public boolean equals(Object o) {
         if (this == o) return true;

services/src/main/java/org/keycloak/organization/admin/resource/OrganizationIdentityProviderResource.java

@@ -72,6 +72,8 @@ public class OrganizationIdentityProviderResource {
     @Consumes(MediaType.APPLICATION_JSON)
     public Response addIdentityProvider(IdentityProviderRepresentation providerRep) {
 
+        auth.realm().requireManageRealm();
+
         IdentityProviderModel identityProvider = organization.getIdentityProvider();
         if (identityProvider != null) {
             throw ErrorResponse.error("Organization already assigned with an identity provider.", Status.BAD_REQUEST);
@@ -103,11 +105,13 @@ public class OrganizationIdentityProviderResource {
     @GET
     @Produces(MediaType.APPLICATION_JSON)
     public IdentityProviderRepresentation getIdentityProvider() {
+        auth.realm().requireManageRealm();
         return Optional.ofNullable(organization.getIdentityProvider()).map(this::toRepresentation).orElse(null);
     }
 
     @DELETE
     public Response delete() {
+        auth.realm().requireManageRealm();
         IdentityProviderModel identityProvider = getIdentityProviderModel();
 
         Response response = getIdentityProviderResource(identityProvider).delete();
@@ -132,6 +136,7 @@ public class OrganizationIdentityProviderResource {
     @PUT
     @Consumes(MediaType.APPLICATION_JSON)
     public Response update(IdentityProviderRepresentation rep) {
+        auth.realm().requireManageRealm();
         IdentityProviderModel identityProvider = getIdentityProviderModel();
 
         if (!rep.getAlias().equals(identityProvider.getAlias()) || (rep.getInternalId() != null && !Objects.equals(rep.getInternalId(), identityProvider.getInternalId()))) {

services/src/main/java/org/keycloak/organization/admin/resource/OrganizationMemberResource.java

@@ -80,6 +80,7 @@ public class OrganizationMemberResource {
     @POST
     @Consumes(MediaType.APPLICATION_JSON)
     public Response addMember(UserRepresentation rep) {
+        auth.realm().requireManageRealm();
         if (rep == null || !Objects.equals(rep.getUsername(), rep.getEmail())) {
             throw ErrorResponse.error("To add a member to the organization it is expected the username and the email is the same.", Status.BAD_REQUEST);
         }
@@ -109,6 +110,7 @@ public class OrganizationMemberResource {
     @GET
     @Produces(MediaType.APPLICATION_JSON)
     public Stream<UserRepresentation> getMembers() {
+        auth.realm().requireManageRealm();
         return provider.getMembersStream(organization).map(this::toRepresentation);
     }
 
@@ -116,6 +118,7 @@ public class OrganizationMemberResource {
     @GET
     @Produces(MediaType.APPLICATION_JSON)
     public UserRepresentation get(@PathParam("id") String id) {
+        auth.realm().requireManageRealm();
         if (StringUtil.isBlank(id)) {
             throw ErrorResponse.error("id cannot be null", Status.BAD_REQUEST);
         }
@@ -126,6 +129,7 @@ public class OrganizationMemberResource {
     @Path("{id}")
     @DELETE
     public Response delete(@PathParam("id") String id) {
+        auth.realm().requireManageRealm();
         if (StringUtil.isBlank(id)) {
             throw ErrorResponse.error("id cannot be null", Status.BAD_REQUEST);
         }
@@ -139,6 +143,7 @@ public class OrganizationMemberResource {
     @PUT
     @Consumes(MediaType.APPLICATION_JSON)
     public Response update(@PathParam("id") String id, UserRepresentation user) {
+        auth.realm().requireManageRealm();
         return new UserResource(session, getMember(id), auth, adminEvent).updateUser(user);
     }
 
@@ -146,6 +151,7 @@ public class OrganizationMemberResource {
     @GET
     @Produces(MediaType.APPLICATION_JSON)
     public OrganizationRepresentation getOrganization(@PathParam("id") String id) {
+        auth.realm().requireManageRealm();
         if (StringUtil.isBlank(id)) {
             throw ErrorResponse.error("id cannot be null", Status.BAD_REQUEST);
         }

services/src/main/java/org/keycloak/organization/admin/resource/OrganizationResource.java

@@ -23,11 +23,9 @@ import java.util.Objects;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
-import jakarta.ws.rs.BadRequestException;
 import jakarta.ws.rs.Consumes;
 import jakarta.ws.rs.DELETE;
 import jakarta.ws.rs.GET;
-import jakarta.ws.rs.NotFoundException;
 import jakarta.ws.rs.POST;
 import jakarta.ws.rs.PUT;
 import jakarta.ws.rs.Path;
@@ -44,6 +42,7 @@ import org.keycloak.models.OrganizationModel;
 import org.keycloak.organization.OrganizationProvider;
 import org.keycloak.representations.idm.OrganizationDomainRepresentation;
 import org.keycloak.representations.idm.OrganizationRepresentation;
+import org.keycloak.services.ErrorResponse;
 import org.keycloak.services.resources.admin.AdminEventBuilder;
 import org.keycloak.services.resources.admin.permissions.AdminPermissionEvaluator;
 import org.keycloak.utils.StringUtil;
@@ -71,8 +70,9 @@ public class OrganizationResource {
     @POST
     @Consumes(MediaType.APPLICATION_JSON)
     public Response create(OrganizationRepresentation organization) {
+        auth.realm().requireManageRealm();
         if (organization == null) {
-            throw new BadRequestException();
+            throw ErrorResponse.error("Organization cannot be null.", Response.Status.BAD_REQUEST);
         }
 
         Set<String> domains = organization.getDomains().stream().map(OrganizationDomainRepresentation::getName).collect(Collectors.toSet());
@@ -88,7 +88,8 @@ public class OrganizationResource {
     public Stream<OrganizationRepresentation> search(
             @Parameter(description = "A String representing an organization internet domain") @QueryParam("domain-name") String domainName
     ) {
-        if (domainName == null || domainName.trim().isEmpty()) {
+        auth.realm().requireManageRealm();
+        if (StringUtil.isBlank(domainName)) {
             return provider.getAllStream().map(this::toRepresentation);
         } else {
             // search for the organization associated with the given domain
@@ -101,8 +102,9 @@ public class OrganizationResource {
     @GET
     @Produces(MediaType.APPLICATION_JSON)
     public OrganizationRepresentation get(@PathParam("id") String id) {
+        auth.realm().requireManageRealm();
         if (StringUtil.isBlank(id)) {
-            throw new BadRequestException();
+            throw ErrorResponse.error("Id cannot be null.", Response.Status.BAD_REQUEST);
         }
 
         return toRepresentation(getOrganization(id));
@@ -111,8 +113,9 @@ public class OrganizationResource {
     @Path("{id}")
     @DELETE
     public Response delete(@PathParam("id") String id) {
+        auth.realm().requireManageRealm();
         if (StringUtil.isBlank(id)) {
-            throw new BadRequestException();
+            throw ErrorResponse.error("Id cannot be null.", Response.Status.BAD_REQUEST);
         }
 
         provider.remove(getOrganization(id));
@@ -124,6 +127,7 @@ public class OrganizationResource {
     @PUT
     @Consumes(MediaType.APPLICATION_JSON)
     public Response update(@PathParam("id") String id, OrganizationRepresentation organization) {
+        auth.realm().requireManageRealm();
         OrganizationModel model = getOrganization(id);
         toModel(organization, model);
 
@@ -143,14 +147,17 @@ public class OrganizationResource {
     }
     
     private OrganizationModel getOrganization(String id) {
+        //checking permissions before trying to find organization to be able to return 403 (instead of 400 or 404)
+        auth.realm().requireManageRealm();
+
         if (id == null) {
-            throw new BadRequestException();
+            throw ErrorResponse.error("Id cannot be null.", Response.Status.BAD_REQUEST);
         }
 
         OrganizationModel model = provider.getById(id);
 
         if (model == null) {
-            throw new NotFoundException();
+            throw ErrorResponse.error("Organization not found.", Response.Status.NOT_FOUND);
         }
 
         return model;

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/AbstractOrganizationTest.java

@@ -99,16 +99,10 @@ public abstract class AbstractOrganizationTest extends AbstractAdminTest  {
     }
 
     protected OrganizationRepresentation createOrganization(String name, String orgDomain) {
-        OrganizationRepresentation org = new OrganizationRepresentation();
+        OrganizationRepresentation org = createRepresentation(name, orgDomain);
-
-        org.setName(name);
 
         String id;
 
-        OrganizationDomainRepresentation domainRep = new OrganizationDomainRepresentation();
-        domainRep.setName(orgDomain);
-        org.addDomain(domainRep);
-
         try (Response response = testRealm().organizations().create(org)) {
             assertEquals(Status.CREATED.getStatusCode(), response.getStatus());
             id = ApiUtil.getCreatedId(response);
@@ -123,6 +117,18 @@ public abstract class AbstractOrganizationTest extends AbstractAdminTest  {
         return org;
     }
 
+    protected OrganizationRepresentation createRepresentation(String name, String orgDomain) {
+        OrganizationRepresentation org = new OrganizationRepresentation();
+
+        org.setName(name);
+
+        OrganizationDomainRepresentation domainRep = new OrganizationDomainRepresentation();
+        domainRep.setName(orgDomain);
+        org.addDomain(domainRep);
+
+        return org;
+    }
+
     protected UserRepresentation addMember(OrganizationResource organization) {
         return addMember(organization, memberEmail);
     }

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/OrganizationTest.java

@@ -19,29 +19,50 @@ package org.keycloak.testsuite.organization.admin;
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.containsInAnyOrder;
+import static org.hamcrest.Matchers.equalTo;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.fail;
 
-import java.util.ArrayList;
+import jakarta.ws.rs.ForbiddenException;
-import java.util.HashMap;
-import java.util.List;
-
 import jakarta.ws.rs.NotFoundException;
 import jakarta.ws.rs.core.Response;
 import jakarta.ws.rs.core.Response.Status;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import org.hamcrest.Matchers;
 import org.junit.Test;
+import org.keycloak.admin.client.Keycloak;
 import org.keycloak.admin.client.resource.OrganizationResource;
+import org.keycloak.admin.client.resource.RealmResource;
 import org.keycloak.common.Profile.Feature;
+import org.keycloak.models.AdminRoles;
+import org.keycloak.models.Constants;
+import org.keycloak.representations.idm.IdentityProviderRepresentation;
 import org.keycloak.representations.idm.OrganizationDomainRepresentation;
 import org.keycloak.representations.idm.OrganizationRepresentation;
+import org.keycloak.representations.idm.RealmRepresentation;
+import org.keycloak.representations.idm.UserRepresentation;
+import org.keycloak.testsuite.admin.ApiUtil;
 import org.keycloak.testsuite.arquillian.annotation.EnableFeature;
+import org.keycloak.testsuite.util.AdminClientUtil;
+import org.keycloak.testsuite.util.UserBuilder;
 
 @EnableFeature(Feature.ORGANIZATION)
 public class OrganizationTest extends AbstractOrganizationTest {
 
+    @Override
+    public void configureTestRealm(RealmRepresentation testRealm) {
+        testRealm.getUsers().add(UserBuilder.create().username("realmAdmin").password("password")
+                .role(Constants.REALM_MANAGEMENT_CLIENT_ID, AdminRoles.MANAGE_REALM)
+                .role(Constants.REALM_MANAGEMENT_CLIENT_ID, AdminRoles.MANAGE_IDENTITY_PROVIDERS)
+                .role(Constants.REALM_MANAGEMENT_CLIENT_ID, AdminRoles.MANAGE_USERS)
+                .build());
+    }
+
     @Test
     public void testUpdate() {
         OrganizationRepresentation expected = createOrganization();
@@ -237,4 +258,139 @@ public class OrganizationTest extends AbstractOrganizationTest {
         assertEquals(1, existing.getDomains().size());
         assertNotNull(existing.getDomain("acme.com"));
     }
+
+    @Test
+    public void permissionsTest() throws Exception {
+        try (
+            Keycloak manageRealmAdminClient = AdminClientUtil.createAdminClient(suiteContext.isAdapterCompatTesting(), 
+                TEST_REALM_NAME, "realmAdmin", "password", Constants.ADMIN_CLI_CLIENT_ID, null);
+            Keycloak userAdminClient = AdminClientUtil.createAdminClient(suiteContext.isAdapterCompatTesting(), 
+                TEST_REALM_NAME, "test-user@localhost", "password", Constants.ADMIN_CLI_CLIENT_ID, null)
+        ) {
+            RealmResource realmAdminResource = manageRealmAdminClient.realm(TEST_REALM_NAME);
+            RealmResource realmUserResource = userAdminClient.realm(TEST_REALM_NAME);
+
+            /* Org */
+            //create org
+            OrganizationRepresentation orgRep = createRepresentation("testOrg", "testOrg.org");
+            String orgId;
+            try (
+                Response userResponse = realmUserResource.organizations().create(orgRep);
+                Response adminResponse = realmAdminResource.organizations().create(orgRep)
+            ) {
+                assertThat(userResponse.getStatus(), equalTo(Status.FORBIDDEN.getStatusCode()));
+                assertThat(adminResponse.getStatus(), equalTo(Status.CREATED.getStatusCode()));
+                orgId = ApiUtil.getCreatedId(adminResponse);
+                getCleanup().addCleanup(() -> testRealm().organizations().get(orgId).delete().close());
+            }
+
+            //search for org
+            try {
+                realmUserResource.organizations().getAll("testOrg.org");
+                fail("Expected ForbiddenException");
+            } catch (ForbiddenException expected) {}
+            assertThat(realmAdminResource.organizations().getAll("testOrg.org"), Matchers.notNullValue());
+
+            //get org
+            try {
+                realmUserResource.organizations().get(orgId).toRepresentation();
+                fail("Expected ForbiddenException");
+            } catch (ForbiddenException expected) {}
+            assertThat(realmAdminResource.organizations().get(orgId).toRepresentation(), Matchers.notNullValue());
+
+            //update org
+            try (Response userResponse = realmUserResource.organizations().get(orgId).update(orgRep)) {
+                assertThat(userResponse.getStatus(), equalTo(Status.FORBIDDEN.getStatusCode()));
+            }
+
+            //delete org
+            try (Response userResponse = realmUserResource.organizations().get(orgId).delete()) {
+                assertThat(userResponse.getStatus(), equalTo(Status.FORBIDDEN.getStatusCode()));
+            }
+
+            /* IdP */
+            IdentityProviderRepresentation idpRep = new IdentityProviderRepresentation();
+            idpRep.setAlias("dummy");
+            idpRep.setProviderId("oidc");
+            //create IdP
+            try (
+                Response userResponse = realmUserResource.organizations().get(orgId).identityProvider().create(idpRep);
+                Response adminResponse = realmAdminResource.organizations().get(orgId).identityProvider().create(idpRep)
+            ) {
+                assertThat(userResponse.getStatus(), equalTo(Status.FORBIDDEN.getStatusCode()));
+                assertThat(adminResponse.getStatus(), equalTo(Status.CREATED.getStatusCode()));
+                getCleanup().addCleanup(() -> testRealm().organizations().get(orgId).identityProvider().delete().close());
+            }
+
+            //get IdP
+            try {
+                //we should get 403, not 400 or 404 etc.
+                realmUserResource.organizations().get("non-existing").identityProvider().toRepresentation();
+                fail("Expected ForbiddenException");
+            } catch (ForbiddenException expected) {}
+            try {
+                realmUserResource.organizations().get(orgId).identityProvider().toRepresentation();
+                fail("Expected ForbiddenException");
+            } catch (ForbiddenException expected) {}
+            assertThat(realmAdminResource.organizations().get(orgId).identityProvider().toRepresentation(), Matchers.notNullValue());
+
+            //update IdP
+            try (Response userResponse = realmUserResource.organizations().get(orgId).identityProvider().update(idpRep)) {
+                assertThat(userResponse.getStatus(), equalTo(Status.FORBIDDEN.getStatusCode()));
+            }
+
+            //delete IdP
+            try (Response userResponse = realmUserResource.organizations().get(orgId).identityProvider().delete()) {
+                assertThat(userResponse.getStatus(), equalTo(Status.FORBIDDEN.getStatusCode()));
+            }
+
+            /* Members */
+            UserRepresentation userRep = UserBuilder.create()
+                    .username("user@testOrg.org")
+                    .email("user@testOrg.org")
+                    .build();
+            String userId;
+
+            //create member
+            try (
+                Response userResponse = realmUserResource.organizations().get(orgId).members().addMember(userRep);
+                Response adminResponse = realmAdminResource.organizations().get(orgId).members().addMember(userRep)
+            ) {
+                assertThat(userResponse.getStatus(), equalTo(Status.FORBIDDEN.getStatusCode()));
+                assertThat(adminResponse.getStatus(), equalTo(Status.CREATED.getStatusCode()));
+                userId = ApiUtil.getCreatedId(adminResponse);
+                assertThat(userId, Matchers.notNullValue());
+                getCleanup().addCleanup(() -> testRealm().organizations().get(orgId).members().member(userId).delete().close());
+            }
+
+            //get members
+            try {
+                //we should get 403, not 400 or 404 etc.
+                realmUserResource.organizations().get("non-existing").members().getAll();
+                fail("Expected ForbiddenException");
+            } catch (ForbiddenException expected) {}
+            try {
+                realmUserResource.organizations().get(orgId).members().getAll();
+                fail("Expected ForbiddenException");
+            } catch (ForbiddenException expected) {}
+            assertThat(realmAdminResource.organizations().get(orgId).members().getAll(), Matchers.notNullValue());
+
+            //get member
+            try {
+                realmUserResource.organizations().get(orgId).members().member(userId).toRepresentation();
+                fail("Expected ForbiddenException");
+            } catch (ForbiddenException expected) {}
+            assertThat(realmAdminResource.organizations().get(orgId).members().member(userId).toRepresentation(), Matchers.notNullValue());
+
+            //update member
+            try (Response userResponse = realmUserResource.organizations().get(orgId).members().member(userId).update(userRep)) {
+                assertThat(userResponse.getStatus(), equalTo(Status.FORBIDDEN.getStatusCode()));
+            }
+
+            //delete member
+            try (Response userResponse = realmUserResource.organizations().get(orgId).members().member(userId).delete()) {
+                assertThat(userResponse.getStatus(), equalTo(Status.FORBIDDEN.getStatusCode()));
+            }
+        }
+    }
 }