Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dynamic sign in options #3869

Merged
merged 29 commits into from
Mar 19, 2024
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
2efc43b
Remove commit
davidosorno Nov 14, 2023
15789df
Create enum for dashboard sign in options
davidosorno Nov 29, 2023
5554d9c
Add sign in options variable
davidosorno Nov 29, 2023
f83bc0b
Test sign in options when requesting Config
davidosorno Nov 29, 2023
0ecc390
Merge branch 'main' into DynamicSignInOptions
davidosorno Nov 30, 2023
986b3e4
Update sign in options
davidosorno Dec 2, 2023
56c708a
Remove sign in option and match front end options name.
davidosorno Dec 2, 2023
a8a6055
Add sign in options when preparing request.
davidosorno Dec 6, 2023
5fa3258
Test dashboard sing in options
davidosorno Dec 6, 2023
4e00dfa
Merge branch 'main' into DynamicSignInOptions
davidosorno Dec 6, 2023
c95a85a
Add license header
davidosorno Dec 8, 2023
9a2a6d5
Merge branch 'main' into DynamicSignInOptions
davidosorno Dec 11, 2023
ce50e93
Merge branch 'main' into DynamicSignInOptions
davidosorno Dec 16, 2023
6fbe6f6
Merge branch 'main' into DynamicSignInOptions
davidosorno Dec 19, 2023
0569551
Add json values for testing
davidosorno Dec 20, 2023
508a905
Improve code readability
davidosorno Dec 20, 2023
d812fdd
Merge branch 'main' into DynamicSignInOptions
davidosorno Jan 3, 2024
ac706f3
Add test to validate when updating sign-in options
davidosorno Jan 3, 2024
2f05fae
Merge branch 'main' into DynamicSignInOptions
davidosorno Jan 16, 2024
5d91e4b
Toggle authentication dashboard sign-in ability
davidosorno Jan 19, 2024
b362782
Merge branch 'main' into DynamicSignInOptions
davidosorno Feb 5, 2024
f384a77
Add asserts to ensure sign-in option gets updated
davidosorno Feb 5, 2024
f6c523b
Remove function based on feedback received
davidosorno Feb 5, 2024
a4111ae
Rename variable to match security config pattern
davidosorno Feb 7, 2024
0fb4726
Rename variable to match security config pattern
davidosorno Feb 7, 2024
8c7343b
Merge branch 'DynamicSignInOptions' of https://github.com/davidosorno…
davidosorno Feb 7, 2024
62fb645
Merge branch 'main' into DynamicSignInOptions
davidosorno Mar 12, 2024
46c296f
Validate sign-in option is configured at backend
davidosorno Mar 12, 2024
0f1792b
Improve code readability using Stream
davidosorno Mar 13, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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,17 @@ 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("/dashboard_signin_options").contains(DashboardSignInOption.BASIC.toString()),
davidosorno marked this conversation as resolved.
Show resolved Hide resolved
is(true)
);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
package org.opensearch.security.dlic.rest.api;

import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.Objects;
Expand All @@ -33,6 +34,7 @@
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;
davidosorno marked this conversation as resolved.
Show resolved Hide resolved
import org.opensearch.security.securityconf.impl.SecurityDynamicConfiguration;
import org.opensearch.security.securityconf.impl.v7.ConfigV7;
import org.opensearch.security.support.ConfigConstants;
Expand All @@ -49,6 +51,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 DASHBOARD_SIGNIN_OPTIONS = "dashboard_signin_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 +122,9 @@ public Map<String, DataType> allowedKeys() {
PRIVATE_TENANT_ENABLED_JSON_PROPERTY,
DataType.BOOLEAN,
MULTITENANCY_ENABLED_JSON_PROPERTY,
DataType.BOOLEAN
DataType.BOOLEAN,
DASHBOARD_SIGNIN_OPTIONS,
DarshitChanpura marked this conversation as resolved.
Show resolved Hide resolved
DataType.ARRAY
);
}
});
Expand All @@ -132,6 +137,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(DASHBOARD_SIGNIN_OPTIONS, config.dynamic.kibana.dashboardSignInOptions)
.endObject();
}

Expand Down Expand Up @@ -177,6 +183,19 @@ 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 (Objects.nonNull(jsonContent.findValue(DASHBOARD_SIGNIN_OPTIONS))
davidosorno marked this conversation as resolved.
Show resolved Hide resolved
&& jsonContent.findValue(DASHBOARD_SIGNIN_OPTIONS).size() > 0) {
davidosorno marked this conversation as resolved.
Show resolved Hide resolved
JsonNode newOptions = jsonContent.findValue(DASHBOARD_SIGNIN_OPTIONS);
davidosorno marked this conversation as resolved.
Show resolved Hide resolved
List<DashboardSignInOption> options = new ArrayList<>();
for (int i = 0; i < newOptions.size(); i++) {
try {
String option = newOptions.get(i).asText();
options.add(DashboardSignInOption.valueOf(option));
} catch (Exception e) {}
}
config.dynamic.kibana.dashboardSignInOptions = options;
davidosorno marked this conversation as resolved.
Show resolved Hide resolved
}

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 Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,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 @@ -619,6 +620,10 @@ public String dashboardsOpenSearchRole() {
return dcm.getDashboardsOpenSearchRole();
}

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

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("dashboard_signin_options", evaluator.getDashboardSignInOptions());
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> getDashboardSignInOptions();

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 @@ -207,6 +208,11 @@ public Multimap<String, ClientBlockRegistry<String>> getAuthBackendClientBlockRe
return Multimaps.unmodifiableMultimap(authBackendClientBlockRegistries);
}

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

@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 @@ -223,6 +224,11 @@ public Multimap<String, ClientBlockRegistry<String>> getAuthBackendClientBlockRe
return Multimaps.unmodifiableMultimap(authBackendClientBlockRegistries);
}

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

@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 @@ -41,6 +43,7 @@

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

public class ConfigV6 {

Expand Down Expand Up @@ -97,6 +100,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> dashboardSignInOptions = Arrays.asList(DashboardSignInOption.BASIC);

@Override
public String toString() {
Expand All @@ -110,6 +115,8 @@ public String toString() {
+ index
+ ", do_not_fail_on_forbidden="
+ do_not_fail_on_forbidden
+ ", dashboard_signin_options="
+ dashboardSignInOptions
+ "]";
}

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 @@ -42,6 +44,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;

public class ConfigV7 {
Expand Down Expand Up @@ -73,6 +76,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.dashboardSignInOptions = c6.dynamic.kibana.dashboardSignInOptions;

dynamic.http = new Http();

Expand Down Expand Up @@ -165,6 +169,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> dashboardSignInOptions = Arrays.asList(DashboardSignInOption.BASIC);

@Override
public String toString() {
Expand All @@ -180,6 +186,8 @@ public String toString() {
+ opendistro_role
+ ", index="
+ index
+ ", dashboard_signin_options="
+ dashboardSignInOptions
+ "]";
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,12 @@
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.contains;
import static org.hamcrest.core.IsEqual.equalTo;
import static org.hamcrest.core.StringContains.containsString;

Expand Down Expand Up @@ -57,6 +59,8 @@ private void verifyTenantUpdate(final Header... header) throws Exception {
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(getSettingResponse.findArrayInJson("dashboard_signin_options"), contains(DashboardSignInOption.BASIC.toString()));
}

@Test
Expand Down Expand Up @@ -148,6 +152,18 @@ 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",
"{\"dashboard_signin_options\": \"BASIC\"}",
header
);
assertThat(signInOptionsNonArrayValue.getStatusCode(), equalTo(HttpStatus.SC_BAD_REQUEST));
davidosorno marked this conversation as resolved.
Show resolved Hide resolved
assertThat(
signInOptionsNonArrayValue.getBody(),
signInOptionsNonArrayValue.findValueInJson("reason"),
containsString("Wrong datatype")
);
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@

import org.opensearch.security.DefaultObjectMapper;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.is;

@RunWith(Parameterized.class)
public class ConfigV6Test {
private final boolean omitDefaults;
Expand All @@ -31,6 +35,9 @@ public static Iterable<Boolean> omitDefaults() {

public void assertEquals(ConfigV6.Kibana expected, JsonNode node) {
Assert.assertEquals(expected.multitenancy_enabled, node.get("multitenancy_enabled").asBoolean());
assertThat(node.get("dashboardSignInOptions").isArray(), is(true));
assertThat(node.get("dashboardSignInOptions").toString(), containsString(expected.dashboardSignInOptions.get(0).toString()));

if (expected.server_username == null) {
Assert.assertNull(node.get("server_username"));
} else {
Expand All @@ -57,6 +64,7 @@ public void assertEquals(ConfigV6.Kibana expected, JsonNode node) {

private void assertEquals(ConfigV6.Kibana expected, ConfigV6.Kibana actual) {
Assert.assertEquals(expected.multitenancy_enabled, actual.multitenancy_enabled);
assertThat(expected.dashboardSignInOptions, is(actual.dashboardSignInOptions));
if (expected.server_username == null) {
// null is restored to default instead of null
Assert.assertEquals(new ConfigV6.Kibana().server_username, actual.server_username);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@

import org.opensearch.security.DefaultObjectMapper;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.is;

@RunWith(Parameterized.class)
public class ConfigV7Test {
private final boolean omitDefaults;
Expand All @@ -31,6 +35,9 @@ public static Iterable<Boolean> omitDefaults() {

public void assertEquals(ConfigV7.Kibana expected, JsonNode node) {
Assert.assertEquals(expected.multitenancy_enabled, node.get("multitenancy_enabled").asBoolean());
assertThat(node.get("dashboardSignInOptions").isArray(), is(true));
assertThat(node.get("dashboardSignInOptions").toString(), containsString(expected.dashboardSignInOptions.get(0).toString()));

if (expected.server_username == null) {
Assert.assertNull(node.get("server_username"));
} else {
Expand All @@ -51,6 +58,7 @@ public void assertEquals(ConfigV7.Kibana expected, JsonNode node) {

private void assertEquals(ConfigV7.Kibana expected, ConfigV7.Kibana actual) {
Assert.assertEquals(expected.multitenancy_enabled, actual.multitenancy_enabled);
assertThat(expected.dashboardSignInOptions, is(actual.dashboardSignInOptions));
if (expected.server_username == null) {
// null is restored to default instead of null
Assert.assertEquals(new ConfigV7.Kibana().server_username, actual.server_username);
Expand Down
Loading