diff --git a/common/src/main/java/org/keycloak/common/Profile.java b/common/src/main/java/org/keycloak/common/Profile.java index 01030ab914..c8632cf07a 100755 --- a/common/src/main/java/org/keycloak/common/Profile.java +++ b/common/src/main/java/org/keycloak/common/Profile.java @@ -17,6 +17,8 @@ package org.keycloak.common; +import static org.keycloak.common.Profile.Type.DEPRECATED; + import org.jboss.logging.Logger; import java.io.File; @@ -38,9 +40,9 @@ public class Profile { DEFAULT, DISABLED_BY_DEFAULT, PREVIEW, - EXPERIMENTAL + EXPERIMENTAL, + DEPRECATED; } - public enum Feature { ACCOUNT2(Type.EXPERIMENTAL), ACCOUNT_API(Type.PREVIEW), @@ -50,7 +52,8 @@ public class Profile { OPENSHIFT_INTEGRATION(Type.PREVIEW), SCRIPTS(Type.PREVIEW), TOKEN_EXCHANGE(Type.PREVIEW), - AUTHZ_DROOLS_POLICY(Type.PREVIEW); + AUTHZ_DROOLS_POLICY(Type.PREVIEW), + UPLOAD_SCRIPTS(DEPRECATED); private Type type; @@ -83,6 +86,7 @@ public class Profile { private final Set disabledFeatures = new HashSet<>(); private final Set previewFeatures = new HashSet<>(); private final Set experimentalFeatures = new HashSet<>(); + private final Set deprecatedFeatures = new HashSet<>(); private Profile() { Config config = new Config(); @@ -99,9 +103,13 @@ public class Profile { disabledFeatures.add(f); } break; + case DEPRECATED: + deprecatedFeatures.add(f); case DISABLED_BY_DEFAULT: if (enabled == null || !enabled) { disabledFeatures.add(f); + } else if (DEPRECATED.equals(f.getType())) { + logger.warnf("Deprecated feature enabled: " + f.name().toLowerCase()); } break; case PREVIEW: @@ -144,6 +152,10 @@ public class Profile { return CURRENT.experimentalFeatures; } + public static Set getDeprecatedFeatures() { + return CURRENT.deprecatedFeatures; + } + public static boolean isFeatureEnabled(Feature feature) { return !CURRENT.disabledFeatures.contains(feature); } diff --git a/common/src/test/java/org/keycloak/common/ProfileTest.java b/common/src/test/java/org/keycloak/common/ProfileTest.java index 72760f5155..b2eb707522 100644 --- a/common/src/test/java/org/keycloak/common/ProfileTest.java +++ b/common/src/test/java/org/keycloak/common/ProfileTest.java @@ -6,7 +6,6 @@ import org.junit.Test; import org.junit.rules.TemporaryFolder; import java.io.File; -import java.io.FileWriter; import java.io.IOException; import java.io.PrintWriter; import java.util.Arrays; @@ -22,9 +21,10 @@ public class ProfileTest { @Test public void checkDefaults() { Assert.assertEquals("community", Profile.getName()); - assertEquals(Profile.getDisabledFeatures(), Profile.Feature.ACCOUNT2, Profile.Feature.ACCOUNT_API, Profile.Feature.ADMIN_FINE_GRAINED_AUTHZ, Profile.Feature.DOCKER, Profile.Feature.SCRIPTS, Profile.Feature.TOKEN_EXCHANGE, Profile.Feature.AUTHZ_DROOLS_POLICY, Profile.Feature.OPENSHIFT_INTEGRATION); + assertEquals(Profile.getDisabledFeatures(), Profile.Feature.ACCOUNT2, Profile.Feature.ACCOUNT_API, Profile.Feature.ADMIN_FINE_GRAINED_AUTHZ, Profile.Feature.DOCKER, Profile.Feature.SCRIPTS, Profile.Feature.TOKEN_EXCHANGE, Profile.Feature.AUTHZ_DROOLS_POLICY, Profile.Feature.OPENSHIFT_INTEGRATION, Profile.Feature.UPLOAD_SCRIPTS); assertEquals(Profile.getPreviewFeatures(), Profile.Feature.ACCOUNT_API, Profile.Feature.ADMIN_FINE_GRAINED_AUTHZ, Profile.Feature.SCRIPTS, Profile.Feature.TOKEN_EXCHANGE, Profile.Feature.AUTHZ_DROOLS_POLICY, Profile.Feature.OPENSHIFT_INTEGRATION); assertEquals(Profile.getExperimentalFeatures(), Profile.Feature.ACCOUNT2); + assertEquals(Profile.getDeprecatedFeatures(), Profile.Feature.UPLOAD_SCRIPTS); } @Test @@ -33,10 +33,12 @@ public class ProfileTest { Assert.assertFalse(Profile.isFeatureEnabled(Profile.Feature.DOCKER)); Assert.assertFalse(Profile.isFeatureEnabled(Profile.Feature.OPENSHIFT_INTEGRATION)); Assert.assertTrue(Profile.isFeatureEnabled(Profile.Feature.IMPERSONATION)); + Assert.assertFalse(Profile.isFeatureEnabled(Profile.Feature.UPLOAD_SCRIPTS)); System.setProperty("keycloak.profile", "preview"); System.setProperty("keycloak.profile.feature.docker", "enabled"); System.setProperty("keycloak.profile.feature.impersonation", "disabled"); + System.setProperty("keycloak.profile.feature.upload_scripts", "enabled"); Profile.init(); @@ -44,10 +46,12 @@ public class ProfileTest { Assert.assertTrue(Profile.isFeatureEnabled(Profile.Feature.DOCKER)); Assert.assertTrue(Profile.isFeatureEnabled(Profile.Feature.OPENSHIFT_INTEGRATION)); Assert.assertFalse(Profile.isFeatureEnabled(Profile.Feature.IMPERSONATION)); + Assert.assertTrue(Profile.isFeatureEnabled(Profile.Feature.UPLOAD_SCRIPTS)); System.getProperties().remove("keycloak.profile"); System.getProperties().remove("keycloak.profile.feature.docker"); System.getProperties().remove("keycloak.profile.feature.impersonation"); + System.getProperties().remove("keycloak.profile.feature.upload_scripts"); Profile.init(); } @@ -57,6 +61,7 @@ public class ProfileTest { Assert.assertEquals("community", Profile.getName()); Assert.assertFalse(Profile.isFeatureEnabled(Profile.Feature.DOCKER)); Assert.assertTrue(Profile.isFeatureEnabled(Profile.Feature.IMPERSONATION)); + Assert.assertFalse(Profile.isFeatureEnabled(Profile.Feature.UPLOAD_SCRIPTS)); File d = temporaryFolder.newFolder(); File f = new File(d, "profile.properties"); @@ -65,6 +70,7 @@ public class ProfileTest { p.setProperty("profile", "preview"); p.setProperty("feature.docker", "enabled"); p.setProperty("feature.impersonation", "disabled"); + p.setProperty("feature.upload_scripts", "enabled"); PrintWriter pw = new PrintWriter(f); p.list(pw); pw.close(); @@ -77,6 +83,7 @@ public class ProfileTest { Assert.assertTrue(Profile.isFeatureEnabled(Profile.Feature.DOCKER)); Assert.assertTrue(Profile.isFeatureEnabled(Profile.Feature.OPENSHIFT_INTEGRATION)); Assert.assertFalse(Profile.isFeatureEnabled(Profile.Feature.IMPERSONATION)); + Assert.assertTrue(Profile.isFeatureEnabled(Profile.Feature.UPLOAD_SCRIPTS)); System.getProperties().remove("jboss.server.config.dir"); diff --git a/services/src/main/java/org/keycloak/authorization/protection/policy/UserManagedPermissionService.java b/services/src/main/java/org/keycloak/authorization/protection/policy/UserManagedPermissionService.java index d9a03760b4..affd74bb01 100644 --- a/services/src/main/java/org/keycloak/authorization/protection/policy/UserManagedPermissionService.java +++ b/services/src/main/java/org/keycloak/authorization/protection/policy/UserManagedPermissionService.java @@ -30,10 +30,8 @@ import javax.ws.rs.Path; import javax.ws.rs.PathParam; import javax.ws.rs.Produces; import javax.ws.rs.QueryParam; -import javax.ws.rs.core.Context; import javax.ws.rs.core.Response; import javax.ws.rs.core.Response.Status; -import javax.ws.rs.core.UriInfo; import org.jboss.resteasy.annotations.cache.NoCache; import org.jboss.resteasy.spi.ResteasyProviderFactory; @@ -47,6 +45,7 @@ import org.keycloak.authorization.model.Policy; import org.keycloak.authorization.model.Resource; import org.keycloak.authorization.model.ResourceServer; import org.keycloak.authorization.store.ResourceStore; +import org.keycloak.common.Profile; import org.keycloak.representations.idm.authorization.UmaPermissionRepresentation; import org.keycloak.services.ErrorResponseException; import org.keycloak.services.resources.admin.AdminEventBuilder; @@ -174,6 +173,12 @@ public class UserManagedPermissionService { if (!resourceScopes.containsAll(scopes)) { throw new ErrorResponseException(OAuthErrorException.INVALID_REQUEST, "Some of the scopes [" + scopes + "] are not valid for resource [" + resourceId + "]", Response.Status.BAD_REQUEST); } + + if (representation.getCondition() != null) { + if (!Profile.isFeatureEnabled(Profile.Feature.UPLOAD_SCRIPTS)) { + throw new ErrorResponseException(OAuthErrorException.INVALID_REQUEST, "Script upload not supported", Status.BAD_REQUEST); + } + } } } diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/ProfileAssume.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/ProfileAssume.java index c5e7a212af..710746739f 100644 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/ProfileAssume.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/ProfileAssume.java @@ -57,6 +57,10 @@ public class ProfileAssume { Assume.assumeTrue("Ignoring test as feature " + feature.name() + " is not enabled", isFeatureEnabled(feature)); } + public static void assumeFeatureDisabled(Profile.Feature feature) { + Assume.assumeTrue("Ignoring test as feature " + feature.name() + " is disabled", !isFeatureEnabled(feature)); + } + public static void assumePreview() { updateProfile(); Assume.assumeTrue("Ignoring test as community/preview profile is not enabled", !profile.equals("product")); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/UserManagedPermissionServiceTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/UserManagedPermissionServiceTest.java index d544645d28..b0c445cba5 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/UserManagedPermissionServiceTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/UserManagedPermissionServiceTest.java @@ -18,7 +18,6 @@ package org.keycloak.testsuite.authz; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -35,6 +34,7 @@ import org.keycloak.authorization.client.resource.AuthorizationResource; import org.keycloak.authorization.client.resource.PolicyResource; import org.keycloak.authorization.client.resource.ProtectionResource; import org.keycloak.authorization.client.util.HttpResponseException; +import org.keycloak.common.Profile; import org.keycloak.representations.AccessToken; import org.keycloak.representations.idm.RealmRepresentation; import org.keycloak.representations.idm.authorization.AuthorizationRequest; @@ -46,6 +46,7 @@ import org.keycloak.representations.idm.authorization.PermissionTicketRepresenta import org.keycloak.representations.idm.authorization.PolicyRepresentation; import org.keycloak.representations.idm.authorization.ResourceRepresentation; import org.keycloak.representations.idm.authorization.UmaPermissionRepresentation; +import org.keycloak.testsuite.ProfileAssume; import org.keycloak.testsuite.util.ClientBuilder; import org.keycloak.testsuite.util.GroupBuilder; import org.keycloak.testsuite.util.RealmBuilder; @@ -114,7 +115,11 @@ public class UserManagedPermissionServiceTest extends AbstractResourceServerTest newPermission.addRole("role_a", "role_b", "role_c", "role_d"); newPermission.addGroup("/group_a", "/group_a/group_b", "/group_c"); newPermission.addClient("client-a", "resource-server-test"); - newPermission.setCondition("$evaluation.grant()"); + + if (Profile.isFeatureEnabled(Profile.Feature.UPLOAD_SCRIPTS)) { + newPermission.setCondition("$evaluation.grant()"); + } + newPermission.addUser("kolo"); ProtectionResource protection = getAuthzClient().protection("marta", "password"); @@ -136,6 +141,12 @@ public class UserManagedPermissionServiceTest extends AbstractResourceServerTest assertTrue(permission.getUsers().containsAll(newPermission.getUsers())); } + @Test + public void testCreateDeprecatedFeaturesEnabled() { + ProfileAssume.assumeFeatureEnabled(Profile.Feature.UPLOAD_SCRIPTS); + testCreate(); + } + @Test public void testUpdate() { ResourceRepresentation resource = new ResourceRepresentation(); @@ -236,18 +247,23 @@ public class UserManagedPermissionServiceTest extends AbstractResourceServerTest assertTrue(permission.getClients().containsAll(updated.getClients())); - permission.setCondition("$evaluation.grant()"); + if (Profile.isFeatureEnabled(Profile.Feature.UPLOAD_SCRIPTS)) { + permission.setCondition("$evaluation.grant()"); - protection.policy(resource.getId()).update(permission); - assertEquals(4, getAssociatedPolicies(permission).size()); - updated = protection.policy(resource.getId()).findById(permission.getId()); + protection.policy(resource.getId()).update(permission); + assertEquals(4, getAssociatedPolicies(permission).size()); + updated = protection.policy(resource.getId()).findById(permission.getId()); - assertEquals(permission.getCondition(), updated.getCondition()); + assertEquals(permission.getCondition(), updated.getCondition()); + } permission.addUser("alice"); protection.policy(resource.getId()).update(permission); - assertEquals(5, getAssociatedPolicies(permission).size()); + + int expectedPolicies = Profile.isFeatureEnabled(Profile.Feature.UPLOAD_SCRIPTS) ? 5 : 4; + + assertEquals(expectedPolicies, getAssociatedPolicies(permission).size()); updated = protection.policy(resource.getId()).findById(permission.getId()); assertEquals(1, updated.getUsers().size()); assertEquals(permission.getUsers(), updated.getUsers()); @@ -255,7 +271,7 @@ public class UserManagedPermissionServiceTest extends AbstractResourceServerTest permission.addUser("kolo"); protection.policy(resource.getId()).update(permission); - assertEquals(5, getAssociatedPolicies(permission).size()); + assertEquals(expectedPolicies, getAssociatedPolicies(permission).size()); updated = protection.policy(resource.getId()).findById(permission.getId()); assertEquals(2, updated.getUsers().size()); assertEquals(permission.getUsers(), updated.getUsers()); @@ -263,7 +279,7 @@ public class UserManagedPermissionServiceTest extends AbstractResourceServerTest permission.removeUser("alice"); protection.policy(resource.getId()).update(permission); - assertEquals(5, getAssociatedPolicies(permission).size()); + assertEquals(expectedPolicies, getAssociatedPolicies(permission).size()); updated = protection.policy(resource.getId()).findById(permission.getId()); assertEquals(1, updated.getUsers().size()); assertEquals(permission.getUsers(), updated.getUsers()); @@ -271,23 +287,25 @@ public class UserManagedPermissionServiceTest extends AbstractResourceServerTest permission.setUsers(null); protection.policy(resource.getId()).update(permission); - assertEquals(4, getAssociatedPolicies(permission).size()); + assertEquals(--expectedPolicies, getAssociatedPolicies(permission).size()); updated = protection.policy(resource.getId()).findById(permission.getId()); assertEquals(permission.getUsers(), updated.getUsers()); - permission.setCondition(null); + if (Profile.isFeatureEnabled(Profile.Feature.UPLOAD_SCRIPTS)) { + permission.setCondition(null); - protection.policy(resource.getId()).update(permission); - assertEquals(3, getAssociatedPolicies(permission).size()); - updated = protection.policy(resource.getId()).findById(permission.getId()); + protection.policy(resource.getId()).update(permission); + assertEquals(--expectedPolicies, getAssociatedPolicies(permission).size()); + updated = protection.policy(resource.getId()).findById(permission.getId()); - assertEquals(permission.getCondition(), updated.getCondition()); + assertEquals(permission.getCondition(), updated.getCondition()); + }; permission.setRoles(null); protection.policy(resource.getId()).update(permission); - assertEquals(2, getAssociatedPolicies(permission).size()); + assertEquals(--expectedPolicies, getAssociatedPolicies(permission).size()); updated = protection.policy(resource.getId()).findById(permission.getId()); assertEquals(permission.getRoles(), updated.getRoles()); @@ -295,7 +313,7 @@ public class UserManagedPermissionServiceTest extends AbstractResourceServerTest permission.setClients(null); protection.policy(resource.getId()).update(permission); - assertEquals(1, getAssociatedPolicies(permission).size()); + assertEquals(--expectedPolicies, getAssociatedPolicies(permission).size()); updated = protection.policy(resource.getId()).findById(permission.getId()); assertEquals(permission.getClients(), updated.getClients()); @@ -312,6 +330,53 @@ public class UserManagedPermissionServiceTest extends AbstractResourceServerTest fail("Expected not found"); } } + + @Test + public void testUpdateDeprecatedFeaturesEnabled() { + ProfileAssume.assumeFeatureEnabled(Profile.Feature.UPLOAD_SCRIPTS); + testUpdate(); + } + + @Test + public void testUploadScriptDisabled() { + ProfileAssume.assumeFeatureDisabled(Profile.Feature.UPLOAD_SCRIPTS); + ResourceRepresentation resource = new ResourceRepresentation(); + + resource.setName("Resource A"); + resource.setOwnerManagedAccess(true); + resource.setOwner("marta"); + resource.addScope("Scope A", "Scope B", "Scope C"); + + resource = getAuthzClient().protection().resource().create(resource); + + UmaPermissionRepresentation newPermission = new UmaPermissionRepresentation(); + + newPermission.setName("Custom User-Managed Permission"); + newPermission.setDescription("Users from specific roles are allowed to access"); + newPermission.setCondition("$evaluation.grant()"); + + ProtectionResource protection = getAuthzClient().protection("marta", "password"); + + try { + protection.policy(resource.getId()).create(newPermission); + fail("Should fail because upload scripts is disabled"); + } catch (Exception ignore) { + + } + + newPermission.setCondition(null); + + UmaPermissionRepresentation representation = protection.policy(resource.getId()).create(newPermission); + + representation.setCondition("$evaluation.grant();"); + + try { + protection.policy(resource.getId()).update(newPermission); + fail("Should fail because upload scripts is disabled"); + } catch (Exception ignore) { + + } + } @Test public void testUserManagedPermission() {