Verify fine-grained admin permissions feature is enabled before check…

…ing fine-grained permissions when creating users
eosc-kc · Dec 23, 2021 · c2a369a · c2a369a
1 parent f51f983
commit c2a369a
Show file tree

Hide file tree

Showing 4 changed files with 117 additions and 29 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,38 +7,42 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 For Keycloak upstream changelog please see https://www.keycloak.org/docs/latest/release_notes/index.html.
 Full Keycloak upstream jira issue can be shown if filtered by Fix version. For example [Keycloak jira issue for 15.0.2 version](https://issues.redhat.com/browse/KEYCLOAK-19161?jql=project%20%3D%20keycloak%20and%20fixVersion%20%3D%2015.0.2)
 
-## [v15.0.2-r1.0.6] - 10-12-2021
+## [v15.0.2-r1.0.7] - 2021-12-23
+### Fixed
+- Verify fine-grained admin permissions feature is enabled before checking fine-grained permissions when creating users
+
+## [v15.0.2-r1.0.6] - 2021-12-10
 ### Added
 - SAML Federation sync mode [EOSC-KC-144](https://github.com/eosc-kc/keycloak/issues/144)
 
-###Changed
+### Changed
 - Increase User Attribute Value length to 4000 [EOSC-KC-132](https://github.com/eosc-kc/keycloak/issues/132)
 - Improve implementation in attribute Identity Provider mapper taking into account emailVerified User field. [EOSC-KC-70](https://github.com/eosc-kc/keycloak/issues/70)
 - FreeMarkerLoginFormsProvider now has an additional common attribute passed to the ftl templates, the "uriInfo"
 
 ### Fixed
 - Fixed SAML principals tooltip
 
-## [v15.0.2-r1.0.5] - 16-11-2021
+## [v15.0.2-r1.0.5] - 2021-11-16
 ### Added
 -the idpLoginFullUrl common attribute passed to the ftl templates for any theme except from the default
 
-## [v15.0.2-r1.0.4] - 4-11-2021
+## [v15.0.2-r1.0.4] - 2021-11-04
 ### Added
 - Attribute Identity Provider mapper taking into account emailVerified User field. [EOSC-KC-70](https://github.com/eosc-kc/keycloak/issues/70)
 
 ### Fixed
 - Fix a bug for Identity Providers in ModelToRepresentation
 
-## [v15.0.2-r1.0.3] - 18-10-2021
+## [v15.0.2-r1.0.3] - 2021-10-18
 ### Added
 - new release created on tag
 
-## [v15.0.2-r1.0.2] - 5-10-2021
+## [v15.0.2-r1.0.2] - 2021-10-05
 ### Fixed
 - Brokered SAML logins fail due to wrong InResponseTo in 15.x. [KEYCLOAK-19143](https://issues.redhat.com/browse/KEYCLOAK-19143)
 
-## [v15.0.2-r1.0.1] - 29-9-2021
+## [v15.0.2-r1.0.1] - 2021-09-29
 ### Added
 - Support for SAML IdP Federation
 - User reaccepting Terms and Conditions. [EOSC-KC-48](https://github.com/eosc-kc/keycloak/issues/48)

diff --git a/services/src/main/java/org/keycloak/services/resources/admin/UsersResource.java b/services/src/main/java/org/keycloak/services/resources/admin/UsersResource.java
@@ -16,12 +16,11 @@
  */
 package org.keycloak.services.resources.admin;
 
-import static org.keycloak.userprofile.UserProfileContext.USER_API;
-
 import org.jboss.logging.Logger;
 import org.jboss.resteasy.annotations.cache.NoCache;
 import org.jboss.resteasy.spi.ResteasyProviderFactory;
 import org.keycloak.common.ClientConnection;
+import org.keycloak.common.Profile;
 import org.keycloak.common.util.ObjectUtil;
 import org.keycloak.events.admin.OperationType;
 import org.keycloak.events.admin.ResourceType;
@@ -56,11 +55,20 @@
 import javax.ws.rs.core.HttpHeaders;
 import javax.ws.rs.core.MediaType;
 import javax.ws.rs.core.Response;
+
+import java.util.Collections;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
 import java.util.Set;
+import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
+import static org.keycloak.models.utils.KeycloakModelUtils.findGroupByPath;
+import static org.keycloak.userprofile.UserProfileContext.USER_API;
+
 /**
  * Base resource for managing users
  *
@@ -108,22 +116,8 @@ public Response createUser(final UserRepresentation rep) {
         // first check if user has manage rights
         try {
             auth.users().requireManage();
-        }
-        catch (ForbiddenException exception) {
-            // if user does not have manage rights, fallback to fine grain admin permissions per group
-            if (rep.getGroups() != null) {
-                // if groups is part of the user rep, check if admin has manage_members and manage_membership on each group
-                for (String groupPath : rep.getGroups()) {
-                    GroupModel group = KeycloakModelUtils.findGroupByPath(realm, groupPath);
-                    if (group != null) {
-                        auth.groups().requireManageMembers(group);
-                        auth.groups().requireManageMembership(group);
-                    } else {
-                        return ErrorResponse.error(String.format("Group %s not found", groupPath), Response.Status.BAD_REQUEST);
-                    }
-                }
-            } else {
-                // propagate exception if no group specified
+        } catch (ForbiddenException exception) {
+            if (!canCreateGroupMembers(rep)) {
                 throw exception;
             }
         }
@@ -192,6 +186,32 @@ public Response createUser(final UserRepresentation rep) {
             return ErrorResponse.error("Could not create user", Response.Status.BAD_REQUEST);
         }
     }
+
+    private boolean canCreateGroupMembers(UserRepresentation rep) {
+        if (!Profile.isFeatureEnabled(Profile.Feature.ADMIN_FINE_GRAINED_AUTHZ)) {
+            return false;
+        }
+
+        List<GroupModel> groups = Optional.ofNullable(rep.getGroups())
+                .orElse(Collections.emptyList())
+                .stream().map(path -> findGroupByPath(realm, path))
+                .filter(Objects::nonNull)
+                .collect(Collectors.toList());
+
+        if (groups.isEmpty()) {
+            return false;
+        }
+
+        // if groups is part of the user rep, check if admin has manage_members and manage_membership on each group
+        // an exception is thrown in case the current user does not have permissions to manage any of the groups
+        for (GroupModel group : groups) {
+            auth.groups().requireManageMembers(group);
+            auth.groups().requireManageMembership(group);
+        }
+
+        return true;
+    }
+
     /**
      * Get representation of the user
      *

diff --git a/...uillian/tests/base/src/test/java/org/keycloak/testsuite/admin/FineGrainAdminUnitTest.java b/...uillian/tests/base/src/test/java/org/keycloak/testsuite/admin/FineGrainAdminUnitTest.java
@@ -40,6 +40,7 @@
 import org.keycloak.models.utils.RepresentationToModel;
 import org.keycloak.representations.idm.ClientRepresentation;
 import org.keycloak.representations.idm.ClientScopeRepresentation;
+import org.keycloak.representations.idm.GroupRepresentation;
 import org.keycloak.representations.idm.RealmRepresentation;
 import org.keycloak.representations.idm.RoleRepresentation;
 import org.keycloak.representations.idm.UserRepresentation;
@@ -58,13 +59,15 @@
 import org.keycloak.testsuite.arquillian.annotation.UncaughtServerErrorExpected;
 import org.keycloak.testsuite.auth.page.AuthRealm;
 import org.keycloak.testsuite.util.AdminClientUtil;
+import org.keycloak.testsuite.util.GroupBuilder;
 import org.keycloak.testsuite.utils.tls.TLSUtils;
 
 import javax.ws.rs.ClientErrorException;
 import javax.ws.rs.WebApplicationException;
 import javax.ws.rs.core.Response;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.stream.Collectors;
@@ -81,6 +84,7 @@
  * @author <a href="mailto:bill@burkecentral.com">Bill Burke</a>
  * @version $Revision: 1 $
  */
+@EnableFeature(Profile.Feature.ADMIN_FINE_GRAINED_AUTHZ)
 public class FineGrainAdminUnitTest extends AbstractKeycloakTest {
 
     public static final String CLIENT_NAME = "application";
@@ -97,6 +101,7 @@ public void addTestRealms(List<RealmRepresentation> testRealms) {
         testRealmRep.setRealm(TEST);
         testRealmRep.setEnabled(true);
         testRealms.add(testRealmRep);
+        testRealmRep.setGroups(Arrays.asList(GroupBuilder.create().name("restricted-group").build()));
     }
 
     public static void setupDemo(KeycloakSession session) {
@@ -656,14 +661,35 @@ public void testRestEvaluation() throws Exception {
                     Assert.assertEquals(403, e.getResponse().getStatus());
                 }
 
-                UserRepresentation newGroupMemberWithAWrongGroup = createUserRepresentation("new-group-member",
+                UserRepresentation newEmptyGroupList = createUserRepresentation("new-group-member",
+                        "new-group-member@keycloak.org", "New", "Member", true);
+                newEmptyGroupList.setGroups(Collections.emptyList());
+
+                try {
+                    ApiUtil.createUserWithAdminClient(realmClient.realm(TEST), newEmptyGroupList);
+                    Assert.fail("should fail with HTTP response code 403 Forbidden");
+                } catch (WebApplicationException e) {
+                    Assert.assertEquals(403, e.getResponse().getStatus());
+                }
+
+                UserRepresentation newGroupMemberWithNonExistentGroup = createUserRepresentation("new-group-member",
                         "new-group-member@keycloak.org", "New", "Member",
                         Arrays.asList("wrong-group"),true);
                 try {
-                    ApiUtil.createUserWithAdminClient(realmClient.realm(TEST), newGroupMemberWithAWrongGroup);
-                    Assert.fail("should fail with HTTP response code 400 Bad Request");
+                    ApiUtil.createUserWithAdminClient(realmClient.realm(TEST), newGroupMemberWithNonExistentGroup);
+                    Assert.fail("should fail with HTTP response code 403 Forbidden");
                 } catch (WebApplicationException e) {
-                    Assert.assertEquals(400, e.getResponse().getStatus());
+                    Assert.assertEquals(403, e.getResponse().getStatus());
+                }
+
+                UserRepresentation newGroupMemberOfNotManagedGroup = createUserRepresentation("new-group-member",
+                        "new-group-member@keycloak.org", "New", "Member",
+                        Arrays.asList("restricted-group"),true);
+                try {
+                    ApiUtil.createUserWithAdminClient(realmClient.realm(TEST), newGroupMemberOfNotManagedGroup);
+                    Assert.fail("should fail with HTTP response code 403 Forbidden");
+                } catch (WebApplicationException e) {
+                    Assert.assertEquals(403, e.getResponse().getStatus());
                 }
 
                 UserRepresentation newGroupMember = createUserRepresentation("new-group-member",

diff --git a/...ntegration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/UserTest.java b/...ntegration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/UserTest.java
@@ -26,6 +26,7 @@
 import org.junit.Rule;
 import org.junit.Test;
 import org.keycloak.TokenVerifier;
+import org.keycloak.admin.client.Keycloak;
 import org.keycloak.admin.client.resource.GroupResource;
 import org.keycloak.admin.client.resource.IdentityProviderResource;
 import org.keycloak.admin.client.resource.RealmResource;
@@ -39,6 +40,7 @@
 import org.keycloak.credential.CredentialModel;
 import org.keycloak.events.admin.OperationType;
 import org.keycloak.events.admin.ResourceType;
+import org.keycloak.jose.jws.JWSInput;
 import org.keycloak.models.Constants;
 import org.keycloak.models.LDAPConstants;
 import org.keycloak.models.PasswordPolicy;
@@ -72,6 +74,7 @@
 import org.keycloak.testsuite.pages.ProceedPage;
 import org.keycloak.testsuite.runonserver.RunHelpers;
 import org.keycloak.testsuite.updaters.Creator;
+import org.keycloak.testsuite.util.AdminClientUtil;
 import org.keycloak.testsuite.util.AdminEventPaths;
 import org.keycloak.testsuite.util.ClientBuilder;
 import org.keycloak.testsuite.util.GreenMailRule;
@@ -89,6 +92,7 @@
 import javax.ws.rs.BadRequestException;
 import javax.ws.rs.ClientErrorException;
 import javax.ws.rs.NotFoundException;
+import javax.ws.rs.WebApplicationException;
 import javax.ws.rs.core.Response;
 import javax.ws.rs.core.UriBuilder;
 import java.io.IOException;
@@ -116,6 +120,7 @@
 import static org.keycloak.testsuite.Assert.assertNames;
 import static org.keycloak.testsuite.arquillian.annotation.AuthServerContainerExclude.AuthServer.QUARKUS;
 import static org.keycloak.testsuite.arquillian.annotation.AuthServerContainerExclude.AuthServer.REMOTE;
+import static org.keycloak.testsuite.auth.page.AuthRealm.TEST;
 
 import org.keycloak.testsuite.arquillian.annotation.AuthServerContainerExclude;
 import org.keycloak.testsuite.arquillian.annotation.AuthServerContainerExclude.AuthServer;
@@ -2495,4 +2500,37 @@ private GroupRepresentation createGroup(RealmResource realm, GroupRepresentation
         group.setId(groupId);
         return group;
     }
+
+    @Test
+    public void failCreateUserUsingRegularUser() throws Exception {
+        String regularUserId = ApiUtil.getCreatedId(
+                testRealm().users().create(UserBuilder.create().username("regular-user").password("password").build()));
+        UserResource regularUser = testRealm().users().get(regularUserId);
+        UserRepresentation regularUserRep = regularUser.toRepresentation();
+
+        try (Keycloak adminClient = AdminClientUtil.createAdminClient(suiteContext.isAdapterCompatTesting(),
+                TEST, regularUserRep.getUsername(), "password", Constants.ADMIN_CLI_CLIENT_ID, null)) {
+            UserRepresentation invalidUser = UserBuilder.create().username("do-not-create-me").build();
+
+            Response response = adminClient.realm(TEST).users().create(invalidUser);
+            assertEquals(Response.Status.FORBIDDEN.getStatusCode(), response.getStatus());
+
+            invalidUser.setGroups(Collections.emptyList());
+            response = adminClient.realm(TEST).users().create(invalidUser);
+
+            assertEquals(Response.Status.FORBIDDEN.getStatusCode(), response.getStatus());
+        }
+    }
+
+    @Test
+    public void testCreateUserDoNotGrantRole() {
+        testRealm().roles().create(RoleBuilder.create().name("realm-role").build());
+
+        UserRepresentation userRep = UserBuilder.create().username("alice").password("password").addRoles("realm-role").build();
+        String userId = ApiUtil.getCreatedId(testRealm().users().create(userRep));
+        UserResource user = testRealm().users().get(userId);
+        List<RoleRepresentation> realmMappings = user.roles().getAll().getRealmMappings();
+
+        assertFalse(realmMappings.stream().map(RoleRepresentation::getName).anyMatch("realm-role"::equals));
+    }
 }