Merge pull request #3513 from pedroigor/KEYCLOAK-3830

[KEYCLOAK-3830] - Properly handle public resources when using security-constraints

adapters/oidc/adapter-core/src/main/java/org/keycloak/adapters/AuthenticatedActionsHandler.java

@@ -142,9 +142,13 @@ public class AuthenticatedActionsHandler {
             AuthorizationContext authorizationContext = policyEnforcer.enforce(facade);
             RefreshableKeycloakSecurityContext session = (RefreshableKeycloakSecurityContext) facade.getSecurityContext();
 
-            session.setAuthorizationContext(authorizationContext);
+            if (session != null) {
+                session.setAuthorizationContext(authorizationContext);
 
-            return  authorizationContext.isGranted();
+                return authorizationContext.isGranted();
+            }
+
+            return true;
         } catch (Exception e) {
             throw new RuntimeException("Failed to enforce policy decisions.", e);
         }

adapters/oidc/adapter-core/src/main/java/org/keycloak/adapters/authorization/AbstractPolicyEnforcer.java

@@ -19,6 +19,7 @@ package org.keycloak.adapters.authorization;
 
 import org.jboss.logging.Logger;
 import org.keycloak.AuthorizationContext;
+import org.keycloak.KeycloakSecurityContext;
 import org.keycloak.adapters.OIDCHttpFacade;
 import org.keycloak.adapters.spi.HttpFacade.Request;
 import org.keycloak.adapters.spi.HttpFacade.Response;
@@ -66,40 +67,51 @@ public abstract class AbstractPolicyEnforcer {
             return createEmptyAuthorizationContext(true);
         }
 
-        AccessToken accessToken = httpFacade.getSecurityContext().getToken();
+        KeycloakSecurityContext securityContext = httpFacade.getSecurityContext();
-        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());
-        PathConfig pathConfig = this.pathMatcher.matches(path, this.paths);
 
-        LOGGER.debugf("Checking permissions for path [%s] with config [%s].", request.getURI(), pathConfig);
+        if (securityContext != null) {
+            AccessToken accessToken = securityContext.getToken();
 
-        if (pathConfig == null) {
+            if (accessToken != null) {
-            if (EnforcementMode.PERMISSIVE.equals(enforcementMode)) {
+                Request request = httpFacade.getRequest();
-                return createAuthorizationContext(accessToken);
+                Response response = httpFacade.getResponse();
+                String pathInfo = URI.create(request.getURI()).getPath().substring(1);
+                String path = pathInfo.substring(pathInfo.indexOf('/'), pathInfo.length());
+                PathConfig pathConfig = this.pathMatcher.matches(path, this.paths);
+
+                LOGGER.debugf("Checking permissions for path [%s] with config [%s].", request.getURI(), pathConfig);
+
+                if (pathConfig == null) {
+                    if (EnforcementMode.PERMISSIVE.equals(enforcementMode)) {
+                        return createAuthorizationContext(accessToken);
+                    }
+
+                    LOGGER.debugf("Could not find a configuration for path [%s]", path);
+                    response.sendError(403, "Could not find a configuration for path [" + path + "].");
+
+                    return createEmptyAuthorizationContext(false);
+                }
+
+                if (EnforcementMode.DISABLED.equals(pathConfig.getEnforcementMode())) {
+                    return createEmptyAuthorizationContext(true);
+                }
+
+                PathConfig actualPathConfig = resolvePathConfig(pathConfig, request);
+                Set<String> requiredScopes = getRequiredScopes(actualPathConfig, request);
+
+                if (isAuthorized(actualPathConfig, requiredScopes, accessToken, httpFacade)) {
+                    try {
+                        return createAuthorizationContext(accessToken);
+                    } catch (Exception e) {
+                        throw new RuntimeException("Error processing path [" + actualPathConfig.getPath() + "].", e);
+                    }
+                }
+
+                if (!challenge(actualPathConfig, requiredScopes, httpFacade)) {
+                    LOGGER.debugf("Sending challenge to the client. Path [%s]", pathConfig);
+                    response.sendError(403, "Authorization failed.");
+                }
             }
-
-            LOGGER.debugf("Could not find a configuration for path [%s]", path);
-            response.sendError(403, "Could not find a configuration for path [" + path + "].");
-
-            return createEmptyAuthorizationContext(false);
-        }
-
-        PathConfig actualPathConfig = resolvePathConfig(pathConfig, request);
-        Set<String> requiredScopes = getRequiredScopes(actualPathConfig, request);
-
-        if (isAuthorized(actualPathConfig, requiredScopes, accessToken, httpFacade)) {
-            try {
-                return createAuthorizationContext(accessToken);
-            } catch (Exception e) {
-                throw new RuntimeException("Error processing path [" + actualPathConfig.getPath() + "].", e);
-            }
-        }
-
-        if (!challenge(actualPathConfig, requiredScopes, httpFacade)) {
-            LOGGER.debugf("Sending challenge to the client. Path [%s]", pathConfig);
-            response.sendError(403, "Authorization failed.");
         }
 
         return createEmptyAuthorizationContext(false);
@@ -125,14 +137,17 @@ public abstract class AbstractPolicyEnforcer {
         }
 
         List<Permission> permissions = authorization.getPermissions();
+        boolean hasPermission = false;
 
         for (Permission permission : permissions) {
             if (permission.getResourceSetId() != null) {
                 if (isResourcePermission(actualPathConfig, permission)) {
+                    hasPermission = true;
+
                     if (actualPathConfig.isInstance() && !matchResourcePermission(actualPathConfig, permission)) {
                         continue;
-
                     }
+
                     if (hasResourceScopePermission(requiredScopes, permission, actualPathConfig)) {
                         LOGGER.debugf("Authorization GRANTED for path [%s]. Permissions [%s].", actualPathConfig, permissions);
                         if (request.getMethod().equalsIgnoreCase("DELETE") && actualPathConfig.isInstance()) {
@@ -143,11 +158,16 @@ public abstract class AbstractPolicyEnforcer {
                 }
             } else {
                 if (hasResourceScopePermission(requiredScopes, permission, actualPathConfig)) {
+                    hasPermission = true;
                     return true;
                 }
             }
         }
 
+        if (!hasPermission && EnforcementMode.PERMISSIVE.equals(actualPathConfig.getEnforcementMode())) {
+            return true;
+        }
+
         LOGGER.debugf("Authorization FAILED for path [%s]. No enough permissions [%s].", actualPathConfig, permissions);
 
         return false;
@@ -218,6 +238,7 @@ public abstract class AbstractPolicyEnforcer {
                 config.setScopes(originalConfig.getScopes());
                 config.setMethods(originalConfig.getMethods());
                 config.setParentConfig(originalConfig);
+                config.setEnforcementMode(originalConfig.getEnforcementMode());
 
                 this.paths.add(config);
 

adapters/oidc/adapter-core/src/main/java/org/keycloak/adapters/authorization/PolicyEnforcer.java

@@ -105,7 +105,16 @@ public class PolicyEnforcer {
     }
 
     private List<PathConfig> configurePaths(ProtectedResource protectedResource, PolicyEnforcerConfig enforcerConfig) {
-        if (enforcerConfig.getPaths().isEmpty()) {
+        boolean loadPathsFromServer = true;
+
+        for (PathConfig pathConfig : enforcerConfig.getPaths()) {
+            if (!PolicyEnforcerConfig.EnforcementMode.DISABLED.equals(pathConfig.getEnforcementMode())) {
+                loadPathsFromServer = false;
+                break;
+            }
+        }
+
+        if (loadPathsFromServer) {
             LOGGER.info("No path provided in configuration.");
             return configureAllPathsForResourceServer(protectedResource);
         } else {

core/src/main/java/org/keycloak/AuthorizationContext.java

@@ -18,9 +18,11 @@
 package org.keycloak;
 
 import org.keycloak.representations.AccessToken;
+import org.keycloak.representations.AccessToken.Authorization;
 import org.keycloak.representations.adapters.config.PolicyEnforcerConfig.PathConfig;
 import org.keycloak.representations.idm.authorization.Permission;
 
+import java.util.Collections;
 import java.util.List;
 
 /**
@@ -44,7 +46,17 @@ public class AuthorizationContext {
     }
 
     public boolean hasPermission(String resourceName, String scopeName) {
-        for (Permission permission : authzToken.getAuthorization().getPermissions()) {
+        if (this.authzToken == null) {
+            return false;
+        }
+
+        Authorization authorization = this.authzToken.getAuthorization();
+
+        if (authorization == null) {
+            return false;
+        }
+
+        for (Permission permission : authorization.getPermissions()) {
             for (PathConfig pathHolder : this.paths) {
                 if (pathHolder.getName().equals(resourceName)) {
                     if (pathHolder.getId().equals(permission.getResourceSetId())) {
@@ -60,7 +72,17 @@ public class AuthorizationContext {
     }
 
     public boolean hasResourcePermission(String resourceName) {
-        for (Permission permission : authzToken.getAuthorization().getPermissions()) {
+        if (this.authzToken == null) {
+            return false;
+        }
+
+        Authorization authorization = this.authzToken.getAuthorization();
+
+        if (authorization == null) {
+            return false;
+        }
+
+        for (Permission permission : authorization.getPermissions()) {
             for (PathConfig pathHolder : this.paths) {
                 if (pathHolder.getName().equals(resourceName)) {
                     if (pathHolder.getId().equals(permission.getResourceSetId())) {
@@ -74,7 +96,17 @@ public class AuthorizationContext {
     }
 
     public boolean hasScopePermission(String scopeName) {
-        for (Permission permission : authzToken.getAuthorization().getPermissions()) {
+        if (this.authzToken == null) {
+            return false;
+        }
+
+        Authorization authorization = this.authzToken.getAuthorization();
+
+        if (authorization == null) {
+            return false;
+        }
+
+        for (Permission permission : authorization.getPermissions()) {
             if (permission.getScopes().contains(scopeName)) {
                 return true;
             }
@@ -84,7 +116,17 @@ public class AuthorizationContext {
     }
 
     public List<Permission> getPermissions() {
-        return this.authzToken.getAuthorization().getPermissions();
+        if (this.authzToken == null) {
+            return Collections.emptyList();
+        }
+
+        Authorization authorization = this.authzToken.getAuthorization();
+
+        if (authorization == null) {
+            return Collections.emptyList();
+        }
+
+        return Collections.unmodifiableList(authorization.getPermissions());
     }
 
     public boolean isGranted() {

core/src/main/java/org/keycloak/representations/adapters/config/PolicyEnforcerConfig.java

@@ -122,6 +122,9 @@ public class PolicyEnforcerConfig {
         private List<String> scopes = Collections.emptyList();
         private String id;
 
+        @JsonProperty("enforcement-mode")
+        private EnforcementMode enforcementMode = EnforcementMode.ENFORCING;
+
         @JsonIgnore
         private PathConfig parentConfig;
 
@@ -173,6 +176,14 @@ public class PolicyEnforcerConfig {
             return id;
         }
 
+        public EnforcementMode getEnforcementMode() {
+            return enforcementMode;
+        }
+
+        public void setEnforcementMode(EnforcementMode enforcementMode) {
+            this.enforcementMode = enforcementMode;
+        }
+
         @Override
         public String toString() {
             return "PathConfig{" +
@@ -181,6 +192,7 @@ public class PolicyEnforcerConfig {
                     ", path='" + path + '\'' +
                     ", scopes=" + scopes +
                     ", id='" + id + '\'' +
+                    ", enforcerMode='" + enforcementMode + '\'' +
                     '}';
         }