Skip to content

Commit 1d4f54e

Browse files
authored
[Backport 2.19] Update restPathMatches to handle case with missing leading slash (opensearch-project#5061) (opensearch-project#5077)
Signed-off-by: Craig Perkins <cwperx@amazon.com>
1 parent c50a90e commit 1d4f54e

File tree

5 files changed

+48
-6
lines changed

5 files changed

+48
-6
lines changed

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

+7
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,13 @@ public void testWhoAmIWithoutGetPermissions() {
138138
}
139139
}
140140

141+
@Test
142+
public void testWhoAmIWithoutGetPermissionsWithoutLeadingSlashInPath() {
143+
try (TestRestClient client = cluster.getRestClient(WHO_AM_I_NO_PERM)) {
144+
assertThat(client.getWithoutLeadingSlash(WHOAMI_PROTECTED_ENDPOINT).getStatusCode(), equalTo(HttpStatus.SC_UNAUTHORIZED));
145+
}
146+
}
147+
141148
@Test
142149
public void testWhoAmIPost() {
143150
try (TestRestClient client = cluster.getRestClient(WHO_AM_I)) {

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

+8
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import java.io.UnsupportedEncodingException;
3333
import java.net.InetAddress;
3434
import java.net.InetSocketAddress;
35+
import java.net.URI;
3536
import java.nio.charset.StandardCharsets;
3637
import java.util.ArrayList;
3738
import java.util.Arrays;
@@ -111,6 +112,13 @@ public HttpResponse get(String path, Header... headers) {
111112
return executeRequest(new HttpGet(getHttpServerUri() + "/" + path), headers);
112113
}
113114

115+
public HttpResponse getWithoutLeadingSlash(String path, Header... headers) {
116+
URI uri = URI.create(getHttpServerUri());
117+
uri = uri.resolve(path);
118+
HttpUriRequest req = new HttpGet(uri);
119+
return executeRequest(req, headers);
120+
}
121+
114122
public HttpResponse getAuthInfo(Header... headers) {
115123
return executeRequest(new HttpGet(getHttpServerUri() + "/_opendistro/_security/authinfo?pretty"), headers);
116124
}

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

+3
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,9 @@ public void onAllowlistingSettingChanged(AllowlistingSettings allowlistingSettin
333333
* @return true if the request path matches the route
334334
*/
335335
private boolean restPathMatches(String requestPath, String handlerPath) {
336+
// Trim leading and trailing slashes
337+
requestPath = requestPath.replaceAll("^/+", "").replaceAll("/+$", "");
338+
handlerPath = handlerPath.replaceAll("^/+", "").replaceAll("/+$", "");
336339
// Check exact match
337340
if (handlerPath.equals(requestPath)) {
338341
return true;

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

+27-6
Original file line numberDiff line numberDiff line change
@@ -72,16 +72,37 @@ public void testRequestPathWithNamedParam() throws InvocationTargetException, Il
7272
}
7373

7474
@Test
75-
public void testRequestPathMismatch() throws InvocationTargetException, IllegalAccessException {
76-
String requestPath = "_plugins/security/api/x/y";
77-
String handlerPath = "_plugins/security/api/z/y";
75+
public void testMatchWithLeadingSlashDifference() throws InvocationTargetException, IllegalAccessException {
76+
String requestPath = "api/v1/resource";
77+
String handlerPath = "/api/v1/resource";
78+
assertTrue((Boolean) restPathMatches.invoke(securityRestFilter, requestPath, handlerPath));
79+
}
80+
81+
@Test
82+
public void testMatchWithTrailingSlashDifference() throws InvocationTargetException, IllegalAccessException {
83+
String requestPath = "/api/v1/resource/";
84+
String handlerPath = "/api/v1/resource";
85+
assertTrue((Boolean) restPathMatches.invoke(securityRestFilter, requestPath, handlerPath));
86+
}
87+
88+
@Test
89+
public void testPathsMatchWithMultipleNamedParameters() throws InvocationTargetException, IllegalAccessException {
90+
String requestPath = "/api/v1/resource/123/details";
91+
String handlerPath = "/api/v1/resource/{id}/details";
92+
assertTrue((Boolean) restPathMatches.invoke(securityRestFilter, requestPath, handlerPath));
93+
}
94+
95+
@Test
96+
public void testPathsDoNotMatchWithNonMatchingNamedParameterSegment() throws InvocationTargetException, IllegalAccessException {
97+
String requestPath = "/api/v1/resource/123/details";
98+
String handlerPath = "/api/v1/resource/{id}/summary";
7899
assertFalse((Boolean) restPathMatches.invoke(securityRestFilter, requestPath, handlerPath));
79100
}
80101

81102
@Test
82-
public void testRequestPathWithExtraSegments() throws InvocationTargetException, IllegalAccessException {
83-
String requestPath = "_plugins/security/api/x/y/z";
84-
String handlerPath = "_plugins/security/api/x/y";
103+
public void testDifferentSegmentCount() throws InvocationTargetException, IllegalAccessException {
104+
String requestPath = "/api/v1/resource/123/extra";
105+
String handlerPath = "/api/v1/resource/{id}";
85106
assertFalse((Boolean) restPathMatches.invoke(securityRestFilter, requestPath, handlerPath));
86107
}
87108
}

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

+3
Original file line numberDiff line numberDiff line change
@@ -100,4 +100,7 @@ public void testDoesCallDelegateOnSuccessfulAuthorization() throws Exception {
100100

101101
verify(testRestHandlerSpy).handleRequest(any(), any(), any());
102102
}
103+
104+
// unit tests for restPathMatches are in RestPathMatchesTests.java
105+
103106
}

0 commit comments

Comments
 (0)