Update restPathMatches to handle case with missing leading slash (#5061)

Signed-off-by: Darshit Chanpura <[email protected]>
Co-authored-by: Darshit Chanpura <[email protected]>

Author: cwperks

src/integrationTest/java/org/opensearch/security/rest/AuthZinRestLayerTests.java

@@ -111,6 +111,15 @@ public void testAccessDeniedForUserWithNoPermissions() {
         }
     }
 
+    @Test
+    public void testShouldFailWithoutPermForPathWithoutLeadingSlashes() {
+        try (TestRestClient client = cluster.getRestClient(NO_PERM)) {
+
+            // Protected Routes plugin
+            assertThat(client.getWithoutLeadingSlash(PROTECTED_API).getStatusCode(), equalTo(HttpStatus.SC_UNAUTHORIZED));
+        }
+    }
+
     /** AuthZ in REST Layer check */
 
     @Test

src/integrationTest/java/org/opensearch/security/rest/WhoAmITests.java

@@ -138,6 +138,13 @@ public void testWhoAmIWithoutGetPermissions() {
         }
     }
 
+    @Test
+    public void testWhoAmIWithoutGetPermissionsWithoutLeadingSlashInPath() {
+        try (TestRestClient client = cluster.getRestClient(WHO_AM_I_NO_PERM)) {
+            assertThat(client.getWithoutLeadingSlash(WHOAMI_PROTECTED_ENDPOINT).getStatusCode(), equalTo(HttpStatus.SC_UNAUTHORIZED));
+        }
+    }
+
     @Test
     public void testWhoAmIPost() {
         try (TestRestClient client = cluster.getRestClient(WHO_AM_I)) {

src/integrationTest/java/org/opensearch/test/framework/cluster/TestRestClient.java

@@ -110,6 +110,12 @@ public HttpResponse get(String path, Header... headers) {
         return executeRequest(new HttpGet(getHttpServerUri() + "/" + path), headers);
     }
 
+    public HttpResponse getWithoutLeadingSlash(String path, Header... headers) {
+        HttpUriRequest req = new HttpGet(getHttpServerUri());
+        req.setPath(path);
+        return executeRequest(req, headers);
+    }
+
     public HttpResponse getAuthInfo(Header... headers) {
         return executeRequest(new HttpGet(getHttpServerUri() + "/_opendistro/_security/authinfo?pretty"), headers);
     }

src/main/java/org/opensearch/security/filter/SecurityRestFilter.java

@@ -333,6 +333,9 @@ public void onAllowlistingSettingChanged(AllowlistingSettings allowlistingSettin
      * @return true if the request path matches the route
      */
     private boolean restPathMatches(String requestPath, String handlerPath) {
+        // Trim leading and trailing slashes
+        requestPath = requestPath.replaceAll("^/+", "").replaceAll("/+$", "");
+        handlerPath = handlerPath.replaceAll("^/+", "").replaceAll("/+$", "");
         // Check exact match
         if (handlerPath.equals(requestPath)) {
             return true;

src/test/java/org/opensearch/security/filter/RestPathMatchesTests.java

@@ -72,16 +72,37 @@ public void testRequestPathWithNamedParam() throws InvocationTargetException, Il
     }
 
     @Test
-    public void testRequestPathMismatch() throws InvocationTargetException, IllegalAccessException {
-        String requestPath = "_plugins/security/api/x/y";
-        String handlerPath = "_plugins/security/api/z/y";
+    public void testMatchWithLeadingSlashDifference() throws InvocationTargetException, IllegalAccessException {
+        String requestPath = "api/v1/resource";
+        String handlerPath = "/api/v1/resource";
+        assertTrue((Boolean) restPathMatches.invoke(securityRestFilter, requestPath, handlerPath));
+    }
+
+    @Test
+    public void testMatchWithTrailingSlashDifference() throws InvocationTargetException, IllegalAccessException {
+        String requestPath = "/api/v1/resource/";
+        String handlerPath = "/api/v1/resource";
+        assertTrue((Boolean) restPathMatches.invoke(securityRestFilter, requestPath, handlerPath));
+    }
+
+    @Test
+    public void testPathsMatchWithMultipleNamedParameters() throws InvocationTargetException, IllegalAccessException {
+        String requestPath = "/api/v1/resource/123/details";
+        String handlerPath = "/api/v1/resource/{id}/details";
+        assertTrue((Boolean) restPathMatches.invoke(securityRestFilter, requestPath, handlerPath));
+    }
+
+    @Test
+    public void testPathsDoNotMatchWithNonMatchingNamedParameterSegment() throws InvocationTargetException, IllegalAccessException {
+        String requestPath = "/api/v1/resource/123/details";
+        String handlerPath = "/api/v1/resource/{id}/summary";
         assertFalse((Boolean) restPathMatches.invoke(securityRestFilter, requestPath, handlerPath));
     }
 
     @Test
-    public void testRequestPathWithExtraSegments() throws InvocationTargetException, IllegalAccessException {
-        String requestPath = "_plugins/security/api/x/y/z";
-        String handlerPath = "_plugins/security/api/x/y";
+    public void testDifferentSegmentCount() throws InvocationTargetException, IllegalAccessException {
+        String requestPath = "/api/v1/resource/123/extra";
+        String handlerPath = "/api/v1/resource/{id}";
         assertFalse((Boolean) restPathMatches.invoke(securityRestFilter, requestPath, handlerPath));
     }
 }

src/test/java/org/opensearch/security/filter/SecurityRestFilterUnitTests.java

@@ -100,4 +100,7 @@ public void testDoesCallDelegateOnSuccessfulAuthorization() throws Exception {
 
         verify(testRestHandlerSpy).handleRequest(any(), any(), any());
     }
+
+    // unit tests for restPathMatches are in RestPathMatchesTests.java
+
 }