8000 Avoid removing static path config from cache by pedroigor · Pull Request #14230 · keycloak/keycloak · GitHub
[go: up one dir, main page]
Skip to content

Avoid removing static path config from cache #14230

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
stianst merged 1 commit into from
Sep 8, 2022
Merged

Avoid removing static path config from cache #14230

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions ...s/oidc/adapter-core/src/main/java/org/keycloak/adapters/authorization/PolicyEnforcer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,12 +191,16 @@ private Map<String, PathConfig> configureDefinedPaths(ProtectedResource protecte

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()) {
Expand Down Expand Up @@ -282,11 +286,13 @@ public PathConfig matches(String targetUri) {
PolicyEnforcerConfig.EnforcementMode enforcementMode = PolicyEnforcerConfig.EnforcementMode.ENFORCING;
ResourceRepresentation targetResource = matchingResources.get(0);
List<PolicyEnforcerConfig.MethodConfig> 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())
Expand All @@ -306,7 +312,8 @@ public PathConfig matches(String targetUri) {
if (methodConfig != null) {
pathConfig.setMethods(methodConfig);
}


pathConfig.setStatic(isStatic);
pathConfig.setEnforcementMode(enforcementMode);
}
} catch (Exception cause) {
Expand Down
73 changes: 71 additions & 2 deletions ...e/src/test/java/org/keycloak/testsuite/admin/client/authorization/PolicyEnforcerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
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;
Expand Down Expand Up @@ -85,13 +84,13 @@
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 <a href="mailto:psilva@redhat.com">Pedro Igor</a>
Expand Down Expand Up @@ -340,6 +339,76 @@ public void testMappedPathEnforcementModeDisabled() {
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"));
Expand Down
31 changes: 31 additions & 0 deletions ...lian/tests/base/src/test/resources/authorization-test/enforcer-disabled-path-nocache.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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"
}
]
}
}
0