Adds and updates tests

Signed-off-by: Darshit Chanpura <[email protected]>
DarshitChanpura · Nov 5, 2024 · e10c2fe · e10c2fe
1 parent 6108c30
commit e10c2fe
Show file tree

Hide file tree

Showing 5 changed files with 120 additions and 4 deletions.
diff --git a/src/integrationTest/java/org/opensearch/security/rest/AuthZinRestLayerTests.java b/src/integrationTest/java/org/opensearch/security/rest/AuthZinRestLayerTests.java
@@ -26,11 +26,13 @@
 import org.opensearch.test.framework.audit.AuditLogsRule;
 import org.opensearch.test.framework.cluster.ClusterManager;
 import org.opensearch.test.framework.cluster.LocalCluster;
+import org.opensearch.test.framework.cluster.RestClientException;
 import org.opensearch.test.framework.cluster.TestRestClient;
 import org.opensearch.test.framework.testplugins.dummy.CustomLegacyTestPlugin;
 import org.opensearch.test.framework.testplugins.dummyprotected.CustomRestProtectedTestPlugin;
 
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
 import static org.opensearch.rest.RestRequest.Method.GET;
 import static org.opensearch.rest.RestRequest.Method.POST;
@@ -40,6 +42,7 @@
 import static org.opensearch.test.framework.TestSecurityConfig.AuthcDomain.AUTHC_HTTPBASIC_INTERNAL;
 import static org.opensearch.test.framework.audit.AuditMessagePredicate.privilegePredicateRESTLayer;
 import static org.opensearch.test.framework.audit.AuditMessagePredicate.privilegePredicateTransportLayer;
+import static org.junit.Assert.assertThrows;
 
 @RunWith(com.carrotsearch.randomizedtesting.RandomizedRunner.class)
 @ThreadLeakScope(ThreadLeakScope.Scope.NONE)
@@ -111,6 +114,17 @@ public void testAccessDeniedForUserWithNoPermissions() {
         }
     }
 
+    @Test
+    public void testShouldFailWithoutPermForPathWithoutLeadingSlashes() {
+        try (TestRestClient client = cluster.getRestClient(NO_PERM)) {
+
+            // Protected Routes plugin
+            Exception exception = assertThrows(RestClientException.class, () -> { client.getWithoutLeadingSlash(PROTECTED_API); });
+
+            assertThat(exception.getMessage(), containsString("Error occured during HTTP request execution"));
+        }
+    }
+
     /** AuthZ in REST Layer check */
 
     @Test

diff --git a/src/integrationTest/java/org/opensearch/security/rest/WhoAmITests.java b/src/integrationTest/java/org/opensearch/security/rest/WhoAmITests.java
@@ -37,21 +37,21 @@
 import org.opensearch.test.framework.audit.AuditLogsRule;
 import org.opensearch.test.framework.cluster.ClusterManager;
 import org.opensearch.test.framework.cluster.LocalCluster;
+import org.opensearch.test.framework.cluster.RestClientException;
 import org.opensearch.test.framework.cluster.TestRestClient;
 
 import joptsimple.internal.Strings;
 
 import static org.hamcrest.MatcherAssert.assertThat;
-import static org.hamcrest.Matchers.equalTo;
-import static org.hamcrest.Matchers.greaterThan;
-import static org.hamcrest.Matchers.lessThan;
+import static org.hamcrest.Matchers.*;
 import static org.opensearch.rest.RestRequest.Method.GET;
 import static org.opensearch.security.auditlog.impl.AuditCategory.GRANTED_PRIVILEGES;
 import static org.opensearch.security.auditlog.impl.AuditCategory.MISSING_PRIVILEGES;
 import static org.opensearch.test.framework.TestSecurityConfig.AuthcDomain.AUTHC_HTTPBASIC_INTERNAL;
 import static org.opensearch.test.framework.audit.AuditMessagePredicate.grantedPrivilege;
 import static org.opensearch.test.framework.audit.AuditMessagePredicate.privilegePredicateRESTLayer;
 import static org.opensearch.test.framework.audit.AuditMessagePredicate.userAuthenticatedPredicate;
+import static org.junit.Assert.assertThrows;
 import static org.junit.Assert.assertTrue;
 
 @RunWith(com.carrotsearch.randomizedtesting.RandomizedRunner.class)
@@ -138,6 +138,18 @@ public void testWhoAmIWithoutGetPermissions() {
         }
     }
 
+    @Test
+    public void testWhoAmIWithoutGetPermissionsWithoutLeadingSlashInPath() {
+        try (TestRestClient client = cluster.getRestClient(WHO_AM_I_NO_PERM)) {
+            Exception exception = assertThrows(
+                RestClientException.class,
+                () -> { client.getWithoutLeadingSlash(WHOAMI_PROTECTED_ENDPOINT); }
+            );
+
+            assertThat(exception.getMessage(), containsString("Error occured during HTTP request execution"));
+        }
+    }
+
     @Test
     public void testWhoAmIPost() {
         try (TestRestClient client = cluster.getRestClient(WHO_AM_I)) {

diff --git a/src/integrationTest/java/org/opensearch/test/framework/cluster/TestRestClient.java b/src/integrationTest/java/org/opensearch/test/framework/cluster/TestRestClient.java
@@ -110,6 +110,10 @@ public HttpResponse get(String path, Header... headers) {
         return executeRequest(new HttpGet(getHttpServerUri() + "/" + path), headers);
     }
 
+    public void getWithoutLeadingSlash(String path, Header... headers) {
+        executeRequest(new HttpGet(getHttpServerUri() + path), headers);
+    }
+
     public HttpResponse getAuthInfo(Header... headers) {
         return executeRequest(new HttpGet(getHttpServerUri() + "/_opendistro/_security/authinfo?pretty"), headers);
     }

diff --git a/src/main/java/org/opensearch/security/filter/SecurityRestFilter.java b/src/main/java/org/opensearch/security/filter/SecurityRestFilter.java
@@ -335,7 +335,7 @@ public void onAllowlistingSettingChanged(AllowlistingSettings allowlistingSettin
      * @param handlerPath The path from the {@link RestHandler.Route}
      * @return true if the request path matches the route
      */
-    private boolean restPathMatches(String requestPath, String handlerPath) {
+    boolean restPathMatches(String requestPath, String handlerPath) {
         // Trim leading and trailing slashes
         requestPath = requestPath.replaceAll("^/+", "").replaceAll("/+$", "");
         handlerPath = handlerPath.replaceAll("^/+", "").replaceAll("/+$", "");

diff --git a/src/test/java/org/opensearch/security/filter/SecurityRestFilterUnitTests.java b/src/test/java/org/opensearch/security/filter/SecurityRestFilterUnitTests.java
@@ -15,6 +15,8 @@
 
 import org.junit.Before;
 import org.junit.Test;
+import org.junit.experimental.runners.Enclosed;
+import org.junit.runner.RunWith;
 
 import org.opensearch.client.node.NodeClient;
 import org.opensearch.common.settings.Settings;
@@ -41,6 +43,7 @@
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.verify;
 
+@RunWith(Enclosed.class)
 public class SecurityRestFilterUnitTests {
 
     SecurityRestFilter sf;
@@ -100,4 +103,87 @@ public void testDoesCallDelegateOnSuccessfulAuthorization() throws Exception {
 
         verify(testRestHandlerSpy).handleRequest(any(), any(), any());
     }
+
+    // Mini Test Suite for restPathMatches
+    public static class RestPathMatchesTests {
+
+        private SecurityRestFilter sf;
+
+        @Before
+        public void setUp() {
+            sf = new SecurityRestFilter(
+                mock(BackendRegistry.class),
+                mock(RestLayerPrivilegesEvaluator.class),
+                mock(AuditLog.class),
+                mock(ThreadPool.class),
+                mock(PrincipalExtractor.class),
+                Settings.EMPTY,
+                mock(Path.class),
+                mock(CompatConfig.class)
+            );
+        }
+
+        @Test
+        public void testExactPathMatch() {
+            String requestPath = "/api/v1/resource";
+            String handlerPath = "/api/v1/resource";
+            assertTrue(sf.restPathMatches(requestPath, handlerPath));
+        }
+
+        @Test
+        public void testPathsDoNotMatch() {
+            String requestPath = "/api/v1/resource";
+            String handlerPath = "/api/v2/resource";
+            assertFalse(sf.restPathMatches(requestPath, handlerPath));
+        }
+
+        @Test
+        public void testMatchWithLeadingSlashDifference() {
+            String requestPath = "api/v1/resource";
+            String handlerPath = "/api/v1/resource";
+            assertTrue(sf.restPathMatches(requestPath, handlerPath));
+        }
+
+        @Test
+        public void testMatchWithTrailingSlashDifference() {
+            String requestPath = "/api/v1/resource/";
+            String handlerPath = "/api/v1/resource";
+            assertTrue(sf.restPathMatches(requestPath, handlerPath));
+        }
+
+        @Test
+        public void testPathsMatchWithNamedParameter() {
+            String requestPath = "/api/v1/resource/123";
+            String handlerPath = "/api/v1/resource/{id}";
+            assertTrue(sf.restPathMatches(requestPath, handlerPath));
+        }
+
+        @Test
+        public void testPathsDoNotMatchWithDifferentNamedParameter() {
+            String requestPath = "/api/v1/resource/123";
+            String handlerPath = "/api/v1/resource/{name}";
+            assertTrue(sf.restPathMatches(requestPath, handlerPath));
+        }
+
+        @Test
+        public void testDifferentSegmentCount() {
+            String requestPath = "/api/v1/resource/123/extra";
+            String handlerPath = "/api/v1/resource/{id}";
+            assertFalse(sf.restPathMatches(requestPath, handlerPath));
+        }
+
+        @Test
+        public void testPathsMatchWithMultipleNamedParameters() {
+            String requestPath = "/api/v1/resource/123/details";
+            String handlerPath = "/api/v1/resource/{id}/details";
+            assertTrue(sf.restPathMatches(requestPath, handlerPath));
+        }
+
+        @Test
+        public void testPathsDoNotMatchWithNonMatchingNamedParameterSegment() {
+            String requestPath = "/api/v1/resource/123/details";
+            String handlerPath = "/api/v1/resource/{id}/summary";
+            assertFalse(sf.restPathMatches(requestPath, handlerPath));
+        }
+    }
 }