Skip to content

Commit

Permalink
Update restPathMatches to handle case with missing leading slash (#…
Browse files Browse the repository at this point in the history
…5061)

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>
Co-authored-by: Darshit Chanpura <dchanp@amazon.com>
  • Loading branch information
cwperks and DarshitChanpura authored Jan 27, 2025
1 parent 9b73490 commit 772eef6
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -100,4 +100,7 @@ public void testDoesCallDelegateOnSuccessfulAuthorization() throws Exception {

verify(testRestHandlerSpy).handleRequest(any(), any(), any());
}

// unit tests for restPathMatches are in RestPathMatchesTests.java

}

0 comments on commit 772eef6

Please sign in to comment.