diff --git a/server-spi-private/src/main/java/org/keycloak/authorization/policy/evaluation/DefaultEvaluation.java b/server-spi-private/src/main/java/org/keycloak/authorization/policy/evaluation/DefaultEvaluation.java index 6a456b6dc6e..61b359cc7fc 100644 --- a/server-spi-private/src/main/java/org/keycloak/authorization/policy/evaluation/DefaultEvaluation.java +++ b/server-spi-private/src/main/java/org/keycloak/authorization/policy/evaluation/DefaultEvaluation.java @@ -89,23 +89,19 @@ public class DefaultEvaluation implements Evaluation { @Override public void grant() { if (policy != null && Logic.NEGATIVE.equals(policy.getLogic())) { - this.effect = Effect.DENY; + setEffect(Effect.DENY); } else { - this.effect = Effect.PERMIT; + setEffect(Effect.PERMIT); } - - this.decision.onDecision(this); } @Override public void deny() { if (policy != null && Logic.NEGATIVE.equals(policy.getLogic())) { - this.effect = Effect.PERMIT; + setEffect(Effect.PERMIT); } else { - this.effect = Effect.DENY; + setEffect(Effect.DENY); } - - this.decision.onDecision(this); } @Override @@ -274,10 +270,7 @@ public class DefaultEvaluation implements Evaluation { } public void setEffect(Effect effect) { - if (Effect.PERMIT.equals(effect)) { - grant(); - } else { - deny(); - } + this.effect = effect; + this.decision.onDecision(this); } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/PolicyEvaluationTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/PolicyEvaluationTest.java index fedc6e8adb0..412e1ad6cc6 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/PolicyEvaluationTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/PolicyEvaluationTest.java @@ -50,6 +50,7 @@ import org.keycloak.models.ClientModel; import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; import org.keycloak.models.UserModel; +import org.keycloak.models.utils.KeycloakModelUtils; import org.keycloak.models.utils.RepresentationToModel; import org.keycloak.protocol.oidc.OIDCLoginProtocol; import org.keycloak.protocol.oidc.mappers.GroupMembershipMapper; @@ -58,7 +59,10 @@ import org.keycloak.representations.idm.GroupRepresentation; import org.keycloak.representations.idm.ProtocolMapperRepresentation; import org.keycloak.representations.idm.RealmRepresentation; import org.keycloak.representations.idm.authorization.JSPolicyRepresentation; +import org.keycloak.representations.idm.authorization.Logic; +import org.keycloak.representations.idm.authorization.Permission; import org.keycloak.representations.idm.authorization.ResourcePermissionRepresentation; +import org.keycloak.representations.idm.authorization.ScopePermissionRepresentation; import org.keycloak.representations.idm.authorization.TimePolicyRepresentation; import org.keycloak.testsuite.runonserver.RunOnServerDeployment; import org.keycloak.testsuite.util.ClientBuilder; @@ -678,6 +682,53 @@ public class PolicyEvaluationTest extends AbstractAuthzTest { } } + @Test + public void testCachedDecisionsWithNegativePolicies() { + testingClient.server().run(PolicyEvaluationTest::testCachedDecisionsWithNegativePolicies); + } + + public static void testCachedDecisionsWithNegativePolicies(KeycloakSession session) { + session.getContext().setRealm(session.realms().getRealmByName("authz-test")); + AuthorizationProvider authorization = session.getProvider(AuthorizationProvider.class); + ClientModel clientModel = session.realms().getClientByClientId("resource-server-test", session.getContext().getRealm()); + StoreFactory storeFactory = authorization.getStoreFactory(); + ResourceServer resourceServer = storeFactory.getResourceServerStore().findById(clientModel.getId()); + + Scope readScope = storeFactory.getScopeStore().create("read", resourceServer); + Scope writeScope = storeFactory.getScopeStore().create("write", resourceServer); + + JSPolicyRepresentation policy = new JSPolicyRepresentation(); + + policy.setName(KeycloakModelUtils.generateId()); + policy.setCode("$evaluation.grant()"); + policy.setLogic(Logic.NEGATIVE); + + storeFactory.getPolicyStore().create(policy, resourceServer); + + ScopePermissionRepresentation readPermission = new ScopePermissionRepresentation(); + + readPermission.setName(KeycloakModelUtils.generateId()); + readPermission.addScope(readScope.getId()); + readPermission.addPolicy(policy.getName()); + + storeFactory.getPolicyStore().create(readPermission, resourceServer); + + ScopePermissionRepresentation writePermission = new ScopePermissionRepresentation(); + + writePermission.setName(KeycloakModelUtils.generateId()); + writePermission.addScope(writeScope.getId()); + writePermission.addPolicy(policy.getName()); + + storeFactory.getPolicyStore().create(writePermission, resourceServer); + + Resource resource = storeFactory.getResourceStore().create(KeycloakModelUtils.generateId(), resourceServer, resourceServer.getId()); + + PermissionEvaluator evaluator = authorization.evaluators().from(Arrays.asList(new ResourcePermission(resource, Arrays.asList(readScope, writeScope), resourceServer)), createEvaluationContext(session, Collections.emptyMap())); + Collection permissions = evaluator.evaluate(resourceServer, null); + + Assert.assertEquals(0, permissions.size()); + } + private static DefaultEvaluation createEvaluation(KeycloakSession session, AuthorizationProvider authorization, ResourceServer resourceServer, Policy policy) { return createEvaluation(session, authorization, null, resourceServer, policy); }