Don't ignore methods on security constraints mapped to "/"

Author: markt-asf

java/org/apache/catalina/realm/RealmBase.java

@@ -753,7 +753,7 @@ public SecurityConstraint[] findSecurityConstraints(Request request, Context con
 
                 boolean matched = false;
                 for (String pattern : patterns) {
-                    if (pattern.equals("/")) {
+                    if (pattern.equals("/") && securityCollection.findMethod(method)) {
                         matched = true;
                         break;
                     }

test/org/apache/catalina/realm/TestRealmBase.java

@@ -872,4 +872,71 @@ public void testUncoveredMethods() throws IOException {
         Assert.assertFalse(mapRealm.hasResourcePermission(
                 request, response, constraintsPost, null));
     }
+
+
+    @Test
+    public void testDefaultServletConstraints() throws IOException {
+        // Create a constraint that allows GET
+        SecurityConstraint allowConstraint = new SecurityConstraint();
+        SecurityCollection allowCollection = new SecurityCollection();
+        allowCollection.addMethod(Method.GET);
+        allowCollection.addPatternDecoded("/");
+        allowConstraint.addCollection(allowCollection);
+        // Create a constraint that disallows everything but GET
+        SecurityConstraint blockConstraint = new SecurityConstraint();
+        SecurityCollection blockCollection = new SecurityCollection();
+        blockCollection.addOmittedMethod(Method.GET);
+        blockCollection.addPatternDecoded("/");
+        blockConstraint.addCollection(blockCollection);
+        blockConstraint.addAuthRole(ROLE1);
+
+        TesterMapRealm mapRealm = new TesterMapRealm();
+
+        // Set up the mock request and response
+        TesterRequest request = new TesterRequest();
+        Response response = new TesterResponse();
+        Context context = request.getContext();
+        request.getMappingData().context = context;
+
+        // Create the principals
+        List<String> userRoles1 = new ArrayList<>();
+        userRoles1.add(ROLE1);
+        GenericPrincipal gp1 = new GenericPrincipal(USER1, userRoles1);
+
+        List<String> userRoles2 = new ArrayList<>();
+        userRoles2.add(ROLE2);
+        GenericPrincipal gp2 = new GenericPrincipal(USER2, userRoles2);
+
+        // Add the constraints to the context
+        context.addConstraint(allowConstraint);
+        context.addConstraint(blockConstraint);
+
+        // GET should be allowed
+        request.setMethod(Method.GET);
+        SecurityConstraint[] constraints = mapRealm.findSecurityConstraints(request, context);
+
+        request.setUserPrincipal(null);
+        Assert.assertTrue(mapRealm.hasResourcePermission(
+                request, response, constraints, null));
+        request.setUserPrincipal(gp1);
+        Assert.assertTrue(mapRealm.hasResourcePermission(
+                request, response, constraints, null));
+        request.setUserPrincipal(gp2);
+        Assert.assertTrue(mapRealm.hasResourcePermission(
+                request, response, constraints, null));
+
+        // POST should require ROLE1 should be allowed
+        request.setMethod(Method.POST);
+        constraints = mapRealm.findSecurityConstraints(request, context);
+
+        request.setUserPrincipal(null);
+        Assert.assertFalse(mapRealm.hasResourcePermission(
+                request, response, constraints, null));
+        request.setUserPrincipal(gp1);
+        Assert.assertTrue(mapRealm.hasResourcePermission(
+                request, response, constraints, null));
+        request.setUserPrincipal(gp2);
+        Assert.assertFalse(mapRealm.hasResourcePermission(
+                request, response, constraints, null));
+    }
 }

webapps/docs/changelog.xml

@@ -407,6 +407,10 @@
         Ensure atomic session persistence in <code>FileStore</code>. Based on
         pull request <pr>1016</pr> by sahvx655-wq. (markt)
       </fix>
+      <fix>
+        Do not ignore methods configured on security constraints that map to the
+        default servlet. (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Coyote">