Allow TransportConfigUpdateAction when security config initialization…

… has completed (opensearch-project#3810)

Introduces another variable on DynamicConfigFactory called
`bgThreadComplete` that behaves differently than the `initialized`
variable. `bgThreadComplete` is a flag that signals to
TransportConfigUpdateAction that it can start accepting updates.

There are 2 ways the security index can be created from scratch:

1. If `plugins.security.allow_default_init_securityindex` is set to
**true** it will create the security index and load all yaml files
2. If `plugins.security.allow_default_init_securityindex` is set to
**false**, the security index is not created on bootstrap and requires a
user to run securityadmin to initialize security. When securityadmin is
utilized, the cluster does depend on
[TransportConfigUpdateAction](https:/opensearch-project/security/blob/main/src/main/java/org/opensearch/security/tools/SecurityAdmin.java#L975-L977)
to initialize security so there still needs to be an avenue where this
can update the config before `initialized` is set to **true**

This PR sets `bgThreadComplete` to **false** on node startup and
explicitly sets it to **true** once its ok for
TransportConfigUpdateAction to start accepting transport actions. In
case 2) up above, it can be set to **true** before DynamicConfigFactory
is `initialized` so that it can accept requests from securityadmin.

* Category (Enhancement, New feature, Bug fix, Test fix, Refactoring,
Maintenance, Documentation)

Bug fix

- Resolves opensearch-project#3204

- [X] New functionality includes testing
- [ ] New functionality has been documented
- [X] Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and
signing off your commits, please check
[here](https:/opensearch-project/OpenSearch/blob/main/CONTRIBUTING.md#developer-certificate-of-origin).

---------

Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Peter Nied <[email protected]>
Signed-off-by: Peter Nied <[email protected]>
Co-authored-by: Peter Nied <[email protected]>
Co-authored-by: Peter Nied <[email protected]>

src/integrationTest/java/org/opensearch/security/ConfigurationFiles.java

@@ -31,12 +31,13 @@ public static Path createConfigurationDirectory() {
  String[] configurationFiles = {
  "config.yml",
  "action_groups.yml",
- "config.yml",
  "internal_users.yml",
+ "nodes_dn.yml",
  "roles.yml",
  "roles_mapping.yml",
  "security_tenants.yml",
- "tenants.yml" };
+ "tenants.yml",
+ "whitelist.yml" };
  for (String fileName : configurationFiles) {
  Path configFileDestination = tempDirectory.resolve(fileName);
  copyResourceToFile(fileName, configFileDestination.toFile());

src/integrationTest/java/org/opensearch/security/SecurityAdminLauncher.java

@@ -10,6 +10,7 @@
 package org.opensearch.security;
 
 import java.io.File;
+import java.nio.file.Path;
 
 import org.opensearch.security.tools.SecurityAdmin;
 import org.opensearch.test.framework.certificate.TestCertificates;
@@ -44,4 +45,21 @@ public int updateRoleMappings(File roleMappingsConfigurationFile) throws Excepti
 
  return SecurityAdmin.execute(commandLineArguments);
  }
+
+ public int runSecurityAdmin(Path configurationFolder) throws Exception {
+ String[] commandLineArguments = {
+ "-cacert",
+ certificates.getRootCertificate().getAbsolutePath(),
+ "-cert",
+ certificates.getAdminCertificate().getAbsolutePath(),
+ "-key",
+ certificates.getAdminKey(null).getAbsolutePath(),
+ "-nhnv",
+ "-p",
+ String.valueOf(port),
+ "-cd",
+ configurationFolder.toString() };
+
+ return SecurityAdmin.execute(commandLineArguments);
+ }
 }

src/integrationTest/java/org/opensearch/security/SecurityConfigurationBootstrapTests.java

@@ -0,0 +1,159 @@
+/*
+ * 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.security;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.time.Duration;
+import java.util.List;
+import java.util.Map;
+
+import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope;
+import com.google.common.collect.ImmutableMap;
+import org.apache.commons.io.FileUtils;
+import org.awaitility.Awaitility;
+import org.junit.AfterClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import org.opensearch.action.admin.cluster.health.ClusterHealthRequest;
+import org.opensearch.security.securityconf.impl.CType;
+import org.opensearch.security.support.ConfigConstants;
+import org.opensearch.security.support.ConfigHelper;
+import org.opensearch.test.framework.TestSecurityConfig.User;
+import org.opensearch.test.framework.cluster.ClusterManager;
+import org.opensearch.test.framework.cluster.ContextHeaderDecoratorClient;
+import org.opensearch.test.framework.cluster.LocalCluster;
+import org.opensearch.test.framework.cluster.TestRestClient;
+
+import static org.apache.http.HttpStatus.SC_SERVICE_UNAVAILABLE;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.equalTo;
+import static org.opensearch.security.configuration.ConfigurationRepository.DEFAULT_CONFIG_VERSION;
+import static org.opensearch.security.support.ConfigConstants.OPENDISTRO_SECURITY_DEFAULT_CONFIG_INDEX;
+import static org.opensearch.security.support.ConfigConstants.SECURITY_ALLOW_DEFAULT_INIT_SECURITYINDEX;
+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.security.support.ConfigConstants.SECURITY_UNSUPPORTED_DELAY_INITIALIZATION_SECONDS;
+import static org.opensearch.test.framework.TestSecurityConfig.Role.ALL_ACCESS;
+
+@RunWith(com.carrotsearch.randomizedtesting.RandomizedRunner.class)
+@ThreadLeakScope(ThreadLeakScope.Scope.NONE)
+public class SecurityConfigurationBootstrapTests {
+
+ private final static Path configurationFolder = ConfigurationFiles.createConfigurationDirectory();
+ private static final User USER_ADMIN = new User("admin").roles(ALL_ACCESS);
+
+ private static LocalCluster createCluster(final Map<String, Object> nodeSettings) {
+ var cluster = new LocalCluster.Builder().clusterManager(ClusterManager.THREE_CLUSTER_MANAGERS)
+ .loadConfigurationIntoIndex(false)
+ .defaultConfigurationInitDirectory(configurationFolder.toString())
+ .nodeSettings(
+ ImmutableMap.<String, Object>builder()
+ .put(SECURITY_RESTAPI_ROLES_ENABLED, List.of("user_" + USER_ADMIN.getName() + "__" + ALL_ACCESS.getName()))
+ .putAll(nodeSettings)
+ .build()
+ )
+ .build();
+
+ cluster.before(); // normally invoked by JUnit rules when run as a class rule - this starts the cluster
+ return cluster;
+ }
+
+ @AfterClass
+ public static void cleanConfigurationDirectory() throws IOException {
+ FileUtils.deleteDirectory(configurationFolder.toFile());
+ }
+
+ @Test
+ public void testInitializeWithSecurityAdminWhenNoBackgroundInitialization() throws Exception {
+ final var nodeSettings = ImmutableMap.<String, Object>builder()
+ .put(SECURITY_ALLOW_DEFAULT_INIT_SECURITYINDEX, false)
+ .put(SECURITY_BACKGROUND_INIT_IF_SECURITYINDEX_NOT_EXIST, false)
+ .build();
+ try (final LocalCluster cluster = createCluster(nodeSettings)) {
+ try (final TestRestClient client = cluster.getRestClient(USER_ADMIN)) {
+ final var rolesMapsResponse = client.get("_plugins/_security/api/rolesmapping/readall");
+ assertThat(rolesMapsResponse.getStatusCode(), equalTo(SC_SERVICE_UNAVAILABLE));
+ assertThat(rolesMapsResponse.getBody(), containsString("OpenSearch Security not initialized"));
+ }
+
+ final var securityAdminLauncher = new SecurityAdminLauncher(cluster.getHttpPort(), cluster.getTestCertificates());
+ final int exitCode = securityAdminLauncher.runSecurityAdmin(configurationFolder);
+ assertThat(exitCode, equalTo(0));
+
+ try (final TestRestClient client = cluster.getRestClient(USER_ADMIN)) {
+ Awaitility.await()
+ .alias("Waiting for rolemapping 'readall' availability.")
+ .until(() -> client.get("_plugins/_security/api/rolesmapping/readall").getStatusCode(), equalTo(200));
+ }
+ }
+ }
+
+ @Test
+ public void shouldStillLoadSecurityConfigDuringBootstrapAndActiveConfigUpdateRequests() throws Exception {
+ final var nodeSettings = ImmutableMap.<String, Object>builder()
+ .put(SECURITY_ALLOW_DEFAULT_INIT_SECURITYINDEX, true)
+ .put(SECURITY_UNSUPPORTED_DELAY_INITIALIZATION_SECONDS, 5)
+ .build();
+ try (final LocalCluster cluster = createCluster(nodeSettings)) {
+ try (final TestRestClient client = cluster.getRestClient(USER_ADMIN)) {
+ cluster.getInternalNodeClient()
+ .admin()
+ .cluster()
+ .health(new ClusterHealthRequest(OPENDISTRO_SECURITY_DEFAULT_CONFIG_INDEX).waitForGreenStatus())
+ .actionGet();
+
+ // Make sure the cluster is unavalaible to authenticate with the security plugin even though it is green
+ final var authResponseWhenUnconfigured = client.getAuthInfo();
+ authResponseWhenUnconfigured.assertStatusCode(503);
+
+ final var internalNodeClient = new ContextHeaderDecoratorClient(
+ cluster.getInternalNodeClient(),
+ Map.of(ConfigConstants.OPENDISTRO_SECURITY_CONF_REQUEST_HEADER, "true")
+ );
+ final var filesToUpload = ImmutableMap.<String, CType>builder()
+ .put("action_groups.yml", CType.ACTIONGROUPS)
+ .put("config.yml", CType.CONFIG)
+ .put("roles.yml", CType.ROLES)
+ .put("tenants.yml", CType.TENANTS)
+ .build();
+
+ final String defaultInitDirectory = System.getProperty("security.default_init.dir") + "/";
+ filesToUpload.forEach((fileName, ctype) -> {
+ try {
+ ConfigHelper.uploadFile(
+ internalNodeClient,
+ defaultInitDirectory + fileName,
+ OPENDISTRO_SECURITY_DEFAULT_CONFIG_INDEX,
+ ctype,
+ DEFAULT_CONFIG_VERSION
+ );
+ } catch (final Exception ex) {
+ throw new RuntimeException(ex);
+ }
+ });
+
+ Awaitility.await().alias("Load default configuration").pollInterval(Duration.ofMillis(100)).until(() -> {
+ // After the configuration has been loaded, the rest clients should be able to connect successfully
+ cluster.triggerConfigurationReloadForCTypes(
+ internalNodeClient,
+ List.of(CType.ACTIONGROUPS, CType.CONFIG, CType.ROLES, CType.TENANTS),
+ true
+ );
+ try (final TestRestClient freshClient = cluster.getRestClient(USER_ADMIN)) {
+ return client.getAuthInfo().getStatusCode();
+ }
+ }, equalTo(200));
+ }
+ }
+ }
+}

src/integrationTest/java/org/opensearch/security/SecurityConfigurationTests.java

@@ -70,7 +70,7 @@ public class SecurityConfigurationTests {
  SECURITY_RESTAPI_ROLES_ENABLED,
  List.of("user_" + USER_ADMIN.getName() + "__" + ALL_ACCESS.getName()),
  SECURITY_BACKGROUND_INIT_IF_SECURITYINDEX_NOT_EXIST,
- true
+ false
  )
  )
  .build();

src/integrationTest/java/org/opensearch/security/api/CreateResetPasswordTest.java

@@ -63,7 +63,7 @@ public class CreateResetPasswordTest {
  SECURITY_RESTAPI_ROLES_ENABLED,
  List.of("user_" + USER_ADMIN.getName() + "__" + ALL_ACCESS.getName()),
  SECURITY_BACKGROUND_INIT_IF_SECURITYINDEX_NOT_EXIST,
- true,
+ false,
  ConfigConstants.SECURITY_RESTAPI_PASSWORD_VALIDATION_REGEX,
  CUSTOM_PASSWORD_REGEX,
  ConfigConstants.SECURITY_RESTAPI_PASSWORD_VALIDATION_ERROR_MESSAGE,

src/integrationTest/java/org/opensearch/security/http/OnBehalfOfJwtAuthenticationTest.java

@@ -48,7 +48,7 @@
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.not;
 import static org.hamcrest.Matchers.notNullValue;
-import static org.opensearch.security.support.ConfigConstants.SECURITY_ALLOW_DEFAULT_INIT_SECURITYINDEX;
+import static org.opensearch.security.support.ConfigConstants.SECURITY_BACKGROUND_INIT_IF_SECURITYINDEX_NOT_EXIST;
 import static org.opensearch.security.support.ConfigConstants.SECURITY_RESTAPI_ADMIN_ENABLED;
 import static org.opensearch.security.support.ConfigConstants.SECURITY_RESTAPI_ROLES_ENABLED;
 import static org.opensearch.test.framework.TestSecurityConfig.AuthcDomain.AUTHC_HTTPBASIC_INTERNAL;
@@ -128,8 +128,8 @@ private static OnBehalfOfConfig defaultOnBehalfOfConfig() {
  .users(ADMIN_USER, OBO_USER, OBO_USER_NO_PERM, HOST_MAPPING_OBO_USER)
  .nodeSettings(
  Map.of(
- SECURITY_ALLOW_DEFAULT_INIT_SECURITYINDEX,
- true,
+ SECURITY_BACKGROUND_INIT_IF_SECURITYINDEX_NOT_EXIST,
+ false,
  SECURITY_RESTAPI_ROLES_ENABLED,
  ADMIN_USER.getRoleNames(),
  SECURITY_RESTAPI_ADMIN_ENABLED,

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

@@ -133,7 +133,7 @@ public String getSnapshotDirPath() {
  }
 
  @Override
- public void before() throws Throwable {
+ public void before() {
  if (localOpenSearchCluster == null) {
  for (LocalCluster dependency : clusterDependencies) {
  if (!dependency.isStarted()) {
@@ -155,12 +155,12 @@ public void before() throws Throwable {
 
  @Override
  protected void after() {
- System.clearProperty(INIT_CONFIGURATION_DIR);
  close();
  }
 
  @Override
  public void close() {
+ System.clearProperty(INIT_CONFIGURATION_DIR);
  if (localOpenSearchCluster != null && localOpenSearchCluster.isStarted()) {
  try {
  localOpenSearchCluster.destroy();
@@ -297,6 +297,16 @@ private static void triggerConfigurationReload(Client client) {
  }
  }
 
+ public void triggerConfigurationReloadForCTypes(Client client, List<CType> cTypes, boolean ignoreFailures) {
+ ConfigUpdateResponse configUpdateResponse = client.execute(
+ ConfigUpdateAction.INSTANCE,
+ new ConfigUpdateRequest(cTypes.stream().map(CType::toLCString).toArray(String[]::new))
+ ).actionGet();
+ if (!ignoreFailures && configUpdateResponse.hasFailures()) {
+ throw new RuntimeException("ConfigUpdateResponse produced failures: " + configUpdateResponse.failures());
+ }
+ }
+
  public CertificateData getAdminCertificate() {
  return testCertificates.getAdminCertificateData();
  }

src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java

@@ -1793,6 +1793,14 @@ public List<Setting<?>> getSettings() {
  Property.Filtered
  )
  );
+ settings.add(
+ Setting.intSetting(
+ ConfigConstants.SECURITY_UNSUPPORTED_DELAY_INITIALIZATION_SECONDS,
+ 0,
+ Property.NodeScope,
+ Property.Filtered
+ )
+ );
 
  // system integration
  settings.add(