Ensure RealmBase finds all matching extension based constraints

Author: markt-asf

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

@@ -666,8 +666,6 @@ public SecurityConstraint[] findSecurityConstraints(Request request, Context con
                         constraints[i].included(uri, method));
             }
 
-            boolean matched = false;
-            int pos = -1;
             for (int j = 0; j < collection.length; j++) {
                 String[] patterns = collection[j].findPatterns();
 
@@ -677,6 +675,7 @@ public SecurityConstraint[] findSecurityConstraints(Request request, Context con
                     continue;
                 }
 
+                boolean matched = false;
                 for (int k = 0; k < patterns.length && !matched; k++) {
                     String pattern = patterns[k];
                     if (pattern.startsWith("*.")) {
@@ -686,19 +685,18 @@ public SecurityConstraint[] findSecurityConstraints(Request request, Context con
                                 uri.length() - dot == pattern.length() - 1) {
                             if (pattern.regionMatches(1, uri, dot, uri.length() - dot)) {
                                 matched = true;
-                                pos = j;
                             }
                         }
                     }
                 }
-            }
-            if (matched) {
-                found = true;
-                if (collection[pos].findMethod(method)) {
-                    if (results == null) {
-                        results = new ArrayList<>();
+                if (matched) {
+                    found = true;
+                    if (collection[j].findMethod(method)) {
+                        if (results == null) {
+                            results = new ArrayList<>();
+                        }
+                        results.add(constraints[i]);
                     }
-                    results.add(constraints[i]);
                 }
             }
         }

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

@@ -790,4 +790,86 @@ public void testHttpConstraint() throws IOException {
         Assert.assertFalse(mapRealm.hasResourcePermission(
                 request, response, constraintsDelete, null));
     }
+
+
+    @Test
+    public void testUncoveredMethods() throws IOException {
+        // Create a constraint for ROLE1
+        SecurityConstraint constraint = new SecurityConstraint();
+        constraint.addAuthRole(ROLE1);
+        // Add a collection for GET
+        SecurityCollection getCollection = new SecurityCollection();
+        getCollection.addMethod(Method.GET);
+        getCollection.addPatternDecoded("*.html");
+        constraint.addCollection(getCollection);
+        // Add a collection for POST
+        SecurityCollection postCollection = new SecurityCollection();
+        postCollection.addMethod(Method.POST);
+        postCollection.addPatternDecoded("*.html");
+        constraint.addCollection(postCollection);
+
+        TesterMapRealm mapRealm = new TesterMapRealm();
+
+        // Set up the mock request and response
+        TesterRequest request = new TesterRequest();
+        Response response = new TesterResponse();
+        Context context = request.getContext();
+        context.addSecurityRole(ROLE1);
+        context.addSecurityRole(ROLE2);
+        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);
+
+        List<String> userRoles99 = new ArrayList<>();
+        GenericPrincipal gp99 = new GenericPrincipal(USER99, userRoles99);
+
+        // Add the constraint to the context
+        context.addConstraint(constraint);
+
+
+        // Only user1 should be able to perform a GET
+        request.setMethod(Method.GET);
+
+        SecurityConstraint[] constraintsGet =
+                mapRealm.findSecurityConstraints(request, context);
+
+        request.setUserPrincipal(null);
+        Assert.assertFalse(mapRealm.hasResourcePermission(
+                request, response, constraintsGet, null));
+        request.setUserPrincipal(gp1);
+        Assert.assertTrue(mapRealm.hasResourcePermission(
+                request, response, constraintsGet, null));
+        request.setUserPrincipal(gp2);
+        Assert.assertFalse(mapRealm.hasResourcePermission(
+                request, response, constraintsGet, null));
+        request.setUserPrincipal(gp99);
+        Assert.assertFalse(mapRealm.hasResourcePermission(
+                request, response, constraintsGet, null));
+
+        // Only user1 should be able to perform a POST
+        request.setMethod(Method.POST);
+
+        SecurityConstraint[] constraintsPost =
+                mapRealm.findSecurityConstraints(request, context);
+
+        request.setUserPrincipal(null);
+        Assert.assertFalse(mapRealm.hasResourcePermission(
+                request, response, constraintsPost, null));
+        request.setUserPrincipal(gp1);
+        Assert.assertTrue(mapRealm.hasResourcePermission(
+                request, response, constraintsPost, null));
+        request.setUserPrincipal(gp2);
+        Assert.assertFalse(mapRealm.hasResourcePermission(
+                request, response, constraintsPost, null));
+        request.setUserPrincipal(gp99);
+        Assert.assertFalse(mapRealm.hasResourcePermission(
+                request, response, constraintsPost, null));
+    }
 }

test/org/apache/tomcat/unittest/TesterRequest.java

@@ -31,6 +31,7 @@
 import org.apache.catalina.Context;
 import org.apache.catalina.connector.Request;
 import org.apache.catalina.session.StandardSession;
+import org.apache.tomcat.util.buf.MessageBytes;
 
 public class TesterRequest extends Request {
 
@@ -81,6 +82,15 @@ public String getRequestURI() {
         return "/level1/level2/foo.html";
     }
 
+
+    @Override
+    public MessageBytes getRequestPathMB() {
+        MessageBytes result = MessageBytes.newInstance();
+        result.setString(getRequestURI());
+        return result;
+    }
+
+
     @Override
     public String getDecodedRequestURI() {
         // Decoding not required

webapps/docs/changelog.xml

@@ -172,6 +172,10 @@
       <fix>
         Correct the handling of invalid users with DIGEST authentication. (markt)
       </fix>
+      <fix>
+        Ensure <code>RealmBase</code> finds all matching extension based
+        security constraints. (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Coyote">