[KEYCLOAK-8172] - Evaluation not considering scopes inherited from pa…

…rent resources
keycloak · Oct 24, 2018 · 2af9d00 · stickycode · Jan 29, 2019 · pedroigor
1 parent 7e12b60
commit 2af9d00
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 13 deletions.
diff --git a/...c/main/java/org/keycloak/authorization/policy/evaluation/DecisionPermissionCollector.java b/...c/main/java/org/keycloak/authorization/policy/evaluation/DecisionPermissionCollector.java
@@ -76,6 +76,11 @@ public void onComplete(Result result) {
                         for (Scope scope : requestedScopes) {
                             if (policyScopes.contains(scope)) {
                                 grantedScopes.add(scope);
+                                // we need to grant any scope granted by a permission in case it is not explicitly
+                                // associated with the resource. For instance, resources inheriting scopes from parent resources.
+                                if (!resource.getScopes().contains(scope)) {
+                                    deniedScopes.remove(scope);
+                                }
                             }
                         }
                     } else if (isResourcePermission(policy)) {

diff --git a/services/src/main/java/org/keycloak/authorization/util/Permissions.java b/services/src/main/java/org/keycloak/authorization/util/Permissions.java
@@ -118,7 +118,7 @@ public static ResourcePermission createResourcePermissions(Resource resource, Co
         if (requestedScopes.isEmpty()) {
             scopes = populateTypedScopes(resource, authorization);
         } else {
-            scopes = requestedScopes.stream().filter(scope -> resource.getScopes().contains(scope)).collect(Collectors.toList());
+            scopes = populateTypedScopes(resource, requestedScopes.stream().filter(scope -> resource.getScopes().contains(scope)).collect(Collectors.toList()), authorization);
         }
 
         return new ResourcePermission(resource, scopes, resource.getResourceServer(), request.getClaims());
@@ -135,25 +135,32 @@ public static ResourcePermission createResourcePermissions(Resource resource, Au
     }
 
     private static List<Scope> populateTypedScopes(Resource resource, AuthorizationProvider authorization) {
-        List<Scope> scopes = new LinkedList<>(resource.getScopes());
+        return populateTypedScopes(resource, resource.getScopes(), authorization);
+    }
+
+    private static List<Scope> populateTypedScopes(Resource resource, List<Scope> defaultScopes, AuthorizationProvider authorization) {
         String type = resource.getType();
         ResourceServer resourceServer = resource.getResourceServer();
 
+        if (type == null || resource.getOwner().equals(resourceServer.getId())) {
+            return new ArrayList<>(defaultScopes);
+        }
+
+        List<Scope> scopes = new ArrayList<>(defaultScopes);
+
         // check if there is a typed resource whose scopes are inherited by the resource being requested. In this case, we assume that parent resource
         // is owned by the resource server itself
-        if (type != null && !resource.getOwner().equals(resourceServer.getId())) {
-            StoreFactory storeFactory = authorization.getStoreFactory();
-            ResourceStore resourceStore = storeFactory.getResourceStore();
-            resourceStore.findByType(type, resourceServer.getId(), resource1 -> {
-                if (resource1.getOwner().equals(resourceServer.getId())) {
-                    for (Scope typeScope : resource1.getScopes()) {
-                        if (!scopes.contains(typeScope)) {
-                            scopes.add(typeScope);
-                        }
+        StoreFactory storeFactory = authorization.getStoreFactory();
+        ResourceStore resourceStore = storeFactory.getResourceStore();
+        resourceStore.findByType(type, resourceServer.getId(), resource1 -> {
+            if (resource1.getOwner().equals(resourceServer.getId())) {
+                for (Scope typeScope : resource1.getScopes()) {
+                    if (!scopes.contains(typeScope)) {
+                        scopes.add(typeScope);
                     }
                 }
-            });
-        }
+            }
+        });
 
         return scopes;
     }