Skip to content

Commit

Permalink
[Backport 2.x] Dynamic sign in options (#4137)
Browse files Browse the repository at this point in the history
Backport 25e2e51 from #3869.

---------

Signed-off-by: David Osorno <[email protected]>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Signed-off-by: Craig Perkins <[email protected]>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Craig Perkins <[email protected]>
  • Loading branch information
3 people authored Mar 26, 2024
1 parent 3c70762 commit 80cee28
Show file tree
Hide file tree
Showing 16 changed files with 232 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import org.junit.Test;
import org.junit.runner.RunWith;

import org.opensearch.security.securityconf.impl.DashboardSignInOption;
import org.opensearch.test.framework.TestSecurityConfig;
import org.opensearch.test.framework.TestSecurityConfig.Role;
import org.opensearch.test.framework.cluster.ClusterManager;
Expand All @@ -25,6 +26,7 @@

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.is;
import static org.opensearch.security.rest.DashboardsInfoAction.DEFAULT_PASSWORD_MESSAGE;
import static org.opensearch.security.rest.DashboardsInfoAction.DEFAULT_PASSWORD_REGEX;
import static org.opensearch.test.framework.TestSecurityConfig.AuthcDomain.AUTHC_HTTPBASIC_INTERNAL;
Expand Down Expand Up @@ -53,4 +55,14 @@ public void testDashboardsInfoValidationMessage() throws Exception {
assertThat(response.getTextFromJsonBody("/password_validation_regex"), equalTo(DEFAULT_PASSWORD_REGEX));
}
}

@Test
public void testDashboardsInfoContainsSignInOptions() 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.getTextArrayFromJsonBody("/sign_in_options").contains(DashboardSignInOption.BASIC.toString()), is(true));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.IntStream;

import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
Expand All @@ -33,8 +34,10 @@
import org.opensearch.security.dlic.rest.validation.RequestContentValidator;
import org.opensearch.security.dlic.rest.validation.RequestContentValidator.DataType;
import org.opensearch.security.securityconf.impl.CType;
import org.opensearch.security.securityconf.impl.DashboardSignInOption;
import org.opensearch.security.securityconf.impl.SecurityDynamicConfiguration;
import org.opensearch.security.securityconf.impl.v7.ConfigV7;
import org.opensearch.security.securityconf.impl.v7.ConfigV7.Authc;
import org.opensearch.security.support.ConfigConstants;
import org.opensearch.threadpool.ThreadPool;

Expand All @@ -49,6 +52,7 @@ public class MultiTenancyConfigApiAction extends AbstractApiAction {
public static final String DEFAULT_TENANT_JSON_PROPERTY = "default_tenant";
public static final String PRIVATE_TENANT_ENABLED_JSON_PROPERTY = "private_tenant_enabled";
public static final String MULTITENANCY_ENABLED_JSON_PROPERTY = "multitenancy_enabled";
public static final String SIGN_IN_OPTIONS = "sign_in_options";

private static final List<Route> ROUTES = addRoutesPrefix(
ImmutableList.of(new Route(GET, "/tenancy/config"), new Route(PUT, "/tenancy/config"))
Expand Down Expand Up @@ -119,7 +123,9 @@ public Map<String, DataType> allowedKeys() {
PRIVATE_TENANT_ENABLED_JSON_PROPERTY,
DataType.BOOLEAN,
MULTITENANCY_ENABLED_JSON_PROPERTY,
DataType.BOOLEAN
DataType.BOOLEAN,
SIGN_IN_OPTIONS,
DataType.ARRAY
);
}
});
Expand All @@ -132,6 +138,7 @@ private ToXContent multitenancyContent(final ConfigV7 config) {
.field(DEFAULT_TENANT_JSON_PROPERTY, config.dynamic.kibana.default_tenant)
.field(PRIVATE_TENANT_ENABLED_JSON_PROPERTY, config.dynamic.kibana.private_tenant_enabled)
.field(MULTITENANCY_ENABLED_JSON_PROPERTY, config.dynamic.kibana.multitenancy_enabled)
.field(SIGN_IN_OPTIONS, config.dynamic.kibana.sign_in_options)
.endObject();
}

Expand Down Expand Up @@ -177,6 +184,12 @@ private void updateAndValidatesValues(final ConfigV7 config, final JsonNode json
if (Objects.nonNull(jsonContent.findValue(MULTITENANCY_ENABLED_JSON_PROPERTY))) {
config.dynamic.kibana.multitenancy_enabled = jsonContent.findValue(MULTITENANCY_ENABLED_JSON_PROPERTY).asBoolean();
}
if (jsonContent.hasNonNull(SIGN_IN_OPTIONS) && jsonContent.findValue(SIGN_IN_OPTIONS).isEmpty() == false) {
JsonNode newOptions = jsonContent.findValue(SIGN_IN_OPTIONS);
List<DashboardSignInOption> options = getNewSignInOptions(newOptions, config.dynamic.authc);
config.dynamic.kibana.sign_in_options = options;
}

final String defaultTenant = Optional.ofNullable(config.dynamic.kibana.default_tenant).map(String::toLowerCase).orElse("");

if (!config.dynamic.kibana.private_tenant_enabled && ConfigConstants.TENANCY_PRIVATE_TENANT_NAME.equals(defaultTenant)) {
Expand All @@ -202,4 +215,20 @@ private void updateAndValidatesValues(final ConfigV7 config, final JsonNode json
}
}

private List<DashboardSignInOption> getNewSignInOptions(JsonNode newOptions, Authc authc) {

Set<String> domains = authc.getDomains().keySet();

return IntStream.range(0, newOptions.size()).mapToObj(newOptions::get).map(JsonNode::asText).filter(option -> {
// Checking if the new sign-in options are set in backend.
if (option.equals(DashboardSignInOption.ANONYMOUS.toString())
|| domains.stream().anyMatch(domain -> domain.contains(option.toLowerCase()))) {
return true;
} else {
throw new IllegalArgumentException(
"Validation failure: " + option.toUpperCase() + " authentication provider is not available for this cluster."
);
}
}).map(DashboardSignInOption::valueOf).collect(Collectors.toList());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@
import org.opensearch.security.securityconf.ConfigModel;
import org.opensearch.security.securityconf.DynamicConfigModel;
import org.opensearch.security.securityconf.SecurityRoles;
import org.opensearch.security.securityconf.impl.DashboardSignInOption;
import org.opensearch.security.support.ConfigConstants;
import org.opensearch.security.support.WildcardMatcher;
import org.opensearch.security.user.User;
Expand Down Expand Up @@ -621,6 +622,10 @@ public String dashboardsOpenSearchRole() {
return dcm.getDashboardsOpenSearchRole();
}

public List<DashboardSignInOption> getSignInOptions() {
return dcm.getSignInOptions();
}

private Set<String> evaluateAdditionalIndexPermissions(final ActionRequest request, final String originalAction) {
// --- check inner bulk requests
final Set<String> additionalPermissionsRequired = new HashSet<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ public void accept(RestChannel channel) throws Exception {
builder.field("multitenancy_enabled", evaluator.multitenancyEnabled());
builder.field("private_tenant_enabled", evaluator.privateTenantEnabled());
builder.field("default_tenant", evaluator.dashboardsDefaultTenant());
builder.field("sign_in_options", evaluator.getSignInOptions());
builder.field(
"password_validation_error_message",
client.settings().get(ConfigConstants.SECURITY_RESTAPI_PASSWORD_VALIDATION_ERROR_MESSAGE, DEFAULT_PASSWORD_MESSAGE)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
import org.opensearch.security.http.HTTPClientCertAuthenticator;
import org.opensearch.security.http.HTTPProxyAuthenticator;
import org.opensearch.security.http.proxy.HTTPExtendedProxyAuthenticator;
import org.opensearch.security.securityconf.impl.DashboardSignInOption;

public abstract class DynamicConfigModel {

Expand Down Expand Up @@ -105,6 +106,8 @@ public abstract class DynamicConfigModel {

public abstract Multimap<String, ClientBlockRegistry<String>> getAuthBackendClientBlockRegistries();

public abstract List<DashboardSignInOption> getSignInOptions();

public abstract Settings getDynamicOnBehalfOfSettings();

protected final Map<String, String> authImplMap = new HashMap<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
import org.opensearch.security.auth.HTTPAuthenticator;
import org.opensearch.security.auth.blocking.ClientBlockRegistry;
import org.opensearch.security.auth.internal.InternalAuthenticationBackend;
import org.opensearch.security.securityconf.impl.DashboardSignInOption;
import org.opensearch.security.securityconf.impl.v6.ConfigV6;
import org.opensearch.security.securityconf.impl.v6.ConfigV6.Authc;
import org.opensearch.security.securityconf.impl.v6.ConfigV6.AuthcDomain;
Expand Down Expand Up @@ -205,6 +206,11 @@ public Multimap<String, ClientBlockRegistry<String>> getAuthBackendClientBlockRe
return Multimaps.unmodifiableMultimap(authBackendClientBlockRegistries);
}

@Override
public List<DashboardSignInOption> getSignInOptions() {
return config.dynamic.kibana.sign_in_options;
}

@Override
public Settings getDynamicOnBehalfOfSettings() {
return Settings.EMPTY;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
import org.opensearch.security.auth.internal.NoOpAuthenticationBackend;
import org.opensearch.security.configuration.ClusterInfoHolder;
import org.opensearch.security.http.OnBehalfOfAuthenticator;
import org.opensearch.security.securityconf.impl.DashboardSignInOption;
import org.opensearch.security.securityconf.impl.v7.ConfigV7;
import org.opensearch.security.securityconf.impl.v7.ConfigV7.Authc;
import org.opensearch.security.securityconf.impl.v7.ConfigV7.AuthcDomain;
Expand Down Expand Up @@ -221,6 +222,11 @@ public Multimap<String, ClientBlockRegistry<String>> getAuthBackendClientBlockRe
return Multimaps.unmodifiableMultimap(authBackendClientBlockRegistries);
}

@Override
public List<DashboardSignInOption> getSignInOptions() {
return config.dynamic.kibana.sign_in_options;
}

@Override
public Settings getDynamicOnBehalfOfSettings() {
return Settings.builder()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/*
* 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.securityconf.impl;

public enum DashboardSignInOption {
BASIC("basic"),
SAML("saml"),
OPENID("openid"),
ANONYMOUS("anonymous");

private String option;

DashboardSignInOption(String option) {
this.option = option;
}

public String getOption() {
return option;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,10 @@

package org.opensearch.security.securityconf.impl.v6;

import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.regex.Pattern;

Expand All @@ -43,6 +45,7 @@

import org.opensearch.security.DefaultObjectMapper;
import org.opensearch.security.auth.internal.InternalAuthenticationBackend;
import org.opensearch.security.securityconf.impl.DashboardSignInOption;
import org.opensearch.security.setting.DeprecatedSettings;

public class ConfigV6 {
Expand Down Expand Up @@ -100,6 +103,8 @@ public static class Kibana {
public String opendistro_role = null;
public String index = ".kibana";
public boolean do_not_fail_on_forbidden;
@JsonInclude(JsonInclude.Include.NON_NULL)
public List<DashboardSignInOption> sign_in_options = Arrays.asList(DashboardSignInOption.BASIC);

@Override
public String toString() {
Expand All @@ -113,6 +118,8 @@ public String toString() {
+ index
+ ", do_not_fail_on_forbidden="
+ do_not_fail_on_forbidden
+ ", sign_in_options="
+ sign_in_options
+ "]";
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,10 @@

package org.opensearch.security.securityconf.impl.v7;

import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
Expand All @@ -44,6 +46,7 @@

import org.opensearch.security.DefaultObjectMapper;
import org.opensearch.security.auth.internal.InternalAuthenticationBackend;
import org.opensearch.security.securityconf.impl.DashboardSignInOption;
import org.opensearch.security.securityconf.impl.v6.ConfigV6;
import org.opensearch.security.setting.DeprecatedSettings;

Expand Down Expand Up @@ -76,6 +79,7 @@ public ConfigV7(ConfigV6 c6) {
dynamic.kibana.private_tenant_enabled = true;
dynamic.kibana.default_tenant = "";
dynamic.kibana.server_username = c6.dynamic.kibana.server_username;
dynamic.kibana.sign_in_options = c6.dynamic.kibana.sign_in_options;

dynamic.http = new Http();

Expand Down Expand Up @@ -168,6 +172,8 @@ public static class Kibana {
public String server_username = "kibanaserver";
public String opendistro_role = null;
public String index = ".kibana";
@JsonInclude(JsonInclude.Include.NON_NULL)
public List<DashboardSignInOption> sign_in_options = Arrays.asList(DashboardSignInOption.BASIC);

@Override
public String toString() {
Expand All @@ -183,6 +189,8 @@ public String toString() {
+ opendistro_role
+ ", index="
+ index
+ ", sign_in_options="
+ sign_in_options
+ "]";
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,13 @@
import org.apache.http.HttpStatus;
import org.junit.Test;

import org.opensearch.security.securityconf.impl.DashboardSignInOption;
import org.opensearch.security.support.ConfigConstants;
import org.opensearch.security.test.helper.rest.RestHelper.HttpResponse;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.hasItem;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.core.IsEqual.equalTo;
import static org.hamcrest.core.StringContains.containsString;

Expand Down Expand Up @@ -54,9 +57,43 @@ private void verifyTenantUpdate(final Header... header) throws Exception {
setPrivateTenantAsDefaultResponse.getStatusCode(),
equalTo(HttpStatus.SC_OK)
);
assertThat(getDashboardsinfoResponse.findArrayInJson("sign_in_options"), hasItem(DashboardSignInOption.BASIC.toString()));
assertThat(getDashboardsinfoResponse.findArrayInJson("sign_in_options"), not(hasItem(DashboardSignInOption.SAML.toString())));
assertThat(getDashboardsinfoResponse.findArrayInJson("sign_in_options"), not(hasItem(DashboardSignInOption.OPENID.toString())));

final HttpResponse updateDashboardSignInOptions = rh.executePutRequest(
"/_plugins/_security/api/tenancy/config",
"{\"sign_in_options\": [\"BASIC\", \"OPENID\"]}",
header
);
assertThat(updateDashboardSignInOptions.getBody(), updateDashboardSignInOptions.getStatusCode(), equalTo(HttpStatus.SC_OK));

getDashboardsinfoResponse = rh.executeGetRequest("/_plugins/_security/dashboardsinfo", ADMIN_FULL_ACCESS_USER);
assertThat(getDashboardsinfoResponse.getStatusCode(), equalTo(HttpStatus.SC_OK));
assertThat(getDashboardsinfoResponse.findValueInJson("default_tenant"), equalTo("Private"));

assertThat(getDashboardsinfoResponse.findArrayInJson("sign_in_options"), hasItem((DashboardSignInOption.BASIC.toString())));
assertThat(getDashboardsinfoResponse.findArrayInJson("sign_in_options"), hasItem((DashboardSignInOption.OPENID.toString())));

final HttpResponse updateUnavailableSignInOption = rh.executePutRequest(
"/_plugins/_security/api/tenancy/config",
"{\"sign_in_options\": [\"BASIC\", \"SAML\"]}",
header
);
assertThat(updateUnavailableSignInOption.getStatusCode(), equalTo(HttpStatus.SC_BAD_REQUEST));
assertThat(
updateUnavailableSignInOption.findValueInJson("error.reason"),
containsString("Validation failure: SAML authentication provider is not available for this cluster.")
);

// Ensuring the sign in options array has not been modified due to the Bad Request response.
getDashboardsinfoResponse = rh.executeGetRequest("/_plugins/_security/dashboardsinfo", ADMIN_FULL_ACCESS_USER);
assertThat(getDashboardsinfoResponse.getStatusCode(), equalTo(HttpStatus.SC_OK));

assertThat(getDashboardsinfoResponse.findArrayInJson("sign_in_options").size(), equalTo(2));
assertThat(getDashboardsinfoResponse.findArrayInJson("sign_in_options"), hasItem(DashboardSignInOption.BASIC.toString()));
assertThat(getDashboardsinfoResponse.findArrayInJson("sign_in_options"), hasItem(DashboardSignInOption.OPENID.toString()));
assertThat(getDashboardsinfoResponse.findArrayInJson("sign_in_options"), not(hasItem(DashboardSignInOption.SAML.toString())));
}

@Test
Expand Down Expand Up @@ -148,6 +185,30 @@ private void verifyTenantUpdateFailed(final Header... header) throws Exception {
setRandomStringAsDefaultTenant.findValueInJson("error.reason"),
containsString("Default tenant should be selected from one of the available tenants.")
);

final HttpResponse signInOptionsNonArrayValue = rh.executePutRequest(
"/_plugins/_security/api/tenancy/config",
"{\"sign_in_options\": \"BASIC\"}",
header
);
assertThat(signInOptionsNonArrayValue.getStatusCode(), equalTo(HttpStatus.SC_BAD_REQUEST));
assertThat(
signInOptionsNonArrayValue.getBody(),
signInOptionsNonArrayValue.findValueInJson("reason"),
containsString("Wrong datatype")
);

final HttpResponse invalidSignInOption = rh.executePutRequest(
"/_plugins/_security/api/tenancy/config",
"{\"sign_in_options\": [\"INVALID_OPTION\"]}",
header
);
assertThat(invalidSignInOption.getStatusCode(), equalTo(HttpStatus.SC_BAD_REQUEST));
assertThat(
invalidSignInOption.getBody(),
invalidSignInOption.findValueInJson("error.reason"),
containsString("authentication provider is not available for this cluster")
);
}

@Test
Expand Down
Loading

0 comments on commit 80cee28

Please sign in to comment.