diff --git a/adapters/oidc/adapter-core/src/main/java/org/keycloak/adapters/authorization/AbstractPolicyEnforcer.java b/adapters/oidc/adapter-core/src/main/java/org/keycloak/adapters/authorization/AbstractPolicyEnforcer.java index 0186e187c7..1673aa6034 100644 --- a/adapters/oidc/adapter-core/src/main/java/org/keycloak/adapters/authorization/AbstractPolicyEnforcer.java +++ b/adapters/oidc/adapter-core/src/main/java/org/keycloak/adapters/authorization/AbstractPolicyEnforcer.java @@ -17,6 +17,13 @@ */ package org.keycloak.adapters.authorization; +import java.net.URI; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; + import org.jboss.logging.Logger; import org.keycloak.AuthorizationContext; import org.keycloak.KeycloakSecurityContext; @@ -32,13 +39,6 @@ import org.keycloak.representations.adapters.config.PolicyEnforcerConfig.Enforce import org.keycloak.representations.adapters.config.PolicyEnforcerConfig.PathConfig; import org.keycloak.representations.idm.authorization.Permission; -import java.net.URI; -import java.util.ArrayList; -import java.util.Collections; -import java.util.HashSet; -import java.util.List; -import java.util.Set; - /** * @author Pedro Igor */ @@ -48,7 +48,7 @@ public abstract class AbstractPolicyEnforcer { private final PolicyEnforcerConfig enforcerConfig; private final PolicyEnforcer policyEnforcer; - private List paths; + private Map paths; private AuthzClient authzClient; private PathMatcher pathMatcher; @@ -57,7 +57,7 @@ public abstract class AbstractPolicyEnforcer { this.enforcerConfig = policyEnforcer.getEnforcerConfig(); this.authzClient = policyEnforcer.getClient(); this.pathMatcher = new PathMatcher(); - this.paths = new ArrayList<>(policyEnforcer.getPaths()); + this.paths = policyEnforcer.getPaths(); } public AuthorizationContext authorize(OIDCHttpFacade httpFacade) { @@ -75,8 +75,7 @@ public abstract class AbstractPolicyEnforcer { if (accessToken != null) { Request request = httpFacade.getRequest(); Response response = httpFacade.getResponse(); - String pathInfo = URI.create(request.getURI()).getPath().substring(1); - String path = pathInfo.substring(pathInfo.indexOf('/'), pathInfo.length()); + String path = getPath(request); PathConfig pathConfig = this.pathMatcher.matches(path, this.paths); LOGGER.debugf("Checking permissions for path [%s] with config [%s].", request.getURI(), pathConfig); @@ -122,12 +121,9 @@ public abstract class AbstractPolicyEnforcer { protected boolean isAuthorized(PathConfig actualPathConfig, Set requiredScopes, AccessToken accessToken, OIDCHttpFacade httpFacade) { Request request = httpFacade.getRequest(); PolicyEnforcerConfig enforcerConfig = getEnforcerConfig(); - String accessDeniedPath = enforcerConfig.getOnDenyRedirectTo(); - if (accessDeniedPath != null) { - if (request.getURI().contains(accessDeniedPath)) { - return true; - } + if (isDefaultAccessDeniedUri(request, enforcerConfig)) { + return true; } AccessToken.Authorization authorization = accessToken.getAuthorization(); @@ -173,6 +169,17 @@ public abstract class AbstractPolicyEnforcer { return false; } + private boolean isDefaultAccessDeniedUri(Request request, PolicyEnforcerConfig enforcerConfig) { + String accessDeniedPath = enforcerConfig.getOnDenyRedirectTo(); + + if (accessDeniedPath != null) { + if (request.getURI().contains(accessDeniedPath)) { + return true; + } + } + return false; + } + private boolean hasResourceScopePermission(Set requiredScopes, Permission permission, PathConfig actualPathConfig) { Set allowedScopes = permission.getScopes(); return (allowedScopes.containsAll(requiredScopes) || allowedScopes.isEmpty()); @@ -220,27 +227,23 @@ public abstract class AbstractPolicyEnforcer { } private PathConfig resolvePathConfig(PathConfig originalConfig, Request request) { + String path = getPath(request); + if (originalConfig.hasPattern()) { - String pathInfo = URI.create(request.getURI()).getPath().substring(1); - String path = pathInfo.substring(pathInfo.indexOf('/'), pathInfo.length()); ProtectedResource resource = this.authzClient.protection().resource(); Set search = resource.findByFilter("uri=" + path); if (!search.isEmpty()) { // resource does exist on the server, cache it ResourceRepresentation targetResource = resource.findById(search.iterator().next()).getResourceDescription(); - PathConfig config = new PathConfig(); + PathConfig config = PolicyEnforcer.createPathConfig(targetResource); - config.setId(targetResource.getId()); - config.setName(targetResource.getName()); - config.setType(targetResource.getType()); - config.setPath(targetResource.getUri()); config.setScopes(originalConfig.getScopes()); config.setMethods(originalConfig.getMethods()); config.setParentConfig(originalConfig); config.setEnforcementMode(originalConfig.getEnforcementMode()); - this.paths.add(config); + this.policyEnforcer.addPath(config); return config; } @@ -249,6 +252,11 @@ public abstract class AbstractPolicyEnforcer { return originalConfig; } + private String getPath(Request request) { + String pathInfo = URI.create(request.getURI()).getPath().substring(1); + return pathInfo.substring(pathInfo.indexOf('/'), pathInfo.length()); + } + private Set getRequiredScopes(PathConfig pathConfig, Request request) { Set requiredScopes = new HashSet<>(); diff --git a/adapters/oidc/adapter-core/src/main/java/org/keycloak/adapters/authorization/KeycloakAdapterPolicyEnforcer.java b/adapters/oidc/adapter-core/src/main/java/org/keycloak/adapters/authorization/KeycloakAdapterPolicyEnforcer.java index a12fc84123..b6df2eafb3 100644 --- a/adapters/oidc/adapter-core/src/main/java/org/keycloak/adapters/authorization/KeycloakAdapterPolicyEnforcer.java +++ b/adapters/oidc/adapter-core/src/main/java/org/keycloak/adapters/authorization/KeycloakAdapterPolicyEnforcer.java @@ -17,6 +17,10 @@ */ package org.keycloak.adapters.authorization; +import java.util.ArrayList; +import java.util.HashSet; +import java.util.Set; + import org.jboss.logging.Logger; import org.keycloak.adapters.KeycloakDeployment; import org.keycloak.adapters.OIDCHttpFacade; @@ -34,10 +38,6 @@ import org.keycloak.representations.AccessToken; import org.keycloak.representations.adapters.config.PolicyEnforcerConfig.PathConfig; import org.keycloak.representations.idm.authorization.Permission; -import java.util.ArrayList; -import java.util.HashSet; -import java.util.Set; - /** * @author Pedro Igor */ @@ -51,39 +51,34 @@ public class KeycloakAdapterPolicyEnforcer extends AbstractPolicyEnforcer { @Override protected boolean isAuthorized(PathConfig pathConfig, Set requiredScopes, AccessToken accessToken, OIDCHttpFacade httpFacade) { - int retry = 2; AccessToken original = accessToken; - while (retry > 0) { - if (super.isAuthorized(pathConfig, requiredScopes, accessToken, httpFacade)) { - return true; - } - - accessToken = requestAuthorizationToken(pathConfig, requiredScopes, httpFacade); - - if (accessToken == null) { - return false; - } - - AccessToken.Authorization authorization = original.getAuthorization(); - - if (authorization == null) { - authorization = new AccessToken.Authorization(); - authorization.setPermissions(new ArrayList()); - } - - AccessToken.Authorization newAuthorization = accessToken.getAuthorization(); - - if (newAuthorization != null) { - authorization.getPermissions().addAll(newAuthorization.getPermissions()); - } - - original.setAuthorization(authorization); - - retry--; + if (super.isAuthorized(pathConfig, requiredScopes, accessToken, httpFacade)) { + return true; } - return false; + accessToken = requestAuthorizationToken(pathConfig, requiredScopes, httpFacade); + + if (accessToken == null) { + return false; + } + + AccessToken.Authorization authorization = original.getAuthorization(); + + if (authorization == null) { + authorization = new AccessToken.Authorization(); + authorization.setPermissions(new ArrayList()); + } + + AccessToken.Authorization newAuthorization = accessToken.getAuthorization(); + + if (newAuthorization != null) { + authorization.getPermissions().addAll(newAuthorization.getPermissions()); + } + + original.setAuthorization(authorization); + + return super.isAuthorized(pathConfig, requiredScopes, accessToken, httpFacade); } @Override @@ -108,7 +103,7 @@ public class KeycloakAdapterPolicyEnforcer extends AbstractPolicyEnforcer { KeycloakDeployment deployment = getPolicyEnforcer().getDeployment(); if (getEnforcerConfig().getUserManagedAccess() != null) { - LOGGER.debug("Obtaining authorization for authenticated user."); + LOGGER.debug("Obtaining authorization for authenticated user."); PermissionRequest permissionRequest = new PermissionRequest(); permissionRequest.setResourceSetId(pathConfig.getId()); @@ -136,12 +131,14 @@ public class KeycloakAdapterPolicyEnforcer extends AbstractPolicyEnforcer { permissionRequest.setResourceSetId(pathConfig.getId()); permissionRequest.setResourceSetName(pathConfig.getName()); permissionRequest.setScopes(new HashSet<>(pathConfig.getScopes())); + LOGGER.debugf("Sending entitlements request: resource_set_id [%s], resource_set_name [%s], scopes [%s].", permissionRequest.getResourceSetId(), permissionRequest.getResourceSetName(), permissionRequest.getScopes()); request.addPermission(permissionRequest); EntitlementResponse authzResponse = authzClient.entitlement(accessToken).get(authzClient.getConfiguration().getClientId(), request); return AdapterRSATokenVerifier.verifyToken(authzResponse.getRpt(), deployment); } } } catch (AuthorizationDeniedException e) { + LOGGER.debug("Authorization denied", e); return null; } catch (Exception e) { throw new RuntimeException("Unexpected error during authorization request.", e); diff --git a/adapters/oidc/adapter-core/src/main/java/org/keycloak/adapters/authorization/PathMatcher.java b/adapters/oidc/adapter-core/src/main/java/org/keycloak/adapters/authorization/PathMatcher.java index 5ac1b79acc..8865892b28 100644 --- a/adapters/oidc/adapter-core/src/main/java/org/keycloak/adapters/authorization/PathMatcher.java +++ b/adapters/oidc/adapter-core/src/main/java/org/keycloak/adapters/authorization/PathMatcher.java @@ -19,7 +19,7 @@ package org.keycloak.adapters.authorization; import org.keycloak.representations.adapters.config.PolicyEnforcerConfig.PathConfig; -import java.util.List; +import java.util.Map; /** * @author Pedro Igor @@ -28,10 +28,16 @@ class PathMatcher { private static final String ANY_RESOURCE_PATTERN = "/*"; - PathConfig matches(final String requestedUri, List paths) { + PathConfig matches(final String requestedUri, Map paths) { + PathConfig pathConfig = paths.get(requestedUri); + + if (pathConfig != null) { + return pathConfig; + } + PathConfig actualConfig = null; - for (PathConfig entry : paths) { + for (PathConfig entry : paths.values()) { String protectedUri = entry.getPath(); String selectedUri = null; 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 37b8f3dd30..f8a5d295e5 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 @@ -34,8 +34,10 @@ import org.keycloak.representations.idm.authorization.Permission; import java.util.ArrayList; import java.util.Collections; +import java.util.HashMap; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Set; /** @@ -48,7 +50,7 @@ public class PolicyEnforcer { private final KeycloakDeployment deployment; private final AuthzClient authzClient; private final PolicyEnforcerConfig enforcerConfig; - private final List paths; + private final Map paths; public PolicyEnforcer(KeycloakDeployment deployment, AdapterConfig adapterConfig) { this.deployment = deployment; @@ -58,7 +60,7 @@ public class PolicyEnforcer { if (LOGGER.isDebugEnabled()) { LOGGER.debug("Initialization complete. Path configurations:"); - for (PathConfig pathConfig : this.paths) { + for (PathConfig pathConfig : this.paths.values()) { LOGGER.debug(pathConfig); } } @@ -96,15 +98,19 @@ public class PolicyEnforcer { return authzClient; } - public List getPaths() { - return Collections.unmodifiableList(paths); + public Map getPaths() { + return paths; + } + + void addPath(PathConfig pathConfig) { + paths.put(pathConfig.getPath(), pathConfig); } KeycloakDeployment getDeployment() { return deployment; } - private List configurePaths(ProtectedResource protectedResource, PolicyEnforcerConfig enforcerConfig) { + private Map configurePaths(ProtectedResource protectedResource, PolicyEnforcerConfig enforcerConfig) { boolean loadPathsFromServer = true; for (PathConfig pathConfig : enforcerConfig.getPaths()) { @@ -123,8 +129,8 @@ public class PolicyEnforcer { } } - private List configureDefinedPaths(ProtectedResource protectedResource, PolicyEnforcerConfig enforcerConfig) { - List paths = new ArrayList<>(); + private Map configureDefinedPaths(ProtectedResource protectedResource, PolicyEnforcerConfig enforcerConfig) { + Map paths = Collections.synchronizedMap(new HashMap()); for (PathConfig pathConfig : enforcerConfig.getPaths()) { Set search; @@ -172,7 +178,7 @@ public class PolicyEnforcer { PathConfig existingPath = null; - for (PathConfig current : paths) { + for (PathConfig current : paths.values()) { if (current.getId().equals(pathConfig.getId()) && current.getPath().equals(pathConfig.getPath())) { existingPath = current; break; @@ -180,7 +186,7 @@ public class PolicyEnforcer { } if (existingPath == null) { - paths.add(pathConfig); + paths.put(pathConfig.getPath(), pathConfig); } else { existingPath.getMethods().addAll(pathConfig.getMethods()); existingPath.getScopes().addAll(pathConfig.getScopes()); @@ -190,23 +196,24 @@ public class PolicyEnforcer { return paths; } - private List configureAllPathsForResourceServer(ProtectedResource protectedResource) { + private Map configureAllPathsForResourceServer(ProtectedResource protectedResource) { LOGGER.info("Querying the server for all resources associated with this application."); - List paths = new ArrayList<>(); + Map paths = Collections.synchronizedMap(new HashMap()); for (String id : protectedResource.findAll()) { RegistrationResponse response = protectedResource.findById(id); ResourceRepresentation resourceDescription = response.getResourceDescription(); if (resourceDescription.getUri() != null) { - paths.add(createPathConfig(resourceDescription)); + PathConfig pathConfig = createPathConfig(resourceDescription); + paths.put(pathConfig.getPath(), pathConfig); } } return paths; } - private PathConfig createPathConfig(ResourceRepresentation resourceDescription) { + static PathConfig createPathConfig(ResourceRepresentation resourceDescription) { PathConfig pathConfig = new PathConfig(); pathConfig.setId(resourceDescription.getId()); diff --git a/core/src/main/java/org/keycloak/AuthorizationContext.java b/core/src/main/java/org/keycloak/AuthorizationContext.java index a14594bc46..93f3ff1f75 100644 --- a/core/src/main/java/org/keycloak/AuthorizationContext.java +++ b/core/src/main/java/org/keycloak/AuthorizationContext.java @@ -24,6 +24,7 @@ import org.keycloak.representations.idm.authorization.Permission; import java.util.Collections; import java.util.List; +import java.util.Map; /** * @author Pedro Igor @@ -31,10 +32,10 @@ import java.util.List; public class AuthorizationContext { private final AccessToken authzToken; - private final List paths; + private final Map paths; private boolean granted; - public AuthorizationContext(AccessToken authzToken, List paths) { + public AuthorizationContext(AccessToken authzToken, Map paths) { this.authzToken = authzToken; this.paths = paths; this.granted = true; @@ -57,7 +58,7 @@ public class AuthorizationContext { } for (Permission permission : authorization.getPermissions()) { - for (PathConfig pathHolder : this.paths) { + for (PathConfig pathHolder : this.paths.values()) { if (pathHolder.getName().equals(resourceName)) { if (pathHolder.getId().equals(permission.getResourceSetId())) { if (permission.getScopes().contains(scopeName)) { @@ -83,7 +84,7 @@ public class AuthorizationContext { } for (Permission permission : authorization.getPermissions()) { - for (PathConfig pathHolder : this.paths) { + for (PathConfig pathHolder : this.paths.values()) { if (pathHolder.getName().equals(resourceName)) { if (pathHolder.getId().equals(permission.getResourceSetId())) { return true; diff --git a/core/src/main/java/org/keycloak/representations/adapters/config/PolicyEnforcerConfig.java b/core/src/main/java/org/keycloak/representations/adapters/config/PolicyEnforcerConfig.java index db874c096e..a1bc1790bd 100644 --- a/core/src/main/java/org/keycloak/representations/adapters/config/PolicyEnforcerConfig.java +++ b/core/src/main/java/org/keycloak/representations/adapters/config/PolicyEnforcerConfig.java @@ -112,7 +112,7 @@ public class PolicyEnforcerConfig { public void setOnDenyRedirectTo(String onDenyRedirectTo) { this.onDenyRedirectTo = onDenyRedirectTo; } - + public static class PathConfig { private String name; diff --git a/examples/authz/servlet-authz/src/main/webapp/WEB-INF/web.xml b/examples/authz/servlet-authz/src/main/webapp/WEB-INF/web.xml index 14d0615978..e4ce0f1fb0 100644 --- a/examples/authz/servlet-authz/src/main/webapp/WEB-INF/web.xml +++ b/examples/authz/servlet-authz/src/main/webapp/WEB-INF/web.xml @@ -13,16 +13,8 @@ user - - - - - - All Resources - /* - - admin + user_premium @@ -39,9 +31,13 @@ user + + user_premium + + 403 /accessDenied.jsp - + \ No newline at end of file diff --git a/services/src/main/java/org/keycloak/authorization/admin/representation/PolicyEvaluationResponse.java b/services/src/main/java/org/keycloak/authorization/admin/representation/PolicyEvaluationResponse.java index 83c00b8c1f..ac6a97b4c5 100644 --- a/services/src/main/java/org/keycloak/authorization/admin/representation/PolicyEvaluationResponse.java +++ b/services/src/main/java/org/keycloak/authorization/admin/representation/PolicyEvaluationResponse.java @@ -64,7 +64,7 @@ public class PolicyEvaluationResponse { AccessToken accessToken = identity.getAccessToken(); AccessToken.Authorization authorizationData = new AccessToken.Authorization(); - authorizationData.setPermissions(Permissions.permits(results, authorization, resourceServer.getId())); + authorizationData.setPermissions(Permissions.allPermits(results, authorization, resourceServer)); accessToken.setAuthorization(authorizationData); response.rpt = accessToken; diff --git a/services/src/main/java/org/keycloak/authorization/entitlement/EntitlementService.java b/services/src/main/java/org/keycloak/authorization/entitlement/EntitlementService.java index 579417d100..894c3c7da1 100644 --- a/services/src/main/java/org/keycloak/authorization/entitlement/EntitlementService.java +++ b/services/src/main/java/org/keycloak/authorization/entitlement/EntitlementService.java @@ -179,7 +179,7 @@ public class EntitlementService { @Override protected void onComplete(List results) { - List entitlements = Permissions.allPermits(results, authorization, resourceServer); + List entitlements = Permissions.permits(results, authorization, resourceServer.getId()); if (entitlements.isEmpty()) { HashMap error = new HashMap<>(); diff --git a/testsuite/integration-arquillian/test-apps/servlet-authz/src/main/webapp/WEB-INF/web.xml b/testsuite/integration-arquillian/test-apps/servlet-authz/src/main/webapp/WEB-INF/web.xml index 14d0615978..2378ca59b5 100644 --- a/testsuite/integration-arquillian/test-apps/servlet-authz/src/main/webapp/WEB-INF/web.xml +++ b/testsuite/integration-arquillian/test-apps/servlet-authz/src/main/webapp/WEB-INF/web.xml @@ -13,16 +13,8 @@ user - - - - - - All Resources - /* - - admin + user_premium @@ -39,6 +31,10 @@ user + + user_premium + + 403 /accessDenied.jsp diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/authorization/EnforcerConfigTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/authorization/EnforcerConfigTest.java index f1e407f9f0..081bb79305 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/authorization/EnforcerConfigTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/authorization/EnforcerConfigTest.java @@ -27,6 +27,7 @@ import org.keycloak.testsuite.AbstractKeycloakTest; import org.keycloak.testsuite.ProfileAssume; import java.util.List; +import java.util.Map; import static org.junit.Assert.assertEquals; import static org.keycloak.testsuite.util.IOUtil.loadRealm; @@ -49,8 +50,8 @@ public class EnforcerConfigTest extends AbstractKeycloakTest { public void testMultiplePathsWithSameName() throws Exception{ KeycloakDeployment deployment = KeycloakDeploymentBuilder.build(getClass().getResourceAsStream("/authorization-test/enforcer-config-paths-same-name.json")); PolicyEnforcer policyEnforcer = deployment.getPolicyEnforcer(); - List paths = policyEnforcer.getPaths(); + Map paths = policyEnforcer.getPaths(); assertEquals(1, paths.size()); - assertEquals(4, paths.get(0).getMethods().size()); + assertEquals(4, paths.values().iterator().next().getMethods().size()); } }