[KEYCLOAK-10870] - Deprecate support for JavaScript policy support from UMA policy endpoint

Conflicts:
	testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/UserManagedPermissionServiceTest.java

(cherry picked from commit 13923a7683cb666d2842bc61429c23409c1493b6)
This commit is contained in:
Pedro Igor 2019-08-14 09:43:50 -03:00 committed by Stian Thorgersen
parent 0e0177136c
commit bad9e29c15
5 changed files with 118 additions and 25 deletions

View file

@ -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<Feature> disabledFeatures = new HashSet<>();
private final Set<Feature> previewFeatures = new HashSet<>();
private final Set<Feature> experimentalFeatures = new HashSet<>();
private final Set<Feature> 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<Feature> getDeprecatedFeatures() {
return CURRENT.deprecatedFeatures;
}
public static boolean isFeatureEnabled(Feature feature) {
return !CURRENT.disabledFeatures.contains(feature);
}

View file

@ -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");

View file

@ -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);
}
}
}
}

View file

@ -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"));

View file

@ -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() {