keycloak-memoizeit

[KEYCLOAK-8273] - Failed to evaluate permissions when in permissive

9/11/2018 8:08:39 PM

Details

diff --git a/server-spi-private/src/main/java/org/keycloak/authorization/policy/evaluation/DecisionPermissionCollector.java b/server-spi-private/src/main/java/org/keycloak/authorization/policy/evaluation/DecisionPermissionCollector.java
index 3c18191..6265026 100644
--- a/server-spi-private/src/main/java/org/keycloak/authorization/policy/evaluation/DecisionPermissionCollector.java
+++ b/server-spi-private/src/main/java/org/keycloak/authorization/policy/evaluation/DecisionPermissionCollector.java
@@ -160,12 +160,22 @@ public class DecisionPermissionCollector extends AbstractDecisionCollector {
             metadata = request.getMetadata();
         }
 
+        Permission permission;
+
         if (resource != null) {
             String resourceName = metadata == null || metadata.getIncludeResourceName() ? resource.getName() : null;
-            return new Permission(resource.getId(), resourceName, scopes, claims);
+            permission = new Permission(resource.getId(), resourceName, scopes, claims);
+        } else {
+            permission = new Permission(null, null, scopes, claims);
         }
 
-        return new Permission(null, null, scopes, claims);
+        onGrant(permission);
+
+        return permission;
+    }
+
+    protected void onGrant(Permission permission) {
+
     }
 
     private static boolean isResourcePermission(Policy policy) {
diff --git a/server-spi-private/src/main/java/org/keycloak/authorization/policy/evaluation/PermissionTicketAwareDecisionResultCollector.java b/server-spi-private/src/main/java/org/keycloak/authorization/policy/evaluation/PermissionTicketAwareDecisionResultCollector.java
index e6a5ef9..274cca4 100644
--- a/server-spi-private/src/main/java/org/keycloak/authorization/policy/evaluation/PermissionTicketAwareDecisionResultCollector.java
+++ b/server-spi-private/src/main/java/org/keycloak/authorization/policy/evaluation/PermissionTicketAwareDecisionResultCollector.java
@@ -38,6 +38,7 @@ import org.keycloak.authorization.store.StoreFactory;
 import org.keycloak.representations.idm.authorization.AuthorizationRequest;
 import org.keycloak.representations.idm.authorization.Permission;
 import org.keycloak.representations.idm.authorization.PermissionTicketToken;
+import org.keycloak.representations.idm.authorization.PolicyEnforcementMode;
 
 /**
  * @author <a href="mailto:psilva@redhat.com">Pedro Igor</a>
@@ -60,45 +61,27 @@ public class PermissionTicketAwareDecisionResultCollector extends DecisionPermis
     }
 
     @Override
-    public void onDecision(DefaultEvaluation evaluation) {
-        super.onDecision(evaluation);
-        removePermissionsIfGranted(evaluation);
-    }
-
-    /**
-     * Removes permissions (represented by {@code ticket}) granted by any user-managed policy so we don't create unnecessary permission tickets.
-     *
-     * @param evaluation the evaluation
-     */
-    private void removePermissionsIfGranted(DefaultEvaluation evaluation) {
-        if (Effect.PERMIT.equals(evaluation.getEffect())) {
-            Policy policy = evaluation.getParentPolicy();
-
-            if ("uma".equals(policy.getType())) {
-                ResourcePermission grantedPermission = evaluation.getPermission();
-                List<Permission> permissions = ticket.getPermissions();
-
-                Iterator<Permission> itPermissions = permissions.iterator();
-
-                while (itPermissions.hasNext()) {
-                    Permission permission = itPermissions.next();
+    protected void onGrant(Permission grantedPermission) {
+        // Removes permissions (represented by {@code ticket}) granted by any user-managed policy so we don't create unnecessary permission tickets.
+        List<Permission> permissions = ticket.getPermissions();
+        Iterator<Permission> itPermissions = permissions.iterator();
 
-                    if (permission.getResourceId().equals(grantedPermission.getResource().getId())) {
-                        Set<String> scopes = permission.getScopes();
-                        Iterator<String> itScopes = scopes.iterator();
+        while (itPermissions.hasNext()) {
+            Permission permission = itPermissions.next();
 
-                        while (itScopes.hasNext()) {
-                            Scope scope = authorization.getStoreFactory().getScopeStore().findByName(itScopes.next(), resourceServer.getId());
-                            if (policy.getScopes().contains(scope)) {
-                                itScopes.remove();
-                            }
-                        }
+            if (permission.getResourceId() == null || permission.getResourceId().equals(grantedPermission.getResourceId())) {
+                Set<String> scopes = permission.getScopes();
+                Iterator<String> itScopes = scopes.iterator();
 
-                        if (scopes.isEmpty()) {
-                            itPermissions.remove();
-                        }
+                while (itScopes.hasNext()) {
+                    if (grantedPermission.getScopes().contains(itScopes.next())) {
+                        itScopes.remove();
                     }
                 }
+
+                if (scopes.isEmpty()) {
+                    itPermissions.remove();
+                }
             }
         }
     }
diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/UserManagedAccessTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/UserManagedAccessTest.java
index 8b0ac60..2d7a69a 100644
--- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/UserManagedAccessTest.java
+++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/UserManagedAccessTest.java
@@ -39,8 +39,10 @@ import org.keycloak.representations.idm.authorization.AuthorizationResponse;
 import org.keycloak.representations.idm.authorization.JSPolicyRepresentation;
 import org.keycloak.representations.idm.authorization.Permission;
 import org.keycloak.representations.idm.authorization.PermissionTicketRepresentation;
+import org.keycloak.representations.idm.authorization.PolicyEnforcementMode;
 import org.keycloak.representations.idm.authorization.ResourcePermissionRepresentation;
 import org.keycloak.representations.idm.authorization.ResourceRepresentation;
+import org.keycloak.representations.idm.authorization.ResourceServerRepresentation;
 
 /**
  * @author <a href="mailto:psilva@redhat.com">Pedro Igor</a>
@@ -440,4 +442,40 @@ public class UserManagedAccessTest extends AbstractResourceServerTest {
 
         assertEquals(1, permissionTickets.size());
     }
+
+    @Test
+    public void testPermissiveModePermissions() throws Exception {
+        resource = addResource("Resource A");
+
+        try {
+            authorize("kolo", "password", resource.getId(), null);
+            fail("Access should be denied, server in enforcing mode");
+        } catch (AuthorizationDeniedException ade) {
+
+        }
+
+        AuthorizationResource authorizationResource = getClient(getRealm()).authorization();
+        ResourceServerRepresentation settings = authorizationResource.getSettings();
+
+        settings.setPolicyEnforcementMode(PolicyEnforcementMode.PERMISSIVE);
+
+        authorizationResource.update(settings);
+
+        AuthorizationResponse response = authorize("marta", "password", "Resource A", null);
+        String rpt = response.getToken();
+
+        assertNotNull(rpt);
+        assertFalse(response.isUpgraded());
+
+        AccessToken accessToken = toAccessToken(rpt);
+        AccessToken.Authorization authorization = accessToken.getAuthorization();
+
+        assertNotNull(authorization);
+
+        Collection<Permission> permissions = authorization.getPermissions();
+
+        assertNotNull(permissions);
+        assertPermissions(permissions, "Resource A");
+        assertTrue(permissions.isEmpty());
+    }
 }