KEYCLOAK-4208 restructure spring config to match servlet spec. updating jetty, tomcat and undertow

adapters/oidc/spring-boot/pom.xml

@@ -56,7 +56,7 @@
     </dependency>
     <dependency>
       <groupId>org.keycloak</groupId>
-      <artifactId>keycloak-jetty92-adapter</artifactId>
+      <artifactId>keycloak-jetty93-adapter</artifactId>
       <scope>provided</scope>
     </dependency>
 

adapters/oidc/spring-boot/src/main/java/org/keycloak/adapters/springboot/KeycloakSpringBootConfiguration.java

@@ -141,10 +141,10 @@ public class KeycloakSpringBootConfiguration {
             List<io.undertow.servlet.api.SecurityConstraint> undertowSecurityConstraints = new ArrayList<io.undertow.servlet.api.SecurityConstraint>();
             for (KeycloakSpringBootProperties.SecurityConstraint constraintDefinition : keycloakProperties.getSecurityConstraints()) {
 
-                for (KeycloakSpringBootProperties.SecurityCollection collectionDefinition : constraintDefinition.getSecurityCollections()) {
+                io.undertow.servlet.api.SecurityConstraint undertowSecurityConstraint = new io.undertow.servlet.api.SecurityConstraint();
+                undertowSecurityConstraint.addRolesAllowed(constraintDefinition.getAuthRoles());
 
-                    io.undertow.servlet.api.SecurityConstraint undertowSecurityConstraint = new io.undertow.servlet.api.SecurityConstraint();
+                for (KeycloakSpringBootProperties.SecurityCollection collectionDefinition : constraintDefinition.getSecurityCollections()) {
-                    undertowSecurityConstraint.addRolesAllowed(collectionDefinition.getAuthRoles());
 
                     WebResourceCollection webResourceCollection = new WebResourceCollection();
                     webResourceCollection.addHttpMethods(collectionDefinition.getMethods());
@@ -153,8 +153,9 @@ public class KeycloakSpringBootConfiguration {
 
                     undertowSecurityConstraint.addWebResourceCollections(webResourceCollection);
 
-                    undertowSecurityConstraints.add(undertowSecurityConstraint);
                 }
+
+                undertowSecurityConstraints.add(undertowSecurityConstraint);
             }
             return undertowSecurityConstraints;
         }
@@ -174,38 +175,62 @@ public class KeycloakSpringBootConfiguration {
             KeycloakJettyAuthenticator keycloakJettyAuthenticator = new KeycloakJettyAuthenticator();
             keycloakJettyAuthenticator.setConfigResolver(new KeycloakSpringBootConfigResolver());
 
+            /* see org.eclipse.jetty.webapp.StandardDescriptorProcessor#visitSecurityConstraint for an example
+               on how to map servlet spec to Constraints */
+
             List<ConstraintMapping> jettyConstraintMappings = new ArrayList<ConstraintMapping>();
             for (KeycloakSpringBootProperties.SecurityConstraint constraintDefinition : keycloakProperties.getSecurityConstraints()) {
 
                 for (KeycloakSpringBootProperties.SecurityCollection securityCollectionDefinition : constraintDefinition
                         .getSecurityCollections()) {
-
+                    // securityCollection matches servlet spec's web-resource-collection
                     Constraint jettyConstraint = new Constraint();
+
+                    if (constraintDefinition.getAuthRoles().size() > 0) {
+                        jettyConstraint.setAuthenticate(true);
+                        jettyConstraint.setRoles(constraintDefinition.getAuthRoles().toArray(new String[0]));
+                    }
+
                     jettyConstraint.setName(securityCollectionDefinition.getName());
-                    jettyConstraint.setAuthenticate(true);
 
-                    if (securityCollectionDefinition.getName() != null) {
+                    // according to the servlet spec each security-constraint has at least one URL pattern
-                        jettyConstraint.setName(securityCollectionDefinition.getName());
+                    for(String pattern : securityCollectionDefinition.getPatterns()) {
+
+                        /* the following code is asymmetric as Jetty's ConstraintMapping accepts only one allowed HTTP method,
+                           but multiple omitted methods. Therefore we add one ConstraintMapping for each allowed
+                           mapping but only one mapping in the cases of omitted methods or no methods.
+                         */
+
+                        if (securityCollectionDefinition.getMethods().size() > 0) {
+                            // according to the servlet spec we have either methods ...
+                            for(String method : securityCollectionDefinition.getMethods()) {
+                                ConstraintMapping jettyConstraintMapping = new ConstraintMapping();
+                                jettyConstraintMappings.add(jettyConstraintMapping);
+
+                                jettyConstraintMapping.setConstraint(jettyConstraint);
+                                jettyConstraintMapping.setPathSpec(pattern);
+                                jettyConstraintMapping.setMethod(method);
+                            }
+                        } else if (securityCollectionDefinition.getOmittedMethods().size() > 0){
+                            // ... omitted methods ...
+                            ConstraintMapping jettyConstraintMapping = new ConstraintMapping();
+                            jettyConstraintMappings.add(jettyConstraintMapping);
+
+                            jettyConstraintMapping.setConstraint(jettyConstraint);
+                            jettyConstraintMapping.setPathSpec(pattern);
+                            jettyConstraintMapping.setMethodOmissions(
+                                    securityCollectionDefinition.getOmittedMethods().toArray(new String[0]));
+                        } else {
+                            // ... or no methods at all
+                            ConstraintMapping jettyConstraintMapping = new ConstraintMapping();
+                            jettyConstraintMappings.add(jettyConstraintMapping);
+
+                            jettyConstraintMapping.setConstraint(jettyConstraint);
+                            jettyConstraintMapping.setPathSpec(pattern);
+                        }
+
                     }
 
-                    jettyConstraint.setRoles(securityCollectionDefinition.getAuthRoles().toArray(new String[0]));
-
-                    ConstraintMapping jettyConstraintMapping = new ConstraintMapping();
-                    if (securityCollectionDefinition.getPatterns().size() > 0) {
-                        //First pattern wins
-                        jettyConstraintMapping.setPathSpec(securityCollectionDefinition.getPatterns().get(0));
-                        jettyConstraintMapping.setConstraint(jettyConstraint);
-                    }
-
-                    if (securityCollectionDefinition.getMethods().size() > 0) {
-                        //First method wins
-                        jettyConstraintMapping.setMethod(securityCollectionDefinition.getMethods().get(0));
-                    }
-
-                    jettyConstraintMapping.setMethodOmissions(
-                            securityCollectionDefinition.getOmittedMethods().toArray(new String[0]));
-
-                    jettyConstraintMappings.add(jettyConstraintMapping);
                 }
             }
 
@@ -235,12 +260,10 @@ public class KeycloakSpringBootConfiguration {
 
             Set<String> authRoles = new HashSet<String>();
             for (KeycloakSpringBootProperties.SecurityConstraint constraint : keycloakProperties.getSecurityConstraints()) {
-                for (KeycloakSpringBootProperties.SecurityCollection collection : constraint.getSecurityCollections()) {
+                for (String authRole : constraint.getAuthRoles()) {
-                    for (String authRole : collection.getAuthRoles()) {
+                    if (!authRoles.contains(authRole)) {
-                        if (!authRoles.contains(authRole)) {
+                        context.addSecurityRole(authRole);
-                            context.addSecurityRole(authRole);
+                        authRoles.add(authRole);
-                            authRoles.add(authRole);
-                        }
                     }
                 }
             }
@@ -248,6 +271,10 @@ public class KeycloakSpringBootConfiguration {
             for (KeycloakSpringBootProperties.SecurityConstraint constraint : keycloakProperties.getSecurityConstraints()) {
                 SecurityConstraint tomcatConstraint = new SecurityConstraint();
 
+                for (String authRole : constraint.getAuthRoles()) {
+                    tomcatConstraint.addAuthRole(authRole);
+                }
+
                 for (KeycloakSpringBootProperties.SecurityCollection collection : constraint.getSecurityCollections()) {
                     SecurityCollection tomcatSecCollection = new SecurityCollection();
 
@@ -258,10 +285,6 @@ public class KeycloakSpringBootConfiguration {
                         tomcatSecCollection.setDescription(collection.getDescription());
                     }
 
-                    for (String authRole : collection.getAuthRoles()) {
-                        tomcatConstraint.addAuthRole(authRole);
-                    }
-
                     for (String pattern : collection.getPatterns()) {
                         tomcatSecCollection.addPattern(pattern);
                     }

adapters/oidc/spring-boot/src/main/java/org/keycloak/adapters/springboot/KeycloakSpringBootProperties.java

@@ -43,12 +43,20 @@ public class KeycloakSpringBootProperties extends AdapterConfig {
      */
     private List<SecurityConstraint> securityConstraints = new ArrayList<SecurityConstraint>();
 
+    /**
+     * This matches security-constraint of the servlet spec
+     */
     @ConfigurationProperties()
     public static class SecurityConstraint {
         /**
          * A list of security collections
          */
         private List<SecurityCollection> securityCollections = new ArrayList<SecurityCollection>();
+        private List<String> authRoles = new ArrayList<String>();
+
+        public List<String> getAuthRoles() {
+            return authRoles;
+        }
 
         public List<SecurityCollection> getSecurityCollections() {
             return securityCollections;
@@ -57,7 +65,16 @@ public class KeycloakSpringBootProperties extends AdapterConfig {
         public void setSecurityCollections(List<SecurityCollection> securityCollections) {
             this.securityCollections = securityCollections;
         }
+
+        public void setAuthRoles(List<String> authRoles) {
+            this.authRoles = authRoles;
+        }
+
     }
+
+    /**
+     * This matches web-resource-collection of the servlet spec
+     */
     @ConfigurationProperties()
     public static class SecurityCollection {
         /**
@@ -68,10 +85,6 @@ public class KeycloakSpringBootProperties extends AdapterConfig {
          * The description of your security collection
          */
         private String description;
-        /**
-         *  A list of roles that applies for this security collection
-         */
-        private List<String> authRoles = new ArrayList<String>();
         /**
          * A list of URL patterns that should match to apply the security collection
          */
@@ -85,10 +98,6 @@ public class KeycloakSpringBootProperties extends AdapterConfig {
          */
         private List<String> omittedMethods = new ArrayList<String>();
 
-        public List<String> getAuthRoles() {
-            return authRoles;
-        }
-
         public List<String> getPatterns() {
             return patterns;
         }
@@ -117,10 +126,6 @@ public class KeycloakSpringBootProperties extends AdapterConfig {
             this.description = description;
         }
 
-        public void setAuthRoles(List<String> authRoles) {
-            this.authRoles = authRoles;
-        }
-
         public void setPatterns(List<String> patterns) {
             this.patterns = patterns;
         }