From 15bbb46657422d30f13becc6b81f3dc1b09cb644 Mon Sep 17 00:00:00 2001 From: Pedro Igor Date: Thu, 11 Aug 2022 16:36:30 -0300 Subject: [PATCH] Avoid removing static path config from cache Closes #9855 --- .../authorization/PolicyEnforcer.java | 13 +++- .../authorization/PolicyEnforcerTest.java | 73 ++++++++++++++++++- .../enforcer-disabled-path-nocache.json | 31 ++++++++ 3 files changed, 112 insertions(+), 5 deletions(-) create mode 100644 testsuite/integration-arquillian/tests/base/src/test/resources/authorization-test/enforcer-disabled-path-nocache.json 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 6c087f5981..2fd9916f5f 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 @@ -191,12 +191,16 @@ public class PolicyEnforcer { if (resource != null) { pathConfig.setId(resource.getId()); - // if the resource is staticly bound to a resource it means the config can not be invalidated + // if the resource is statically bound to a resource it means the config can not be invalidated if (resourceName != null) { pathConfig.setStatic(true); } } - + + if (PolicyEnforcerConfig.EnforcementMode.DISABLED.equals(pathConfig.getEnforcementMode())) { + pathConfig.setStatic(true); + } + PathConfig existingPath = null; for (PathConfig current : paths.values()) { @@ -282,11 +286,13 @@ public class PolicyEnforcer { PolicyEnforcerConfig.EnforcementMode enforcementMode = PolicyEnforcerConfig.EnforcementMode.ENFORCING; ResourceRepresentation targetResource = matchingResources.get(0); List methodConfig = null; + boolean isStatic = false; if (pathConfig != null) { cipConfig = pathConfig.getClaimInformationPointConfig(); enforcementMode = pathConfig.getEnforcementMode(); methodConfig = pathConfig.getMethods(); + isStatic = pathConfig.isStatic(); } else { for (PathConfig existingPath : paths.values()) { if (targetResource.getId().equals(existingPath.getId()) @@ -306,7 +312,8 @@ public class PolicyEnforcer { if (methodConfig != null) { pathConfig.setMethods(methodConfig); } - + + pathConfig.setStatic(isStatic); pathConfig.setEnforcementMode(enforcementMode); } } catch (Exception cause) { diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/authorization/PolicyEnforcerTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/authorization/PolicyEnforcerTest.java index c010a6532c..8e22bf76fa 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/authorization/PolicyEnforcerTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/authorization/PolicyEnforcerTest.java @@ -20,7 +20,6 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; import static org.keycloak.common.Profile.Feature.AUTHORIZATION; import javax.security.cert.X509Certificate; @@ -85,13 +84,13 @@ import org.keycloak.testsuite.AbstractKeycloakTest; import org.keycloak.testsuite.ProfileAssume; import org.keycloak.testsuite.arquillian.annotation.AuthServerContainerExclude; import org.keycloak.testsuite.arquillian.annotation.AuthServerContainerExclude.AuthServer; -import org.keycloak.testsuite.arquillian.annotation.EnableFeature; import org.keycloak.testsuite.util.ClientBuilder; import org.keycloak.testsuite.util.OAuthClient; import org.keycloak.testsuite.util.RealmBuilder; import org.keycloak.testsuite.util.RoleBuilder; import org.keycloak.testsuite.util.RolesBuilder; import org.keycloak.testsuite.util.UserBuilder; +import org.keycloak.testsuite.util.WaitUtils; /** * @author Pedro Igor @@ -340,6 +339,76 @@ public class PolicyEnforcerTest extends AbstractKeycloakTest { assertTrue(context.isGranted()); } + @Test + public void testDisabledPathNoCache() { + KeycloakDeployment deployment = KeycloakDeploymentBuilder.build(getAdapterConfiguration("enforcer-disabled-path-nocache.json")); + PolicyEnforcer policyEnforcer = deployment.getPolicyEnforcer(); + + OIDCHttpFacade httpFacade = createHttpFacade("/api/resource/public"); + AuthorizationContext context = policyEnforcer.enforce(httpFacade); + assertTrue(context.isGranted()); + + ClientResource clientResource = getClientResource(RESOURCE_SERVER_CLIENT_ID); + ResourceRepresentation resource = clientResource.authorization().resources() + .findByName("Root").get(0); + + clientResource.authorization().resources().resource(resource.getId()).remove(); + + // first request caches the path and the entry is invalidated due to the lifespan + httpFacade = createHttpFacade("/api/resource/all-public"); + context = policyEnforcer.enforce(httpFacade); + assertTrue(context.isGranted()); + + WaitUtils.pause(1000); + + // second request can not fail because entry should not be invalidated + httpFacade = createHttpFacade("/api/resource/all-public"); + context = policyEnforcer.enforce(httpFacade); + assertTrue(context.isGranted()); + } + + @Test + public void testLazyLoadedPathIsCached() { + ClientResource clientResource = getClientResource(RESOURCE_SERVER_CLIENT_ID); + createResource(clientResource, "Static Test Resource", "/api/any-resource/*"); + + ResourcePermissionRepresentation permission = new ResourcePermissionRepresentation(); + + permission.setName("Any Resource Permission"); + permission.addResource("Static Test Resource"); + permission.addPolicy("Always Grant Policy"); + + clientResource.authorization().permissions().resource().create(permission); + + KeycloakDeployment deployment = KeycloakDeploymentBuilder.build(getAdapterConfiguration("enforcer-disabled-path-nocache.json")); + PolicyEnforcer policyEnforcer = deployment.getPolicyEnforcer(); + + oauth.realm(REALM_NAME); + oauth.clientId("public-client-test"); + oauth.doLogin("marta", "password"); + + String code = oauth.getCurrentQuery().get(OAuth2Constants.CODE); + OAuthClient.AccessTokenResponse response = oauth.doAccessTokenRequest(code, null); + String token = response.getAccessToken(); + + OIDCHttpFacade httpFacade = createHttpFacade("/api/any-resource/test", token); + AuthorizationContext context = policyEnforcer.enforce(httpFacade); + assertTrue(context.isGranted()); + + httpFacade = createHttpFacade("/api/any-resource/test", token); + context = policyEnforcer.enforce(httpFacade); + assertTrue(context.isGranted()); + + ResourceRepresentation resource = clientResource.authorization().resources() + .findByName("Static Test Resource").get(0); + + clientResource.authorization().resources().resource(resource.getId()).remove(); + + httpFacade = createHttpFacade("/api/any-resource/test", token); + context = policyEnforcer.enforce(httpFacade); + assertFalse(context.isGranted()); + } + @Test public void testEnforcementModeDisabled() { KeycloakDeployment deployment = KeycloakDeploymentBuilder.build(getAdapterConfiguration("enforcer-disabled-enforce-mode.json")); diff --git a/testsuite/integration-arquillian/tests/base/src/test/resources/authorization-test/enforcer-disabled-path-nocache.json b/testsuite/integration-arquillian/tests/base/src/test/resources/authorization-test/enforcer-disabled-path-nocache.json new file mode 100644 index 0000000000..c20e9fae08 --- /dev/null +++ b/testsuite/integration-arquillian/tests/base/src/test/resources/authorization-test/enforcer-disabled-path-nocache.json @@ -0,0 +1,31 @@ +{ + "realm": "authz-test", + "auth-server-url": "http://localhost:8180/auth", + "ssl-required": "external", + "resource": "resource-server-test", + "credentials": { + "secret": "secret" + }, + "bearer-only": true, + "policy-enforcer": { + "path-cache": { + "lifespan": 1 + }, + "paths": [ + { + "name": "Resource B", + "path": "/api/resource/public", + "enforcement-mode": "DISABLED" + }, + { + "name": "Nonexistent", + "path": "/api/resource/all-public/*", + "enforcement-mode": "DISABLED" + }, + { + "name": "Static Test Resource", + "path": "/api/any-resource/test" + } + ] + } +}