From 4bfaf9c22e785b484d50433a6c353cd900fb024b Mon Sep 17 00:00:00 2001 From: Peter Nied Date: Wed, 11 Oct 2023 11:51:02 -0400 Subject: [PATCH] Forward port integration tests (#3512) Forward ports integration test changes from 2.11 and 2.x branches Signed-off-by: Peter Nied Signed-off-by: Peter Nied Co-authored-by: opensearch-trigger-bot[bot] <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com> --- .github/workflows/ci.yml | 6 +-- .github/workflows/code-hygiene.yml | 2 +- build.gradle | 14 ++++-- .../security/CrossClusterSearchTests.java | 2 + .../security/DoNotFailOnForbiddenTests.java | 9 +++- .../IpBruteForceAttacksPreventionTests.java | 12 ++--- .../security/PointInTimeOperationTest.java | 11 ++-- .../security/ResourceFocusedTests.java | 14 +++--- .../security/SearchOperationTest.java | 14 ++++++ .../security/SecurityRolesTests.java | 2 +- .../UserBruteForceAttacksPreventionTests.java | 4 +- .../security/api/DashboardsInfoTest.java | 9 ++-- .../api/DashboardsInfoWithSettingsTest.java | 22 ++++---- .../security/http/BasicAuthTests.java | 6 +-- .../http/BasicAuthWithoutChallengeTests.java | 2 +- .../http/CertificateAuthenticationTest.java | 2 +- .../security/http/DisabledBasicAuthTests.java | 2 +- .../http/OnBehalfOfJwtAuthenticationTest.java | 17 +++---- .../privileges/PrivilegesEvaluatorTest.java | 2 +- .../cluster/CloseableHttpClientFactory.java | 6 +-- .../cluster/OpenSearchClientProvider.java | 1 - .../framework/cluster/TestRestClient.java | 50 +++++++++---------- .../cluster/TestRestClientConfiguration.java | 25 ++++++---- .../testplugins/AbstractRestHandler.java | 16 +++--- .../http/saml/AuthTokenProcessorHandler.java | 2 +- .../security/auth/BackendRegistry.java | 4 +- .../security/filter/SecurityResponse.java | 22 ++++++++ .../security/filter/SecurityRestFilter.java | 6 +-- .../Netty4HttpRequestHeaderVerifier.java | 2 +- 29 files changed, 169 insertions(+), 117 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fbafa851e2..73d1f12f4c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -87,8 +87,8 @@ jobs: strategy: fail-fast: false matrix: - jdk: [17] - platform: [ubuntu-latest, windows-latest] + jdk: [11, 17] + platform: [ubuntu-latest] # Removed windows https://github.com/opensearch-project/security/issues/3423 runs-on: ${{ matrix.platform }} steps: @@ -103,7 +103,6 @@ jobs: - name: Build and Test uses: gradle/gradle-build-action@v2 - continue-on-error: true # Until retries are enable do not fail the workflow https://github.com/opensearch-project/security/issues/2184 with: cache-disabled: true arguments: | @@ -135,6 +134,7 @@ jobs: cache-disabled: true arguments: | integrationTest -Dbuild.snapshot=false --tests org.opensearch.security.ResourceFocusedTests + backward-compatibility-build: runs-on: ubuntu-latest steps: diff --git a/.github/workflows/code-hygiene.yml b/.github/workflows/code-hygiene.yml index 7aca45f064..1b46c65a63 100644 --- a/.github/workflows/code-hygiene.yml +++ b/.github/workflows/code-hygiene.yml @@ -43,7 +43,7 @@ jobs: - uses: gradle/gradle-build-action@v2 with: cache-disabled: true - arguments: checkstyleMain checkstyleTest + arguments: checkstyleMain checkstyleTest checkstyleIntegrationTest spotbugs: runs-on: ubuntu-latest diff --git a/build.gradle b/build.gradle index fa6abb64e1..719c3cb555 100644 --- a/build.gradle +++ b/build.gradle @@ -432,6 +432,10 @@ configurations { force "com.github.luben:zstd-jni:${versions.zstd}" force "org.xerial.snappy:snappy-java:1.1.10.5" force "com.google.guava:guava:${guava_version}" + + // For integrationTest + force "org.apache.httpcomponents:httpclient:4.5.13" + force "org.apache.httpcomponents:httpcore:4.4.16" } } @@ -447,6 +451,7 @@ sourceSets { srcDir file ('src/integrationTest/java') compileClasspath += sourceSets.main.output runtimeClasspath += sourceSets.main.output + } resources { srcDir file('src/integrationTest/resources') @@ -461,9 +466,7 @@ sourceSets { task integrationTest(type: Test) { doFirst { // Only run resources tests on resource-test CI environments or locally - if (System.getenv('CI_ENVIRONMENT') == 'resource-test' || System.getenv('CI_ENVIRONMENT') == null) { - include '**/ResourceFocusedTests.class' - } else { + if (System.getenv('CI_ENVIRONMENT') != 'resource-test' && System.getenv('CI_ENVIRONMENT') != null) { exclude '**/ResourceFocusedTests.class' } // Only run with retries while in CI systems @@ -648,6 +651,11 @@ dependencies { exclude(group: 'org.hamcrest', module: 'hamcrest') } integrationTestImplementation 'com.unboundid:unboundid-ldapsdk:4.0.14' + integrationTestImplementation "org.apache.httpcomponents:httpclient-cache:4.5.13" + integrationTestImplementation "org.apache.httpcomponents:httpclient:4.5.13" + integrationTestImplementation "org.apache.httpcomponents:fluent-hc:4.5.13" + integrationTestImplementation "org.apache.httpcomponents:httpcore:4.4.13" + integrationTestImplementation "org.apache.httpcomponents:httpasyncclient:4.1.5" //spotless implementation('com.google.googlejavaformat:google-java-format:1.17.0') { diff --git a/src/integrationTest/java/org/opensearch/security/CrossClusterSearchTests.java b/src/integrationTest/java/org/opensearch/security/CrossClusterSearchTests.java index 86d27efa87..d9e4d2b5f3 100644 --- a/src/integrationTest/java/org/opensearch/security/CrossClusterSearchTests.java +++ b/src/integrationTest/java/org/opensearch/security/CrossClusterSearchTests.java @@ -18,6 +18,7 @@ import org.apache.commons.lang3.tuple.Pair; import org.junit.BeforeClass; import org.junit.ClassRule; +import org.junit.Ignore; import org.junit.Test; import org.junit.runner.RunWith; @@ -64,6 +65,7 @@ * This is a parameterized test so that one test class is used to test security plugin behaviour when ccsMinimizeRoundtrips * option is enabled or disabled. Method {@link #parameters()} is a source of parameters values. */ +@Ignore("Setting up two clusters at once seems to be prone to issues where they have port mismatches") @RunWith(com.carrotsearch.randomizedtesting.RandomizedRunner.class) @ThreadLeakScope(ThreadLeakScope.Scope.NONE) public class CrossClusterSearchTests { diff --git a/src/integrationTest/java/org/opensearch/security/DoNotFailOnForbiddenTests.java b/src/integrationTest/java/org/opensearch/security/DoNotFailOnForbiddenTests.java index 138a57e60b..afbb9f38ae 100644 --- a/src/integrationTest/java/org/opensearch/security/DoNotFailOnForbiddenTests.java +++ b/src/integrationTest/java/org/opensearch/security/DoNotFailOnForbiddenTests.java @@ -44,7 +44,14 @@ import org.opensearch.test.framework.cluster.LocalCluster; import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.*; +import static org.hamcrest.Matchers.allOf; +import static org.hamcrest.Matchers.aMapWithSize; +import static org.hamcrest.Matchers.arrayContainingInAnyOrder; +import static org.hamcrest.Matchers.arrayWithSize; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.hasKey; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.nullValue; import static org.opensearch.action.admin.indices.alias.IndicesAliasesRequest.AliasActions.Type.ADD; import static org.opensearch.action.support.WriteRequest.RefreshPolicy.IMMEDIATE; import static org.opensearch.client.RequestOptions.DEFAULT; diff --git a/src/integrationTest/java/org/opensearch/security/IpBruteForceAttacksPreventionTests.java b/src/integrationTest/java/org/opensearch/security/IpBruteForceAttacksPreventionTests.java index 34e79613f6..31f320a654 100644 --- a/src/integrationTest/java/org/opensearch/security/IpBruteForceAttacksPreventionTests.java +++ b/src/integrationTest/java/org/opensearch/security/IpBruteForceAttacksPreventionTests.java @@ -26,8 +26,8 @@ import org.opensearch.test.framework.cluster.TestRestClientConfiguration; import org.opensearch.test.framework.log.LogsRule; -import static org.apache.hc.core5.http.HttpStatus.SC_OK; -import static org.apache.hc.core5.http.HttpStatus.SC_UNAUTHORIZED; +import static org.apache.http.HttpStatus.SC_OK; +import static org.apache.http.HttpStatus.SC_UNAUTHORIZED; import static org.opensearch.test.framework.TestSecurityConfig.AuthcDomain.AUTHC_HTTPBASIC_INTERNAL_WITHOUT_CHALLENGE; import static org.opensearch.test.framework.TestSecurityConfig.Role.ALL_ACCESS; import static org.opensearch.test.framework.cluster.TestRestClientConfiguration.userWithSourceIp; @@ -35,8 +35,8 @@ @RunWith(com.carrotsearch.randomizedtesting.RandomizedRunner.class) @ThreadLeakScope(ThreadLeakScope.Scope.NONE) public class IpBruteForceAttacksPreventionTests { - static final User USER_1 = new User("simple-user-1").roles(ALL_ACCESS); - static final User USER_2 = new User("simple-user-2").roles(ALL_ACCESS); + protected static final User USER_1 = new User("simple-user-1").roles(ALL_ACCESS); + protected static final User USER_2 = new User("simple-user-2").roles(ALL_ACCESS); public static final int ALLOWED_TRIES = 3; public static final int TIME_WINDOW_SECONDS = 3; @@ -50,7 +50,7 @@ public class IpBruteForceAttacksPreventionTests { public static final String CLIENT_IP_8 = "127.0.0.8"; public static final String CLIENT_IP_9 = "127.0.0.9"; - static final AuthFailureListeners listener = new AuthFailureListeners().addRateLimit( + protected static final AuthFailureListeners listener = new AuthFailureListeners().addRateLimit( new RateLimiting("internal_authentication_backend_limiting").type("ip") .allowedTries(ALLOWED_TRIES) .timeWindowSeconds(TIME_WINDOW_SECONDS) @@ -154,7 +154,7 @@ public void shouldReleaseIpAddressLock() throws InterruptedException { } } - void authenticateUserWithIncorrectPassword(String sourceIpAddress, User user, int numberOfRequests) { + private void authenticateUserWithIncorrectPassword(String sourceIpAddress, User user, int numberOfRequests) { var clientConfiguration = new TestRestClientConfiguration().username(user.getName()) .password("incorrect password") .sourceInetAddress(sourceIpAddress); diff --git a/src/integrationTest/java/org/opensearch/security/PointInTimeOperationTest.java b/src/integrationTest/java/org/opensearch/security/PointInTimeOperationTest.java index 3b01ca69bf..3d634c4a5d 100644 --- a/src/integrationTest/java/org/opensearch/security/PointInTimeOperationTest.java +++ b/src/integrationTest/java/org/opensearch/security/PointInTimeOperationTest.java @@ -16,6 +16,7 @@ import org.apache.commons.lang3.tuple.Pair; import org.junit.BeforeClass; import org.junit.ClassRule; +import org.junit.Ignore; import org.junit.Test; import org.junit.runner.RunWith; @@ -179,6 +180,7 @@ public void createPitWithIndexAlias_negative() throws IOException { } } + @Ignore("Pretty sure cleanUpPits is returning before all of the PITs have actually been deleted") @Test public void listAllPits_positive() throws IOException { try (RestHighLevelClient restHighLevelClient = cluster.getRestHighLevelClient(POINT_IN_TIME_USER)) { @@ -245,6 +247,7 @@ public void deletePitCreatedWithIndexAlias_negative() throws IOException { } } + @Ignore("Pretty sure cleanUpPits is returning before all of the PITs have actually been deleted") @Test public void deleteAllPits_positive() throws IOException { try (RestHighLevelClient restHighLevelClient = cluster.getRestHighLevelClient(POINT_IN_TIME_USER)) { @@ -336,7 +339,7 @@ public void listPitSegments_positive() throws IOException { try (TestRestClient restClient = cluster.getRestClient(LIMITED_POINT_IN_TIME_USER)) { String existingPitId = createPitForIndices(FIRST_SONG_INDEX); String body = String.format("{\"pit_id\":[\"%s\"]}", existingPitId); - HttpResponse response = restClient.getWithJsonBody("/_cat/pit_segments", body); + HttpResponse response = restClient.getWithJsonBody("_cat/pit_segments", body); response.assertStatusCode(OK.getStatus()); } @@ -347,7 +350,7 @@ public void listPitSegmentsCreatedWithIndexAlias_positive() throws IOException { try (TestRestClient restClient = cluster.getRestClient(POINT_IN_TIME_USER)) { String existingPitId = createPitForIndices(FIRST_INDEX_ALIAS); String body = String.format("{\"pit_id\":[\"%s\"]}", existingPitId); - HttpResponse response = restClient.getWithJsonBody("/_cat/pit_segments", body); + HttpResponse response = restClient.getWithJsonBody("_cat/pit_segments", body); response.assertStatusCode(OK.getStatus()); } @@ -358,7 +361,7 @@ public void listPitSegments_negative() throws IOException { try (TestRestClient restClient = cluster.getRestClient(LIMITED_POINT_IN_TIME_USER)) { String existingPitId = createPitForIndices(SECOND_SONG_INDEX); String body = String.format("{\"pit_id\":[\"%s\"]}", existingPitId); - HttpResponse response = restClient.getWithJsonBody("/_cat/pit_segments", body); + HttpResponse response = restClient.getWithJsonBody("_cat/pit_segments", body); response.assertStatusCode(FORBIDDEN.getStatus()); } @@ -369,7 +372,7 @@ public void listPitSegmentsCreatedWithIndexAlias_negative() throws IOException { try (TestRestClient restClient = cluster.getRestClient(LIMITED_POINT_IN_TIME_USER)) { String existingPitId = createPitForIndices(SECOND_INDEX_ALIAS); String body = String.format("{\"pit_id\":[\"%s\"]}", existingPitId); - HttpResponse response = restClient.getWithJsonBody("/_cat/pit_segments", body); + HttpResponse response = restClient.getWithJsonBody("_cat/pit_segments", body); response.assertStatusCode(FORBIDDEN.getStatus()); } diff --git a/src/integrationTest/java/org/opensearch/security/ResourceFocusedTests.java b/src/integrationTest/java/org/opensearch/security/ResourceFocusedTests.java index 2ab19d15ce..3899dfa111 100644 --- a/src/integrationTest/java/org/opensearch/security/ResourceFocusedTests.java +++ b/src/integrationTest/java/org/opensearch/security/ResourceFocusedTests.java @@ -139,7 +139,7 @@ private Long runResourceTest( CompletableFuture statPrinter = statsPrinter ? CompletableFuture.runAsync(() -> { while (true) { printStats(); - System.out.println(" & Succesful completions: " + getCount.get()); + System.err.println(" & Succesful completions: " + getCount.get()); try { Thread.sleep(500); } catch (Exception e) { @@ -159,7 +159,7 @@ private Long runResourceTest( if (statsPrinter) { printStats(); - System.out.println(" & Succesful completions: " + getCount.get()); + System.err.println(" & Succesful completions: " + getCount.get()); } return getCount.get(); } @@ -207,7 +207,7 @@ private byte[] createCompressedRequestBody(final RequestBodySize size) { gzipOutputStream.finish(); final byte[] compressedRequestBody = byteArrayOutputStream.toByteArray(); - System.out.println( + System.err.println( "^^^" + String.format( "Original size was %,d bytes, compressed to %,d bytes, ratio %,.2f", @@ -223,7 +223,7 @@ private byte[] createCompressedRequestBody(final RequestBodySize size) { } private void printStats() { - System.out.println("** Stats "); + System.err.println("** Stats "); printMemory(); printMemoryPools(); printGCPools(); @@ -236,21 +236,21 @@ private void printMemory() { final long freeMemory = runtime.freeMemory(); // Amount of free memory final long usedMemory = totalMemory - freeMemory; // Amount of used memory - System.out.println(" Memory Total: " + totalMemory + " Free:" + freeMemory + " Used:" + usedMemory); + System.err.println(" Memory Total: " + totalMemory + " Free:" + freeMemory + " Used:" + usedMemory); } private void printMemoryPools() { List memoryPools = ManagementFactory.getMemoryPoolMXBeans(); for (MemoryPoolMXBean memoryPool : memoryPools) { MemoryUsage usage = memoryPool.getUsage(); - System.out.println(" " + memoryPool.getName() + " USED: " + usage.getUsed() + " MAX: " + usage.getMax()); + System.err.println(" " + memoryPool.getName() + " USED: " + usage.getUsed() + " MAX: " + usage.getMax()); } } private void printGCPools() { List garbageCollectors = ManagementFactory.getGarbageCollectorMXBeans(); for (GarbageCollectorMXBean garbageCollector : garbageCollectors) { - System.out.println(" " + garbageCollector.getName() + " COLLECTION TIME: " + garbageCollector.getCollectionTime()); + System.err.println(" " + garbageCollector.getName() + " COLLECTION TIME: " + garbageCollector.getCollectionTime()); } } diff --git a/src/integrationTest/java/org/opensearch/security/SearchOperationTest.java b/src/integrationTest/java/org/opensearch/security/SearchOperationTest.java index f16d40e905..a38d26800a 100644 --- a/src/integrationTest/java/org/opensearch/security/SearchOperationTest.java +++ b/src/integrationTest/java/org/opensearch/security/SearchOperationTest.java @@ -975,6 +975,7 @@ public void shouldIndexDocumentInBulkRequest_positive() throws IOException { auditLogsRule.assertAtLeast(2, grantedPrivilege(LIMITED_WRITE_USER, "PutMappingRequest"));// sometimes 2 or 4 } + @Ignore("Audit log verification is shown to be flaky in this test") @Test public void shouldIndexDocumentInBulkRequest_partiallyPositive() throws IOException { try (RestHighLevelClient restHighLevelClient = cluster.getRestHighLevelClient(LIMITED_WRITE_USER)) { @@ -1029,6 +1030,7 @@ public void shouldIndexDocumentInBulkRequest_negative() throws IOException { auditLogsRule.assertExactlyOne(missingPrivilege(LIMITED_WRITE_USER, "BulkShardRequest").withIndex(SONG_INDEX_NAME)); } + @Ignore("Audit log verification is shown to be flaky in this test") @Test public void shouldUpdateDocumentsInBulk_positive() throws IOException { try (RestHighLevelClient restHighLevelClient = cluster.getRestHighLevelClient(LIMITED_WRITE_USER)) { @@ -1056,6 +1058,7 @@ public void shouldUpdateDocumentsInBulk_positive() throws IOException { } + @Ignore("Audit log verification is shown to be flaky in this test") @Test public void shouldUpdateDocumentsInBulk_partiallyPositive() throws IOException { try (RestHighLevelClient restHighLevelClient = cluster.getRestHighLevelClient(LIMITED_WRITE_USER)) { @@ -1140,6 +1143,7 @@ public void shouldDeleteDocumentInBulk_positive() throws IOException { auditLogsRule.assertExactly(6, auditPredicate(INDEX_EVENT).withEffectiveUser(LIMITED_WRITE_USER)); } + @Ignore("Audit log verification is shown to be flaky in this test") @Test public void shouldDeleteDocumentInBulk_partiallyPositive() throws IOException { try (RestHighLevelClient restHighLevelClient = cluster.getRestHighLevelClient(LIMITED_WRITE_USER)) { @@ -1198,6 +1202,7 @@ public void shouldDeleteDocumentInBulk_negative() throws IOException { } + @Ignore("Seems like reindixing isn't completing before the test proceeds") @Test public void shouldReindexDocuments_positive() throws IOException { try (RestHighLevelClient restHighLevelClient = cluster.getRestHighLevelClient(REINDEXING_USER)) { @@ -1223,6 +1228,7 @@ public void shouldReindexDocuments_positive() throws IOException { auditLogsRule.assertExactlyOne(missingPrivilege(REINDEXING_USER, "ClearScrollRequest")); } + @Ignore("Seems like reindixing isn't completing before the test proceeds") @Test public void shouldReindexDocuments_negativeSource() throws IOException { try (RestHighLevelClient restHighLevelClient = cluster.getRestHighLevelClient(REINDEXING_USER)) { @@ -1237,6 +1243,7 @@ public void shouldReindexDocuments_negativeSource() throws IOException { auditLogsRule.assertExactlyOne(missingPrivilege(REINDEXING_USER, "SearchRequest")); } + @Ignore("Seems like reindixing isn't completing before the test proceeds") @Test public void shouldReindexDocuments_negativeDestination() throws IOException { try (RestHighLevelClient restHighLevelClient = cluster.getRestHighLevelClient(REINDEXING_USER)) { @@ -1255,6 +1262,7 @@ public void shouldReindexDocuments_negativeDestination() throws IOException { auditLogsRule.assertExactlyOne(missingPrivilege(REINDEXING_USER, "ClearScrollRequest")); } + @Ignore("Seems like reindixing isn't completing before the test proceeds") @Test public void shouldReindexDocuments_negativeSourceAndDestination() throws IOException { try (RestHighLevelClient restHighLevelClient = cluster.getRestHighLevelClient(REINDEXING_USER)) { @@ -1327,6 +1335,7 @@ public void shouldDeleteDocument_negative() throws IOException { } } + @Ignore("Create alias / delete alias isn't resolving in a timely manner for this test") @Test public void shouldCreateAlias_positive() throws IOException { try (RestHighLevelClient restHighLevelClient = cluster.getRestHighLevelClient(LIMITED_READ_USER)) { @@ -1344,6 +1353,7 @@ public void shouldCreateAlias_positive() throws IOException { auditLogsRule.assertExactly(2, auditPredicate(INDEX_EVENT).withEffectiveUser(LIMITED_READ_USER)); } + @Ignore("Create alias / delete alias isn't resolving in a timely manner for this test") @Test public void shouldCreateAlias_negative() throws IOException { try (RestHighLevelClient restHighLevelClient = cluster.getRestHighLevelClient(LIMITED_READ_USER)) { @@ -1361,6 +1371,7 @@ public void shouldCreateAlias_negative() throws IOException { auditLogsRule.assertExactlyOne(missingPrivilege(LIMITED_READ_USER, "IndicesAliasesRequest")); } + @Ignore("Create alias / delete alias isn't resolving in a timely manner for this test") @Test public void shouldDeleteAlias_positive() throws IOException { try (RestHighLevelClient restHighLevelClient = cluster.getRestHighLevelClient(LIMITED_READ_USER)) { @@ -1398,6 +1409,7 @@ public void shouldDeleteAlias_negative() throws IOException { auditLogsRule.assertExactlyOne(missingPrivilege(LIMITED_READ_USER, "IndicesAliasesRequest")); } + @Ignore("Create alias / delete alias isn't resolving in a timely manner for this test") @Test public void shouldCreateIndexTemplate_positive() throws IOException { try (RestHighLevelClient restHighLevelClient = cluster.getRestHighLevelClient(LIMITED_WRITE_USER)) { @@ -1479,6 +1491,7 @@ public void shouldDeleteTemplate_negative() throws IOException { auditLogsRule.assertExactlyOne(missingPrivilege(LIMITED_READ_USER, "DeleteIndexTemplateRequest")); } + @Ignore("Create alias / delete alias isn't resolving in a timely manner for this test") @Test public void shouldUpdateTemplate_positive() throws IOException { try (RestHighLevelClient restHighLevelClient = cluster.getRestHighLevelClient(LIMITED_WRITE_USER)) { @@ -1765,6 +1778,7 @@ public void shouldDeleteSnapshot_negative() throws IOException { auditLogsRule.assertAtLeast(2, grantedPrivilege(LIMITED_WRITE_USER, "GetSnapshotsRequest")); } + @Ignore("Audit log entries verifcation isn't always consistant") @Test public void shouldRestoreSnapshot_positive() throws IOException { final String snapshotName = "restore-snapshot-positive"; diff --git a/src/integrationTest/java/org/opensearch/security/SecurityRolesTests.java b/src/integrationTest/java/org/opensearch/security/SecurityRolesTests.java index fcccaf53b3..ce2376c616 100644 --- a/src/integrationTest/java/org/opensearch/security/SecurityRolesTests.java +++ b/src/integrationTest/java/org/opensearch/security/SecurityRolesTests.java @@ -12,7 +12,7 @@ package org.opensearch.security; import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope; -import org.apache.hc.core5.http.HttpStatus; +import org.apache.http.HttpStatus; import org.junit.ClassRule; import org.junit.Test; import org.junit.runner.RunWith; diff --git a/src/integrationTest/java/org/opensearch/security/UserBruteForceAttacksPreventionTests.java b/src/integrationTest/java/org/opensearch/security/UserBruteForceAttacksPreventionTests.java index 2da3446e75..cb61950ada 100644 --- a/src/integrationTest/java/org/opensearch/security/UserBruteForceAttacksPreventionTests.java +++ b/src/integrationTest/java/org/opensearch/security/UserBruteForceAttacksPreventionTests.java @@ -26,8 +26,8 @@ import org.opensearch.test.framework.cluster.TestRestClient.HttpResponse; import org.opensearch.test.framework.log.LogsRule; -import static org.apache.hc.core5.http.HttpStatus.SC_OK; -import static org.apache.hc.core5.http.HttpStatus.SC_UNAUTHORIZED; +import static org.apache.http.HttpStatus.SC_OK; +import static org.apache.http.HttpStatus.SC_UNAUTHORIZED; import static org.opensearch.test.framework.TestSecurityConfig.AuthcDomain.AUTHC_HTTPBASIC_INTERNAL; import static org.opensearch.test.framework.TestSecurityConfig.Role.ALL_ACCESS; diff --git a/src/integrationTest/java/org/opensearch/security/api/DashboardsInfoTest.java b/src/integrationTest/java/org/opensearch/security/api/DashboardsInfoTest.java index a1dbc611a3..8bfcd3b8a8 100644 --- a/src/integrationTest/java/org/opensearch/security/api/DashboardsInfoTest.java +++ b/src/integrationTest/java/org/opensearch/security/api/DashboardsInfoTest.java @@ -12,7 +12,7 @@ package org.opensearch.security.api; import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope; -import org.apache.hc.core5.http.HttpStatus; +import org.apache.http.HttpStatus; import org.junit.ClassRule; import org.junit.Test; import org.junit.runner.RunWith; @@ -24,7 +24,6 @@ import org.opensearch.test.framework.cluster.TestRestClient; import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.opensearch.security.rest.DashboardsInfoAction.DEFAULT_PASSWORD_MESSAGE; import static org.opensearch.security.rest.DashboardsInfoAction.DEFAULT_PASSWORD_REGEX; @@ -50,10 +49,8 @@ public void testDashboardsInfoValidationMessage() throws Exception { try (TestRestClient client = cluster.getRestClient(DASHBOARDS_USER)) { TestRestClient.HttpResponse response = client.get("_plugins/_security/dashboardsinfo"); assertThat(response.getStatusCode(), equalTo(HttpStatus.SC_OK)); - assertThat(response.getBody(), containsString("password_validation_error_message")); - assertThat(response.getBody(), containsString(DEFAULT_PASSWORD_MESSAGE)); - assertThat(response.getBody(), containsString("password_validation_regex")); - assertThat(response.getBody(), containsString(DEFAULT_PASSWORD_REGEX)); + assertThat(response.getTextFromJsonBody("/password_validation_error_message"), equalTo(DEFAULT_PASSWORD_MESSAGE)); + assertThat(response.getTextFromJsonBody("/password_validation_regex"), equalTo(DEFAULT_PASSWORD_REGEX)); } } } diff --git a/src/integrationTest/java/org/opensearch/security/api/DashboardsInfoWithSettingsTest.java b/src/integrationTest/java/org/opensearch/security/api/DashboardsInfoWithSettingsTest.java index 49f3872420..7807798210 100644 --- a/src/integrationTest/java/org/opensearch/security/api/DashboardsInfoWithSettingsTest.java +++ b/src/integrationTest/java/org/opensearch/security/api/DashboardsInfoWithSettingsTest.java @@ -11,8 +11,13 @@ package org.opensearch.security.api; -import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope; -import org.apache.hc.core5.http.HttpStatus; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.opensearch.test.framework.TestSecurityConfig.AuthcDomain.AUTHC_HTTPBASIC_INTERNAL; + +import java.util.Map; + +import org.apache.http.HttpStatus; import org.junit.ClassRule; import org.junit.Test; import org.junit.runner.RunWith; @@ -23,12 +28,7 @@ import org.opensearch.test.framework.cluster.LocalCluster; import org.opensearch.test.framework.cluster.TestRestClient; -import java.util.Map; - -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.equalTo; -import static org.opensearch.test.framework.TestSecurityConfig.AuthcDomain.AUTHC_HTTPBASIC_INTERNAL; +import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope; @RunWith(com.carrotsearch.randomizedtesting.RandomizedRunner.class) @ThreadLeakScope(ThreadLeakScope.Scope.NONE) @@ -63,10 +63,8 @@ public void testDashboardsInfoValidationMessageWithCustomMessage() throws Except try (TestRestClient client = cluster.getRestClient(DASHBOARDS_USER)) { TestRestClient.HttpResponse response = client.get("_plugins/_security/dashboardsinfo"); assertThat(response.getStatusCode(), equalTo(HttpStatus.SC_OK)); - assertThat(response.getBody(), containsString("password_validation_error_message")); - assertThat(response.getBody(), containsString(CUSTOM_PASSWORD_MESSAGE)); - assertThat(response.getBody(), containsString("password_validation_regex")); - assertThat(response.getBody(), containsString(CUSTOM_PASSWORD_REGEX)); + assertThat(response.getTextFromJsonBody("/password_validation_error_message"), equalTo(CUSTOM_PASSWORD_MESSAGE)); + assertThat(response.getTextFromJsonBody("/password_validation_regex"), equalTo(CUSTOM_PASSWORD_REGEX)); } } } diff --git a/src/integrationTest/java/org/opensearch/security/http/BasicAuthTests.java b/src/integrationTest/java/org/opensearch/security/http/BasicAuthTests.java index 3f4ada4c68..f6b1672bbe 100644 --- a/src/integrationTest/java/org/opensearch/security/http/BasicAuthTests.java +++ b/src/integrationTest/java/org/opensearch/security/http/BasicAuthTests.java @@ -12,7 +12,7 @@ import java.util.List; import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope; -import org.apache.hc.core5.http.HttpHeaders; +import org.apache.http.HttpHeaders; import org.hamcrest.Matchers; import org.junit.ClassRule; import org.junit.Test; @@ -25,8 +25,8 @@ import org.opensearch.test.framework.cluster.TestRestClient; import org.opensearch.test.framework.cluster.TestRestClient.HttpResponse; -import static org.apache.hc.core5.http.HttpStatus.SC_OK; -import static org.apache.hc.core5.http.HttpStatus.SC_UNAUTHORIZED; +import static org.apache.http.HttpStatus.SC_OK; +import static org.apache.http.HttpStatus.SC_UNAUTHORIZED; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsStringIgnoringCase; import static org.hamcrest.Matchers.equalTo; diff --git a/src/integrationTest/java/org/opensearch/security/http/BasicAuthWithoutChallengeTests.java b/src/integrationTest/java/org/opensearch/security/http/BasicAuthWithoutChallengeTests.java index 940f326cc1..d602d0920b 100644 --- a/src/integrationTest/java/org/opensearch/security/http/BasicAuthWithoutChallengeTests.java +++ b/src/integrationTest/java/org/opensearch/security/http/BasicAuthWithoutChallengeTests.java @@ -10,7 +10,7 @@ package org.opensearch.security.http; import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope; -import org.apache.hc.core5.http.HttpHeaders; +import org.apache.http.HttpHeaders; import org.junit.ClassRule; import org.junit.Test; import org.junit.runner.RunWith; diff --git a/src/integrationTest/java/org/opensearch/security/http/CertificateAuthenticationTest.java b/src/integrationTest/java/org/opensearch/security/http/CertificateAuthenticationTest.java index 144a54c4d6..7c4d05b714 100644 --- a/src/integrationTest/java/org/opensearch/security/http/CertificateAuthenticationTest.java +++ b/src/integrationTest/java/org/opensearch/security/http/CertificateAuthenticationTest.java @@ -29,7 +29,7 @@ import org.opensearch.test.framework.cluster.TestRestClient; import org.opensearch.test.framework.cluster.TestRestClient.HttpResponse; -import static org.apache.hc.core5.http.HttpStatus.SC_OK; +import static org.apache.http.HttpStatus.SC_OK; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.hasSize; diff --git a/src/integrationTest/java/org/opensearch/security/http/DisabledBasicAuthTests.java b/src/integrationTest/java/org/opensearch/security/http/DisabledBasicAuthTests.java index 7ae856ba8b..1ae3322a1e 100644 --- a/src/integrationTest/java/org/opensearch/security/http/DisabledBasicAuthTests.java +++ b/src/integrationTest/java/org/opensearch/security/http/DisabledBasicAuthTests.java @@ -19,7 +19,7 @@ import org.opensearch.test.framework.cluster.TestRestClient; import org.opensearch.test.framework.cluster.TestRestClient.HttpResponse; -import static org.apache.hc.core5.http.HttpStatus.SC_UNAUTHORIZED; +import static org.apache.http.HttpStatus.SC_UNAUTHORIZED; import static org.opensearch.security.http.BasicAuthTests.TEST_USER; import static org.opensearch.test.framework.TestSecurityConfig.AuthcDomain.DISABLED_AUTHC_HTTPBASIC_INTERNAL; import static org.opensearch.test.framework.TestSecurityConfig.AuthcDomain.JWT_AUTH_DOMAIN; diff --git a/src/integrationTest/java/org/opensearch/security/http/OnBehalfOfJwtAuthenticationTest.java b/src/integrationTest/java/org/opensearch/security/http/OnBehalfOfJwtAuthenticationTest.java index 6e4fc1c89d..266463d934 100644 --- a/src/integrationTest/java/org/opensearch/security/http/OnBehalfOfJwtAuthenticationTest.java +++ b/src/integrationTest/java/org/opensearch/security/http/OnBehalfOfJwtAuthenticationTest.java @@ -114,7 +114,7 @@ public class OnBehalfOfJwtAuthenticationTest { public void shouldAuthenticateWithOBOTokenEndPoint() { String oboToken = generateOboToken(ADMIN_USER_NAME, DEFAULT_PASSWORD); Header adminOboAuthHeader = new BasicHeader("Authorization", "Bearer " + oboToken); - authenticateWithOboToken(adminOboAuthHeader, ADMIN_USER_NAME, 200); + authenticateWithOboToken(adminOboAuthHeader, ADMIN_USER_NAME, HttpStatus.SC_OK); } @Test @@ -122,7 +122,7 @@ public void shouldNotAuthenticateWithATemperedOBOToken() { String oboToken = generateOboToken(ADMIN_USER_NAME, DEFAULT_PASSWORD); oboToken = oboToken.substring(0, oboToken.length() - 1); // tampering the token Header adminOboAuthHeader = new BasicHeader("Authorization", "Bearer " + oboToken); - authenticateWithOboToken(adminOboAuthHeader, ADMIN_USER_NAME, 401); + authenticateWithOboToken(adminOboAuthHeader, ADMIN_USER_NAME, HttpStatus.SC_UNAUTHORIZED); } @Test @@ -132,7 +132,7 @@ public void shouldNotAuthenticateForUsingOBOTokenToAccessOBOEndpoint() { try (TestRestClient client = cluster.getRestClient(adminOboAuthHeader)) { TestRestClient.HttpResponse response = client.getOnBehalfOfToken(OBO_DESCRIPTION, adminOboAuthHeader); - response.assertStatusCode(401); + response.assertStatusCode(HttpStatus.SC_UNAUTHORIZED); } } @@ -143,7 +143,7 @@ public void shouldNotAuthenticateForUsingOBOTokenToAccessAccountEndpoint() { try (TestRestClient client = cluster.getRestClient(adminOboAuthHeader)) { TestRestClient.HttpResponse response = client.changeInternalUserPassword(CURRENT_AND_NEW_PASSWORDS, adminOboAuthHeader); - response.assertStatusCode(401); + response.assertStatusCode(HttpStatus.SC_UNAUTHORIZED); } } @@ -151,7 +151,7 @@ public void shouldNotAuthenticateForUsingOBOTokenToAccessAccountEndpoint() { public void shouldAuthenticateForNonAdminUserWithOBOPermission() { String oboToken = generateOboToken(OBO_USER_NAME_WITH_PERM, DEFAULT_PASSWORD); Header oboAuthHeader = new BasicHeader("Authorization", "Bearer " + oboToken); - authenticateWithOboToken(oboAuthHeader, OBO_USER_NAME_WITH_PERM, 200); + authenticateWithOboToken(oboAuthHeader, OBO_USER_NAME_WITH_PERM, HttpStatus.SC_OK); } @Test @@ -183,7 +183,7 @@ private String generateOboToken(String username, String password) { client.assertCorrectCredentials(username); TestRestClient.HttpResponse response = client.postJson(OBO_ENDPOINT_PREFIX, OBO_TOKEN_REASON); response.assertStatusCode(200); - Map oboEndPointResponse = response.getBodyAs(Map.class); + Map oboEndPointResponse = (Map) response.getBodyAs(Map.class); assertThat( oboEndPointResponse, allOf(aMapWithSize(3), hasKey("user"), hasKey("authenticationToken"), hasKey("durationSeconds")) @@ -196,11 +196,10 @@ private void authenticateWithOboToken(Header authHeader, String expectedUsername try (TestRestClient client = cluster.getRestClient(authHeader)) { TestRestClient.HttpResponse response = client.getAuthInfo(); response.assertStatusCode(expectedStatusCode); - if (expectedStatusCode == 200) { + assertThat(response.getStatusCode(), equalTo(expectedStatusCode)); + if (expectedStatusCode == HttpStatus.SC_OK) { String username = response.getTextFromJsonBody(POINTER_USERNAME); assertThat(username, equalTo(expectedUsername)); - } else { - Assert.assertTrue(response.getBody().contains("Unauthorized")); } } } diff --git a/src/integrationTest/java/org/opensearch/security/privileges/PrivilegesEvaluatorTest.java b/src/integrationTest/java/org/opensearch/security/privileges/PrivilegesEvaluatorTest.java index 9f9da4366c..2315c979ea 100644 --- a/src/integrationTest/java/org/opensearch/security/privileges/PrivilegesEvaluatorTest.java +++ b/src/integrationTest/java/org/opensearch/security/privileges/PrivilegesEvaluatorTest.java @@ -12,7 +12,7 @@ package org.opensearch.security.privileges; import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope; -import org.apache.hc.core5.http.HttpStatus; +import org.apache.http.HttpStatus; import org.junit.ClassRule; import org.junit.Test; import org.junit.runner.RunWith; diff --git a/src/integrationTest/java/org/opensearch/test/framework/cluster/CloseableHttpClientFactory.java b/src/integrationTest/java/org/opensearch/test/framework/cluster/CloseableHttpClientFactory.java index ed236dcd0c..ee2f3227e3 100644 --- a/src/integrationTest/java/org/opensearch/test/framework/cluster/CloseableHttpClientFactory.java +++ b/src/integrationTest/java/org/opensearch/test/framework/cluster/CloseableHttpClientFactory.java @@ -33,7 +33,7 @@ class CloseableHttpClientFactory { private final HttpRoutePlanner routePlanner; - private final String[] supportedCipherSuit; + private final String[] supportedCipherSuites; public CloseableHttpClientFactory( SSLContext sslContext, @@ -44,7 +44,7 @@ public CloseableHttpClientFactory( this.sslContext = Objects.requireNonNull(sslContext, "SSL context is required."); this.requestConfig = requestConfig; this.routePlanner = routePlanner; - this.supportedCipherSuit = supportedCipherSuit; + this.supportedCipherSuites = supportedCipherSuit; } public CloseableHttpClient getHTTPClient() { @@ -54,7 +54,7 @@ public CloseableHttpClient getHTTPClient() { final SSLConnectionSocketFactory sslsf = new SSLConnectionSocketFactory( this.sslContext, null, - supportedCipherSuit, + supportedCipherSuites, NoopHostnameVerifier.INSTANCE ); diff --git a/src/integrationTest/java/org/opensearch/test/framework/cluster/OpenSearchClientProvider.java b/src/integrationTest/java/org/opensearch/test/framework/cluster/OpenSearchClientProvider.java index 45a68994f8..ddc68f74df 100644 --- a/src/integrationTest/java/org/opensearch/test/framework/cluster/OpenSearchClientProvider.java +++ b/src/integrationTest/java/org/opensearch/test/framework/cluster/OpenSearchClientProvider.java @@ -60,7 +60,6 @@ import org.apache.hc.core5.http.HttpHost; import org.apache.hc.core5.http.nio.ssl.TlsStrategy; import org.apache.hc.core5.reactor.ssl.TlsDetails; - import org.opensearch.client.RestClient; import org.opensearch.client.RestClientBuilder; import org.opensearch.client.RestHighLevelClient; diff --git a/src/integrationTest/java/org/opensearch/test/framework/cluster/TestRestClient.java b/src/integrationTest/java/org/opensearch/test/framework/cluster/TestRestClient.java index f494f00796..afdce66679 100644 --- a/src/integrationTest/java/org/opensearch/test/framework/cluster/TestRestClient.java +++ b/src/integrationTest/java/org/opensearch/test/framework/cluster/TestRestClient.java @@ -28,6 +28,12 @@ package org.opensearch.test.framework.cluster; +import static java.lang.String.format; +import static java.util.Objects.requireNonNull; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.notNullValue; + import java.io.IOException; import java.net.InetAddress; import java.net.InetSocketAddress; @@ -45,8 +51,6 @@ import javax.net.ssl.SSLContext; -import com.fasterxml.jackson.core.JsonProcessingException; -import com.fasterxml.jackson.databind.JsonNode; import org.apache.commons.io.IOUtils; import org.apache.hc.client5.http.classic.methods.HttpDelete; import org.apache.hc.client5.http.classic.methods.HttpGet; @@ -68,17 +72,13 @@ import org.apache.hc.core5.net.URIBuilder; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; - import org.opensearch.common.xcontent.XContentType; import org.opensearch.core.common.Strings; import org.opensearch.core.xcontent.ToXContentObject; import org.opensearch.security.DefaultObjectMapper; -import static java.lang.String.format; -import static java.util.Objects.requireNonNull; -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.notNullValue; +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.JsonNode; /** * A OpenSearch REST client, which is tailored towards use in integration tests. Instances of this class can be @@ -121,16 +121,6 @@ public HttpResponse get(String path, Header... headers) { return get(path, Collections.emptyList(), headers); } - public HttpResponse getWithJsonBody(String path, String body, Header... headers) { - try { - HttpGet httpGet = new HttpGet(new URIBuilder(getHttpServerUri()).setPath(path).build()); - httpGet.setEntity(toStringEntity(body)); - return executeRequest(httpGet, mergeHeaders(CONTENT_TYPE_JSON, headers)); - } catch (URISyntaxException ex) { - throw new RuntimeException("Incorrect URI syntax", ex); - } - } - public HttpResponse getAuthInfo(Header... headers) { return executeRequest(new HttpGet(getHttpServerUri() + "/_opendistro/_security/authinfo?pretty"), headers); } @@ -140,7 +130,7 @@ public HttpResponse getOnBehalfOfToken(String jsonData, Header... headers) { HttpPost httpPost = new HttpPost( new URIBuilder(getHttpServerUri() + "/_plugins/_security/api/generateonbehalfoftoken?pretty").build() ); - httpPost.setEntity(toStringEntity(jsonData)); + httpPost.setEntity(new StringEntity(jsonData)); return executeRequest(httpPost, mergeHeaders(CONTENT_TYPE_JSON, headers)); } catch (URISyntaxException ex) { throw new RuntimeException("Incorrect URI syntax", ex); @@ -150,7 +140,7 @@ public HttpResponse getOnBehalfOfToken(String jsonData, Header... headers) { public HttpResponse changeInternalUserPassword(String jsonData, Header... headers) { try { HttpPut httpPut = new HttpPut(new URIBuilder(getHttpServerUri() + "/_plugins/_security/api/account?pretty").build()); - httpPut.setEntity(toStringEntity(jsonData)); + httpPut.setEntity(new StringEntity(jsonData)); return executeRequest(httpPut, mergeHeaders(CONTENT_TYPE_JSON, headers)); } catch (URISyntaxException ex) { throw new RuntimeException("Incorrect URI syntax", ex); @@ -176,12 +166,20 @@ public HttpResponse options(String path, Header... headers) { public HttpResponse putJson(String path, String body, Header... headers) { HttpPut uriRequest = new HttpPut(getHttpServerUri() + "/" + path); - uriRequest.setEntity(toStringEntity(body)); + uriRequest.setEntity(new StringEntity(body)); return executeRequest(uriRequest, mergeHeaders(CONTENT_TYPE_JSON, headers)); } - private StringEntity toStringEntity(String body) { - return new StringEntity(body); + public HttpResponse getWithJsonBody(String path, String body, Header... headers) { + // Clever workaround to get support for GET with body https://stackoverflow.com/a/25019452/533057 + HttpPost uriRequest = new HttpPost(getHttpServerUri() + "/" + path) { + @Override + public String getMethod() { + return "GET"; + } + }; + uriRequest.setEntity(new StringEntity(body)); + return executeRequest(uriRequest, mergeHeaders(CONTENT_TYPE_JSON, headers)); } public HttpResponse putJson(String path, ToXContentObject body) { @@ -199,7 +197,7 @@ public HttpResponse delete(String path, Header... headers) { public HttpResponse postJson(String path, String body, Header... headers) { HttpPost uriRequest = new HttpPost(getHttpServerUri() + "/" + path); - uriRequest.setEntity(toStringEntity(body)); + uriRequest.setEntity(new StringEntity(body)); return executeRequest(uriRequest, mergeHeaders(CONTENT_TYPE_JSON, headers)); } @@ -214,7 +212,7 @@ public HttpResponse post(String path) { public HttpResponse patch(String path, String body) { HttpPatch uriRequest = new HttpPatch(getHttpServerUri() + "/" + path); - uriRequest.setEntity(toStringEntity(body)); + uriRequest.setEntity(new StringEntity(body)); return executeRequest(uriRequest, CONTENT_TYPE_JSON); } @@ -396,7 +394,7 @@ private JsonNode getJsonNodeAt(String jsonPointer) { try { return toJsonNode().at(jsonPointer); } catch (IOException e) { - throw new IllegalArgumentException("Cound not convert response body to JSON node ", e); + throw new IllegalArgumentException("Cound not convert response body to JSON node '" + getBody() + "'", e); } } diff --git a/src/integrationTest/java/org/opensearch/test/framework/cluster/TestRestClientConfiguration.java b/src/integrationTest/java/org/opensearch/test/framework/cluster/TestRestClientConfiguration.java index 3b75730303..73e61bb1f8 100644 --- a/src/integrationTest/java/org/opensearch/test/framework/cluster/TestRestClientConfiguration.java +++ b/src/integrationTest/java/org/opensearch/test/framework/cluster/TestRestClientConfiguration.java @@ -90,29 +90,34 @@ public TestRestClientConfiguration credentials(UserCredentialsHolder userCredent * @param headers headers * @return builder */ - public TestRestClientConfiguration headers(Header... headers) { - this.headers.addAll(Arrays.asList(Objects.requireNonNull(headers, "Headers are required"))); + public TestRestClientConfiguration header(final String headerName, final String headerValue) { + this.headers.add( + new BasicHeader( + Objects.requireNonNull(headerName, "Header names are required"), + Objects.requireNonNull(headerValue, "Header values are required") + ) + ); return this; } /** * Add HTTP headers which are attached to each HTTP request - * @param headers list of headers + * @param headers headers * @return builder */ - public TestRestClientConfiguration headers(List
headers) { - this.headers.addAll(Objects.requireNonNull(headers, "Cannot add null headers")); + public TestRestClientConfiguration headers(Header... headers) { + this.headers.addAll(Arrays.asList(Objects.requireNonNull(headers, "Headers are required"))); return this; } /** - * Add HTTP header to each request - * @param name header name - * @param value header value + * Add HTTP headers which are attached to each HTTP request + * @param headers list of headers * @return builder */ - public TestRestClientConfiguration header(String name, Object value) { - return headers(new BasicHeader(name, value)); + public TestRestClientConfiguration headers(List
headers) { + this.headers.addAll(Objects.requireNonNull(headers, "Cannot add null headers")); + return this; } /** diff --git a/src/integrationTest/java/org/opensearch/test/framework/testplugins/AbstractRestHandler.java b/src/integrationTest/java/org/opensearch/test/framework/testplugins/AbstractRestHandler.java index e92468af98..2f88585b22 100644 --- a/src/integrationTest/java/org/opensearch/test/framework/testplugins/AbstractRestHandler.java +++ b/src/integrationTest/java/org/opensearch/test/framework/testplugins/AbstractRestHandler.java @@ -1,12 +1,12 @@ /* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - * - */ +* Copyright OpenSearch Contributors +* SPDX-License-Identifier: Apache-2.0 +* +* The OpenSearch Contributors require contributions made to +* this file be licensed under the Apache-2.0 license or a +* compatible open source license. +* +*/ package org.opensearch.test.framework.testplugins; import org.opensearch.ExceptionsHelper; diff --git a/src/main/java/com/amazon/dlic/auth/http/saml/AuthTokenProcessorHandler.java b/src/main/java/com/amazon/dlic/auth/http/saml/AuthTokenProcessorHandler.java index a344c653f3..f545c2425b 100644 --- a/src/main/java/com/amazon/dlic/auth/http/saml/AuthTokenProcessorHandler.java +++ b/src/main/java/com/amazon/dlic/auth/http/saml/AuthTokenProcessorHandler.java @@ -239,7 +239,7 @@ private Optional handleLowLevel(RestRequest restRequest) throw return Optional.of(new SecurityResponse(HttpStatus.SC_OK, SecurityResponse.CONTENT_TYPE_APP_JSON, responseBodyString)); } catch (JsonProcessingException e) { log.warn("Error while parsing JSON for /_opendistro/_security/api/authtoken", e); - return Optional.of(new SecurityResponse(HttpStatus.SC_BAD_REQUEST, null, "JSON could not be parsed")); + return Optional.of(new SecurityResponse(HttpStatus.SC_BAD_REQUEST, new Exception("JSON could not be parsed"))); } } diff --git a/src/main/java/org/opensearch/security/auth/BackendRegistry.java b/src/main/java/org/opensearch/security/auth/BackendRegistry.java index f4abb5361a..5824348225 100644 --- a/src/main/java/org/opensearch/security/auth/BackendRegistry.java +++ b/src/main/java/org/opensearch/security/auth/BackendRegistry.java @@ -204,7 +204,7 @@ public boolean authenticate(final SecurityRequestChannel request) { log.debug("Rejecting REST request because of blocked address: {}", request.getRemoteAddress().orElse(null)); } - request.queueForSending(new SecurityResponse(SC_UNAUTHORIZED, null, "Authentication finally failed")); + request.queueForSending(new SecurityResponse(SC_UNAUTHORIZED, new Exception("Authentication finally failed"))); return false; } @@ -226,7 +226,7 @@ public boolean authenticate(final SecurityRequestChannel request) { if (!isInitialized()) { log.error("Not yet initialized (you may need to run securityadmin)"); - request.queueForSending(new SecurityResponse(SC_SERVICE_UNAVAILABLE, null, "OpenSearch Security not initialized.")); + request.queueForSending(new SecurityResponse(SC_SERVICE_UNAVAILABLE, new Exception("OpenSearch Security not initialized."))); return false; } diff --git a/src/main/java/org/opensearch/security/filter/SecurityResponse.java b/src/main/java/org/opensearch/security/filter/SecurityResponse.java index be80a5d4d4..14c21a9385 100644 --- a/src/main/java/org/opensearch/security/filter/SecurityResponse.java +++ b/src/main/java/org/opensearch/security/filter/SecurityResponse.java @@ -11,9 +11,11 @@ package org.opensearch.security.filter; +import java.io.IOException; import java.util.Map; import org.apache.http.HttpHeaders; +import org.opensearch.common.xcontent.XContentFactory; import org.opensearch.core.rest.RestStatus; import org.opensearch.rest.BytesRestResponse; import org.opensearch.rest.RestResponse; @@ -26,6 +28,12 @@ public class SecurityResponse { private final Map headers; private final String body; + public SecurityResponse(final int status, final Exception e) { + this.status = status; + this.headers = CONTENT_TYPE_APP_JSON; + this.body = generateFailureMessage(e); + } + public SecurityResponse(final int status, final Map headers, final String body) { this.status = status; this.headers = headers; @@ -52,4 +60,18 @@ public RestResponse asRestResponse() { return restResponse; } + protected String generateFailureMessage(final Exception e) { + try { + return XContentFactory.jsonBuilder() + .startObject() + .startObject("error") + .field("status", "error") + .field("reason", e.getMessage()) + .endObject() + .endObject() + .toString(); + } catch (final IOException ioe) { + throw new RuntimeException(ioe); + } + } } diff --git a/src/main/java/org/opensearch/security/filter/SecurityRestFilter.java b/src/main/java/org/opensearch/security/filter/SecurityRestFilter.java index 87fe7f5323..b6475e5ec3 100644 --- a/src/main/java/org/opensearch/security/filter/SecurityRestFilter.java +++ b/src/main/java/org/opensearch/security/filter/SecurityRestFilter.java @@ -247,7 +247,7 @@ public void checkAndAuthenticateRequest(SecurityRequestChannel requestChannel) t log.error(exception.toString()); auditLog.logBadHeaders(requestChannel); - requestChannel.queueForSending(new SecurityResponse(HttpStatus.SC_FORBIDDEN, null, exception.toString())); + requestChannel.queueForSending(new SecurityResponse(HttpStatus.SC_FORBIDDEN, exception)); return; } @@ -256,7 +256,7 @@ public void checkAndAuthenticateRequest(SecurityRequestChannel requestChannel) t log.error(exception.toString()); auditLog.logBadHeaders(requestChannel); - requestChannel.queueForSending(new SecurityResponse(HttpStatus.SC_FORBIDDEN, null, exception.toString())); + requestChannel.queueForSending(new SecurityResponse(HttpStatus.SC_FORBIDDEN, exception)); return; } @@ -276,7 +276,7 @@ public void checkAndAuthenticateRequest(SecurityRequestChannel requestChannel) t } catch (SSLPeerUnverifiedException e) { log.error("No ssl info", e); auditLog.logSSLException(requestChannel, e); - requestChannel.queueForSending(new SecurityResponse(HttpStatus.SC_FORBIDDEN, null, null)); + requestChannel.queueForSending(new SecurityResponse(HttpStatus.SC_FORBIDDEN, new Exception("No ssl info"))); return; } diff --git a/src/main/java/org/opensearch/security/ssl/http/netty/Netty4HttpRequestHeaderVerifier.java b/src/main/java/org/opensearch/security/ssl/http/netty/Netty4HttpRequestHeaderVerifier.java index 16e23ea361..5112ceced3 100644 --- a/src/main/java/org/opensearch/security/ssl/http/netty/Netty4HttpRequestHeaderVerifier.java +++ b/src/main/java/org/opensearch/security/ssl/http/netty/Netty4HttpRequestHeaderVerifier.java @@ -119,7 +119,7 @@ public void channelRead0(ChannelHandlerContext ctx, DefaultHttpRequest msg) thro ctx.channel().attr(IS_AUTHENTICATED).set(Boolean.TRUE); } } catch (final OpenSearchSecurityException e) { - final SecurityResponse earlyResponse = new SecurityResponse(ExceptionsHelper.status(e).getStatus(), null, e.getMessage()); + final SecurityResponse earlyResponse = new SecurityResponse(ExceptionsHelper.status(e).getStatus(), e); ctx.channel().attr(EARLY_RESPONSE).set(earlyResponse); } catch (final SecurityRequestChannelUnsupported srcu) { // Use defaults for unsupported channels