From 53472e097c71ba4aae7d2027e27152a6d1ae6155 Mon Sep 17 00:00:00 2001 From: Sebastian Schuster Date: Mon, 8 Aug 2022 18:45:07 +0200 Subject: [PATCH] 13647 fixed wrong feature flag for checking admin fine-grained authz --- .../admin/permissions/GroupPermissions.java | 2 +- .../admin/permissions/MgmtPermissions.java | 6 +++--- .../admin/permissions/UserPermissions.java | 2 +- .../BrokerLinkAndTokenExchangeTest.java | 6 +----- .../admin/FineGrainAdminUnitTest.java | 7 ------- .../admin/ManagementPermissionsTest.java | 9 ++------- .../keycloak/testsuite/admin/UsersTest.java | 20 +++++++------------ .../oauth/ClientTokenExchangeSAML2Test.java | 9 +-------- .../oauth/ClientTokenExchangeTest.java | 9 +-------- 9 files changed, 17 insertions(+), 53 deletions(-) diff --git a/services/src/main/java/org/keycloak/services/resources/admin/permissions/GroupPermissions.java b/services/src/main/java/org/keycloak/services/resources/admin/permissions/GroupPermissions.java index e99cbf0d7dc..382ec2c0b6a 100644 --- a/services/src/main/java/org/keycloak/services/resources/admin/permissions/GroupPermissions.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/permissions/GroupPermissions.java @@ -61,7 +61,7 @@ class GroupPermissions implements GroupPermissionEvaluator, GroupPermissionManag GroupPermissions(AuthorizationProvider authz, MgmtPermissions root) { this.authz = authz; this.root = root; - if (Profile.isFeatureEnabled(Profile.Feature.AUTHORIZATION)) { + if (Profile.isFeatureEnabled(Profile.Feature.ADMIN_FINE_GRAINED_AUTHZ)) { resourceStore = authz.getStoreFactory().getResourceStore(); policyStore = authz.getStoreFactory().getPolicyStore(); } else { diff --git a/services/src/main/java/org/keycloak/services/resources/admin/permissions/MgmtPermissions.java b/services/src/main/java/org/keycloak/services/resources/admin/permissions/MgmtPermissions.java index 72a2cdc4dd1..772c4d4928d 100644 --- a/services/src/main/java/org/keycloak/services/resources/admin/permissions/MgmtPermissions.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/permissions/MgmtPermissions.java @@ -73,7 +73,7 @@ class MgmtPermissions implements AdminPermissionEvaluator, AdminPermissionManage this.session = session; this.realm = realm; KeycloakSessionFactory keycloakSessionFactory = session.getKeycloakSessionFactory(); - if (Profile.isFeatureEnabled(Profile.Feature.AUTHORIZATION)) { + if (Profile.isFeatureEnabled(Profile.Feature.ADMIN_FINE_GRAINED_AUTHZ)) { AuthorizationProviderFactory factory = (AuthorizationProviderFactory) keycloakSessionFactory.getProviderFactory(AuthorizationProvider.class); this.authz = factory.create(session, realm); } @@ -251,7 +251,7 @@ class MgmtPermissions implements AdminPermissionEvaluator, AdminPermissionManage @Override public ResourceServer realmResourceServer() { - if (!Profile.isFeatureEnabled(Profile.Feature.AUTHORIZATION)) return null; + if (!Profile.isFeatureEnabled(Profile.Feature.ADMIN_FINE_GRAINED_AUTHZ)) return null; if (realmResourceServer != null) return realmResourceServer; ClientModel client = getRealmManagementClient(); if (client == null) return null; @@ -262,7 +262,7 @@ class MgmtPermissions implements AdminPermissionEvaluator, AdminPermissionManage } public ResourceServer initializeRealmResourceServer() { - if (!Profile.isFeatureEnabled(Profile.Feature.AUTHORIZATION)) return null; + if (!Profile.isFeatureEnabled(Profile.Feature.ADMIN_FINE_GRAINED_AUTHZ)) return null; if (realmResourceServer != null) return realmResourceServer; ClientModel client = getRealmManagementClient(); realmResourceServer = authz.getStoreFactory().getResourceServerStore().findByClient(client); diff --git a/services/src/main/java/org/keycloak/services/resources/admin/permissions/UserPermissions.java b/services/src/main/java/org/keycloak/services/resources/admin/permissions/UserPermissions.java index fc5aa8d4825..1154894390f 100644 --- a/services/src/main/java/org/keycloak/services/resources/admin/permissions/UserPermissions.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/permissions/UserPermissions.java @@ -83,7 +83,7 @@ class UserPermissions implements UserPermissionEvaluator, UserPermissionManageme this.session = session; this.authz = authz; this.root = root; - if (Profile.isFeatureEnabled(Profile.Feature.AUTHORIZATION)) { + if (Profile.isFeatureEnabled(Profile.Feature.ADMIN_FINE_GRAINED_AUTHZ)) { policyStore = authz.getStoreFactory().getPolicyStore(); resourceStore = authz.getStoreFactory().getResourceStore(); } else { diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/BrokerLinkAndTokenExchangeTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/BrokerLinkAndTokenExchangeTest.java index 0582cd1a707..4745c90f499 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/BrokerLinkAndTokenExchangeTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/BrokerLinkAndTokenExchangeTest.java @@ -97,6 +97,7 @@ import static org.keycloak.testsuite.admin.ApiUtil.createUserAndResetPasswordWit @AppServerContainer(ContainerConstants.APP_SERVER_EAP6) @AppServerContainer(ContainerConstants.APP_SERVER_EAP71) @EnableFeature(value = Profile.Feature.TOKEN_EXCHANGE, skipRestart = true) +@EnableFeature(value = Profile.Feature.ADMIN_FINE_GRAINED_AUTHZ, skipRestart = true) public class BrokerLinkAndTokenExchangeTest extends AbstractServletsAdapterTest { public static final String CHILD_IDP = "child"; public static final String PARENT_IDP = "parent-idp"; @@ -106,11 +107,6 @@ public class BrokerLinkAndTokenExchangeTest extends AbstractServletsAdapterTest public static final String UNAUTHORIZED_CHILD_CLIENT = "unauthorized-child-client"; public static final String PARENT_CLIENT = "parent-client"; - @BeforeClass - public static void enabled() { - ProfileAssume.assumeFeatureEnabled(Profile.Feature.AUTHORIZATION); - } - @Deployment(name = ClientApp.DEPLOYMENT_NAME) protected static WebArchive accountLink() { return servletDeployment(ClientApp.DEPLOYMENT_NAME, LinkAndExchangeServlet.class, ServletTestUtils.class); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/FineGrainAdminUnitTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/FineGrainAdminUnitTest.java index a83400f99c3..03ac0868ff6 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/FineGrainAdminUnitTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/FineGrainAdminUnitTest.java @@ -18,7 +18,6 @@ package org.keycloak.testsuite.admin; import org.hamcrest.Matchers; import org.junit.Assert; -import org.junit.BeforeClass; import org.junit.Test; import org.keycloak.admin.client.Keycloak; import org.keycloak.authorization.AuthorizationProvider; @@ -53,7 +52,6 @@ import org.keycloak.services.resources.admin.permissions.AdminPermissions; import org.keycloak.services.resources.admin.permissions.ClientPermissionManagement; import org.keycloak.services.resources.admin.permissions.GroupPermissionManagement; import org.keycloak.testsuite.AbstractKeycloakTest; -import org.keycloak.testsuite.ProfileAssume; import org.keycloak.testsuite.arquillian.annotation.EnableFeature; import org.keycloak.testsuite.arquillian.annotation.UncaughtServerErrorExpected; import org.keycloak.testsuite.auth.page.AuthRealm; @@ -88,11 +86,6 @@ public class FineGrainAdminUnitTest extends AbstractKeycloakTest { public static final String CLIENT_NAME = "application"; - @BeforeClass - public static void enabled() { - ProfileAssume.assumeFeatureEnabled(Profile.Feature.AUTHORIZATION); - } - @Override public void addTestRealms(List testRealms) { RealmRepresentation testRealmRep = new RealmRepresentation(); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/ManagementPermissionsTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/ManagementPermissionsTest.java index bc3c690516f..b5e3d1a0adf 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/ManagementPermissionsTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/ManagementPermissionsTest.java @@ -16,7 +16,6 @@ */ package org.keycloak.testsuite.admin; -import org.junit.BeforeClass; import org.junit.Test; import org.keycloak.admin.client.resource.ClientResource; import org.keycloak.admin.client.resource.GroupResource; @@ -25,7 +24,7 @@ import org.keycloak.admin.client.resource.RoleResource; import org.keycloak.common.Profile; import org.keycloak.representations.idm.*; import org.keycloak.testsuite.AbstractTestRealmKeycloakTest; -import org.keycloak.testsuite.ProfileAssume; +import org.keycloak.testsuite.arquillian.annotation.EnableFeature; import javax.ws.rs.core.Response; @@ -36,13 +35,9 @@ import static org.junit.Assert.assertTrue; /** * @author Leon Graser */ +@EnableFeature(value = Profile.Feature.ADMIN_FINE_GRAINED_AUTHZ, skipRestart = true) public class ManagementPermissionsTest extends AbstractTestRealmKeycloakTest { - @BeforeClass - public static void enabled() { - ProfileAssume.assumeFeatureEnabled(Profile.Feature.AUTHORIZATION); - } - @Override public void configureTestRealm(RealmRepresentation testRealm) { diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/UsersTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/UsersTest.java index 0c4ee9f2106..138c689ad4c 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/UsersTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/UsersTest.java @@ -32,7 +32,7 @@ import org.keycloak.representations.idm.authorization.DecisionStrategy; import org.keycloak.representations.idm.authorization.PolicyRepresentation; import org.keycloak.representations.idm.authorization.ScopePermissionRepresentation; import org.keycloak.representations.idm.authorization.UserPolicyRepresentation; -import org.keycloak.testsuite.ProfileAssume; +import org.keycloak.testsuite.arquillian.annotation.EnableFeature; import org.keycloak.testsuite.util.AdminClientUtil; import java.io.IOException; @@ -224,17 +224,15 @@ public class UsersTest extends AbstractAdminTest { } @Test + @EnableFeature(value = Profile.Feature.ADMIN_FINE_GRAINED_AUTHZ, skipRestart = true) public void countUsersWithGroupViewPermission() throws CertificateException, NoSuchAlgorithmException, KeyStoreException, KeyManagementException, IOException { - ProfileAssume.assumeFeatureEnabled(Profile.Feature.AUTHORIZATION); - RealmResource testRealmResource = setupTestEnvironmentWithPermissions(true); assertThat(testRealmResource.users().count(), is(3)); } @Test + @EnableFeature(value = Profile.Feature.ADMIN_FINE_GRAINED_AUTHZ, skipRestart = true) public void countUsersBySearchWithGroupViewPermission() throws CertificateException, NoSuchAlgorithmException, KeyStoreException, KeyManagementException, IOException { - ProfileAssume.assumeFeatureEnabled(Profile.Feature.AUTHORIZATION); - RealmResource testRealmResource = setupTestEnvironmentWithPermissions(true); //search all assertThat(testRealmResource.users().count("user"), is(3)); @@ -256,9 +254,8 @@ public class UsersTest extends AbstractAdminTest { } @Test + @EnableFeature(value = Profile.Feature.ADMIN_FINE_GRAINED_AUTHZ, skipRestart = true) public void countUsersByFiltersWithGroupViewPermission() throws CertificateException, NoSuchAlgorithmException, KeyStoreException, KeyManagementException, IOException { - ProfileAssume.assumeFeatureEnabled(Profile.Feature.AUTHORIZATION); - RealmResource testRealmResource = setupTestEnvironmentWithPermissions(true); //search username assertThat(testRealmResource.users().count(null, null, null, "user"), is(3)); @@ -293,17 +290,15 @@ public class UsersTest extends AbstractAdminTest { } @Test + @EnableFeature(value = Profile.Feature.ADMIN_FINE_GRAINED_AUTHZ, skipRestart = true) public void countUsersWithNoViewPermission() throws CertificateException, NoSuchAlgorithmException, KeyStoreException, IOException, KeyManagementException { - ProfileAssume.assumeFeatureEnabled(Profile.Feature.AUTHORIZATION); - RealmResource testRealmResource = setupTestEnvironmentWithPermissions(false); assertThat(testRealmResource.users().count(), is(0)); } @Test + @EnableFeature(value = Profile.Feature.ADMIN_FINE_GRAINED_AUTHZ, skipRestart = true) public void countUsersBySearchWithNoViewPermission() throws CertificateException, NoSuchAlgorithmException, KeyStoreException, KeyManagementException, IOException { - ProfileAssume.assumeFeatureEnabled(Profile.Feature.AUTHORIZATION); - RealmResource testRealmResource = setupTestEnvironmentWithPermissions(false); //search all assertThat(testRealmResource.users().count("user"), is(0)); @@ -325,9 +320,8 @@ public class UsersTest extends AbstractAdminTest { } @Test + @EnableFeature(value = Profile.Feature.ADMIN_FINE_GRAINED_AUTHZ, skipRestart = true) public void countUsersByFiltersWithNoViewPermission() throws CertificateException, NoSuchAlgorithmException, KeyStoreException, KeyManagementException, IOException { - ProfileAssume.assumeFeatureEnabled(Profile.Feature.AUTHORIZATION); - RealmResource testRealmResource = setupTestEnvironmentWithPermissions(false); //search username assertThat(testRealmResource.users().count(null, null, null, "user"), is(0)); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/ClientTokenExchangeSAML2Test.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/ClientTokenExchangeSAML2Test.java index 0408c45ab8e..e245b08cbd2 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/ClientTokenExchangeSAML2Test.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/ClientTokenExchangeSAML2Test.java @@ -17,7 +17,6 @@ package org.keycloak.testsuite.oauth; -import org.junit.BeforeClass; import org.junit.Rule; import org.junit.Test; import org.keycloak.OAuth2Constants; @@ -54,7 +53,6 @@ import org.keycloak.services.resources.admin.permissions.AdminPermissions; import org.keycloak.testsuite.AbstractKeycloakTest; import org.keycloak.testsuite.Assert; import org.keycloak.testsuite.AssertEvents; -import org.keycloak.testsuite.ProfileAssume; import org.keycloak.testsuite.arquillian.annotation.AuthServerContainerExclude; import org.keycloak.testsuite.arquillian.annotation.AuthServerContainerExclude.AuthServer; import org.keycloak.testsuite.arquillian.annotation.EnableFeature; @@ -78,7 +76,6 @@ import java.util.List; import java.util.Map; import static org.junit.Assert.assertNotNull; -import static org.keycloak.common.Profile.Feature.AUTHORIZATION; import static org.keycloak.models.ImpersonationSessionNote.IMPERSONATOR_ID; import static org.keycloak.models.ImpersonationSessionNote.IMPERSONATOR_USERNAME; import static org.keycloak.protocol.saml.SamlProtocol.SAML_ASSERTION_CONSUMER_URL_POST_ATTRIBUTE; @@ -89,6 +86,7 @@ import static org.keycloak.testsuite.auth.page.AuthRealm.TEST; */ @AuthServerContainerExclude(AuthServer.REMOTE) @EnableFeature(value = Profile.Feature.TOKEN_EXCHANGE, skipRestart = true) +@EnableFeature(value = Profile.Feature.ADMIN_FINE_GRAINED_AUTHZ, skipRestart = true) public class ClientTokenExchangeSAML2Test extends AbstractKeycloakTest { private static final String SAML_SIGNED_TARGET = "http://localhost:8080/saml-signed-assertion/"; @@ -104,11 +102,6 @@ public class ClientTokenExchangeSAML2Test extends AbstractKeycloakTest { @Rule public AssertEvents events = new AssertEvents(this); - @BeforeClass - public static void enabled() { - ProfileAssume.assumeFeatureEnabled(AUTHORIZATION); - } - @Override public void addTestRealms(List testRealms) { RealmRepresentation testRealmRep = new RealmRepresentation(); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/ClientTokenExchangeTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/ClientTokenExchangeTest.java index 37a4504f88a..1b698605ad8 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/ClientTokenExchangeTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/ClientTokenExchangeTest.java @@ -17,7 +17,6 @@ package org.keycloak.testsuite.oauth; -import org.junit.BeforeClass; import org.junit.Rule; import org.junit.Test; import org.keycloak.OAuth2Constants; @@ -49,7 +48,6 @@ import org.keycloak.services.resources.admin.permissions.AdminPermissions; import org.keycloak.testsuite.AbstractKeycloakTest; import org.keycloak.testsuite.Assert; import org.keycloak.testsuite.AssertEvents; -import org.keycloak.testsuite.ProfileAssume; import org.keycloak.testsuite.admin.ApiUtil; import org.keycloak.testsuite.arquillian.annotation.AuthServerContainerExclude; import org.keycloak.testsuite.arquillian.annotation.DisableFeature; @@ -71,7 +69,6 @@ import static org.hamcrest.Matchers.instanceOf; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; -import static org.keycloak.common.Profile.Feature.AUTHORIZATION; import static org.keycloak.models.ImpersonationSessionNote.IMPERSONATOR_ID; import static org.keycloak.models.ImpersonationSessionNote.IMPERSONATOR_USERNAME; import org.keycloak.testsuite.arquillian.annotation.AuthServerContainerExclude.AuthServer; @@ -83,16 +80,12 @@ import static org.keycloak.testsuite.auth.page.AuthRealm.TEST; */ @AuthServerContainerExclude(AuthServer.REMOTE) @EnableFeature(value = Profile.Feature.TOKEN_EXCHANGE, skipRestart = true) +@EnableFeature(value = Profile.Feature.ADMIN_FINE_GRAINED_AUTHZ, skipRestart = true) public class ClientTokenExchangeTest extends AbstractKeycloakTest { @Rule public AssertEvents events = new AssertEvents(this); - @BeforeClass - public static void enabled() { - ProfileAssume.assumeFeatureEnabled(AUTHORIZATION); - } - @Test @UncaughtServerErrorExpected @DisableFeature(value = Profile.Feature.TOKEN_EXCHANGE, skipRestart = true)