diff --git a/CHANGELOG.md b/CHANGELOG.md index 6eb0f8e66a..ba84bc3150 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), * [Resource Sharing] Reverts @Inject pattern usage for ResourceSharingExtension to client accessor pattern. ([#5576](https://github.com/opensearch-project/security/pull/5576)) * Inject user custom attributes when injecting user and role information to the thread context ([#5560](https://github.com/opensearch-project/security/pull/5560)) * Allow any plugin system request when `plugins.security.system_indices.enabled` is set to `false` ([#5579](https://github.com/opensearch-project/security/pull/5579)) +* Fix compilation issue after change to Subject interface in core and bump to 3.2.0 ([#5423](https://github.com/opensearch-project/security/pull/5423)) +* Provide SecureHttpTransportParameters to complement SecureTransportParameters counterpart ([#5432](https://github.com/opensearch-project/security/pull/5432)) +* Use isClusterPerm instead of requestedResolved.isLocalAll() to determine if action is a cluster action ([#5445](https://github.com/opensearch-project/security/pull/5445)) +* Fix config update with deprecated config types failing in mixed clusters ([#5456](https://github.com/opensearch-project/security/pull/5456)) +* Fix usage of jwt_clock_skew_tolerance_seconds in HTTPJwtAuthenticator ([#5506](https://github.com/opensearch-project/security/pull/5506)) +* Fix partial cache update post snapshot restore[#5478](https://github.com/opensearch-project/security/pull/5478) ### Refactoring diff --git a/src/integrationTest/java/org/opensearch/security/SecurityIndexSnapshotRestoreTests.java b/src/integrationTest/java/org/opensearch/security/SecurityIndexSnapshotRestoreTests.java new file mode 100644 index 0000000000..3e01325b77 --- /dev/null +++ b/src/integrationTest/java/org/opensearch/security/SecurityIndexSnapshotRestoreTests.java @@ -0,0 +1,189 @@ +/* + * 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. + * + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + +package org.opensearch.security; + +import java.util.List; +import java.util.Map; +import java.util.concurrent.ExecutionException; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.junit.After; +import org.junit.Before; +import org.junit.ClassRule; +import org.junit.Test; + +import org.opensearch.OpenSearchStatusException; +import org.opensearch.action.admin.cluster.repositories.delete.DeleteRepositoryRequest; +import org.opensearch.action.admin.cluster.repositories.put.PutRepositoryRequest; +import org.opensearch.action.admin.cluster.snapshots.restore.RestoreSnapshotRequest; +import org.opensearch.action.admin.cluster.snapshots.restore.RestoreSnapshotResponse; +import org.opensearch.action.admin.indices.create.CreateIndexRequest; +import org.opensearch.action.admin.indices.create.CreateIndexResponse; +import org.opensearch.action.admin.indices.delete.DeleteIndexRequest; +import org.opensearch.action.get.GetRequest; +import org.opensearch.action.get.GetResponse; +import org.opensearch.action.index.IndexRequest; +import org.opensearch.action.support.WriteRequest; +import org.opensearch.client.RequestOptions; +import org.opensearch.client.RestHighLevelClient; +import org.opensearch.common.xcontent.XContentType; +import org.opensearch.core.rest.RestStatus; +import org.opensearch.security.api.AbstractApiIntegrationTest; +import org.opensearch.security.support.ConfigConstants; +import org.opensearch.test.framework.TestSecurityConfig; +import org.opensearch.test.framework.cluster.ClusterManager; +import org.opensearch.test.framework.cluster.LocalCluster; +import org.opensearch.test.framework.cluster.TestRestClient; +import org.opensearch.transport.client.Client; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.opensearch.security.CrossClusterSearchTests.TYPE_ATTRIBUTE; +import static org.opensearch.security.SearchOperationTest.TEST_SNAPSHOT_REPOSITORY_NAME; +import static org.opensearch.security.support.ConfigConstants.SECURITY_BACKGROUND_INIT_IF_SECURITYINDEX_NOT_EXIST; +import static org.opensearch.security.support.ConfigConstants.SECURITY_RESTAPI_ROLES_ENABLED; +import static org.opensearch.test.framework.TestSecurityConfig.AuthcDomain.AUTHC_HTTPBASIC_INTERNAL; +import static org.opensearch.test.framework.TestSecurityConfig.Role.ALL_ACCESS; +import static org.opensearch.test.framework.TestSecurityConfig.User.USER_ADMIN; +import static org.opensearch.test.framework.matcher.GetResponseMatchers.containDocument; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +public class SecurityIndexSnapshotRestoreTests extends AbstractApiIntegrationTest { + private static final Logger log = LogManager.getLogger(SecurityIndexSnapshotRestoreTests.class); + + private static final String TEST_INDEX_NAME = "my_index_001"; + private static final String DOC_ID = "doc_id"; + + private static final TestSecurityConfig.User ADMIN_USER = new TestSecurityConfig.User("admin").roles(ALL_ACCESS) + .attr(TYPE_ATTRIBUTE, "administrative"); + + private static final TestSecurityConfig.User LIMITED_READ_USER_1 = new TestSecurityConfig.User("limited_read_user").roles( + new TestSecurityConfig.Role("limited-reader").indexPermissions("indices:data/read*").on(TEST_INDEX_NAME) + ); + + private static final TestSecurityConfig.User LIMITED_READ_USER_2 = new TestSecurityConfig.User("user2"); + + private static final TestSecurityConfig.Role LIMITED_READ_USER_2_ROLE = new TestSecurityConfig.Role("limited-reader_2") + .indexPermissions("indices:data/read*") + .on(TEST_INDEX_NAME); + + private String securityIndex; + + @ClassRule + public static LocalCluster cluster = new LocalCluster.Builder().clusterManager(ClusterManager.THREE_CLUSTER_MANAGERS) + .authc(AUTHC_HTTPBASIC_INTERNAL) + .users(ADMIN_USER, LIMITED_READ_USER_1) + .anonymousAuth(false) + .nodeSettings( + Map.of( + SECURITY_RESTAPI_ROLES_ENABLED, + List.of("user_" + USER_ADMIN.getName() + "__" + ALL_ACCESS.getName()), + SECURITY_BACKGROUND_INIT_IF_SECURITYINDEX_NOT_EXIST, + false + ) + ) + .build(); + + @Before + public void setUp() throws Exception { + securityIndex = ConfigConstants.OPENDISTRO_SECURITY_DEFAULT_CONFIG_INDEX; + + try (Client client = cluster.getInternalNodeClient()) { + client.admin() + .cluster() + .putRepository( + new PutRepositoryRequest(TEST_SNAPSHOT_REPOSITORY_NAME).type("fs") + .settings(Map.of("location", cluster.getSnapshotDirPath())) + ) + .actionGet(); + + CreateIndexResponse createIndexResponse = client.admin().indices().create(new CreateIndexRequest(TEST_INDEX_NAME)).actionGet(); + assertTrue(createIndexResponse.isAcknowledged()); + + client.index( + new IndexRequest(TEST_INDEX_NAME).id(DOC_ID) + .source("{\"message\": \"test document 1\"}", XContentType.JSON) + .setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE) + ).actionGet(); + } + } + + @After + public void cleanData() throws ExecutionException, InterruptedException { + try (Client client = cluster.getInternalNodeClient()) { + client.admin().indices().delete(new DeleteIndexRequest(TEST_INDEX_NAME)).actionGet(); + + client.admin().cluster().deleteRepository(new DeleteRepositoryRequest(TEST_SNAPSHOT_REPOSITORY_NAME)).actionGet(); + } + } + + @Test + public void testSecurityCacheReloadAfterRestore() throws Exception { + // 1. Read data in custom index with LIMITED_READ_USER_1 + try (RestHighLevelClient restHighLevelClient = cluster.getRestHighLevelClient(LIMITED_READ_USER_1)) { + GetResponse response = restHighLevelClient.get(new GetRequest(TEST_INDEX_NAME, DOC_ID), RequestOptions.DEFAULT); + assertThat(response, containDocument(TEST_INDEX_NAME, DOC_ID)); + } + + // 2. Create snapshot of security index + try (RestHighLevelClient restHighLevelClient = cluster.getRestHighLevelClient(ADMIN_USER)) { + SnapshotSteps steps = new SnapshotSteps(restHighLevelClient); + steps.createSnapshot(TEST_SNAPSHOT_REPOSITORY_NAME, "test-snap", securityIndex); + steps.waitForSnapshotCreation(TEST_SNAPSHOT_REPOSITORY_NAME, "test-snap"); + } + + // 3. Add new role and user to security index (This is not in snapshot created above) + try (TestRestClient client = cluster.getRestClient(ADMIN_USER)) { + client.createRole(LIMITED_READ_USER_2_ROLE.getName(), LIMITED_READ_USER_2_ROLE).assertStatusCode(201); + client.createUser(LIMITED_READ_USER_2.getName(), LIMITED_READ_USER_2).assertStatusCode(201); + client.assignRoleToUser(LIMITED_READ_USER_2.getName(), "limited-reader_2").assertStatusCode(200); + } + + // 4. Read data in custom index with LIMITED_READ_USER_2 + try (RestHighLevelClient restHighLevelClient = cluster.getRestHighLevelClient(LIMITED_READ_USER_2)) { + GetResponse response = restHighLevelClient.get(new GetRequest(TEST_INDEX_NAME, DOC_ID), RequestOptions.DEFAULT); + assertThat(response, containDocument(TEST_INDEX_NAME, DOC_ID)); + } + + // 5. Delete security index + try (Client client = cluster.getInternalNodeClient()) { + DeleteIndexRequest deleteRequest = new DeleteIndexRequest(securityIndex); + client.admin().indices().delete(deleteRequest).actionGet(); + } + + // 6. Restore security index + try (Client client = cluster.getInternalNodeClient()) { + RestoreSnapshotRequest restoreRequest = new RestoreSnapshotRequest(TEST_SNAPSHOT_REPOSITORY_NAME, "test-snap") + .waitForCompletion(true) + .indices(securityIndex); // restore security index + + RestoreSnapshotResponse restoreResponse = client.admin().cluster().restoreSnapshot(restoreRequest).actionGet(); + + // Verify restore was successful + assertEquals(RestStatus.OK, restoreResponse.status()); + } + + // 7. Read data in custom index with LIMITED_READ_USER_1 because it was in snapshot + try (RestHighLevelClient restHighLevelClient = cluster.getRestHighLevelClient(LIMITED_READ_USER_1)) { + GetResponse response = restHighLevelClient.get(new GetRequest(TEST_INDEX_NAME, DOC_ID), RequestOptions.DEFAULT); + assertThat(response, containDocument(TEST_INDEX_NAME, DOC_ID)); + } + + // 8. Should get 401 error to read custom index with LIMITED_READ_USER_2 because it was not in snapshot + try (RestHighLevelClient restHighLevelClient = cluster.getRestHighLevelClient(LIMITED_READ_USER_2)) { + restHighLevelClient.get(new GetRequest(TEST_INDEX_NAME, DOC_ID), RequestOptions.DEFAULT); + } catch (OpenSearchStatusException exception) { + assertEquals(RestStatus.UNAUTHORIZED, exception.status()); // Verify it's a 401 + } + } +} diff --git a/src/main/java/org/opensearch/security/configuration/ConfigurationRepository.java b/src/main/java/org/opensearch/security/configuration/ConfigurationRepository.java index 737f86ba74..87335b7f9a 100644 --- a/src/main/java/org/opensearch/security/configuration/ConfigurationRepository.java +++ b/src/main/java/org/opensearch/security/configuration/ConfigurationRepository.java @@ -681,15 +681,12 @@ public void afterIndexShardStarted(IndexShard indexShard) { // Check if this is a security index shard if (securityIndex.equals(index.getName())) { - // Only trigger on primary shard to avoid multiple reloads - if (indexShard.routingEntry() != null && indexShard.routingEntry().primary()) { - threadPool.generic().execute(() -> { - if (isSecurityIndexRestoredFromSnapshot(clusterService, index, securityIndex)) { - LOGGER.info("Security index primary shard {} started - config reloading for snapshot restore", shardId); - reloadConfiguration(CType.values()); - } - }); - } + threadPool.generic().execute(() -> { + if (isSecurityIndexRestoredFromSnapshot(clusterService, index, securityIndex)) { + LOGGER.info("Security index shard {} started - config reloading for snapshot restore", shardId); + reloadConfiguration(CType.values()); + } + }); } } } diff --git a/src/test/java/org/opensearch/security/configuration/ConfigurationRepositoryTest.java b/src/test/java/org/opensearch/security/configuration/ConfigurationRepositoryTest.java index 555c495955..294e4b2fd1 100644 --- a/src/test/java/org/opensearch/security/configuration/ConfigurationRepositoryTest.java +++ b/src/test/java/org/opensearch/security/configuration/ConfigurationRepositoryTest.java @@ -43,7 +43,6 @@ import org.opensearch.cluster.metadata.Metadata; import org.opensearch.cluster.node.DiscoveryNode; import org.opensearch.cluster.node.DiscoveryNodes; -import org.opensearch.cluster.routing.ShardRouting; import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.Priority; import org.opensearch.common.settings.Settings; @@ -597,7 +596,6 @@ public void getConfigurationsFromIndex_SecurityIndexNotInitiallyReady() throws I public void afterIndexShardStarted_whenSecurityIndexUpdated() throws InterruptedException, TimeoutException { Settings settings = Settings.builder().build(); IndexShard indexShard = mock(IndexShard.class); - ShardRouting shardRouting = mock(ShardRouting.class); ShardId shardId = mock(ShardId.class); Index index = mock(Index.class); ClusterState mockClusterState = mock(ClusterState.class); @@ -611,20 +609,11 @@ public void afterIndexShardStarted_whenSecurityIndexUpdated() throws Interrupted when(indexShard.shardId()).thenReturn(shardId); when(shardId.getIndex()).thenReturn(index); when(index.getName()).thenReturn(ConfigConstants.OPENDISTRO_SECURITY_DEFAULT_CONFIG_INDEX); - when(indexShard.routingEntry()).thenReturn(shardRouting); when(clusterService.state()).thenReturn(mockClusterState); when(mockClusterState.custom(RestoreInProgress.TYPE)).thenReturn(mockRestore); when(threadPool.generic()).thenReturn(executorService); - // when replica shard updated - when(shardRouting.primary()).thenReturn(false); - configurationRepository.afterIndexShardStarted(indexShard); - verify(executorService, never()).execute(any()); - verify(configurationRepository, never()).reloadConfiguration(any()); - - // when primary shard updated doReturn(true).when(configurationRepository).reloadConfiguration(any()); - when(shardRouting.primary()).thenReturn(true); when(mockRestore.iterator()).thenReturn(Collections.singletonList(mockEntry).iterator()); when(mockEntry.indices()).thenReturn(Collections.singletonList(ConfigConstants.OPENDISTRO_SECURITY_DEFAULT_CONFIG_INDEX)); ArgumentCaptor successRunnableCaptor = ArgumentCaptor.forClass(Runnable.class); @@ -637,7 +626,6 @@ public void afterIndexShardStarted_whenSecurityIndexUpdated() throws Interrupted Mockito.reset(configurationRepository, executorService); ArgumentCaptor errorRunnableCaptor = ArgumentCaptor.forClass(Runnable.class); when(clusterService.state()).thenThrow(new RuntimeException("ClusterState exception")); - when(shardRouting.primary()).thenReturn(true); configurationRepository.afterIndexShardStarted(indexShard); verify(executorService).execute(errorRunnableCaptor.capture()); errorRunnableCaptor.getValue().run();