From 6539160bc82c404390e45fc49aec1a7103567682 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Thu, 11 Apr 2024 12:35:35 -0400 Subject: [PATCH 1/2] Ensure that challenge response contains body Signed-off-by: Craig Perkins --- .../opensearch/security/http/BasicAuthTests.java | 13 +++++++++++++ .../security/http/HTTPBasicAuthenticator.java | 6 +++++- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/integrationTest/java/org/opensearch/security/http/BasicAuthTests.java b/src/integrationTest/java/org/opensearch/security/http/BasicAuthTests.java index 1e424ab115..a9888d281e 100644 --- a/src/integrationTest/java/org/opensearch/security/http/BasicAuthTests.java +++ b/src/integrationTest/java/org/opensearch/security/http/BasicAuthTests.java @@ -106,6 +106,19 @@ public void testBrowserShouldRequestForCredentials() { } } + @Test + public void shouldRespondWithChallengeWhenNoCredentialsArePresent() { + try (TestRestClient client = cluster.getRestClient()) { + HttpResponse response = client.getAuthInfo(); + + assertThat(response, is(notNullValue())); + response.assertStatusCode(SC_UNAUTHORIZED); + assertThat(response.getHeader("WWW-Authenticate"), is(notNullValue())); + assertThat(response.getHeader("WWW-Authenticate").getValue(), equalTo("Basic realm=\"OpenSearch Security\"")); + assertThat(response.getBody(), equalTo("Unauthorized")); + } + } + @Test public void testUserShouldNotHaveAssignedCustomAttributes() { try (TestRestClient client = cluster.getRestClient(TEST_USER)) { diff --git a/src/main/java/org/opensearch/security/http/HTTPBasicAuthenticator.java b/src/main/java/org/opensearch/security/http/HTTPBasicAuthenticator.java index ff07db147e..4aec26db3d 100644 --- a/src/main/java/org/opensearch/security/http/HTTPBasicAuthenticator.java +++ b/src/main/java/org/opensearch/security/http/HTTPBasicAuthenticator.java @@ -68,7 +68,11 @@ public AuthCredentials extractCredentials(final SecurityRequest request, final T @Override public Optional reRequestAuthentication(final SecurityRequest request, AuthCredentials creds) { return Optional.of( - new SecurityResponse(HttpStatus.SC_UNAUTHORIZED, Map.of("WWW-Authenticate", "Basic realm=\"OpenSearch Security\""), "") + new SecurityResponse( + HttpStatus.SC_UNAUTHORIZED, + Map.of("WWW-Authenticate", "Basic realm=\"OpenSearch Security\""), + "Unauthorized" + ) ); } From a05a28c9ff68fd7a90adabc928b59dc74e79cbfb Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Mon, 15 Apr 2024 20:06:51 -0400 Subject: [PATCH 2/2] Only return status code in resource-focused tests Signed-off-by: Craig Perkins --- .../org/opensearch/security/ResourceFocusedTests.java | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/integrationTest/java/org/opensearch/security/ResourceFocusedTests.java b/src/integrationTest/java/org/opensearch/security/ResourceFocusedTests.java index 61a1e32023..7264a33542 100644 --- a/src/integrationTest/java/org/opensearch/security/ResourceFocusedTests.java +++ b/src/integrationTest/java/org/opensearch/security/ResourceFocusedTests.java @@ -41,6 +41,8 @@ import org.opensearch.test.framework.cluster.LocalCluster; import org.opensearch.test.framework.cluster.TestRestClient; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; import static org.opensearch.action.support.WriteRequest.RefreshPolicy.IMMEDIATE; import static org.opensearch.test.framework.TestSecurityConfig.AuthcDomain.AUTHC_HTTPBASIC_INTERNAL; import static org.opensearch.test.framework.TestSecurityConfig.Role.ALL_ACCESS; @@ -127,11 +129,13 @@ private void runResourceTest( final var requests = AsyncActions.generate(() -> { final HttpPost post = new HttpPost(client.getHttpServerUri() + requestPath); post.setEntity(new ByteArrayEntity(compressedRequestBody, ContentType.APPLICATION_JSON)); - return client.executeRequest(post); + TestRestClient.HttpResponse response = client.executeRequest(post); + return response.getStatusCode(); }, parrallelism, totalNumberOfRequests); - AsyncActions.getAll(requests, 2, TimeUnit.MINUTES) - .forEach((response) -> { response.assertStatusCode(HttpStatus.SC_UNAUTHORIZED); }); + AsyncActions.getAll(requests, 2, TimeUnit.MINUTES).forEach((responseCode) -> { + assertThat(responseCode, equalTo(HttpStatus.SC_UNAUTHORIZED)); + }); } }