From 7ebcc69cb9923fc08cc1c6c9efdbeca821eee4ba Mon Sep 17 00:00:00 2001 From: pedroigor Date: Wed, 25 Apr 2018 18:16:37 -0300 Subject: [PATCH] [KEYCLOAK-7148] - Associate sub resources to a parent resource --- .../authorization/PolicyEnforcer.java | 10 +++ .../org/keycloak/common/util/PathMatcher.java | 20 +++++- .../servlet-policy-enforcer-authz-realm.json | 16 +++++ .../src/main/webapp/WEB-INF/keycloak.json | 4 ++ .../AbstractServletPolicyEnforcerTest.java | 61 +++++++++++++++---- ...ResourceManagementWithAuthzClientTest.java | 18 ++++-- 6 files changed, 111 insertions(+), 18 deletions(-) diff --git a/adapters/oidc/adapter-core/src/main/java/org/keycloak/adapters/authorization/PolicyEnforcer.java b/adapters/oidc/adapter-core/src/main/java/org/keycloak/adapters/authorization/PolicyEnforcer.java index 2ccdf28a91..d5e46ad7f8 100644 --- a/adapters/oidc/adapter-core/src/main/java/org/keycloak/adapters/authorization/PolicyEnforcer.java +++ b/adapters/oidc/adapter-core/src/main/java/org/keycloak/adapters/authorization/PolicyEnforcer.java @@ -281,8 +281,18 @@ public class PolicyEnforcer { protected PathConfig resolvePathConfig(PathConfig originalConfig, String path) { if (originalConfig.hasPattern()) { ProtectedResource resource = authzClient.protection().resource(); + + // search by an exact match List search = resource.findByUri(path); + // if exact match not found, try to obtain from current path the parent path. + // if path is /resource/1/test and pattern from pathConfig is /resource/{id}/*, parent path is /resource/1 + // this logic allows to match sub resources of a resource instance (/resource/1) to the parent resource, + // so any permission granted to parent also applies to sub resources + if (search.isEmpty()) { + search = resource.findByUri(buildUriFromTemplate(originalConfig.getPath(), path, true)); + } + if (!search.isEmpty()) { ResourceRepresentation targetResource = search.get(0); PathConfig config = PathConfig.createPathConfig(targetResource); diff --git a/common/src/main/java/org/keycloak/common/util/PathMatcher.java b/common/src/main/java/org/keycloak/common/util/PathMatcher.java index d56c408cb9..6e0423f835 100644 --- a/common/src/main/java/org/keycloak/common/util/PathMatcher.java +++ b/common/src/main/java/org/keycloak/common/util/PathMatcher.java @@ -46,7 +46,7 @@ public abstract class PathMatcher

{ } if (isTemplate(expectedUri)) { - String templateUri = buildUriFromTemplate(expectedUri, targetUri); + String templateUri = buildUriFromTemplate(expectedUri, targetUri, false); if (templateUri != null) { int length = expectedUri.split("\\/").length; @@ -144,9 +144,14 @@ public abstract class PathMatcher

{ return false; } - public String buildUriFromTemplate(String expectedUri, String targetUri) { + protected String buildUriFromTemplate(String template, String targetUri, boolean onlyFirstParam) { + String expectedUri = template; int patternStartIndex = expectedUri.indexOf("{"); + if (expectedUri.endsWith("/*")) { + expectedUri = expectedUri.substring(0, expectedUri.length() - 2); + } + if (patternStartIndex == -1 || patternStartIndex >= targetUri.length()) { return null; } @@ -195,6 +200,10 @@ public abstract class PathMatcher

{ } i = expectedUri.indexOf('}', i); + + if (i == expectedUri.lastIndexOf('}') && onlyFirstParam) { + return String.valueOf(matchingUri).substring(0, matchingUriLastIndex); + } } else { if (c == '/') { paramIndex++; @@ -204,6 +213,13 @@ public abstract class PathMatcher

{ } if (matchingUri[matchingUri.length - 1] == '\u0000') { + if (template.endsWith("*")) { + StringBuilder firstParam = new StringBuilder(String.valueOf(matchingUri).substring(0, matchingUriLastIndex)); + + firstParam.append(targetUri.substring(firstParam.length())); + + return firstParam.toString(); + } return null; } diff --git a/testsuite/integration-arquillian/test-apps/servlet-policy-enforcer/servlet-policy-enforcer-authz-realm.json b/testsuite/integration-arquillian/test-apps/servlet-policy-enforcer/servlet-policy-enforcer-authz-realm.json index bad1b2595f..9e984b5631 100644 --- a/testsuite/integration-arquillian/test-apps/servlet-policy-enforcer/servlet-policy-enforcer-authz-realm.json +++ b/testsuite/integration-arquillian/test-apps/servlet-policy-enforcer/servlet-policy-enforcer-authz-realm.json @@ -115,6 +115,11 @@ { "name": "Pattern 14", "uri": "/keycloak-6623/sub-resource/*" + }, + { + "name": "Pattern 15", + "type": "pattern-15", + "uri": "/keycloak-7148/{id}" } ], "policies": [ @@ -286,6 +291,17 @@ "resources": "[\"Pattern 14\"]", "applyPolicies": "[\"Default Policy\"]" } + }, + { + "name": "Pattern 15 Permission", + "type": "resource", + "logic": "POSITIVE", + "decisionStrategy": "UNANIMOUS", + "config": { + "defaultResourceType": "pattern-15", + "default": "true", + "applyPolicies": "[\"Default Policy\"]" + } } ], "scopes": [] diff --git a/testsuite/integration-arquillian/test-apps/servlet-policy-enforcer/src/main/webapp/WEB-INF/keycloak.json b/testsuite/integration-arquillian/test-apps/servlet-policy-enforcer/src/main/webapp/WEB-INF/keycloak.json index d6e7f000de..43735e0453 100644 --- a/testsuite/integration-arquillian/test-apps/servlet-policy-enforcer/src/main/webapp/WEB-INF/keycloak.json +++ b/testsuite/integration-arquillian/test-apps/servlet-policy-enforcer/src/main/webapp/WEB-INF/keycloak.json @@ -69,6 +69,10 @@ { "name": "Pattern 13", "path": "/keycloak-6623/*" + }, + { + "name": "Pattern 15", + "path": "/keycloak-7148/{id}/*" } ] } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/example/authorization/AbstractServletPolicyEnforcerTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/example/authorization/AbstractServletPolicyEnforcerTest.java index 9afed016cc..648f36caf6 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/example/authorization/AbstractServletPolicyEnforcerTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/example/authorization/AbstractServletPolicyEnforcerTest.java @@ -73,7 +73,7 @@ public abstract class AbstractServletPolicyEnforcerTest extends AbstractExampleA } @Test - public void testPattern1() throws Exception { + public void testPattern1() { performTests(() -> { login("alice", "alice"); @@ -93,7 +93,7 @@ public abstract class AbstractServletPolicyEnforcerTest extends AbstractExampleA } @Test - public void testPattern2() throws Exception { + public void testPattern2() { performTests(() -> { login("alice", "alice"); @@ -117,7 +117,7 @@ public abstract class AbstractServletPolicyEnforcerTest extends AbstractExampleA } @Test - public void testPattern3() throws Exception { + public void testPattern3() { performTests(() -> { login("alice", "alice"); @@ -153,7 +153,7 @@ public abstract class AbstractServletPolicyEnforcerTest extends AbstractExampleA } @Test - public void testPattern4() throws Exception { + public void testPattern4() { performTests(() -> { login("alice", "alice"); @@ -173,7 +173,7 @@ public abstract class AbstractServletPolicyEnforcerTest extends AbstractExampleA } @Test - public void testPattern5() throws Exception { + public void testPattern5() { performTests(() -> { login("alice", "alice"); @@ -197,7 +197,7 @@ public abstract class AbstractServletPolicyEnforcerTest extends AbstractExampleA } @Test - public void testPattern6() throws Exception { + public void testPattern6() { performTests(() -> { login("alice", "alice"); @@ -221,7 +221,7 @@ public abstract class AbstractServletPolicyEnforcerTest extends AbstractExampleA } @Test - public void testPattern7() throws Exception { + public void testPattern7() { performTests(() -> { login("alice", "alice"); @@ -245,7 +245,7 @@ public abstract class AbstractServletPolicyEnforcerTest extends AbstractExampleA } @Test - public void testPattern8() throws Exception { + public void testPattern8() { performTests(() -> { login("alice", "alice"); @@ -265,7 +265,7 @@ public abstract class AbstractServletPolicyEnforcerTest extends AbstractExampleA } @Test - public void testPattern9() throws Exception { + public void testPattern9() { performTests(() -> { login("alice", "alice"); @@ -285,7 +285,7 @@ public abstract class AbstractServletPolicyEnforcerTest extends AbstractExampleA } @Test - public void testPattern10() throws Exception { + public void testPattern10() { performTests(() -> { login("alice", "alice"); @@ -309,7 +309,7 @@ public abstract class AbstractServletPolicyEnforcerTest extends AbstractExampleA } @Test - public void testPattern11UsingResourceInstancePermission() throws Exception { + public void testPattern11UsingResourceInstancePermission() { performTests(() -> { login("alice", "alice"); navigateTo("/api/v1/resource-a"); @@ -406,6 +406,45 @@ public abstract class AbstractServletPolicyEnforcerTest extends AbstractExampleA }); } + @Test + public void testPathWithPatternSlashAllAndResourceInstance() { + performTests(() -> { + ResourceRepresentation resource = new ResourceRepresentation("Pattern 15 Instance"); + + resource.setType("pattern-15"); + resource.setUri("/keycloak-7148/1"); + resource.setOwner("alice"); + + getAuthorizationResource().resources().create(resource).close(); + + login("alice", "alice"); + navigateTo("/keycloak-7148/1"); + assertFalse(wasDenied()); + navigateTo("/keycloak-7148/1/sub-a/2"); + assertFalse(wasDenied()); + navigateTo("/keycloak-7148/1/sub-a"); + assertFalse(wasDenied()); + navigateTo("/keycloak-7148/1/sub-a/2/sub-b"); + assertFalse(wasDenied()); + + updatePermissionPolicies("Pattern 15 Permission", "Deny Policy"); + + login("alice", "alice"); + navigateTo("/keycloak-7148/1"); + assertTrue(wasDenied()); + navigateTo("/keycloak-7148/1/sub-a/2"); + assertTrue(wasDenied()); + navigateTo("/keycloak-7148/1/sub-a"); + assertTrue(wasDenied()); + navigateTo("/keycloak-7148/1/sub-a/2/sub-b"); + assertTrue(wasDenied()); + + // does not exist + navigateTo("/keycloak-7148/2"); + assertTrue(wasDenied()); + }); + } + private void navigateTo(String path) { this.driver.navigate().to(getResourceServerUrl() + path); } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/authorization/ResourceManagementWithAuthzClientTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/authorization/ResourceManagementWithAuthzClientTest.java index bbff687b9b..5800071844 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/authorization/ResourceManagementWithAuthzClientTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/authorization/ResourceManagementWithAuthzClientTest.java @@ -44,7 +44,9 @@ public class ResourceManagementWithAuthzClientTest extends ResourceManagementTes public void testFindMatchingUri() { doCreateResource(new ResourceRepresentation("/*", Collections.emptySet(), "/*", null)); doCreateResource(new ResourceRepresentation("/resources/*", Collections.emptySet(), "/resources/*", null)); - doCreateResource(new ResourceRepresentation("/resources/{pattern}/*", Collections.emptySet(), "/resources/{pattern}/*", null)); + doCreateResource(new ResourceRepresentation("/resources-a/*", Collections.emptySet(), "/resources-a/*", null)); + doCreateResource(new ResourceRepresentation("/resources-b/{pattern}", Collections.emptySet(), "/resources-b/{pattern}", null)); + doCreateResource(new ResourceRepresentation("/resources-c/{pattern}/*", Collections.emptySet(), "/resources-c/{pattern}/*", null)); doCreateResource(new ResourceRepresentation("/resources/{pattern}/{pattern}/*", Collections.emptySet(), "/resources/{pattern}/{pattern}/*", null)); doCreateResource(new ResourceRepresentation("/resources/{pattern}/sub-resources/{pattern}/*", Collections.emptySet(), "/resources/{pattern}/sub-resources/{pattern}/*", null)); doCreateResource(new ResourceRepresentation("/resources/{pattern}/sub-resource", Collections.emptySet(), "/resources/{pattern}/sub-resources/{pattern}/*", null)); @@ -57,11 +59,11 @@ public class ResourceManagementWithAuthzClientTest extends ResourceManagementTes assertEquals(1, resources.size()); assertEquals("/*", resources.get(0).getUri()); - resources = authzClient.protection().resource().findByMatchingUri("/resources/test"); + resources = authzClient.protection().resource().findByMatchingUri("/resources-a/test"); assertNotNull(resources); assertEquals(1, resources.size()); - assertEquals("/resources/*", resources.get(0).getUri()); + assertEquals("/resources-a/*", resources.get(0).getUri()); resources = authzClient.protection().resource().findByMatchingUri("/resources"); @@ -69,11 +71,17 @@ public class ResourceManagementWithAuthzClientTest extends ResourceManagementTes assertEquals(1, resources.size()); assertEquals("/resources/*", resources.get(0).getUri()); - resources = authzClient.protection().resource().findByMatchingUri("/resources/a/b"); + resources = authzClient.protection().resource().findByMatchingUri("/resources-b/a"); assertNotNull(resources); assertEquals(1, resources.size()); - assertEquals("/resources/{pattern}/*", resources.get(0).getUri()); + assertEquals("/resources-b/{pattern}", resources.get(0).getUri()); + + resources = authzClient.protection().resource().findByMatchingUri("/resources-c/a/b"); + + assertNotNull(resources); + assertEquals(1, resources.size()); + assertEquals("/resources-c/{pattern}/*", resources.get(0).getUri()); resources = authzClient.protection().resource().findByMatchingUri("/resources/a/b/c");