From 63aa09887c9a652f9cca3346e293ba772e9d35c3 Mon Sep 17 00:00:00 2001 From: Dave Lago Date: Thu, 11 May 2023 11:57:48 -0400 Subject: [PATCH 1/5] Adding tenets to CONTRIBUTING.md (#2762) Signed-off-by: Dave Lago --- CONTRIBUTING.md | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 202f8977b7..a7e7d89e9a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -8,3 +8,25 @@ OpenSearch is a community project that is built and maintained by people just li Visit the following link(s) for more information on specific practices: - [Triaging](./TRIAGING.md) + +## How we work + +#### Quality and security form the foundation of our efforts +* We deliver quality security solutions by understanding issues thoroughly, acting iteratively, and validating solutions through rigorous testing. +* We hold security to the highest standards of quality. Quality is the first step in building trust with users, stakeholders, and our community as a whole. +* We move swiftly to solve problems. But we don’t sacrifice quality to achieve quick wins, meet performance indicators, or get the bragging rights related to launching popular, high-profile features. + + + +#### Privacy won’t be compromised +* Privacy is a key part of security, and we make sure it isn’t compromised in the pursuit of creating benefits. +* When we make decisions, we carefully consider any potential impacts to privacy and make sure those decisions protect our users’ and stakeholders’ data. +* We maintain a focus on creating software that empowers cluster administrators to keep their sensitive data safe. + + + +#### Transparent collaboration creates secure outcomes +* Transparent collaboration promotes community inclusion, creates a space for concise and authentic communication, and supports accountability. +* We operate through transparent collaboration. We believe a secure product is built through diverse perspectives, knowledge sharing, candid discussions, and doing our work in the open when and where it’s safe to do so. +* We are relationship builders who create safe, respectful, and accessible spaces for everyone so we can engage and work towards the common goal of building secure solutions. +* When circumstances do require privacy, we make every effort to quickly resolve those requirements and return circumstances to a state of full visibility with our community and collaborators. From 48c4f960293db4f7cc73f733e070c4e048af1b6d Mon Sep 17 00:00:00 2001 From: Sean Kao Date: Fri, 12 May 2023 12:27:10 -0700 Subject: [PATCH 2/5] Add default roles for sql plugin (#2729) Signed-off-by: Sean Kao --- config/roles.yml | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/config/roles.yml b/config/roles.yml index 00ebdf6edb..e4eb3ac535 100644 --- a/config/roles.yml +++ b/config/roles.yml @@ -136,6 +136,19 @@ observability_full_access: - 'cluster:admin/opensearch/observability/delete' - 'cluster:admin/opensearch/observability/get' +# Allows users to all PPL functionality +ppl_full_access: + reserved: true + cluster_permissions: + - 'cluster:admin/opensearch/ppl' + index_permissions: + - index_patterns: + - '*' + allowed_actions: + - 'indices:admin/mappings/get' + - 'indices:data/read/search*' + - 'indices:monitor/settings/get' + # Allows users to read and download Reports reports_instances_read_access: reserved: true @@ -228,6 +241,16 @@ cross_cluster_replication_follower_full_access: - "indices:admin/plugins/replication/index/update" - "indices:admin/plugins/replication/index/status_check" +# Allows users to use all cross cluster search functionality at remote cluster +cross_cluster_search_remote_full_access: + reserved: true + index_permissions: + - index_patterns: + - '*' + allowed_actions: + - 'indices:admin/shards/search_shards' + - 'indices:data/read/search' + # Allow users to read ML stats/models/tasks ml_read_access: reserved: true From 7c4e06d323135ef8ab616ff083c420ba26287f18 Mon Sep 17 00:00:00 2001 From: Paras Jain Date: Sat, 13 May 2023 07:17:35 +0530 Subject: [PATCH 3/5] `deserializeSafeFromHeader` uses `context.getHeader(headerName)` instead of `context.getHeaders()` (#2768) --- .../org/opensearch/security/support/HeaderHelper.java | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/src/main/java/org/opensearch/security/support/HeaderHelper.java b/src/main/java/org/opensearch/security/support/HeaderHelper.java index a4ee123347..1a7484a781 100644 --- a/src/main/java/org/opensearch/security/support/HeaderHelper.java +++ b/src/main/java/org/opensearch/security/support/HeaderHelper.java @@ -27,7 +27,6 @@ package org.opensearch.security.support; import java.io.Serializable; -import java.util.Map; import com.google.common.base.Strings; @@ -57,15 +56,8 @@ public static String getSafeFromHeader(final ThreadContext context, final String return null; } - String headerValue = null; - - Map headers = context.getHeaders(); - if (!headers.containsKey(headerName) || (headerValue = headers.get(headerName)) == null) { - return null; - } - if (isInterClusterRequest(context) || isTrustedClusterRequest(context) || isDirectRequest(context)) { - return headerValue; + return context.getHeader(headerName); } return null; From 94db5dbb0e2351b6d2b31b2a0969863acbad9ebe Mon Sep 17 00:00:00 2001 From: Andrey Pleskach Date: Mon, 15 May 2023 17:49:51 +0200 Subject: [PATCH 4/5] Fix multitency config update (#2758) Moved multi-tenancy to REST API implementation Signed-off-by: Andrey Pleskach --- .../security/OpenSearchSecurityPlugin.java | 10 - .../security/action/tenancy/EmptyRequest.java | 35 --- .../tenancy/TenancyConfigRestHandler.java | 65 ------ .../tenancy/TenancyConfigRetrieveActions.java | 24 -- .../TenancyConfigRetrieveResponse.java | 70 ------ .../TenancyConfigRetrieveTransportAction.java | 63 ----- .../tenancy/TenancyConfigUpdateAction.java | 26 --- .../tenancy/TenancyConfigUpdateRequest.java | 69 ------ .../TenancyConfigUpdateTransportAction.java | 155 ------------- .../action/tenancy/TenancyConfigs.java | 18 -- .../rest/api/MultiTenancyConfigApiAction.java | 217 ++++++++++++++++++ .../dlic/rest/api/SecurityRestApiActions.java | 1 + .../MultiTenancyConfigValidator.java | 32 +++ .../security/support/ConfigConstants.java | 4 +- .../rest/api/MultiTenancyConfigApiTest.java | 170 ++++++++++++++ .../test/TenancyDefaultTenantTests.java | 96 -------- .../test/TenancyMultitenancyEnabledTests.java | 43 ++-- .../TenancyPrivateTenantEnabledTests.java | 55 ++--- .../resources/multitenancy/internal_users.yml | 6 + src/test/resources/multitenancy/roles.yml | 2 + .../resources/multitenancy/roles_mapping.yml | 5 + 21 files changed, 472 insertions(+), 694 deletions(-) delete mode 100644 src/main/java/org/opensearch/security/action/tenancy/EmptyRequest.java delete mode 100644 src/main/java/org/opensearch/security/action/tenancy/TenancyConfigRestHandler.java delete mode 100644 src/main/java/org/opensearch/security/action/tenancy/TenancyConfigRetrieveActions.java delete mode 100644 src/main/java/org/opensearch/security/action/tenancy/TenancyConfigRetrieveResponse.java delete mode 100644 src/main/java/org/opensearch/security/action/tenancy/TenancyConfigRetrieveTransportAction.java delete mode 100644 src/main/java/org/opensearch/security/action/tenancy/TenancyConfigUpdateAction.java delete mode 100644 src/main/java/org/opensearch/security/action/tenancy/TenancyConfigUpdateRequest.java delete mode 100644 src/main/java/org/opensearch/security/action/tenancy/TenancyConfigUpdateTransportAction.java delete mode 100644 src/main/java/org/opensearch/security/action/tenancy/TenancyConfigs.java create mode 100644 src/main/java/org/opensearch/security/dlic/rest/api/MultiTenancyConfigApiAction.java create mode 100644 src/main/java/org/opensearch/security/dlic/rest/validation/MultiTenancyConfigValidator.java create mode 100644 src/test/java/org/opensearch/security/dlic/rest/api/MultiTenancyConfigApiTest.java delete mode 100644 src/test/java/org/opensearch/security/multitenancy/test/TenancyDefaultTenantTests.java diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java index 815f96f0c5..2db8b5d2eb 100644 --- a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java +++ b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java @@ -117,11 +117,6 @@ import org.opensearch.search.query.QuerySearchResult; import org.opensearch.security.action.configupdate.ConfigUpdateAction; import org.opensearch.security.action.configupdate.TransportConfigUpdateAction; -import org.opensearch.security.action.tenancy.TenancyConfigRestHandler; -import org.opensearch.security.action.tenancy.TenancyConfigRetrieveActions; -import org.opensearch.security.action.tenancy.TenancyConfigRetrieveTransportAction; -import org.opensearch.security.action.tenancy.TenancyConfigUpdateAction; -import org.opensearch.security.action.tenancy.TenancyConfigUpdateTransportAction; import org.opensearch.security.action.whoami.TransportWhoAmIAction; import org.opensearch.security.action.whoami.WhoAmIAction; import org.opensearch.security.auditlog.AuditLog; @@ -480,7 +475,6 @@ public List getRestHandlers(Settings settings, RestController restC Objects.requireNonNull(cs), Objects.requireNonNull(adminDns), Objects.requireNonNull(cr))); handlers.add(new SecurityConfigUpdateAction(settings, restController, Objects.requireNonNull(threadPool), adminDns, configPath, principalExtractor)); handlers.add(new SecurityWhoAmIAction(settings, restController, Objects.requireNonNull(threadPool), adminDns, configPath, principalExtractor)); - handlers.add(new TenancyConfigRestHandler()); handlers.addAll( SecurityRestApiActions.getHandler( settings, @@ -518,10 +512,6 @@ public UnaryOperator getRestHandlerWrapper(final ThreadContext thre if(!disabled && !SSLConfig.isSslOnlyMode()) { actions.add(new ActionHandler<>(ConfigUpdateAction.INSTANCE, TransportConfigUpdateAction.class)); actions.add(new ActionHandler<>(WhoAmIAction.INSTANCE, TransportWhoAmIAction.class)); - - actions.add(new ActionHandler<>(TenancyConfigRetrieveActions.INSTANCE, TenancyConfigRetrieveTransportAction.class)); - actions.add(new ActionHandler<>(TenancyConfigUpdateAction.INSTANCE, TenancyConfigUpdateTransportAction.class)); - } return actions; } diff --git a/src/main/java/org/opensearch/security/action/tenancy/EmptyRequest.java b/src/main/java/org/opensearch/security/action/tenancy/EmptyRequest.java deleted file mode 100644 index 607216a8c3..0000000000 --- a/src/main/java/org/opensearch/security/action/tenancy/EmptyRequest.java +++ /dev/null @@ -1,35 +0,0 @@ -/* - * 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.action.tenancy; - -import java.io.IOException; - -import org.opensearch.action.ActionRequest; -import org.opensearch.action.ActionRequestValidationException; -import org.opensearch.common.io.stream.StreamInput; - -public class EmptyRequest extends ActionRequest { - - public EmptyRequest(final StreamInput in) throws IOException { - super(in); - } - - public EmptyRequest() throws IOException { - super(); - } - - @Override - public ActionRequestValidationException validate() - { - return null; - } -} diff --git a/src/main/java/org/opensearch/security/action/tenancy/TenancyConfigRestHandler.java b/src/main/java/org/opensearch/security/action/tenancy/TenancyConfigRestHandler.java deleted file mode 100644 index 0a00d16694..0000000000 --- a/src/main/java/org/opensearch/security/action/tenancy/TenancyConfigRestHandler.java +++ /dev/null @@ -1,65 +0,0 @@ -/* - * 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.action.tenancy; - -import java.io.IOException; -import java.util.List; - -import com.google.common.collect.ImmutableList; - -import org.opensearch.client.node.NodeClient; -import org.opensearch.rest.BaseRestHandler; -import org.opensearch.rest.RestRequest; -import org.opensearch.rest.action.RestToXContentListener; - -import static org.opensearch.rest.RestRequest.Method.GET; -import static org.opensearch.rest.RestRequest.Method.PUT; - -public class TenancyConfigRestHandler extends BaseRestHandler { - - public TenancyConfigRestHandler() { - super(); - } - - @Override - public String getName() { - return "Multi Tenancy actions to Retrieve / Update configs."; - } - - @Override - public List routes() { - return ImmutableList.of( - new Route(GET, "/_plugins/_security/api/tenancy/config"), - new Route(PUT, "/_plugins/_security/api/tenancy/config") - ); - } - - @Override - protected RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient nodeClient) throws IOException { - - switch (request.method()) { - case GET: - return channel -> nodeClient.execute( - TenancyConfigRetrieveActions.INSTANCE, - new EmptyRequest(), - new RestToXContentListener<>(channel)); - case PUT: - return channel -> nodeClient.execute( - TenancyConfigUpdateAction.INSTANCE, - TenancyConfigUpdateRequest.fromXContent(request.contentParser()), - new RestToXContentListener<>(channel)); - default: - throw new RuntimeException("Not implemented"); - } - } - -} diff --git a/src/main/java/org/opensearch/security/action/tenancy/TenancyConfigRetrieveActions.java b/src/main/java/org/opensearch/security/action/tenancy/TenancyConfigRetrieveActions.java deleted file mode 100644 index 796f233f13..0000000000 --- a/src/main/java/org/opensearch/security/action/tenancy/TenancyConfigRetrieveActions.java +++ /dev/null @@ -1,24 +0,0 @@ -/* - * 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.action.tenancy; - -import org.opensearch.action.ActionType; - -public class TenancyConfigRetrieveActions extends ActionType { - - public static final TenancyConfigRetrieveActions INSTANCE = new TenancyConfigRetrieveActions(); - public static final String NAME = "cluster:feature/tenancy/config/read"; - - protected TenancyConfigRetrieveActions() { - super(NAME, TenancyConfigRetrieveResponse::new); - } -} diff --git a/src/main/java/org/opensearch/security/action/tenancy/TenancyConfigRetrieveResponse.java b/src/main/java/org/opensearch/security/action/tenancy/TenancyConfigRetrieveResponse.java deleted file mode 100644 index 463cc7d831..0000000000 --- a/src/main/java/org/opensearch/security/action/tenancy/TenancyConfigRetrieveResponse.java +++ /dev/null @@ -1,70 +0,0 @@ -/* - * 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.action.tenancy; - -import java.io.IOException; - -import org.opensearch.action.ActionResponse; -import org.opensearch.common.Strings; -import org.opensearch.common.io.stream.StreamInput; -import org.opensearch.common.io.stream.StreamOutput; -import org.opensearch.common.xcontent.XContentType; -import org.opensearch.core.xcontent.ToXContentObject; -import org.opensearch.core.xcontent.XContentBuilder; - -public class TenancyConfigRetrieveResponse extends ActionResponse implements ToXContentObject { - - public TenancyConfigs tenancyConfigs = new TenancyConfigs(); - - public TenancyConfigRetrieveResponse(final StreamInput in) throws IOException { - super(in); - this.tenancyConfigs.multitenancy_enabled = in.readOptionalBoolean(); - this.tenancyConfigs.private_tenant_enabled = in.readOptionalBoolean(); - this.tenancyConfigs.default_tenant = in.readOptionalString(); - } - - public TenancyConfigRetrieveResponse(final TenancyConfigs tenancyConfigs) { - this.tenancyConfigs = tenancyConfigs; - } - - public TenancyConfigs getMultitenancyConfig() { - return tenancyConfigs; - } - - public Boolean getMultitenancyEnabled() { return tenancyConfigs.multitenancy_enabled; } - - public Boolean getPrivateTenantEnabled() { return tenancyConfigs.private_tenant_enabled; } - - public String getDefaultTenant() { return tenancyConfigs.default_tenant; } - - @Override - public void writeTo(final StreamOutput out) throws IOException { - out.writeBoolean(getMultitenancyEnabled()); - out.writeBoolean(getPrivateTenantEnabled()); - out.writeString(getDefaultTenant()); - } - - @Override - public String toString() { - return Strings.toString(XContentType.JSON, this, true, true); - } - - @Override - public XContentBuilder toXContent(final XContentBuilder builder, final Params params) throws IOException { - builder.startObject(); - builder.field("multitenancy_enabled", getMultitenancyEnabled()); - builder.field("private_tenant_enabled", getPrivateTenantEnabled()); - builder.field("default_tenant", getDefaultTenant()); - builder.endObject(); - return builder; - } -} diff --git a/src/main/java/org/opensearch/security/action/tenancy/TenancyConfigRetrieveTransportAction.java b/src/main/java/org/opensearch/security/action/tenancy/TenancyConfigRetrieveTransportAction.java deleted file mode 100644 index a68bbae85e..0000000000 --- a/src/main/java/org/opensearch/security/action/tenancy/TenancyConfigRetrieveTransportAction.java +++ /dev/null @@ -1,63 +0,0 @@ -/* - * 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.action.tenancy; - -import java.util.Collections; - -import org.opensearch.action.ActionListener; -import org.opensearch.action.support.ActionFilters; -import org.opensearch.action.support.HandledTransportAction; -import org.opensearch.common.inject.Inject; -import org.opensearch.common.settings.Settings; -import org.opensearch.security.configuration.ConfigurationRepository; -import org.opensearch.security.securityconf.impl.CType; -import org.opensearch.security.securityconf.impl.SecurityDynamicConfiguration; -import org.opensearch.security.securityconf.impl.v7.ConfigV7; -import org.opensearch.tasks.Task; -import org.opensearch.transport.TransportService; - -public class TenancyConfigRetrieveTransportAction - extends HandledTransportAction { - - private final ConfigurationRepository config; - - @Inject - public TenancyConfigRetrieveTransportAction(final Settings settings, - final TransportService transportService, - final ActionFilters actionFilters, - final ConfigurationRepository config) { - super(TenancyConfigRetrieveActions.NAME, transportService, actionFilters, EmptyRequest::new); - - this.config = config; - } - - /** Load the configuration from the security index and return a copy */ - protected final SecurityDynamicConfiguration load() { - return config.getConfigurationsFromIndex(Collections.singleton(CType.CONFIG), false).get(CType.CONFIG).deepClone(); - } - - @Override - protected void doExecute(final Task task, final EmptyRequest request, final ActionListener listener) { - - // Get the security configuration and lookup the config setting state - final SecurityDynamicConfiguration dynamicConfig = load(); - ConfigV7 config = (ConfigV7)dynamicConfig.getCEntry("config"); - - final TenancyConfigs tenancyConfigs= new TenancyConfigs(); - - tenancyConfigs.multitenancy_enabled = config.dynamic.kibana.multitenancy_enabled; - tenancyConfigs.private_tenant_enabled = config.dynamic.kibana.private_tenant_enabled; - tenancyConfigs.default_tenant = config.dynamic.kibana.default_tenant; - - listener.onResponse(new TenancyConfigRetrieveResponse(tenancyConfigs)); - } -} diff --git a/src/main/java/org/opensearch/security/action/tenancy/TenancyConfigUpdateAction.java b/src/main/java/org/opensearch/security/action/tenancy/TenancyConfigUpdateAction.java deleted file mode 100644 index 73d515b7d8..0000000000 --- a/src/main/java/org/opensearch/security/action/tenancy/TenancyConfigUpdateAction.java +++ /dev/null @@ -1,26 +0,0 @@ -/* - * 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.action.tenancy; - -import org.opensearch.action.ActionType; - -public class TenancyConfigUpdateAction extends ActionType { - - public static final TenancyConfigUpdateAction INSTANCE = new TenancyConfigUpdateAction(); - public static final String NAME = "cluster:feature/tenancy/config/update"; - - - protected TenancyConfigUpdateAction() - { - super(NAME, TenancyConfigRetrieveResponse::new); - } -} diff --git a/src/main/java/org/opensearch/security/action/tenancy/TenancyConfigUpdateRequest.java b/src/main/java/org/opensearch/security/action/tenancy/TenancyConfigUpdateRequest.java deleted file mode 100644 index 2d03f698a6..0000000000 --- a/src/main/java/org/opensearch/security/action/tenancy/TenancyConfigUpdateRequest.java +++ /dev/null @@ -1,69 +0,0 @@ -/* - * 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.action.tenancy; -import java.io.IOException; - -import org.opensearch.action.ActionRequest; -import org.opensearch.action.ActionRequestValidationException; -import org.opensearch.common.io.stream.StreamInput; -import org.opensearch.core.ParseField; -import org.opensearch.core.xcontent.ConstructingObjectParser; -import org.opensearch.core.xcontent.XContentParser; - -public class TenancyConfigUpdateRequest extends ActionRequest { - - private TenancyConfigs tenancyConfigs = new TenancyConfigs(); - - public TenancyConfigUpdateRequest(final StreamInput in) throws IOException { - super(in); - in.readOptionalBoolean(); - in.readOptionalBoolean(); - in.readOptionalString(); - } - - public TenancyConfigUpdateRequest(final Boolean multitenancy_enabled, final Boolean private_tenant_enabled, final String default_tenant) { - super(); - this.tenancyConfigs.multitenancy_enabled = multitenancy_enabled; - this.tenancyConfigs.private_tenant_enabled = private_tenant_enabled; - this.tenancyConfigs.default_tenant = default_tenant; - } - - public TenancyConfigs getTenancyConfigs() { - return tenancyConfigs; - } - - @Override - public ActionRequestValidationException validate() { - if (getTenancyConfigs() == null) { - final ActionRequestValidationException validationException = new ActionRequestValidationException(); - validationException.addValidationError("Missing tenancy configs"); - return validationException; - } - return null; - } - - private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>( - TenancyConfigUpdateRequest.class.getName(), - args -> new TenancyConfigUpdateRequest((Boolean)args[0], (Boolean) args[1], (String) args[2]) - ); - - static { - PARSER.declareBoolean(ConstructingObjectParser.optionalConstructorArg(), new ParseField("multitenancy_enabled")); - PARSER.declareBoolean(ConstructingObjectParser.optionalConstructorArg(), new ParseField("private_tenant_enabled")); - PARSER.declareString(ConstructingObjectParser.optionalConstructorArg(), new ParseField("default_tenant")); - - } - - public static TenancyConfigUpdateRequest fromXContent(final XContentParser parser) { - return PARSER.apply(parser, null); - } -} diff --git a/src/main/java/org/opensearch/security/action/tenancy/TenancyConfigUpdateTransportAction.java b/src/main/java/org/opensearch/security/action/tenancy/TenancyConfigUpdateTransportAction.java deleted file mode 100644 index 1d4b563ca4..0000000000 --- a/src/main/java/org/opensearch/security/action/tenancy/TenancyConfigUpdateTransportAction.java +++ /dev/null @@ -1,155 +0,0 @@ -/* - * 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.action.tenancy; - -import java.io.IOException; -import java.util.Collections; -import java.util.HashSet; -import java.util.Set; - -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; - -import org.opensearch.action.ActionListener; -import org.opensearch.action.index.IndexResponse; -import org.opensearch.action.support.ActionFilters; -import org.opensearch.action.support.HandledTransportAction; -import org.opensearch.client.Client; -import org.opensearch.common.inject.Inject; -import org.opensearch.common.settings.Settings; -import org.opensearch.common.util.concurrent.ThreadContext; -import org.opensearch.security.configuration.ConfigurationRepository; -import org.opensearch.security.dlic.rest.api.AbstractApiAction; -import org.opensearch.security.securityconf.impl.CType; -import org.opensearch.security.securityconf.impl.SecurityDynamicConfiguration; -import org.opensearch.security.securityconf.impl.v7.ConfigV7; -import org.opensearch.security.support.ConfigConstants; -import org.opensearch.tasks.Task; -import org.opensearch.threadpool.ThreadPool; -import org.opensearch.transport.TransportService; - -public class TenancyConfigUpdateTransportAction extends HandledTransportAction { - - private static final Logger log = LogManager.getLogger(TenancyConfigUpdateTransportAction.class); - - private final String securityIndex; - private final ConfigurationRepository config; - private final Client client; - private final ThreadPool pool; - - @Inject - public TenancyConfigUpdateTransportAction(final Settings settings, - final TransportService transportService, - final ActionFilters actionFilters, - final ConfigurationRepository config, - final ThreadPool pool, - final Client client) { - super(TenancyConfigUpdateAction.NAME, transportService, actionFilters, TenancyConfigUpdateRequest::new); - - this.securityIndex = settings.get(ConfigConstants.SECURITY_CONFIG_INDEX_NAME, ConfigConstants.OPENDISTRO_SECURITY_DEFAULT_CONFIG_INDEX); - - this.config = config; - this.client = client; - this.pool = pool; - } - - /** Load the configuration from the security index and return a copy */ - protected final SecurityDynamicConfiguration load() { - return config.getConfigurationsFromIndex(Collections.singleton(CType.CONFIG), false).get(CType.CONFIG).deepClone(); - } - - private Set getAcceptableDefaultTenants() { - Set acceptableDefaultTenants = new HashSet(); - acceptableDefaultTenants.add(ConfigConstants.TENANCY_GLOBAL_TENANT_DEFAULT_NAME); - acceptableDefaultTenants.add(ConfigConstants.TENANCY_GLOBAL_TENANT_NAME); - acceptableDefaultTenants.add(ConfigConstants.TENANCY_PRIVATE_TENANT_NAME); - return acceptableDefaultTenants; - } - - private Set getAllConfiguredTenantNames() { - - return this.config.getConfiguration(CType.TENANTS).getCEntries().keySet(); - } - - protected void validate(ConfigV7 updatedConfig) { - if(!updatedConfig.dynamic.kibana.private_tenant_enabled && (updatedConfig.dynamic.kibana.default_tenant).equals(ConfigConstants.TENANCY_PRIVATE_TENANT_NAME)) { - throw new IllegalArgumentException("Private tenant can not be disabled if it is the default tenant."); - } - - Set acceptableDefaultTenants = getAcceptableDefaultTenants(); - - if(acceptableDefaultTenants.contains(updatedConfig.dynamic.kibana.default_tenant)) { - return; - } - - Set availableTenants = getAllConfiguredTenantNames(); - - if(!availableTenants.contains(updatedConfig.dynamic.kibana.default_tenant)){ - throw new IllegalArgumentException(updatedConfig.dynamic.kibana.default_tenant + " can not be set to default tenant. Default tenant should be selected from one of the available tenants."); - } - - } - - @Override - protected void doExecute(final Task task, final TenancyConfigUpdateRequest request, final ActionListener listener) { - - // Get the current security config and prepare the config with the updated value - final SecurityDynamicConfiguration dynamicConfig = load(); - final ConfigV7 config = (ConfigV7)dynamicConfig.getCEntry("config"); - - final TenancyConfigs tenancyConfigs = request.getTenancyConfigs(); - if(tenancyConfigs.multitenancy_enabled != null) - { - config.dynamic.kibana.multitenancy_enabled = tenancyConfigs.multitenancy_enabled; - } - - if(tenancyConfigs.private_tenant_enabled != null) - { - config.dynamic.kibana.private_tenant_enabled = tenancyConfigs.private_tenant_enabled; - } - - if(tenancyConfigs.default_tenant != null) - { - config.dynamic.kibana.default_tenant = tenancyConfigs.default_tenant; - } - - validate(config); - - dynamicConfig.putCEntry("config", config); - - // When performing an update to the configuration run as admin - try (final ThreadContext.StoredContext stashedContext = pool.getThreadContext().stashContext()) { - // Update the security configuration and make sure the cluster has fully refreshed - AbstractApiAction.saveAndUpdateConfigs(this.securityIndex, this.client, CType.CONFIG, dynamicConfig, new ActionListener(){ - - @Override - public void onResponse(final IndexResponse response) { - // After processing the request, restore the user context - stashedContext.close(); - try { - // Lookup the current value and notify the listener - client.execute(TenancyConfigRetrieveActions.INSTANCE, new EmptyRequest(), listener); - } catch (IOException ioe) { - log.error(ioe); - listener.onFailure(ioe); - } - } - - @Override - public void onFailure(Exception e) { - log.error(e); - listener.onFailure(e); - } - }); - } - } -} diff --git a/src/main/java/org/opensearch/security/action/tenancy/TenancyConfigs.java b/src/main/java/org/opensearch/security/action/tenancy/TenancyConfigs.java deleted file mode 100644 index 4e8fc41ef4..0000000000 --- a/src/main/java/org/opensearch/security/action/tenancy/TenancyConfigs.java +++ /dev/null @@ -1,18 +0,0 @@ -/* - * 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.action.tenancy; - -public class TenancyConfigs { - public Boolean multitenancy_enabled; - public Boolean private_tenant_enabled; - public String default_tenant; -} diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/MultiTenancyConfigApiAction.java b/src/main/java/org/opensearch/security/dlic/rest/api/MultiTenancyConfigApiAction.java new file mode 100644 index 0000000000..e5ec82c245 --- /dev/null +++ b/src/main/java/org/opensearch/security/dlic/rest/api/MultiTenancyConfigApiAction.java @@ -0,0 +1,217 @@ +/* + * 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.dlic.rest.api; + +import java.io.IOException; +import java.nio.file.Path; +import java.util.List; +import java.util.Objects; +import java.util.Optional; +import java.util.Set; +import java.util.stream.Collectors; + +import com.fasterxml.jackson.databind.JsonNode; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; + +import org.opensearch.action.index.IndexResponse; +import org.opensearch.client.Client; +import org.opensearch.cluster.service.ClusterService; +import org.opensearch.common.bytes.BytesReference; +import org.opensearch.common.settings.Settings; +import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.rest.BytesRestResponse; +import org.opensearch.rest.RestChannel; +import org.opensearch.rest.RestController; +import org.opensearch.rest.RestRequest; +import org.opensearch.rest.RestStatus; +import org.opensearch.security.auditlog.AuditLog; +import org.opensearch.security.configuration.AdminDNs; +import org.opensearch.security.configuration.ConfigurationRepository; +import org.opensearch.security.dlic.rest.validation.AbstractConfigurationValidator; +import org.opensearch.security.dlic.rest.validation.MultiTenancyConfigValidator; +import org.opensearch.security.privileges.PrivilegesEvaluator; +import org.opensearch.security.securityconf.impl.CType; +import org.opensearch.security.securityconf.impl.SecurityDynamicConfiguration; +import org.opensearch.security.securityconf.impl.v7.ConfigV7; +import org.opensearch.security.ssl.transport.PrincipalExtractor; +import org.opensearch.security.support.ConfigConstants; +import org.opensearch.threadpool.ThreadPool; + +import static org.opensearch.rest.RestRequest.Method.GET; +import static org.opensearch.rest.RestRequest.Method.PUT; +import static org.opensearch.security.dlic.rest.support.Utils.addRoutesPrefix; + + +public class MultiTenancyConfigApiAction extends AbstractApiAction { + + private static final List ROUTES = addRoutesPrefix( + ImmutableList.of( + new Route(GET, "/tenancy/config"), + new Route(PUT, "/tenancy/config") + ) + ); + + private final static Set ACCEPTABLE_DEFAULT_TENANTS = ImmutableSet.of( + ConfigConstants.TENANCY_GLOBAL_TENANT_DEFAULT_NAME, + ConfigConstants.TENANCY_GLOBAL_TENANT_NAME, + ConfigConstants.TENANCY_PRIVATE_TENANT_NAME + ); + + @Override + public String getName() { + return "Multi Tenancy actions to Retrieve / Update configs."; + } + + @Override + public List routes() { + return ROUTES; + } + + public MultiTenancyConfigApiAction( + final Settings settings, final Path configPath, + final RestController controller, final Client client, + final AdminDNs adminDNs, final ConfigurationRepository cl, + final ClusterService cs, final PrincipalExtractor principalExtractor, + final PrivilegesEvaluator evaluator, final ThreadPool threadPool, + final AuditLog auditLog) { + super(settings, configPath, controller, client, adminDNs, cl, cs, principalExtractor, evaluator, threadPool, auditLog); + } + + @Override + protected AbstractConfigurationValidator getValidator(RestRequest request, BytesReference ref, Object... params) { + return new MultiTenancyConfigValidator(request, ref, settings, params); + } + + @Override + protected Endpoint getEndpoint() { + return Endpoint.TENANTS; + } + + @Override + protected String getResourceName() { + return null; + } + + @Override + protected CType getConfigName() { + return CType.CONFIG; + } + + @Override + protected void handleDelete(final RestChannel channel, + final RestRequest request, + final Client client, + final JsonNode content) throws IOException { + notImplemented(channel, RestRequest.Method.DELETE); + } + + private void multitenancyResponse(final ConfigV7 config, final RestChannel channel) { + try (final XContentBuilder contentBuilder = channel.newBuilder()) { + channel.sendResponse( + new BytesRestResponse( + RestStatus.OK, + contentBuilder + .startObject() + .field( + MultiTenancyConfigValidator.DEFAULT_TENANT_JSON_PROPERTY, + config.dynamic.kibana.default_tenant + ).field( + MultiTenancyConfigValidator.PRIVATE_TENANT_ENABLED_JSON_PROPERTY, + config.dynamic.kibana.private_tenant_enabled + ).field( + MultiTenancyConfigValidator.MULTITENANCY_ENABLED_JSON_PROPERTY, + config.dynamic.kibana.multitenancy_enabled + ).endObject() + ) + ); + } catch (final Exception e) { + internalErrorResponse(channel, e.getMessage()); + log.error("Error handle request ", e); + } + } + + + @Override + protected void handleGet(final RestChannel channel, + final RestRequest request, + final Client client, + final JsonNode content) throws IOException { + final SecurityDynamicConfiguration dynamicConfiguration = load(CType.CONFIG, false); + final ConfigV7 config = (ConfigV7) dynamicConfiguration.getCEntry(CType.CONFIG.toLCString()); + multitenancyResponse(config, channel); + } + + @Override + protected void handlePut(final RestChannel channel, + final RestRequest request, + final Client client, + final JsonNode content) throws IOException { + final SecurityDynamicConfiguration dynamicConfiguration = (SecurityDynamicConfiguration) + load(CType.CONFIG, false); + final ConfigV7 config = dynamicConfiguration.getCEntry(CType.CONFIG.toLCString()); + updateAndValidatesValues(config, content); + dynamicConfiguration.putCEntry(CType.CONFIG.toLCString(), config); + saveAndUpdateConfigs( + this.securityIndexName, + client, + getConfigName(), + dynamicConfiguration, + new OnSucessActionListener<>(channel) { + @Override + public void onResponse(IndexResponse response) { + multitenancyResponse(config, channel); + } + } + ); + } + + private void updateAndValidatesValues(final ConfigV7 config, final JsonNode jsonContent) { + if (Objects.nonNull(jsonContent.findValue(MultiTenancyConfigValidator.DEFAULT_TENANT_JSON_PROPERTY))) { + config.dynamic.kibana.default_tenant = + jsonContent.findValue(MultiTenancyConfigValidator.DEFAULT_TENANT_JSON_PROPERTY).asText(); + } + if (Objects.nonNull(jsonContent.findValue(MultiTenancyConfigValidator.PRIVATE_TENANT_ENABLED_JSON_PROPERTY))) { + config.dynamic.kibana.private_tenant_enabled = + jsonContent.findValue(MultiTenancyConfigValidator.PRIVATE_TENANT_ENABLED_JSON_PROPERTY).booleanValue(); + } + if (Objects.nonNull(jsonContent.findValue(MultiTenancyConfigValidator.MULTITENANCY_ENABLED_JSON_PROPERTY))) { + config.dynamic.kibana.multitenancy_enabled = + jsonContent.findValue(MultiTenancyConfigValidator.MULTITENANCY_ENABLED_JSON_PROPERTY).asBoolean(); + } + 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)) { + throw new IllegalArgumentException("Private tenant can not be disabled if it is the default tenant."); + } + + if (ACCEPTABLE_DEFAULT_TENANTS.contains(defaultTenant)) { + return; + } + + final Set availableTenants = + cl.getConfiguration(CType.TENANTS) + .getCEntries() + .keySet() + .stream() + .map(String::toLowerCase) + .collect(Collectors.toSet()); + if (!availableTenants.contains(defaultTenant)) { + throw new IllegalArgumentException(config.dynamic.kibana.default_tenant + " can not be set to default tenant. Default tenant should be selected from one of the available tenants."); + } + } + +} diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/SecurityRestApiActions.java b/src/main/java/org/opensearch/security/dlic/rest/api/SecurityRestApiActions.java index e73d28e865..cd489cab4d 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/api/SecurityRestApiActions.java +++ b/src/main/java/org/opensearch/security/dlic/rest/api/SecurityRestApiActions.java @@ -64,6 +64,7 @@ public static Collection getHandler(final Settings settings, handlers.add(new WhitelistApiAction(settings, configPath, controller, client, adminDns, cr, cs, principalExtractor, evaluator, threadPool, auditLog)); handlers.add(new AllowlistApiAction(settings, configPath, controller, client, adminDns, cr, cs, principalExtractor, evaluator, threadPool, auditLog)); handlers.add(new AuditApiAction(settings, configPath, controller, client, adminDns, cr, cs, principalExtractor, evaluator, threadPool, auditLog)); + handlers.add(new MultiTenancyConfigApiAction(settings, configPath, controller, client, adminDns, cr, cs, principalExtractor, evaluator, threadPool, auditLog)); handlers.add(new SecuritySSLCertsAction(settings, configPath, controller, client, adminDns, cr, cs, principalExtractor, evaluator, threadPool, auditLog, securityKeyStore, certificatesReloadEnabled)); return Collections.unmodifiableCollection(handlers); } diff --git a/src/main/java/org/opensearch/security/dlic/rest/validation/MultiTenancyConfigValidator.java b/src/main/java/org/opensearch/security/dlic/rest/validation/MultiTenancyConfigValidator.java new file mode 100644 index 0000000000..42870f1c13 --- /dev/null +++ b/src/main/java/org/opensearch/security/dlic/rest/validation/MultiTenancyConfigValidator.java @@ -0,0 +1,32 @@ +/* + * 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.dlic.rest.validation; + +import org.opensearch.common.bytes.BytesReference; +import org.opensearch.common.settings.Settings; +import org.opensearch.rest.RestRequest; + +public class MultiTenancyConfigValidator extends AbstractConfigurationValidator { + + 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 MultiTenancyConfigValidator(RestRequest request, BytesReference ref, Settings opensearchSettings, Object... param) { + super(request, ref, opensearchSettings, param); + this.payloadMandatory = true; + allowedKeys.put(DEFAULT_TENANT_JSON_PROPERTY, DataType.STRING); + allowedKeys.put(PRIVATE_TENANT_ENABLED_JSON_PROPERTY, DataType.BOOLEAN); + allowedKeys.put(MULTITENANCY_ENABLED_JSON_PROPERTY, DataType.BOOLEAN); + } + +} diff --git a/src/main/java/org/opensearch/security/support/ConfigConstants.java b/src/main/java/org/opensearch/security/support/ConfigConstants.java index 7ed780f413..a58639bc0a 100644 --- a/src/main/java/org/opensearch/security/support/ConfigConstants.java +++ b/src/main/java/org/opensearch/security/support/ConfigConstants.java @@ -290,8 +290,8 @@ public enum RolesMappingResolution { public static final String SECURITY_SYSTEM_INDICES_KEY = "plugins.security.system_indices.indices"; public static final List SECURITY_SYSTEM_INDICES_DEFAULT = Collections.emptyList(); - public static final String TENANCY_PRIVATE_TENANT_NAME = "Private"; - public static final String TENANCY_GLOBAL_TENANT_NAME = "Global"; + public static final String TENANCY_PRIVATE_TENANT_NAME = "private"; + public static final String TENANCY_GLOBAL_TENANT_NAME = "global"; public static final String TENANCY_GLOBAL_TENANT_DEFAULT_NAME = ""; public static Set getSettingAsSet(final Settings settings, final String key, final List defaultList, final boolean ignoreCaseForNone) { diff --git a/src/test/java/org/opensearch/security/dlic/rest/api/MultiTenancyConfigApiTest.java b/src/test/java/org/opensearch/security/dlic/rest/api/MultiTenancyConfigApiTest.java new file mode 100644 index 0000000000..24aa8737c6 --- /dev/null +++ b/src/test/java/org/opensearch/security/dlic/rest/api/MultiTenancyConfigApiTest.java @@ -0,0 +1,170 @@ +/* + * 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.dlic.rest.api; + +import org.apache.hc.core5.http.Header; +import org.apache.hc.core5.http.HttpStatus; +import org.junit.Test; + +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.core.IsEqual.equalTo; +import static org.hamcrest.core.StringContains.containsString; + +public class MultiTenancyConfigApiTest extends AbstractRestApiUnitTest { + + private static final Header ADMIN_FULL_ACCESS_USER = encodeBasicHeader("admin_all_access", "admin_all_access"); + private static final Header USER_NO_REST_API_ACCESS = encodeBasicHeader("admin", "admin"); + + private void verifyTenantUpdate(final Header... header) throws Exception { + final HttpResponse getSettingResponse = rh.executeGetRequest("/_plugins/_security/api/tenancy/config", header); + assertThat(getSettingResponse.getBody(), getSettingResponse.getStatusCode(), equalTo(HttpStatus.SC_OK)); + assertThat( + getSettingResponse.getBody(), + getSettingResponse.findValueInJson("default_tenant"), + equalTo(ConfigConstants.TENANCY_GLOBAL_TENANT_DEFAULT_NAME) + ); + + HttpResponse getDashboardsinfoResponse = rh.executeGetRequest("/_plugins/_security/dashboardsinfo", header); + assertThat(getDashboardsinfoResponse.getStatusCode(), equalTo(HttpStatus.SC_OK)); + assertThat( + getDashboardsinfoResponse.getBody(), + getDashboardsinfoResponse.findValueInJson("default_tenant"), + equalTo(ConfigConstants.TENANCY_GLOBAL_TENANT_DEFAULT_NAME) + ); + + final HttpResponse setPrivateTenantAsDefaultResponse = rh.executePutRequest( + "/_plugins/_security/api/tenancy/config", + "{\"default_tenant\": \"Private\"}", header + ); + assertThat( + setPrivateTenantAsDefaultResponse.getBody(), + setPrivateTenantAsDefaultResponse.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")); + } + + @Test + public void testUpdateSuperAdmin() throws Exception { + setupWithRestRoles(); + rh.keystore = "restapi/kirk-keystore.jks"; + rh.sendAdminCertificate = true; + verifyTenantUpdate(); + } + + @Test + public void testUpdateRestAPIAdmin() throws Exception { + setupWithRestRoles(); + rh.sendAdminCertificate = false; + verifyTenantUpdate(ADMIN_FULL_ACCESS_USER); + } + + + private void verifyTenantUpdateFailed(final Header... header) throws Exception { + final HttpResponse disablePrivateTenantResponse = rh.executePutRequest( + "/_plugins/_security/api/tenancy/config", + "{\"private_tenant_enabled\":false}", header + ); + assertThat(disablePrivateTenantResponse.getBody(), disablePrivateTenantResponse.getStatusCode(), equalTo(HttpStatus.SC_OK)); + + final HttpResponse setPrivateTenantAsDefaultFailResponse = rh.executePutRequest( + "/_plugins/_security/api/tenancy/config", + "{\"default_tenant\": \"Private\"}", header + ); + assertThat(setPrivateTenantAsDefaultFailResponse.getBody(), setPrivateTenantAsDefaultFailResponse.getStatusCode(), equalTo(HttpStatus.SC_BAD_REQUEST)); + assertThat( + setPrivateTenantAsDefaultFailResponse.getBody(), + setPrivateTenantAsDefaultFailResponse.findValueInJson("error.reason"), + containsString("Private tenant can not be disabled if it is the default tenant.") + ); + + final HttpResponse enablePrivateTenantResponse = rh.executePutRequest( + "/_plugins/_security/api/tenancy/config", + "{\"private_tenant_enabled\":true}", + header + ); + assertThat(enablePrivateTenantResponse.getBody(), enablePrivateTenantResponse.getStatusCode(), equalTo(HttpStatus.SC_OK)); + + final HttpResponse setPrivateTenantAsDefaultResponse = rh.executePutRequest( + "/_plugins/_security/api/tenancy/config", + "{\"default_tenant\": \"Private\"}", + header + ); + assertThat(setPrivateTenantAsDefaultResponse.getBody(), setPrivateTenantAsDefaultResponse.getStatusCode(), equalTo(HttpStatus.SC_OK)); + final HttpResponse updatePrivateSettingResponse = + rh.executePutRequest("/_plugins/_security/api/tenancy/config", "{\"private_tenant_enabled\":false}", header); + assertThat(updatePrivateSettingResponse.getStatusCode(), equalTo(HttpStatus.SC_BAD_REQUEST)); + assertThat(updatePrivateSettingResponse.findValueInJson("error.reason"), containsString("Private tenant can not be disabled if it is the default tenant.")); + + final HttpResponse getSettingResponseAfterUpdate = rh.executeGetRequest("/_plugins/_security/api/tenancy/config", header); + assertThat(getSettingResponseAfterUpdate.getBody(), getSettingResponseAfterUpdate.getStatusCode(), equalTo(HttpStatus.SC_OK)); + assertThat( + getSettingResponseAfterUpdate.getBody(), + getSettingResponseAfterUpdate.findValueInJson("default_tenant"), + equalTo("Private") + ); + + final HttpResponse getDashboardsinfoResponse = rh.executeGetRequest("/_plugins/_security/dashboardsinfo", header); + assertThat( + getDashboardsinfoResponse.getBody(), + getDashboardsinfoResponse.findValueInJson("default_tenant"), + equalTo("Private") + ); + + final HttpResponse setRandomStringAsDefaultTenant = rh.executePutRequest( + "/_plugins/_security/api/tenancy/config", + "{\"default_tenant\": \"NonExistentTenant\"}", + header + ); + assertThat(setRandomStringAsDefaultTenant.getStatusCode(), equalTo(HttpStatus.SC_BAD_REQUEST)); + assertThat(setPrivateTenantAsDefaultFailResponse.getBody(), + setRandomStringAsDefaultTenant.findValueInJson("error.reason"), + containsString("Default tenant should be selected from one of the available tenants.") + ); + } + + @Test + public void testDefaultTenantUpdateFailedSuperAdmin() throws Exception { + setupWithRestRoles(); + rh.keystore = "restapi/kirk-keystore.jks"; + rh.sendAdminCertificate = true; + verifyTenantUpdateFailed(); + } + + @Test + public void testDefaultTenantUpdateFailedRestAPIAdmin() throws Exception { + setupWithRestRoles(); + rh.sendAdminCertificate = false; + verifyTenantUpdateFailed(ADMIN_FULL_ACCESS_USER); + } + + @Test + public void testForbiddenAccess() throws Exception { + setupWithRestRoles(); + + rh.sendAdminCertificate = false; + HttpResponse getSettingResponse = rh.executeGetRequest("/_plugins/_security/api/tenancy/config", USER_NO_REST_API_ACCESS); + assertThat(getSettingResponse.getBody(), getSettingResponse.getStatusCode(), equalTo(HttpStatus.SC_FORBIDDEN)); + HttpResponse updateSettingResponse = rh.executePutRequest( + "/_plugins/_security/api/tenancy/config", + "{\"default_tenant\": \"Private\"}", USER_NO_REST_API_ACCESS + ); + assertThat(getSettingResponse.getBody(), updateSettingResponse.getStatusCode(), equalTo(HttpStatus.SC_FORBIDDEN)); + } + + +} diff --git a/src/test/java/org/opensearch/security/multitenancy/test/TenancyDefaultTenantTests.java b/src/test/java/org/opensearch/security/multitenancy/test/TenancyDefaultTenantTests.java deleted file mode 100644 index b4d6aa4b59..0000000000 --- a/src/test/java/org/opensearch/security/multitenancy/test/TenancyDefaultTenantTests.java +++ /dev/null @@ -1,96 +0,0 @@ -/* - * 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.multitenancy.test; - -import org.apache.hc.core5.http.Header; -import org.apache.hc.core5.http.HttpStatus; -import org.junit.Test; - -import org.opensearch.security.support.ConfigConstants; -import org.opensearch.security.test.SingleClusterTest; -import org.opensearch.security.test.helper.rest.RestHelper.HttpResponse; - -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.core.IsEqual.equalTo; -import static org.hamcrest.core.StringContains.containsString; - -public class TenancyDefaultTenantTests extends SingleClusterTest { - private final Header asAdminUser = encodeBasicHeader("admin", "admin"); - private final Header asUser = encodeBasicHeader("kirk", "kirk"); - - @Override - protected String getResourceFolder() { - return "multitenancy"; - } - - @Test - public void testDefaultTenantUpdate() throws Exception { - setup(); - - final HttpResponse getSettingResponse = nonSslRestHelper().executeGetRequest("/_plugins/_security/api/tenancy/config", asAdminUser); - assertThat(getSettingResponse.getStatusCode(), equalTo(HttpStatus.SC_OK)); - assertThat(getSettingResponse.findValueInJson("default_tenant"), equalTo(ConfigConstants.TENANCY_GLOBAL_TENANT_DEFAULT_NAME)); - - HttpResponse getDashboardsinfoResponse = nonSslRestHelper().executeGetRequest("/_plugins/_security/dashboardsinfo", asAdminUser); - assertThat(getDashboardsinfoResponse.getStatusCode(), equalTo(HttpStatus.SC_OK)); - assertThat(getDashboardsinfoResponse.findValueInJson("default_tenant"), equalTo(ConfigConstants.TENANCY_GLOBAL_TENANT_DEFAULT_NAME)); - - final HttpResponse setPrivateTenantAsDefaultResponse = nonSslRestHelper().executePutRequest("/_plugins/_security/api/tenancy/config", "{\"default_tenant\": \"Private\"}", asAdminUser); - assertThat(setPrivateTenantAsDefaultResponse.getStatusCode(), equalTo(HttpStatus.SC_OK)); - getDashboardsinfoResponse = nonSslRestHelper().executeGetRequest("/_plugins/_security/dashboardsinfo", asAdminUser); - assertThat(getDashboardsinfoResponse.getStatusCode(), equalTo(HttpStatus.SC_OK)); - assertThat(getDashboardsinfoResponse.findValueInJson("default_tenant"), equalTo(ConfigConstants.TENANCY_PRIVATE_TENANT_NAME)); - } - - @Test - public void testDefaultTenant_UpdateFailed() throws Exception { - setup(); - - final HttpResponse disablePrivateTenantResponse = nonSslRestHelper().executePutRequest("/_plugins/_security/api/tenancy/config", "{\"private_tenant_enabled\":false}", asAdminUser); - assertThat(disablePrivateTenantResponse.getStatusCode(), equalTo(HttpStatus.SC_OK)); - - - final HttpResponse setPrivateTenantAsDefaultFailResponse = nonSslRestHelper().executePutRequest("/_plugins/_security/api/tenancy/config", "{\"default_tenant\": \"Private\"}", asAdminUser); - assertThat(setPrivateTenantAsDefaultFailResponse.getStatusCode(), equalTo(HttpStatus.SC_BAD_REQUEST)); - assertThat(setPrivateTenantAsDefaultFailResponse.findValueInJson("error.reason"), containsString("Private tenant can not be disabled if it is the default tenant.")); - - final HttpResponse enablePrivateTenantResponse = nonSslRestHelper().executePutRequest("/_plugins/_security/api/tenancy/config", "{\"private_tenant_enabled\":true}", asAdminUser); - assertThat(enablePrivateTenantResponse.getStatusCode(), equalTo(HttpStatus.SC_OK)); - - final HttpResponse setPrivateTenantAsDefaultResponse = nonSslRestHelper().executePutRequest("/_plugins/_security/api/tenancy/config", "{\"default_tenant\": \"Private\"}", asAdminUser); - assertThat(setPrivateTenantAsDefaultResponse.getStatusCode(), equalTo(HttpStatus.SC_OK)); - - final HttpResponse getSettingResponseAfterUpdate = nonSslRestHelper().executeGetRequest("/_plugins/_security/api/tenancy/config", asAdminUser); - assertThat(getSettingResponseAfterUpdate.getStatusCode(), equalTo(HttpStatus.SC_OK)); - assertThat(getSettingResponseAfterUpdate.findValueInJson("default_tenant"), equalTo(ConfigConstants.TENANCY_PRIVATE_TENANT_NAME)); - - HttpResponse getDashboardsinfoResponse = nonSslRestHelper().executeGetRequest("/_plugins/_security/dashboardsinfo", asAdminUser); - assertThat(getDashboardsinfoResponse.findValueInJson("default_tenant"),equalTo(ConfigConstants.TENANCY_PRIVATE_TENANT_NAME)); - - final HttpResponse setRandomStringAsDefaultTenant = nonSslRestHelper().executePutRequest("/_plugins/_security/api/tenancy/config", "{\"default_tenant\": \"NonExistentTenant\"}", asAdminUser); - assertThat(setRandomStringAsDefaultTenant.getStatusCode(), equalTo(HttpStatus.SC_BAD_REQUEST)); - assertThat(setRandomStringAsDefaultTenant.findValueInJson("error.reason"), containsString("Default tenant should be selected from one of the available tenants.")); - - } - @Test - public void testForbiddenAccess() throws Exception { - setup(); - - final HttpResponse getSettingResponse = nonSslRestHelper().executeGetRequest("/_plugins/_security/api/tenancy/config", asUser); - assertThat(getSettingResponse.getStatusCode(), equalTo(HttpStatus.SC_FORBIDDEN)); - assertThat(getSettingResponse.findValueInJson("error.reason"), containsString("no permissions for [cluster:feature/tenancy/config/read]")); - - final HttpResponse updateSettingResponse = nonSslRestHelper().executePutRequest("/_plugins/_security/api/tenancy/config", "{\"default_tenant\": \"Private\"}", asUser); - assertThat(updateSettingResponse.getStatusCode(), equalTo(HttpStatus.SC_FORBIDDEN)); - assertThat(updateSettingResponse.findValueInJson("error.reason"), containsString("no permissions for [cluster:feature/tenancy/config/update]")); - } -} diff --git a/src/test/java/org/opensearch/security/multitenancy/test/TenancyMultitenancyEnabledTests.java b/src/test/java/org/opensearch/security/multitenancy/test/TenancyMultitenancyEnabledTests.java index 033d38ed41..bd9664a84c 100644 --- a/src/test/java/org/opensearch/security/multitenancy/test/TenancyMultitenancyEnabledTests.java +++ b/src/test/java/org/opensearch/security/multitenancy/test/TenancyMultitenancyEnabledTests.java @@ -16,17 +16,19 @@ import org.apache.hc.core5.http.message.BasicHeader; import org.junit.Test; +import org.opensearch.common.settings.Settings; +import org.opensearch.security.test.DynamicSecurityConfig; import org.opensearch.security.test.SingleClusterTest; import org.opensearch.security.test.helper.rest.RestHelper.HttpResponse; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.core.IsEqual.equalTo; -import static org.hamcrest.core.StringContains.containsString; public class TenancyMultitenancyEnabledTests extends SingleClusterTest { - private final Header asAdminUser = encodeBasicHeader("admin", "admin"); - private final Header asUser = encodeBasicHeader("kirk", "kirk"); - private final Header onUserTenant = new BasicHeader("securitytenant", "__user__"); + + private static final Header AS_REST_API_USER = encodeBasicHeader("user_rest_api_access", "user_rest_api_access"); + private static final Header AS_USER = encodeBasicHeader("admin", "admin"); + private static final Header ON_USER_TENANT = new BasicHeader("securitytenant", "__user__"); private static String createIndexPatternDoc(final String title) { return "{"+ @@ -44,46 +46,37 @@ protected String getResourceFolder() { @Test public void testMultitenancyDisabled_endToEndTest() throws Exception { - setup(); + setup(Settings.EMPTY, + new DynamicSecurityConfig(), + Settings.builder().put("plugins.security.restapi.roles_enabled.0", "security_rest_api_access").build(), + true); - final HttpResponse getSettingResponse = nonSslRestHelper().executeGetRequest("/_plugins/_security/api/tenancy/config", asAdminUser); + final HttpResponse getSettingResponse = nonSslRestHelper().executeGetRequest("/_plugins/_security/api/tenancy/config", AS_REST_API_USER); assertThat(getSettingResponse.getStatusCode(), equalTo(HttpStatus.SC_OK)); assertThat(getSettingResponse.findValueInJson("multitenancy_enabled"), equalTo("true")); - HttpResponse getDashboardsinfoResponse = nonSslRestHelper().executeGetRequest("/_plugins/_security/dashboardsinfo", asAdminUser); + HttpResponse getDashboardsinfoResponse = nonSslRestHelper().executeGetRequest("/_plugins/_security/dashboardsinfo", AS_USER); assertThat(getDashboardsinfoResponse.findValueInJson("multitenancy_enabled"),equalTo("true")); - final HttpResponse createDocInGlobalTenantResponse = nonSslRestHelper().executePostRequest(".kibana/_doc?refresh=true", createIndexPatternDoc("globalIndex"), asAdminUser); + final HttpResponse createDocInGlobalTenantResponse = nonSslRestHelper().executePostRequest(".kibana/_doc?refresh=true", createIndexPatternDoc("globalIndex"), AS_USER); assertThat(createDocInGlobalTenantResponse.getStatusCode(), equalTo(HttpStatus.SC_CREATED)); - final HttpResponse createDocInUserTenantResponse = nonSslRestHelper().executePostRequest(".kibana/_doc?refresh=true", createIndexPatternDoc("userIndex"), onUserTenant, asAdminUser); + final HttpResponse createDocInUserTenantResponse = nonSslRestHelper().executePostRequest(".kibana/_doc?refresh=true", createIndexPatternDoc("userIndex"), ON_USER_TENANT, AS_USER); assertThat(createDocInUserTenantResponse.getStatusCode(), equalTo(HttpStatus.SC_CREATED)); - final HttpResponse searchInUserTenantWithMutlitenancyEnabled = nonSslRestHelper().executeGetRequest(".kibana/_search", onUserTenant, asAdminUser); + final HttpResponse searchInUserTenantWithMutlitenancyEnabled = nonSslRestHelper().executeGetRequest(".kibana/_search", ON_USER_TENANT, AS_USER); assertThat(searchInUserTenantWithMutlitenancyEnabled.getStatusCode(), equalTo(HttpStatus.SC_OK)); assertThat(searchInUserTenantWithMutlitenancyEnabled.findValueInJson("hits.hits[0]._source.index-pattern.title"), equalTo("userIndex")); - final HttpResponse updateMutlitenancyToDisabled = nonSslRestHelper().executePutRequest("/_plugins/_security/api/tenancy/config", "{\"multitenancy_enabled\": \"false\"}", asAdminUser); + final HttpResponse updateMutlitenancyToDisabled = nonSslRestHelper().executePutRequest("/_plugins/_security/api/tenancy/config", "{\"multitenancy_enabled\": \"false\"}", AS_REST_API_USER); assertThat(updateMutlitenancyToDisabled.getStatusCode(), equalTo(HttpStatus.SC_OK)); assertThat(updateMutlitenancyToDisabled.findValueInJson("multitenancy_enabled"), equalTo("false")); - getDashboardsinfoResponse = nonSslRestHelper().executeGetRequest("/_plugins/_security/dashboardsinfo", asAdminUser); + getDashboardsinfoResponse = nonSslRestHelper().executeGetRequest("/_plugins/_security/dashboardsinfo", AS_USER); assertThat(getDashboardsinfoResponse.findValueInJson("multitenancy_enabled"),equalTo("false")); - final HttpResponse searchInUserTenantWithMutlitenancyDisabled = nonSslRestHelper().executeGetRequest(".kibana/_search", onUserTenant, asAdminUser); + final HttpResponse searchInUserTenantWithMutlitenancyDisabled = nonSslRestHelper().executeGetRequest(".kibana/_search", ON_USER_TENANT, AS_USER); assertThat(searchInUserTenantWithMutlitenancyDisabled.getStatusCode(), equalTo(HttpStatus.SC_OK)); assertThat(searchInUserTenantWithMutlitenancyDisabled.findValueInJson("hits.hits[0]._source.index-pattern.title"), equalTo("globalIndex")); } - @Test - public void testForbiddenAccess() throws Exception { - setup(); - - final HttpResponse getSettingResponse = nonSslRestHelper().executeGetRequest("/_plugins/_security/api/tenancy/config", asUser); - assertThat(getSettingResponse.getStatusCode(), equalTo(HttpStatus.SC_FORBIDDEN)); - assertThat(getSettingResponse.findValueInJson("error.reason"), containsString("no permissions for [cluster:feature/tenancy/config/read]")); - - final HttpResponse updateSettingResponse = nonSslRestHelper().executePutRequest("/_plugins/_security/api/tenancy/config", "{\"multitenancy_enabled\": \"false\"}", asUser); - assertThat(updateSettingResponse.getStatusCode(), equalTo(HttpStatus.SC_FORBIDDEN)); - assertThat(updateSettingResponse.findValueInJson("error.reason"), containsString("no permissions for [cluster:feature/tenancy/config/update]")); - } } diff --git a/src/test/java/org/opensearch/security/multitenancy/test/TenancyPrivateTenantEnabledTests.java b/src/test/java/org/opensearch/security/multitenancy/test/TenancyPrivateTenantEnabledTests.java index c6927dbbf8..599586239c 100644 --- a/src/test/java/org/opensearch/security/multitenancy/test/TenancyPrivateTenantEnabledTests.java +++ b/src/test/java/org/opensearch/security/multitenancy/test/TenancyPrivateTenantEnabledTests.java @@ -16,6 +16,8 @@ import org.apache.hc.core5.http.message.BasicHeader; import org.junit.Test; +import org.opensearch.common.settings.Settings; +import org.opensearch.security.test.DynamicSecurityConfig; import org.opensearch.security.test.SingleClusterTest; import org.opensearch.security.test.helper.rest.RestHelper.HttpResponse; @@ -24,9 +26,10 @@ import static org.hamcrest.core.StringContains.containsString; public class TenancyPrivateTenantEnabledTests extends SingleClusterTest { - private final Header asAdminUser = encodeBasicHeader("admin", "admin"); - private final Header asUser = encodeBasicHeader("kirk", "kirk"); - private final Header onUserTenant = new BasicHeader("securitytenant", "__user__"); + private static final Header AS_REST_API_USER = encodeBasicHeader("user_rest_api_access", "user_rest_api_access"); + private static final Header AS_ADMIN_USER = encodeBasicHeader("admin", "admin"); + private static final Header AS_USER = encodeBasicHeader("kirk", "kirk"); + private static final Header ON_USER_TENANT = new BasicHeader("securitytenant", "__user__"); private static String createIndexPatternDoc(final String title) { return "{"+ @@ -43,59 +46,39 @@ protected String getResourceFolder() { } @Test - public void testPrivateTenantDisabled_Update() throws Exception { - setup(); + public void testPrivateTenantDisabled_Update_EndToEnd() throws Exception { + setup(Settings.EMPTY, + new DynamicSecurityConfig(), + Settings.builder().put("plugins.security.restapi.roles_enabled.0", "security_rest_api_access").build(), + true); - final HttpResponse getSettingResponse = nonSslRestHelper().executeGetRequest("/_plugins/_security/api/tenancy/config", asAdminUser); + final HttpResponse getSettingResponse = nonSslRestHelper().executeGetRequest("/_plugins/_security/api/tenancy/config", AS_REST_API_USER); assertThat(getSettingResponse.getStatusCode(), equalTo(HttpStatus.SC_OK)); assertThat(getSettingResponse.findValueInJson("private_tenant_enabled"), equalTo("true")); - HttpResponse getDashboardsinfoResponse = nonSslRestHelper().executeGetRequest("/_plugins/_security/dashboardsinfo", asAdminUser); + HttpResponse getDashboardsinfoResponse = nonSslRestHelper().executeGetRequest("/_plugins/_security/dashboardsinfo", AS_ADMIN_USER); assertThat(getDashboardsinfoResponse.findValueInJson("private_tenant_enabled"), equalTo("true")); - final HttpResponse createDocInGlobalTenantResponse = nonSslRestHelper().executePostRequest(".kibana/_doc?refresh=true", createIndexPatternDoc("globalIndex"), asAdminUser); + final HttpResponse createDocInGlobalTenantResponse = nonSslRestHelper().executePostRequest(".kibana/_doc?refresh=true", createIndexPatternDoc("globalIndex"), AS_ADMIN_USER); assertThat(createDocInGlobalTenantResponse.getStatusCode(), equalTo(HttpStatus.SC_CREATED)); - final HttpResponse createDocInUserTenantResponse = nonSslRestHelper().executePostRequest(".kibana/_doc?refresh=true", createIndexPatternDoc("userIndex"), onUserTenant, asUser); + final HttpResponse createDocInUserTenantResponse = nonSslRestHelper().executePostRequest(".kibana/_doc?refresh=true", createIndexPatternDoc("userIndex"), ON_USER_TENANT, AS_USER); assertThat(createDocInUserTenantResponse.getStatusCode(), equalTo(HttpStatus.SC_CREATED)); - final HttpResponse searchInUserTenantWithPrivateTenantEnabled = nonSslRestHelper().executeGetRequest(".kibana/_search", onUserTenant, asUser); + final HttpResponse searchInUserTenantWithPrivateTenantEnabled = nonSslRestHelper().executeGetRequest(".kibana/_search", ON_USER_TENANT, AS_USER); assertThat(searchInUserTenantWithPrivateTenantEnabled.getStatusCode(), equalTo(HttpStatus.SC_OK)); assertThat(searchInUserTenantWithPrivateTenantEnabled.findValueInJson("hits.hits[0]._source.index-pattern.title"), equalTo("userIndex")); - final HttpResponse disablePrivateTenantResponse = nonSslRestHelper().executePutRequest("/_plugins/_security/api/tenancy/config", "{\"private_tenant_enabled\": \"false\"}", asAdminUser); + final HttpResponse disablePrivateTenantResponse = nonSslRestHelper().executePutRequest("/_plugins/_security/api/tenancy/config", "{\"private_tenant_enabled\": \"false\"}", AS_REST_API_USER); assertThat(disablePrivateTenantResponse.getStatusCode(), equalTo(HttpStatus.SC_OK)); assertThat(disablePrivateTenantResponse.findValueInJson("private_tenant_enabled"), equalTo("false")); - getDashboardsinfoResponse = nonSslRestHelper().executeGetRequest("/_plugins/_security/dashboardsinfo", asAdminUser); + getDashboardsinfoResponse = nonSslRestHelper().executeGetRequest("/_plugins/_security/dashboardsinfo", AS_ADMIN_USER); assertThat(getDashboardsinfoResponse.findValueInJson("private_tenant_enabled"),equalTo("false")); - final HttpResponse searchInUserTenantWithPrivateTenantDisabled = nonSslRestHelper().executeGetRequest(".kibana/_search", onUserTenant, asUser); + final HttpResponse searchInUserTenantWithPrivateTenantDisabled = nonSslRestHelper().executeGetRequest(".kibana/_search", ON_USER_TENANT, AS_USER); assertThat(searchInUserTenantWithPrivateTenantDisabled.getStatusCode(), equalTo(HttpStatus.SC_FORBIDDEN)); assertThat(searchInUserTenantWithPrivateTenantDisabled.findValueInJson("error.reason"), containsString("no permissions for [indices:data/read/search] and User")); } - @Test - public void testPrivateTenantDisabled_UpdateFailed() throws Exception { - setup(); - - final HttpResponse setPrivateTenantAsDefaultResponse = nonSslRestHelper().executePutRequest("/_plugins/_security/api/tenancy/config", "{\"default_tenant\": \"Private\"}", asAdminUser); - assertThat(setPrivateTenantAsDefaultResponse.getStatusCode(), equalTo(HttpStatus.SC_OK)); - final HttpResponse updateSettingResponse = nonSslRestHelper().executePutRequest("/_plugins/_security/api/tenancy/config", "{\"private_tenant_enabled\":false}", asAdminUser); - assertThat(updateSettingResponse.getStatusCode(), equalTo(HttpStatus.SC_BAD_REQUEST)); - assertThat(updateSettingResponse.findValueInJson("error.reason"), containsString("Private tenant can not be disabled if it is the default tenant.")); - } - - @Test - public void testForbiddenAccess() throws Exception { - setup(); - - final HttpResponse getSettingResponse = nonSslRestHelper().executeGetRequest("/_plugins/_security/api/tenancy/config", asUser); - assertThat(getSettingResponse.getStatusCode(), equalTo(HttpStatus.SC_FORBIDDEN)); - assertThat(getSettingResponse.findValueInJson("error.reason"), containsString("no permissions for [cluster:feature/tenancy/config/read]")); - - final HttpResponse updateSettingResponse = nonSslRestHelper().executePutRequest("/_plugins/_security/api/tenancy/config", "{\"private_tenant_enabled\": false}", asUser); - assertThat(updateSettingResponse.getStatusCode(), equalTo(HttpStatus.SC_FORBIDDEN)); - assertThat(updateSettingResponse.findValueInJson("error.reason"), containsString("no permissions for [cluster:feature/tenancy/config/update]")); - } } diff --git a/src/test/resources/multitenancy/internal_users.yml b/src/test/resources/multitenancy/internal_users.yml index 809d268710..af96b2b8ee 100644 --- a/src/test/resources/multitenancy/internal_users.yml +++ b/src/test/resources/multitenancy/internal_users.yml @@ -156,3 +156,9 @@ user_tenant_parameters_substitution: attributes: attribute1: "tenant_parameters_substitution" description: "PR# 819 / Issue#817" +user_rest_api_access: + hash: "$2y$12$aHkyhk95XbrMCByYYVAlrek1thXpTDuVKJW01vdLYPh6kyR36j7x6" + reserved: false + hidden: false + backend_roles: [] + description: "REST API Access" diff --git a/src/test/resources/multitenancy/roles.yml b/src/test/resources/multitenancy/roles.yml index efc17f7464..1baa77cc8b 100644 --- a/src/test/resources/multitenancy/roles.yml +++ b/src/test/resources/multitenancy/roles.yml @@ -2,6 +2,8 @@ _meta: type: "roles" config_version: 2 +security_rest_api_access: + reserved: true opendistro_security_own_index: reserved: false hidden: false diff --git a/src/test/resources/multitenancy/roles_mapping.yml b/src/test/resources/multitenancy/roles_mapping.yml index a7d2867db1..397171a360 100644 --- a/src/test/resources/multitenancy/roles_mapping.yml +++ b/src/test/resources/multitenancy/roles_mapping.yml @@ -2,6 +2,11 @@ _meta: type: "rolesmapping" config_version: 2 +security_rest_api_access: + reserved: false + hidden: false + users: + - "user_rest_api_access" opendistro_security_human_resources: reserved: false hidden: false From 15860b65f33861f875b62ed57b6d8b7ab3c8a65d Mon Sep 17 00:00:00 2001 From: Andrey Pleskach Date: Mon, 15 May 2023 20:23:41 +0200 Subject: [PATCH 5/5] Add score based password verification (#2557) Signed-off-by: Andrey Pleskach --- build.gradle | 1 + .../security/SecurityConfigurationTests.java | 4 +- .../security/OpenSearchSecurityPlugin.java | 14 ++ .../AbstractConfigurationValidator.java | 22 +- .../rest/validation/CredentialsValidator.java | 52 ++--- .../rest/validation/PasswordValidator.java | 185 ++++++++++++++++ .../security/support/ConfigConstants.java | 4 +- .../RestApiComplianceAuditlogTest.java | 13 +- .../rest/api/AbstractRestApiUnitTest.java | 15 +- .../dlic/rest/api/AccountApiTest.java | 2 +- .../dlic/rest/api/ActionGroupsApiTest.java | 38 ++-- .../security/dlic/rest/api/RolesApiTest.java | 39 ++-- .../dlic/rest/api/RolesMappingApiTest.java | 52 ++--- .../security/dlic/rest/api/UserApiTest.java | 201 ++++++++++++------ .../validation/PasswordValidatorTest.java | 197 +++++++++++++++++ 15 files changed, 661 insertions(+), 178 deletions(-) create mode 100644 src/main/java/org/opensearch/security/dlic/rest/validation/PasswordValidator.java create mode 100644 src/test/java/org/opensearch/security/dlic/rest/validation/PasswordValidatorTest.java diff --git a/build.gradle b/build.gradle index 8cfe97ea80..ef187e290f 100644 --- a/build.gradle +++ b/build.gradle @@ -395,6 +395,7 @@ dependencies { implementation ('org.opensaml:opensaml-saml-impl:3.4.5') { exclude(group: 'org.apache.velocity', module: 'velocity') } + implementation "com.nulab-inc:zxcvbn:1.7.0" testImplementation 'org.opensaml:opensaml-messaging-impl:3.4.5' implementation 'org.opensaml:opensaml-messaging-api:3.4.5' runtimeOnly 'org.opensaml:opensaml-profile-api:3.4.5' diff --git a/src/integrationTest/java/org/opensearch/security/SecurityConfigurationTests.java b/src/integrationTest/java/org/opensearch/security/SecurityConfigurationTests.java index 2caf05536b..9d86266f8e 100644 --- a/src/integrationTest/java/org/opensearch/security/SecurityConfigurationTests.java +++ b/src/integrationTest/java/org/opensearch/security/SecurityConfigurationTests.java @@ -49,10 +49,10 @@ public class SecurityConfigurationTests { .roles(new Role("limited-role").indexPermissions("indices:data/read/search", "indices:data/read/get").on("user-${user.name}")); public static final String LIMITED_USER_INDEX = "user-" + LIMITED_USER.getName(); public static final String ADDITIONAL_USER_1 = "additional00001"; - public static final String ADDITIONAL_PASSWORD_1 = ADDITIONAL_USER_1; + public static final String ADDITIONAL_PASSWORD_1 = "user 1 fair password"; public static final String ADDITIONAL_USER_2 = "additional2"; - public static final String ADDITIONAL_PASSWORD_2 = ADDITIONAL_USER_2; + public static final String ADDITIONAL_PASSWORD_2 = "user 2 fair password"; public static final String CREATE_USER_BODY = "{\"password\": \"%s\",\"opendistro_security_roles\": []}"; public static final String INTERNAL_USERS_RESOURCE = "_plugins/_security/api/internalusers/"; public static final String ID_1 = "one"; diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java index 2db8b5d2eb..89e8ec31ac 100644 --- a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java +++ b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java @@ -138,6 +138,7 @@ import org.opensearch.security.configuration.Salt; import org.opensearch.security.configuration.SecurityFlsDlsIndexSearcherWrapper; import org.opensearch.security.dlic.rest.api.SecurityRestApiActions; +import org.opensearch.security.dlic.rest.validation.PasswordValidator; import org.opensearch.security.filter.SecurityFilter; import org.opensearch.security.filter.SecurityRestFilter; import org.opensearch.security.http.SecurityHttpServerTransport; @@ -1043,6 +1044,19 @@ public List> getSettings() { settings.add(Setting.simpleString(ConfigConstants.SECURITY_RESTAPI_PASSWORD_VALIDATION_REGEX, Property.NodeScope, Property.Filtered)); settings.add(Setting.simpleString(ConfigConstants.SECURITY_RESTAPI_PASSWORD_VALIDATION_ERROR_MESSAGE, Property.NodeScope, Property.Filtered)); + settings.add( + Setting.intSetting( + ConfigConstants.SECURITY_RESTAPI_PASSWORD_MIN_LENGTH, + -1, -1, Property.NodeScope, Property.Filtered) + ); + settings.add( + Setting.simpleString( + ConfigConstants.SECURITY_RESTAPI_PASSWORD_SCORE_BASED_VALIDATION_STRENGTH, + PasswordValidator.ScoreStrength.STRONG.name(), + PasswordValidator.ScoreStrength::fromConfiguration, + Property.NodeScope, Property.Filtered + ) + ); // Compliance settings.add(Setting.listSetting(ConfigConstants.OPENDISTRO_SECURITY_COMPLIANCE_HISTORY_WRITE_WATCHED_INDICES, Collections.emptyList(), Function.identity(), Property.NodeScope)); //not filtered here diff --git a/src/main/java/org/opensearch/security/dlic/rest/validation/AbstractConfigurationValidator.java b/src/main/java/org/opensearch/security/dlic/rest/validation/AbstractConfigurationValidator.java index 81942d9c11..e3221de7e6 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/validation/AbstractConfigurationValidator.java +++ b/src/main/java/org/opensearch/security/dlic/rest/validation/AbstractConfigurationValidator.java @@ -249,9 +249,19 @@ public XContentBuilder errorsAsXContent(RestChannel channel) { break; case INVALID_PASSWORD: builder.field("status", "error"); - builder.field("reason", opensearchSettings.get(ConfigConstants.SECURITY_RESTAPI_PASSWORD_VALIDATION_ERROR_MESSAGE, + builder.field("reason", opensearchSettings.get( + ConfigConstants.SECURITY_RESTAPI_PASSWORD_VALIDATION_ERROR_MESSAGE, "Password does not match minimum criteria")); break; + case WEAK_PASSWORD: + case SIMILAR_PASSWORD: + builder.field("status", "error"); + builder.field( + "reason", + opensearchSettings.get( + ConfigConstants.SECURITY_RESTAPI_PASSWORD_VALIDATION_ERROR_MESSAGE, + errorType.message)); + break; case WRONG_DATATYPE: builder.field("status", "error"); builder.field("reason", ErrorType.WRONG_DATATYPE.getMessage()); @@ -289,8 +299,14 @@ public static enum DataType { } public static enum ErrorType { - NONE("ok"), INVALID_CONFIGURATION("Invalid configuration"), INVALID_PASSWORD("Invalid password"), WRONG_DATATYPE("Wrong datatype"), - BODY_NOT_PARSEABLE("Could not parse content of request."), PAYLOAD_NOT_ALLOWED("Request body not allowed for this action."), + NONE("ok"), + INVALID_CONFIGURATION("Invalid configuration"), + INVALID_PASSWORD("Invalid password"), + WEAK_PASSWORD("Weak password"), + SIMILAR_PASSWORD("Password is similar to user name"), + WRONG_DATATYPE("Wrong datatype"), + BODY_NOT_PARSEABLE("Could not parse content of request."), + PAYLOAD_NOT_ALLOWED("Request body not allowed for this action."), PAYLOAD_MANDATORY("Request body required for this action."), SECURITY_NOT_INITIALIZED("Security index not initialized"), NULL_ARRAY_ELEMENT("`null` is not allowed as json array element"); diff --git a/src/main/java/org/opensearch/security/dlic/rest/validation/CredentialsValidator.java b/src/main/java/org/opensearch/security/dlic/rest/validation/CredentialsValidator.java index db95660448..a54f1947f0 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/validation/CredentialsValidator.java +++ b/src/main/java/org/opensearch/security/dlic/rest/validation/CredentialsValidator.java @@ -12,7 +12,6 @@ package org.opensearch.security.dlic.rest.validation; import java.util.Map; -import java.util.regex.Pattern; import org.opensearch.common.Strings; import org.opensearch.common.bytes.BytesReference; @@ -22,17 +21,21 @@ import org.opensearch.common.xcontent.XContentType; import org.opensearch.rest.RestRequest; import org.opensearch.security.ssl.util.Utils; -import org.opensearch.security.support.ConfigConstants; /** * Validator for validating password and hash present in the payload */ public class CredentialsValidator extends AbstractConfigurationValidator { - public CredentialsValidator(final RestRequest request, BytesReference ref, final Settings opensearchSettings, + private final PasswordValidator passwordValidator; + + public CredentialsValidator(final RestRequest request, + final BytesReference ref, + final Settings opensearchSettings, Object... param) { super(request, ref, opensearchSettings, param); this.payloadMandatory = true; + this.passwordValidator = PasswordValidator.of(opensearchSettings); allowedKeys.put("hash", DataType.STRING); allowedKeys.put("password", DataType.STRING); } @@ -46,49 +49,29 @@ public boolean validate() { if (!super.validate()) { return false; } - - final String regex = this.opensearchSettings.get(ConfigConstants.SECURITY_RESTAPI_PASSWORD_VALIDATION_REGEX, null); - if ((request.method() == RestRequest.Method.PUT || request.method() == RestRequest.Method.PATCH) && this.content != null && this.content.length() > 1) { try { final Map contentAsMap = XContentHelper.convertToMap(this.content, false, XContentType.JSON).v2(); - String password = (String) contentAsMap.get("password"); + final String password = (String) contentAsMap.get("password"); if (password != null) { // Password is not allowed to be empty if present. if (password.isEmpty()) { this.errorType = ErrorType.INVALID_PASSWORD; return false; } - - if (!Strings.isNullOrEmpty(regex)) { - // Password can be null for an existing user. Regex will validate password if present - if (!Pattern.compile("^"+regex+"$").matcher(password).matches()) { - if(log.isDebugEnabled()) { - log.debug("Regex does not match password"); - } - this.errorType = ErrorType.INVALID_PASSWORD; - return false; - } - - final String username = Utils.coalesce(request.param("name"), hasParams() ? (String) param[0] : null); - final boolean isDebugEnabled = log.isDebugEnabled(); - - if (username == null || username.isEmpty()) { - if (isDebugEnabled) { - log.debug("Unable to validate username because no user is given"); - } - return false; - } - - if (username.toLowerCase().equals(password.toLowerCase())) { - if (isDebugEnabled) { - log.debug("Username must not match password"); - } - this.errorType = ErrorType.INVALID_PASSWORD; - return false; + final String username = Utils.coalesce(request.param("name"), hasParams() ? (String) param[0] : null); + if (Strings.isNullOrEmpty(username)) { + if (log.isDebugEnabled()) { + log.debug("Unable to validate username because no user is given"); } + return false; + } + final ErrorType passwordValidationResult = passwordValidator.validate(username, password); + if (passwordValidationResult != ErrorType.NONE) { + this.errorType = passwordValidationResult; + return false; } } } catch (NotXContentException e) { @@ -99,4 +82,5 @@ public boolean validate() { } return true; } + } diff --git a/src/main/java/org/opensearch/security/dlic/rest/validation/PasswordValidator.java b/src/main/java/org/opensearch/security/dlic/rest/validation/PasswordValidator.java new file mode 100644 index 0000000000..a59a4e6d40 --- /dev/null +++ b/src/main/java/org/opensearch/security/dlic/rest/validation/PasswordValidator.java @@ -0,0 +1,185 @@ +/* + * 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.dlic.rest.validation; + +import java.util.List; +import java.util.Locale; +import java.util.Objects; +import java.util.StringJoiner; +import java.util.function.Predicate; +import java.util.regex.Pattern; + +import com.google.common.collect.ImmutableList; +import com.nulabinc.zxcvbn.Strength; +import com.nulabinc.zxcvbn.Zxcvbn; +import com.nulabinc.zxcvbn.matchers.Match; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + +import org.opensearch.common.Strings; +import org.opensearch.common.settings.Settings; +import org.opensearch.security.dlic.rest.validation.AbstractConfigurationValidator.ErrorType; + +import static org.opensearch.security.support.ConfigConstants.SECURITY_RESTAPI_PASSWORD_MIN_LENGTH; +import static org.opensearch.security.support.ConfigConstants.SECURITY_RESTAPI_PASSWORD_SCORE_BASED_VALIDATION_STRENGTH; +import static org.opensearch.security.support.ConfigConstants.SECURITY_RESTAPI_PASSWORD_VALIDATION_REGEX; + +public class PasswordValidator { + + private static final int MAX_LENGTH = 100; + + /** + * Checks a username similarity and a password + * names and passwords like: + * - some_user_name/456Some_uSer_Name_1234 + * - some_user_name/some_user_name_Ydfge + * - some_user_name/eman_resu_emos + * are similar + * "user_inputs" - is a default dictionary zxcvbn creates for checking similarity + */ + private final static Predicate USERNAME_SIMILARITY_CHECK = m -> + m.pattern == com.nulabinc.zxcvbn.Pattern.Dictionary && "user_inputs".equals(m.dictionaryName); + + private final Logger logger = LogManager.getLogger(this.getClass()); + + private final int minPasswordLength; + + private final Pattern passwordRegexpPattern; + + private final ScoreStrength scoreStrength; + + private final Zxcvbn zxcvbn; + + private PasswordValidator(final int minPasswordLength, + final Pattern passwordRegexpPattern, + final ScoreStrength scoreStrength) { + this.minPasswordLength = minPasswordLength; + this.passwordRegexpPattern = passwordRegexpPattern; + this.scoreStrength = scoreStrength; + this.zxcvbn = new Zxcvbn(); + } + + public static PasswordValidator of(final Settings settings) { + final String passwordRegex = settings.get(SECURITY_RESTAPI_PASSWORD_VALIDATION_REGEX, null); + final ScoreStrength scoreStrength = ScoreStrength.fromConfiguration( + settings.get(SECURITY_RESTAPI_PASSWORD_SCORE_BASED_VALIDATION_STRENGTH, ScoreStrength.STRONG.name()) + ); + final int minPasswordLength = settings.getAsInt(SECURITY_RESTAPI_PASSWORD_MIN_LENGTH, -1); + return new PasswordValidator( + minPasswordLength, + !Strings.isNullOrEmpty(passwordRegex) ? Pattern.compile(String.format("^%s$", passwordRegex)) : null, + scoreStrength); + } + + ErrorType validate(final String username, final String password) { + if (minPasswordLength > 0 && password.length() < minPasswordLength) { + logger.debug( + "Password is too short, the minimum required length is {}, but current length is {}", + minPasswordLength, + password.length() + ); + return ErrorType.INVALID_PASSWORD; + } + if (password.length() > MAX_LENGTH) { + logger.debug( + "Password is too long, the maximum required length is {}, but current length is {}", + MAX_LENGTH, + password.length() + ); + return ErrorType.INVALID_PASSWORD; + } + if (Objects.nonNull(passwordRegexpPattern) + && !passwordRegexpPattern.matcher(password).matches()) { + logger.debug("Regex does not match password"); + return ErrorType.INVALID_PASSWORD; + } + final Strength strength = zxcvbn.measure(password, ImmutableList.of(username)); + if (strength.getScore() < scoreStrength.score()) { + logger.debug( + "Password is weak the required score is {}, but current is {}", + scoreStrength, + ScoreStrength.fromScore(strength.getScore()) + ); + return ErrorType.WEAK_PASSWORD; + } + final boolean similar = strength.getSequence() + .stream() + .anyMatch(USERNAME_SIMILARITY_CHECK); + if (similar) { + logger.debug("Password is too similar to the user name {}", username); + return ErrorType.SIMILAR_PASSWORD; + } + return ErrorType.NONE; + } + + public enum ScoreStrength { + + // The weak score defines here only for debugging information + // and doesn't use as a configuration setting value. + WEAK(0, "too guessable: risky password"), + FAIR(1, "very guessable: protection from throttled online attacks"), + GOOD(2, "somewhat guessable: protection from unthrottled online attacks"), + STRONG(3, "safely unguessable: moderate protection from offline slow-hash scenario"), + VERY_STRONG(4, "very unguessable: strong protection from offline slow-hash scenario"); + + private final int score; + + private final String description; + + static final List CONFIGURATION_VALUES = ImmutableList.of(FAIR, STRONG, VERY_STRONG); + + static final String EXPECTED_CONFIGURATION_VALUES = + new StringJoiner(",") + .add(FAIR.name().toLowerCase(Locale.ROOT)) + .add(STRONG.name().toLowerCase(Locale.ROOT)) + .add(VERY_STRONG.name().toLowerCase(Locale.ROOT)) + .toString(); + + private ScoreStrength(final int score, final String description) { + this.score = score; + this.description = description; + } + + public static ScoreStrength fromScore(final int score) { + for (final ScoreStrength strength : values()) { + if (strength.score == score) + return strength; + } + throw new IllegalArgumentException("Unknown score " + score); + } + + public static ScoreStrength fromConfiguration(final String value) { + for (final ScoreStrength strength : CONFIGURATION_VALUES) { + if (strength.name().equalsIgnoreCase(value)) + return strength; + } + throw new IllegalArgumentException( + String.format( + "Setting [%s] cannot be used with the configured: %s. Expected one of [%s]", + SECURITY_RESTAPI_PASSWORD_SCORE_BASED_VALIDATION_STRENGTH, + value, + EXPECTED_CONFIGURATION_VALUES + ) + ); + } + + @Override + public String toString() { + return String.format("Password strength score %s. %s", score, description); + } + + public int score() { + return this.score; + } + + } +} diff --git a/src/main/java/org/opensearch/security/support/ConfigConstants.java b/src/main/java/org/opensearch/security/support/ConfigConstants.java index a58639bc0a..83281c9d0b 100644 --- a/src/main/java/org/opensearch/security/support/ConfigConstants.java +++ b/src/main/java/org/opensearch/security/support/ConfigConstants.java @@ -255,7 +255,9 @@ public enum RolesMappingResolution { public static final String SECURITY_RESTAPI_ENDPOINTS_DISABLED = "plugins.security.restapi.endpoints_disabled"; public static final String SECURITY_RESTAPI_PASSWORD_VALIDATION_REGEX = "plugins.security.restapi.password_validation_regex"; public static final String SECURITY_RESTAPI_PASSWORD_VALIDATION_ERROR_MESSAGE = "plugins.security.restapi.password_validation_error_message"; - + public static final String SECURITY_RESTAPI_PASSWORD_MIN_LENGTH = "plugins.security.restapi.password_min_length"; + public static final String SECURITY_RESTAPI_PASSWORD_SCORE_BASED_VALIDATION_STRENGTH = "plugins.security.restapi.password_score_based_validation_strength"; + // Illegal Opcodes from here on public static final String SECURITY_UNSUPPORTED_DISABLE_REST_AUTH_INITIALLY = "plugins.security.unsupported.disable_rest_auth_initially"; public static final String SECURITY_UNSUPPORTED_DISABLE_INTERTRANSPORT_AUTH_INITIALLY = "plugins.security.unsupported.disable_intertransport_auth_initially"; public static final String SECURITY_UNSUPPORTED_PASSIVE_INTERTRANSPORT_AUTH_INITIALLY = "plugins.security.unsupported.passive_intertransport_auth_initially"; diff --git a/src/test/java/org/opensearch/security/auditlog/compliance/RestApiComplianceAuditlogTest.java b/src/test/java/org/opensearch/security/auditlog/compliance/RestApiComplianceAuditlogTest.java index 0a90f2f396..37ef283ccb 100644 --- a/src/test/java/org/opensearch/security/auditlog/compliance/RestApiComplianceAuditlogTest.java +++ b/src/test/java/org/opensearch/security/auditlog/compliance/RestApiComplianceAuditlogTest.java @@ -42,7 +42,7 @@ public void testRestApiRolesEnabled() throws Exception { setup(additionalSettings); TestAuditlogImpl.clear(); - String body = "{ \"password\":\"test\",\"backend_roles\":[\"role1\",\"role2\"] }"; + String body = "{ \"password\":\"some new password\",\"backend_roles\":[\"role1\",\"role2\"] }"; HttpResponse response = rh.executePutRequest("_opendistro/_security/api/internalusers/compuser?pretty", body, encodeBasicHeader("admin", "admin")); Thread.sleep(1500); System.out.println(TestAuditlogImpl.sb.toString()); @@ -71,7 +71,7 @@ public void testRestApiRolesDisabled() throws Exception { setup(additionalSettings); TestAuditlogImpl.clear(); - String body = "{ \"password\":\"test\",\"backend_roles\":[\"role1\",\"role2\"] }"; + String body = "{ \"password\":\"some new password\",\"backend_roles\":[\"role1\",\"role2\"] }"; rh.enableHTTPClientSSL = true; rh.trustHTTPServerCertificate = true; @@ -169,12 +169,13 @@ public void testRestApiNewUser() throws Exception { setup(additionalSettings); TestAuditlogImpl.clear(); - String body = "{ \"password\":\"test\",\"backend_roles\":[\"role1\",\"role2\"] }"; + String body = "{ \"password\":\"some new password\",\"backend_roles\":[\"role1\",\"role2\"] }"; System.out.println("exec"); - HttpResponse response = rh.executePutRequest("_opendistro/_security/api/internalusers/compuser?pretty", body, encodeBasicHeader("admin", "admin")); + HttpResponse response = rh.executePutRequest("_opendistro/_security/api/internalusers/compuser?pretty", + body, encodeBasicHeader("admin", "admin")); Thread.sleep(1500); System.out.println(TestAuditlogImpl.sb.toString()); - Assert.assertEquals(HttpStatus.SC_CREATED, response.getStatusCode()); + Assert.assertEquals(response.getBody(), HttpStatus.SC_CREATED, response.getStatusCode()); Assert.assertTrue(TestAuditlogImpl.messages.size()+"", TestAuditlogImpl.messages.isEmpty()); } @@ -241,7 +242,7 @@ public void testBCryptHashRedaction() throws Exception { // create internal user and verify no BCrypt hash is present in audit logs TestAuditlogImpl.clear(); - rh.executePutRequest("/_opendistro/_security/api/internalusers/test", "{ \"password\":\"test\"}"); + rh.executePutRequest("/_opendistro/_security/api/internalusers/test", "{ \"password\":\"some new user password\"}"); Assert.assertEquals(1, TestAuditlogImpl.messages.size()); Assert.assertFalse(AuditMessage.BCRYPT_HASH.matcher(TestAuditlogImpl.sb.toString()).matches()); } diff --git a/src/test/java/org/opensearch/security/dlic/rest/api/AbstractRestApiUnitTest.java b/src/test/java/org/opensearch/security/dlic/rest/api/AbstractRestApiUnitTest.java index a1cb20d99c..7b590f1d46 100644 --- a/src/test/java/org/opensearch/security/dlic/rest/api/AbstractRestApiUnitTest.java +++ b/src/test/java/org/opensearch/security/dlic/rest/api/AbstractRestApiUnitTest.java @@ -16,6 +16,7 @@ import java.util.Collection; import java.util.HashMap; import java.util.Map; +import java.util.Objects; import java.util.stream.Stream; import com.fasterxml.jackson.core.JsonParseException; @@ -32,12 +33,15 @@ import org.opensearch.plugins.Plugin; import org.opensearch.security.DefaultObjectMapper; import org.opensearch.security.auditlog.AuditTestUtils; +import org.opensearch.security.dlic.rest.validation.PasswordValidator; import org.opensearch.security.test.DynamicSecurityConfig; import org.opensearch.security.test.SingleClusterTest; import org.opensearch.security.test.helper.file.FileHelper; import org.opensearch.security.test.helper.rest.RestHelper; import org.opensearch.security.test.helper.rest.RestHelper.HttpResponse; +import static org.opensearch.security.support.ConfigConstants.SECURITY_RESTAPI_PASSWORD_SCORE_BASED_VALIDATION_STRENGTH; + public abstract class AbstractRestApiUnitTest extends SingleClusterTest { protected RestHelper rh = null; @@ -53,6 +57,7 @@ protected final void setup() throws Exception { Settings.Builder builder = Settings.builder(); builder.put("plugins.security.ssl.http.enabled", true) + .put(SECURITY_RESTAPI_PASSWORD_SCORE_BASED_VALIDATION_STRENGTH, PasswordValidator.ScoreStrength.FAIR.name()) .put("plugins.security.ssl.http.keystore_filepath", FileHelper.getAbsoluteFilePathFromClassPath("restapi/node-0-keystore.jks")) .put("plugins.security.ssl.http.truststore_filepath", @@ -89,6 +94,7 @@ protected final void setupWithRestRoles(Settings nodeOverride) throws Exception Settings.Builder builder = Settings.builder(); builder.put("plugins.security.ssl.http.enabled", true) + .put(SECURITY_RESTAPI_PASSWORD_SCORE_BASED_VALIDATION_STRENGTH, PasswordValidator.ScoreStrength.FAIR.name()) .put("plugins.security.ssl.http.keystore_filepath", FileHelper.getAbsoluteFilePathFromClassPath("restapi/node-0-keystore.jks")) .put("plugins.security.ssl.http.truststore_filepath", @@ -130,12 +136,19 @@ protected void deleteUser(String username) throws Exception { } protected void addUserWithPassword(String username, String password, int status) throws Exception { + addUserWithPassword(username, password, status, null); + } + + protected void addUserWithPassword(String username, String password, int status, String message) throws Exception { boolean sendAdminCertificate = rh.sendAdminCertificate; rh.sendAdminCertificate = true; HttpResponse response = rh.executePutRequest("/_opendistro/_security/api/internalusers/" + username, "{\"password\": \"" + password + "\"}", new Header[0]); Assert.assertEquals(status, response.getStatusCode()); rh.sendAdminCertificate = sendAdminCertificate; + if (Objects.nonNull(message)) { + Assert.assertTrue(response.getBody().contains(message)); + } } protected void addUserWithPassword(String username, String password, String[] roles, int status) throws Exception { @@ -220,7 +233,7 @@ protected String checkWriteAccess(int status, String username, String password, String payload = "{\"value\" : \"true\"}"; HttpResponse response = rh.executePutRequest(action, payload, encodeBasicHeader(username, password)); int returnedStatus = response.getStatusCode(); - Assert.assertEquals(status, returnedStatus); + Assert.assertEquals(response.getBody(), status, returnedStatus); return response.getBody(); } diff --git a/src/test/java/org/opensearch/security/dlic/rest/api/AccountApiTest.java b/src/test/java/org/opensearch/security/dlic/rest/api/AccountApiTest.java index 0b91aa35af..9f6bcb65c9 100644 --- a/src/test/java/org/opensearch/security/dlic/rest/api/AccountApiTest.java +++ b/src/test/java/org/opensearch/security/dlic/rest/api/AccountApiTest.java @@ -45,7 +45,7 @@ public void testGetAccount() throws Exception { // arrange setup(); final String testUser = "test-user"; - final String testPass = "test-pass"; + final String testPass = "some password for user"; addUserWithPassword(testUser, testPass, HttpStatus.SC_CREATED); // test - unauthorized access as credentials are missing. diff --git a/src/test/java/org/opensearch/security/dlic/rest/api/ActionGroupsApiTest.java b/src/test/java/org/opensearch/security/dlic/rest/api/ActionGroupsApiTest.java index abb45df268..926ee23f28 100644 --- a/src/test/java/org/opensearch/security/dlic/rest/api/ActionGroupsApiTest.java +++ b/src/test/java/org/opensearch/security/dlic/rest/api/ActionGroupsApiTest.java @@ -53,9 +53,9 @@ public void testActionGroupsApi() throws Exception { setupStarfleetIndex(); // add user picard, role starfleet, maps to opendistro_security_role_starfleet - addUserWithPassword("picard", "picard", new String[] { "starfleet" }, HttpStatus.SC_CREATED); - checkReadAccess(HttpStatus.SC_OK, "picard", "picard", "sf", "_doc", 0); - checkWriteAccess(HttpStatus.SC_FORBIDDEN, "picard", "picard", "sf", "_doc", 0); + addUserWithPassword("picard", "picardpicardpicard", new String[] { "starfleet" }, HttpStatus.SC_CREATED); + checkReadAccess(HttpStatus.SC_OK, "picard", "picardpicardpicard", "sf", "_doc", 0); + checkWriteAccess(HttpStatus.SC_FORBIDDEN, "picard", "picardpicardpicard", "sf", "_doc", 0); rh.sendAdminCertificate = true; verifyGetForSuperAdmin(new Header[0]); rh.sendAdminCertificate = true; @@ -122,21 +122,21 @@ void verifyDeleteForSuperAdmin(final Header[] header, final boolean userAdminCer Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); rh.sendAdminCertificate = false; - checkReadAccess(HttpStatus.SC_FORBIDDEN, "picard", "picard", "sf", "_doc", 0); + checkReadAccess(HttpStatus.SC_FORBIDDEN, "picard", "picardpicardpicard", "sf", "_doc", 0); // put picard in captains role. Role opendistro_security_role_captains uses the CRUD_UT // action group // which uses READ_UT and WRITE action groups. We removed READ_UT, so only // WRITE is possible - addUserWithPassword("picard", "picard", new String[] { "captains" }, HttpStatus.SC_OK); - checkWriteAccess(HttpStatus.SC_OK, "picard", "picard", "sf", "_doc", 0); - checkReadAccess(HttpStatus.SC_FORBIDDEN, "picard", "picard", "sf", "_doc", 0); + addUserWithPassword("picard", "picardpicardpicard", new String[] { "captains" }, HttpStatus.SC_OK); + checkWriteAccess(HttpStatus.SC_OK, "picard", "picardpicardpicard", "sf", "_doc", 0); + checkReadAccess(HttpStatus.SC_FORBIDDEN, "picard", "picardpicardpicard", "sf", "_doc", 0); // now remove also CRUD_UT groups, write also not possible anymore rh.sendAdminCertificate = true; response = rh.executeDeleteRequest(ENDPOINT+"/CRUD_UT", new Header[0]); rh.sendAdminCertificate = false; - checkWriteAccess(HttpStatus.SC_FORBIDDEN, "picard", "picard", "sf", "_doc", 0); - checkReadAccess(HttpStatus.SC_FORBIDDEN, "picard", "picard", "sf", "_doc", 0); + checkWriteAccess(HttpStatus.SC_FORBIDDEN, "picard", "picardpicardpicard", "sf", "_doc", 0); + checkReadAccess(HttpStatus.SC_FORBIDDEN, "picard", "picardpicardpicard", "sf", "_doc", 0); } void verifyPutForSuperAdmin(final Header[] header, final boolean userAdminCert) throws Exception { @@ -161,8 +161,8 @@ void verifyPutForSuperAdmin(final Header[] header, final boolean userAdminCert) rh.sendAdminCertificate = false; // write access allowed again, read forbidden, since READ_UT group is still missing - checkReadAccess(HttpStatus.SC_FORBIDDEN, "picard", "picard", "sf", "_doc", 0); - checkWriteAccess(HttpStatus.SC_OK, "picard", "picard", "sf", "_doc", 0); + checkReadAccess(HttpStatus.SC_FORBIDDEN, "picard", "picardpicardpicard", "sf", "_doc", 0); + checkWriteAccess(HttpStatus.SC_OK, "picard", "picardpicardpicard", "sf", "_doc", 0); // restore READ_UT action groups rh.sendAdminCertificate = userAdminCert; @@ -171,8 +171,8 @@ void verifyPutForSuperAdmin(final Header[] header, final boolean userAdminCert) rh.sendAdminCertificate = false; // read/write allowed again - checkReadAccess(HttpStatus.SC_OK, "picard", "picard", "sf", "_doc", 0); - checkWriteAccess(HttpStatus.SC_OK, "picard", "picard", "sf", "_doc", 0); + checkReadAccess(HttpStatus.SC_OK, "picard", "picardpicardpicard", "sf", "_doc", 0); + checkWriteAccess(HttpStatus.SC_OK, "picard", "picardpicardpicard", "sf", "_doc", 0); // -- PUT, new JSON format including readonly flag, disallowed in REST API rh.sendAdminCertificate = userAdminCert; @@ -370,9 +370,9 @@ public void testActionGroupsApiForRestAdmin() throws Exception { final Header restApiAdminHeader = encodeBasicHeader("rest_api_admin_user", "rest_api_admin_user"); // add user picard, role starfleet, maps to opendistro_security_role_starfleet - addUserWithPassword("picard", "picard", new String[] { "starfleet" }, HttpStatus.SC_CREATED); - checkReadAccess(HttpStatus.SC_OK, "picard", "picard", "sf", "_doc", 0); - checkWriteAccess(HttpStatus.SC_FORBIDDEN, "picard", "picard", "sf", "_doc", 0); + addUserWithPassword("picard", "picardpicardpicard", new String[] { "starfleet" }, HttpStatus.SC_CREATED); + checkReadAccess(HttpStatus.SC_OK, "picard", "picardpicardpicard", "sf", "_doc", 0); + checkWriteAccess(HttpStatus.SC_FORBIDDEN, "picard", "picardpicardpicard", "sf", "_doc", 0); verifyGetForSuperAdmin(new Header[] {restApiAdminHeader}); verifyDeleteForSuperAdmin(new Header[]{restApiAdminHeader}, false); verifyPutForSuperAdmin(new Header[]{restApiAdminHeader}, false); @@ -388,9 +388,9 @@ public void testActionGroupsApiForActionGroupsRestApiAdmin() throws Exception { final Header restApiAdminActionGroupsHeader = encodeBasicHeader("rest_api_admin_actiongroups", "rest_api_admin_actiongroups"); // add user picard, role starfleet, maps to opendistro_security_role_starfleet - addUserWithPassword("picard", "picard", new String[] { "starfleet" }, HttpStatus.SC_CREATED); - checkReadAccess(HttpStatus.SC_OK, "picard", "picard", "sf", "_doc", 0); - checkWriteAccess(HttpStatus.SC_FORBIDDEN, "picard", "picard", "sf", "_doc", 0); + addUserWithPassword("picard", "picardpicardpicard", new String[] { "starfleet" }, HttpStatus.SC_CREATED); + checkReadAccess(HttpStatus.SC_OK, "picard", "picardpicardpicard", "sf", "_doc", 0); + checkWriteAccess(HttpStatus.SC_FORBIDDEN, "picard", "picardpicardpicard", "sf", "_doc", 0); verifyGetForSuperAdmin(new Header[] {restApiAdminActionGroupsHeader}); verifyDeleteForSuperAdmin(new Header[]{restApiAdminActionGroupsHeader}, false); verifyPutForSuperAdmin(new Header[]{restApiAdminActionGroupsHeader}, false); diff --git a/src/test/java/org/opensearch/security/dlic/rest/api/RolesApiTest.java b/src/test/java/org/opensearch/security/dlic/rest/api/RolesApiTest.java index 5a6d1f297c..a85566dc91 100644 --- a/src/test/java/org/opensearch/security/dlic/rest/api/RolesApiTest.java +++ b/src/test/java/org/opensearch/security/dlic/rest/api/RolesApiTest.java @@ -151,9 +151,9 @@ public void testRolesApi() throws Exception { setupStarfleetIndex(); // add user picard, role starfleet, maps to opendistro_security_role_starfleet - addUserWithPassword("picard", "picard", new String[]{"starfleet", "captains"}, HttpStatus.SC_CREATED); - checkReadAccess(HttpStatus.SC_OK, "picard", "picard", "sf", "_doc", 0); - checkWriteAccess(HttpStatus.SC_OK, "picard", "picard", "sf", "_doc", 0); + addUserWithPassword("picard", "picardpicardpicardpicard", new String[]{"starfleet", "captains"}, HttpStatus.SC_CREATED); + checkReadAccess(HttpStatus.SC_OK, "picard", "picardpicardpicardpicard", "sf", "_doc", 0); + checkWriteAccess(HttpStatus.SC_OK, "picard", "picardpicardpicardpicard", "sf", "_doc", 0); rh.sendAdminCertificate = true; verifyGetForSuperAdmin(new Header[0]); @@ -220,17 +220,17 @@ void verifyDeleteForSuperAdmin(final Header[] header, final boolean sendAdminCer Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); rh.sendAdminCertificate = false; // user has only role starfleet left, role has READ access only - checkWriteAccess(HttpStatus.SC_FORBIDDEN, "picard", "picard", "sf", "_doc", 1); + checkWriteAccess(HttpStatus.SC_FORBIDDEN, "picard", "picardpicardpicardpicard", "sf", "_doc", 1); // ES7 only supports one doc type, but OpenSearch permission checks run first // So we also get a 403 FORBIDDEN when tring to add new document type - checkWriteAccess(HttpStatus.SC_FORBIDDEN, "picard", "picard", "sf", "_doc", 0); + checkWriteAccess(HttpStatus.SC_FORBIDDEN, "picard", "picardpicardpicardpicard", "sf", "_doc", 0); rh.sendAdminCertificate = sendAdminCert; // remove also starfleet role, nothing is allowed anymore response = rh.executeDeleteRequest(ENDPOINT + "/roles/opendistro_security_role_starfleet", header); Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); - checkReadAccess(HttpStatus.SC_FORBIDDEN, "picard", "picard", "sf", "_doc", 0); - checkWriteAccess(HttpStatus.SC_FORBIDDEN, "picard", "picard", "sf", "_doc", 0); + checkReadAccess(HttpStatus.SC_FORBIDDEN, "picard", "picardpicardpicardpicard", "sf", "_doc", 0); + checkWriteAccess(HttpStatus.SC_FORBIDDEN, "picard", "picardpicardpicardpicard", "sf", "_doc", 0); } void verifyPutForSuperAdmin(final Header[] header, final boolean sendAdminCert) throws Exception { @@ -282,14 +282,14 @@ void verifyPutForSuperAdmin(final Header[] header, final boolean sendAdminCert) FileHelper.loadFile("restapi/roles_starfleet.json"), header); Assert.assertEquals(HttpStatus.SC_CREATED, response.getStatusCode()); rh.sendAdminCertificate = false; - checkReadAccess(HttpStatus.SC_OK, "picard", "picard", "sf", "_doc", 0); + checkReadAccess(HttpStatus.SC_OK, "picard", "picardpicardpicardpicard", "sf", "_doc", 0); // now picard is only in opendistro_security_role_starfleet, which has write access to // all indices. We collapse all document types in ODFE7 so this permission in the // starfleet role grants all permissions: // _doc: // - 'indices:*' - checkWriteAccess(HttpStatus.SC_OK, "picard", "picard", "sf", "_doc", 0); + checkWriteAccess(HttpStatus.SC_OK, "picard", "picardpicardpicardpicard", "sf", "_doc", 0); rh.sendAdminCertificate = sendAdminCert; @@ -298,19 +298,14 @@ void verifyPutForSuperAdmin(final Header[] header, final boolean sendAdminCert) FileHelper.loadFile("restapi/roles_captains.json"), header); Assert.assertEquals(HttpStatus.SC_CREATED, response.getStatusCode()); rh.sendAdminCertificate = false; - checkReadAccess(HttpStatus.SC_OK, "picard", "picard", "sf", "_doc", 0); - checkWriteAccess(HttpStatus.SC_OK, "picard", "picard", "sf", "_doc", 0); + checkReadAccess(HttpStatus.SC_OK, "picard", "picardpicardpicardpicard", "sf", "_doc", 0); + checkWriteAccess(HttpStatus.SC_OK, "picard", "picardpicardpicardpicard", "sf", "_doc", 0); rh.sendAdminCertificate = sendAdminCert; response = rh.executePutRequest(ENDPOINT + "/roles/opendistro_security_role_starfleet_captains", FileHelper.loadFile("restapi/roles_complete_invalid.json"), header); Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode()); -// rh.sendAdminCertificate = sendAdminCert; -// response = rh.executePutRequest(ENDPOINT + "/roles/opendistro_security_role_starfleet_captains", -// FileHelper.loadFile("restapi/roles_multiple.json"), header); -// Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode()); - response = rh.executePutRequest(ENDPOINT + "/roles/opendistro_security_role_starfleet_captains", FileHelper.loadFile("restapi/roles_multiple_2.json"), header); Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode()); @@ -530,9 +525,9 @@ public void testRolesApiWithAllRestApiPermissions() throws Exception { setupStarfleetIndex(); // add user picard, role starfleet, maps to opendistro_security_role_starfleet - addUserWithPassword("picard", "picard", new String[]{"starfleet", "captains"}, HttpStatus.SC_CREATED); - checkReadAccess(HttpStatus.SC_OK, "picard", "picard", "sf", "_doc", 0); - checkWriteAccess(HttpStatus.SC_OK, "picard", "picard", "sf", "_doc", 0); + addUserWithPassword("picard", "picardpicardpicardpicard", new String[]{"starfleet", "captains"}, HttpStatus.SC_CREATED); + checkReadAccess(HttpStatus.SC_OK, "picard", "picardpicardpicardpicard", "sf", "_doc", 0); + checkWriteAccess(HttpStatus.SC_OK, "picard", "picardpicardpicardpicard", "sf", "_doc", 0); verifyGetForSuperAdmin(new Header[]{restApiAdminHeader}); verifyDeleteForSuperAdmin(new Header[]{restApiAdminHeader}, false); @@ -550,9 +545,9 @@ public void testRolesApiWithRestApiRolePermission() throws Exception { setupStarfleetIndex(); // add user picard, role starfleet, maps to opendistro_security_role_starfleet - addUserWithPassword("picard", "picard", new String[]{"starfleet", "captains"}, HttpStatus.SC_CREATED); - checkReadAccess(HttpStatus.SC_OK, "picard", "picard", "sf", "_doc", 0); - checkWriteAccess(HttpStatus.SC_OK, "picard", "picard", "sf", "_doc", 0); + addUserWithPassword("picard", "picardpicardpicardpicard", new String[]{"starfleet", "captains"}, HttpStatus.SC_CREATED); + checkReadAccess(HttpStatus.SC_OK, "picard", "picardpicardpicardpicard", "sf", "_doc", 0); + checkWriteAccess(HttpStatus.SC_OK, "picard", "picardpicardpicardpicard", "sf", "_doc", 0); verifyGetForSuperAdmin(new Header[]{restApiRolesHeader}); diff --git a/src/test/java/org/opensearch/security/dlic/rest/api/RolesMappingApiTest.java b/src/test/java/org/opensearch/security/dlic/rest/api/RolesMappingApiTest.java index 44e160e151..3bc647bf12 100644 --- a/src/test/java/org/opensearch/security/dlic/rest/api/RolesMappingApiTest.java +++ b/src/test/java/org/opensearch/security/dlic/rest/api/RolesMappingApiTest.java @@ -53,8 +53,8 @@ public void testRolesMappingApi() throws Exception { setupStarfleetIndex(); // add user picard, role captains initially maps to // opendistro_security_role_starfleet_captains and opendistro_security_role_starfleet - addUserWithPassword("picard", "picard", new String[] { "captains" }, HttpStatus.SC_CREATED); - checkWriteAccess(HttpStatus.SC_CREATED, "picard", "picard", "sf", "_doc", 1); + addUserWithPassword("picard", "picardpicardpicard", new String[] { "captains" }, HttpStatus.SC_CREATED); + checkWriteAccess(HttpStatus.SC_CREATED, "picard", "picardpicardpicard", "sf", "_doc", 1); // TODO: only one doctype allowed for ES6 //checkWriteAccess(HttpStatus.SC_CREATED, "picard", "picard", "sf", "_doc", 1); rh.sendAdminCertificate = true; @@ -65,35 +65,35 @@ public void testRolesMappingApi() throws Exception { verifyPutForSuperAdmin(new Header[0]); verifyPatchForSuperAdmin(new Header[0]); // mapping with several backend roles, one of the is captain - deleteAndputNewMapping(new Header[0],"rolesmapping_backendroles_captains_list.json", true); + deleteAndPutNewMapping(new Header[0],"rolesmapping_backendroles_captains_list.json", true); checkAllSfAllowed(); // mapping with one backend role, captain - deleteAndputNewMapping(new Header[0],"rolesmapping_backendroles_captains_single.json", true); + deleteAndPutNewMapping(new Header[0],"rolesmapping_backendroles_captains_single.json", true); checkAllSfAllowed(); // mapping with several users, one is picard - deleteAndputNewMapping(new Header[0],"rolesmapping_users_picard_list.json", true); + deleteAndPutNewMapping(new Header[0],"rolesmapping_users_picard_list.json", true); checkAllSfAllowed(); // just user picard - deleteAndputNewMapping(new Header[0],"rolesmapping_users_picard_single.json", true); + deleteAndPutNewMapping(new Header[0],"rolesmapping_users_picard_single.json", true); checkAllSfAllowed(); // hosts - deleteAndputNewMapping(new Header[0],"rolesmapping_hosts_list.json", true); + deleteAndPutNewMapping(new Header[0],"rolesmapping_hosts_list.json", true); checkAllSfAllowed(); // hosts - deleteAndputNewMapping(new Header[0],"rolesmapping_hosts_single.json", true); + deleteAndPutNewMapping(new Header[0],"rolesmapping_hosts_single.json", true); checkAllSfAllowed(); // full settings, access - deleteAndputNewMapping(new Header[0],"rolesmapping_all_access.json", true); + deleteAndPutNewMapping(new Header[0],"rolesmapping_all_access.json", true); checkAllSfAllowed(); // full settings, no access - deleteAndputNewMapping(new Header[0],"rolesmapping_all_noaccess.json", true); + deleteAndPutNewMapping(new Header[0],"rolesmapping_all_noaccess.json", true); checkAllSfForbidden(); } @@ -107,8 +107,8 @@ public void testRolesMappingApiWithFullPermissions() throws Exception { setupStarfleetIndex(); // add user picard, role captains initially maps to // opendistro_security_role_starfleet_captains and opendistro_security_role_starfleet - addUserWithPassword("picard", "picard", new String[] { "captains" }, HttpStatus.SC_CREATED); - checkWriteAccess(HttpStatus.SC_CREATED, "picard", "picard", "sf", "_doc", 1); + addUserWithPassword("picard", "picardpicardpicard", new String[] { "captains" }, HttpStatus.SC_CREATED); + checkWriteAccess(HttpStatus.SC_CREATED, "picard", "picardpicardpicard", "sf", "_doc", 1); // TODO: only one doctype allowed for ES6 //checkWriteAccess(HttpStatus.SC_CREATED, "picard", "picard", "sf", "_doc", 1); @@ -117,35 +117,35 @@ public void testRolesMappingApiWithFullPermissions() throws Exception { verifyPutForSuperAdmin(new Header[]{restApiAdminHeader}); verifyPatchForSuperAdmin(new Header[]{restApiAdminHeader}); // mapping with several backend roles, one of the is captain - deleteAndputNewMapping(new Header[]{restApiAdminHeader}, "rolesmapping_backendroles_captains_list.json", false); + deleteAndPutNewMapping(new Header[]{restApiAdminHeader}, "rolesmapping_backendroles_captains_list.json", false); checkAllSfAllowed(); // mapping with one backend role, captain - deleteAndputNewMapping(new Header[]{restApiAdminHeader},"rolesmapping_backendroles_captains_single.json", true); + deleteAndPutNewMapping(new Header[]{restApiAdminHeader},"rolesmapping_backendroles_captains_single.json", true); checkAllSfAllowed(); // mapping with several users, one is picard - deleteAndputNewMapping(new Header[]{restApiAdminHeader},"rolesmapping_users_picard_list.json", true); + deleteAndPutNewMapping(new Header[]{restApiAdminHeader},"rolesmapping_users_picard_list.json", true); checkAllSfAllowed(); // just user picard - deleteAndputNewMapping(new Header[]{restApiAdminHeader},"rolesmapping_users_picard_single.json", true); + deleteAndPutNewMapping(new Header[]{restApiAdminHeader},"rolesmapping_users_picard_single.json", true); checkAllSfAllowed(); // hosts - deleteAndputNewMapping(new Header[]{restApiAdminHeader},"rolesmapping_hosts_list.json", true); + deleteAndPutNewMapping(new Header[]{restApiAdminHeader},"rolesmapping_hosts_list.json", true); checkAllSfAllowed(); // hosts - deleteAndputNewMapping(new Header[]{restApiAdminHeader},"rolesmapping_hosts_single.json", true); + deleteAndPutNewMapping(new Header[]{restApiAdminHeader},"rolesmapping_hosts_single.json", true); checkAllSfAllowed(); // full settings, access - deleteAndputNewMapping(new Header[]{restApiAdminHeader},"rolesmapping_all_access.json", true); + deleteAndPutNewMapping(new Header[]{restApiAdminHeader},"rolesmapping_all_access.json", true); checkAllSfAllowed(); // full settings, no access - deleteAndputNewMapping(new Header[]{restApiAdminHeader},"rolesmapping_all_noaccess.json", true); + deleteAndPutNewMapping(new Header[]{restApiAdminHeader},"rolesmapping_all_noaccess.json", true); checkAllSfForbidden(); } @@ -221,7 +221,7 @@ void verifyDeleteForSuperAdmin(final Header[] header, final boolean useAdminCert // now picard is only in opendistro_security_role_starfleet, which has write access to // public, but not to _doc - checkWriteAccess(HttpStatus.SC_FORBIDDEN, "picard", "picard", "sf", "_doc", 1); + checkWriteAccess(HttpStatus.SC_FORBIDDEN, "picard", "picardpicardpicard", "sf", "_doc", 1); // TODO: only one doctype allowed for ES6 // checkWriteAccess(HttpStatus.SC_OK, "picard", "picard", "sf", "_doc", 1); @@ -382,17 +382,17 @@ void verifyPatchForSuperAdmin(final Header[] header) throws Exception { private void checkAllSfAllowed() throws Exception { rh.sendAdminCertificate = false; - checkReadAccess(HttpStatus.SC_OK, "picard", "picard", "sf", "_doc", 1); - checkWriteAccess(HttpStatus.SC_OK, "picard", "picard", "sf", "_doc", 1); + checkReadAccess(HttpStatus.SC_OK, "picard", "picardpicardpicard", "sf", "_doc", 1); + checkWriteAccess(HttpStatus.SC_OK, "picard", "picardpicardpicard", "sf", "_doc", 1); } private void checkAllSfForbidden() throws Exception { rh.sendAdminCertificate = false; - checkReadAccess(HttpStatus.SC_FORBIDDEN, "picard", "picard", "sf", "_doc", 1); - checkWriteAccess(HttpStatus.SC_FORBIDDEN, "picard", "picard", "sf", "_doc", 1); + checkReadAccess(HttpStatus.SC_FORBIDDEN, "picard", "picardpicardpicard", "sf", "_doc", 1); + checkWriteAccess(HttpStatus.SC_FORBIDDEN, "picard", "picardpicardpicard", "sf", "_doc", 1); } - private HttpResponse deleteAndputNewMapping(final Header[] header, final String fileName, final boolean useAdminCert) throws Exception { + private HttpResponse deleteAndPutNewMapping(final Header[] header, final String fileName, final boolean useAdminCert) throws Exception { rh.sendAdminCertificate = useAdminCert; HttpResponse response = rh.executeDeleteRequest(ENDPOINT + "/rolesmapping/opendistro_security_role_starfleet_captains", header); diff --git a/src/test/java/org/opensearch/security/dlic/rest/api/UserApiTest.java b/src/test/java/org/opensearch/security/dlic/rest/api/UserApiTest.java index 60c2ccde6f..abcda9a69c 100644 --- a/src/test/java/org/opensearch/security/dlic/rest/api/UserApiTest.java +++ b/src/test/java/org/opensearch/security/dlic/rest/api/UserApiTest.java @@ -24,6 +24,7 @@ import org.opensearch.common.settings.Settings; import org.opensearch.common.xcontent.XContentType; import org.opensearch.security.dlic.rest.validation.AbstractConfigurationValidator; +import org.opensearch.security.dlic.rest.validation.PasswordValidator; import org.opensearch.security.securityconf.impl.CType; import org.opensearch.security.support.ConfigConstants; import org.opensearch.security.test.helper.file.FileHelper; @@ -89,15 +90,22 @@ public void testSecurityRoles() throws Exception { .executeGetRequest(ENDPOINT + "/" + CType.INTERNALUSERS.toLCString()); Assert.assertEquals(response.getBody(), HttpStatus.SC_OK, response.getStatusCode()); Settings settings = Settings.builder().loadFromSource(response.getBody(), XContentType.JSON).build(); - Assert.assertEquals(USER_SETTING_SIZE, settings.size()); - response = rh.executePatchRequest(ENDPOINT + "/internalusers", "[{ \"op\": \"add\", \"path\": \"/newuser\", \"value\": {\"password\": \"newuser\", \"opendistro_security_roles\": [\"opendistro_security_all_access\"] } }]", new Header[0]); + Assert.assertEquals(133, settings.size()); + response = rh.executePatchRequest( + ENDPOINT + "/internalusers", + "[{ \"op\": \"add\", \"path\": \"/newuser\", " + + "\"value\": {\"password\": \"fair password for the user\", " + + "\"opendistro_security_roles\": [\"opendistro_security_all_access\"] } }]", + new Header[0] + ); Assert.assertEquals(response.getBody(), HttpStatus.SC_OK, response.getStatusCode()); response = rh.executeGetRequest(ENDPOINT + "/internalusers/newuser", new Header[0]); Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); Assert.assertTrue(response.getBody().contains("\"opendistro_security_roles\":[\"opendistro_security_all_access\"]")); - checkGeneralAccess(HttpStatus.SC_OK, "newuser", "newuser"); + checkGeneralAccess(HttpStatus.SC_OK, "newuser", "fair password for the user"); + } @Test @@ -108,7 +116,8 @@ public void testParallelPutRequests() throws Exception { rh.keystore = "restapi/kirk-keystore.jks"; rh.sendAdminCertificate = true; - HttpResponse[] responses = rh.executeMultipleAsyncPutRequest(10, ENDPOINT + "/internalusers/test1", "{\"password\":\"test1\"}"); + HttpResponse[] responses = rh.executeMultipleAsyncPutRequest(10, + ENDPOINT + "/internalusers/test1", "{\"password\":\"test1test1test1test1test1test1\"}"); boolean created = false; for (HttpResponse response : responses) { int sc = response.getStatusCode(); @@ -254,7 +263,7 @@ private void verifyPatch(final boolean sendAdminCert, Header... restAdminHeader) // PATCH password rh.sendAdminCertificate = sendAdminCert; - response = rh.executePatchRequest(ENDPOINT + "/internalusers/test", "[{ \"op\": \"add\", \"path\": \"/password\", \"value\": \"neu\" }]", restAdminHeader); + response = rh.executePatchRequest(ENDPOINT + "/internalusers/test", "[{ \"op\": \"add\", \"path\": \"/password\", \"value\": \"neu password 42\" }]", restAdminHeader); Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); response = rh.executeGetRequest(ENDPOINT + "/internalusers/test", restAdminHeader); Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); @@ -292,7 +301,7 @@ private void verifyPatch(final boolean sendAdminCert, Header... restAdminHeader) // PATCH rh.sendAdminCertificate = sendAdminCert; response = rh.executePatchRequest(ENDPOINT + "/internalusers", - "[{ \"op\": \"add\", \"path\": \"/bulknew1\", \"value\": {\"password\": \"bla\", \"backend_roles\": [\"vulcan\"] } }]", restAdminHeader); + "[{ \"op\": \"add\", \"path\": \"/bulknew1\", \"value\": {\"password\": \"bla bla bla password 42\", \"backend_roles\": [\"vulcan\"] } }]", restAdminHeader); Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); response = rh.executeGetRequest(ENDPOINT + "/internalusers/bulknew1", restAdminHeader); Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); @@ -382,11 +391,11 @@ private void verifyPatch(final boolean sendAdminCert, Header... restAdminHeader) // use password instead of hash rh.sendAdminCertificate = sendAdminCert; - addUserWithPassword("nagilum", "correctpassword", HttpStatus.SC_CREATED); + addUserWithPassword("nagilum", "correct password 42", HttpStatus.SC_CREATED); rh.sendAdminCertificate = false; checkGeneralAccess(HttpStatus.SC_UNAUTHORIZED, "nagilum", "wrongpassword"); - checkGeneralAccess(HttpStatus.SC_OK, "nagilum", "correctpassword"); + checkGeneralAccess(HttpStatus.SC_OK, "nagilum", "correct password 42"); deleteUser("nagilum"); @@ -490,24 +499,24 @@ private void verifyRoles(final boolean sendAdminCert, Header... header) throws E // use backendroles when creating user. User picard does not exist in // the internal user DB // and is also not assigned to any role by username - addUserWithPassword("picard", "picard", HttpStatus.SC_CREATED); + addUserWithPassword("picard", "picardpicardpicardpicardpicard", HttpStatus.SC_CREATED); // changed in ES5, you now need cluster:monitor/main which pucard does not have - checkGeneralAccess(HttpStatus.SC_FORBIDDEN, "picard", "picard"); + checkGeneralAccess(HttpStatus.SC_FORBIDDEN, "picard", "picardpicardpicardpicardpicard"); // check read access to starfleet index and _doc type, must fail - checkReadAccess(HttpStatus.SC_FORBIDDEN, "picard", "picard", "sf", "_doc", 0); + checkReadAccess(HttpStatus.SC_FORBIDDEN, "picard", "picardpicardpicardpicardpicard", "sf", "_doc", 0); // overwrite user picard, and give him role "starfleet". - addUserWithPassword("picard", "picard", new String[]{"starfleet"}, HttpStatus.SC_OK); + addUserWithPassword("picard", "picardpicardpicardpicardpicard", new String[]{"starfleet"}, HttpStatus.SC_OK); - checkReadAccess(HttpStatus.SC_OK, "picard", "picard", "sf", "_doc", 0); - checkWriteAccess(HttpStatus.SC_FORBIDDEN, "picard", "picard", "sf", "_doc", 1); + checkReadAccess(HttpStatus.SC_OK, "picard", "picardpicardpicardpicardpicard", "sf", "_doc", 0); + checkWriteAccess(HttpStatus.SC_FORBIDDEN, "picard", "picardpicardpicardpicardpicard", "sf", "_doc", 1); // overwrite user picard, and give him role "starfleet" plus "captains. Now // document can be created. - addUserWithPassword("picard", "picard", new String[]{"starfleet", "captains"}, HttpStatus.SC_OK); - checkReadAccess(HttpStatus.SC_OK, "picard", "picard", "sf", "_doc", 0); - checkWriteAccess(HttpStatus.SC_CREATED, "picard", "picard", "sf", "_doc", 1); + addUserWithPassword("picard", "picardpicardpicardpicardpicard", new String[]{"starfleet", "captains"}, HttpStatus.SC_OK); + checkReadAccess(HttpStatus.SC_OK, "picard", "picardpicardpicardpicardpicard", "sf", "_doc", 0); + checkWriteAccess(HttpStatus.SC_CREATED, "picard", "picardpicardpicardpicardpicard", "sf", "_doc", 1); rh.sendAdminCertificate = sendAdminCert; response = rh.executeGetRequest(ENDPOINT + "/internalusers/picard", header); @@ -520,8 +529,8 @@ private void verifyRoles(final boolean sendAdminCert, Header... header) throws E Assert.assertTrue(roles.contains("starfleet")); Assert.assertTrue(roles.contains("captains")); - addUserWithPassword("$1aAAAAAAAAC", "$1aAAAAAAAAC", HttpStatus.SC_CREATED); - addUserWithPassword("abc", "abc", HttpStatus.SC_CREATED); + addUserWithPassword("some_additional_user", "$1aAAAAAAAAC", HttpStatus.SC_CREATED); + addUserWithPassword("abc", "abcabcabcabc42", HttpStatus.SC_CREATED); // check tabs in json response = rh.executePutRequest(ENDPOINT + "/internalusers/userwithtabs", "\t{\"hash\": \t \"123\"\t} ", header); @@ -565,13 +574,15 @@ public void testUserApiWithRestInternalUsersAdminPermissions() throws Exception } @Test - public void testPasswordRules() throws Exception { + public void testRegExpPasswordRules() throws Exception { Settings nodeSettings = Settings.builder() .put(ConfigConstants.SECURITY_RESTAPI_PASSWORD_VALIDATION_ERROR_MESSAGE, "xxx") .put(ConfigConstants.SECURITY_RESTAPI_PASSWORD_VALIDATION_REGEX, "(?=.*[A-Z])(?=.*[^a-zA-Z\\\\d])(?=.*[0-9])(?=.*[a-z]).{8,}") + .put(ConfigConstants.SECURITY_RESTAPI_PASSWORD_SCORE_BASED_VALIDATION_STRENGTH, + PasswordValidator.ScoreStrength.FAIR.name()) .build(); setup(nodeSettings); @@ -582,73 +593,137 @@ public void testPasswordRules() throws Exception { // initial configuration, 6 users HttpResponse response = rh .executeGetRequest("_plugins/_security/api/" + CType.INTERNALUSERS.toLCString()); - Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); + Assert.assertEquals(response.getBody(), HttpStatus.SC_OK, response.getStatusCode()); Settings settings = Settings.builder().loadFromSource(response.getBody(), XContentType.JSON).build(); Assert.assertEquals(USER_SETTING_SIZE, settings.size()); - addUserWithPassword("tooshoort", "", HttpStatus.SC_BAD_REQUEST); - addUserWithPassword("tooshoort", "123", HttpStatus.SC_BAD_REQUEST); - addUserWithPassword("tooshoort", "1234567", HttpStatus.SC_BAD_REQUEST); - addUserWithPassword("tooshoort", "1Aa%", HttpStatus.SC_BAD_REQUEST); - addUserWithPassword("no-nonnumeric", "123456789", HttpStatus.SC_BAD_REQUEST); - addUserWithPassword("no-uppercase", "a123456789", HttpStatus.SC_BAD_REQUEST); - addUserWithPassword("no-lowercase", "A123456789", HttpStatus.SC_BAD_REQUEST); - addUserWithPassword("ok1", "a%A123456789", HttpStatus.SC_CREATED); - addUserWithPassword("ok2", "$aA123456789", HttpStatus.SC_CREATED); - addUserWithPassword("ok3", "$Aa123456789", HttpStatus.SC_CREATED); - addUserWithPassword("ok4", "$1aAAAAAAAAA", HttpStatus.SC_CREATED); - addUserWithPassword("empty_password_no_hash", "", HttpStatus.SC_BAD_REQUEST); + verifyCouldNotCreatePasswords(HttpStatus.SC_BAD_REQUEST); + verifyCanCreatePasswords(); + verifySimilarity("xxx"); + addUserWithPasswordAndHash("empty_password", "", "$%^123", HttpStatus.SC_BAD_REQUEST); addUserWithPasswordAndHash("null_password", null, "$%^123", HttpStatus.SC_BAD_REQUEST); - response = rh.executePatchRequest(PLUGINS_PREFIX + "/api/internalusers", "[{ \"op\": \"add\", \"path\": \"/ok4\", \"value\": {\"password\": \"bla\", \"backend_roles\": [\"vulcan\"] } }]", new Header[0]); - Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode()); + response = rh.executeGetRequest(PLUGINS_PREFIX + "/api/internalusers/nothinghthere?pretty", new Header[0]); + Assert.assertEquals(HttpStatus.SC_NOT_FOUND, response.getStatusCode()); + Assert.assertTrue(response.getBody().contains("NOT_FOUND")); + } + private void verifyCouldNotCreatePasswords(final int expectedStatus) throws Exception { + addUserWithPassword("tooshoort", "", expectedStatus); + addUserWithPassword("tooshoort", "123",expectedStatus); + addUserWithPassword("tooshoort", "1234567", expectedStatus); + addUserWithPassword("tooshoort", "1Aa%", expectedStatus); + addUserWithPassword("no-nonnumeric", "123456789", expectedStatus); + addUserWithPassword("no-uppercase", "a123456789", expectedStatus); + addUserWithPassword("no-lowercase", "A123456789", expectedStatus); + addUserWithPassword("empty_password_no_hash", "", expectedStatus); + HttpResponse response = rh.executePatchRequest( + PLUGINS_PREFIX + "/api/internalusers", + "[{ \"op\": \"add\", \"path\": \"/ok4\", \"value\": {\"password\": \"bla\", \"backend_roles\": [\"vulcan\"] } }]", + new Header[0] + ); + Assert.assertEquals(response.getBody(), expectedStatus, response.getStatusCode()); response = rh.executePatchRequest(PLUGINS_PREFIX + "/api/internalusers", "[{ \"op\": \"replace\", \"path\": \"/ok4\", \"value\": {\"password\": \"bla\", \"backend_roles\": [\"vulcan\"] } }]", new Header[0]); - Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode()); - - addUserWithPassword("ok4", "123", HttpStatus.SC_BAD_REQUEST); - - response = rh.executePatchRequest(PLUGINS_PREFIX + "/api/internalusers", "[{ \"op\": \"add\", \"path\": \"/ok4\", \"value\": {\"password\": \"$1aAAAAAAAAB\", \"backend_roles\": [\"vulcan\"] } }]", new Header[0]); - Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); - - addUserWithPassword("ok4", "$1aAAAAAAAAC", HttpStatus.SC_OK); + Assert.assertEquals(response.getBody(), expectedStatus, response.getStatusCode()); + addUserWithPassword("ok4", "123", expectedStatus); //its not allowed to use the username as password (case insensitive) response = rh.executePatchRequest(PLUGINS_PREFIX + "/api/internalusers", "[{ \"op\": \"add\", \"path\": \"/$1aAAAAAAAAB\", \"value\": {\"password\": \"$1aAAAAAAAAB\", \"backend_roles\": [\"vulcan\"] } }]", new Header[0]); - Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode()); - addUserWithPassword("$1aAAAAAAAAC", "$1aAAAAAAAAC", HttpStatus.SC_BAD_REQUEST); - addUserWithPassword("$1aAAAAAAAac", "$1aAAAAAAAAC", HttpStatus.SC_BAD_REQUEST); - addUserWithPassword(URLEncoder.encode("$1aAAAAAAAac%", "UTF-8"), "$1aAAAAAAAAC%", HttpStatus.SC_BAD_REQUEST); - addUserWithPassword(URLEncoder.encode("$1aAAAAAAAac%!=\"/\\;:test&~@^", "UTF-8").replace("+", "%2B"), "$1aAAAAAAAac%!=\\\"/\\\\;:test&~@^", HttpStatus.SC_BAD_REQUEST); - addUserWithPassword(URLEncoder.encode("$1aAAAAAAAac%!=\"/\\;: test&", "UTF-8"), "$1aAAAAAAAac%!=\\\"/\\\\;: test&123", HttpStatus.SC_BAD_REQUEST); - - response = rh.executeGetRequest(PLUGINS_PREFIX + "/api/internalusers/nothinghthere?pretty", new Header[0]); - Assert.assertEquals(HttpStatus.SC_NOT_FOUND, response.getStatusCode()); - Assert.assertTrue(response.getBody().contains("NOT_FOUND")); - + Assert.assertEquals(response.getBody(), expectedStatus, response.getStatusCode()); + addUserWithPassword("$1aAAAAAAAAC", "$1aAAAAAAAAC", expectedStatus); + addUserWithPassword("$1aAAAAAAAac", "$1aAAAAAAAAC", expectedStatus); + addUserWithPassword(URLEncoder.encode("$1aAAAAAAAac%", "UTF-8"), "$1aAAAAAAAAC%", expectedStatus); + addUserWithPassword(URLEncoder.encode("$1aAAAAAAAac%!=\"/\\;:test&~@^", "UTF-8").replace("+", "%2B"), "$1aAAAAAAAac%!=\\\"/\\\\;:test&~@^", expectedStatus); + addUserWithPassword(URLEncoder.encode("$1aAAAAAAAac%!=\"/\\;: test&", "UTF-8"), "$1aAAAAAAAac%!=\\\"/\\\\;: test&123", expectedStatus); String patchPayload = "[ " + "{ \"op\": \"add\", \"path\": \"/testuser1\", \"value\": { \"password\": \"$aA123456789\", \"backend_roles\": [\"testrole1\"] } }," + "{ \"op\": \"add\", \"path\": \"/testuser2\", \"value\": { \"password\": \"testpassword2\", \"backend_roles\": [\"testrole2\"] } }" + "]"; response = rh.executePatchRequest(PLUGINS_PREFIX + "/api/internalusers", patchPayload, new BasicHeader("Content-Type", "application/json")); - Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode()); + Assert.assertEquals(expectedStatus, response.getStatusCode()); Assert.assertTrue(response.getBody().contains("error")); Assert.assertTrue(response.getBody().contains("xxx")); response = rh.executePutRequest(PLUGINS_PREFIX + "/api/internalusers/ok1", "{\"backend_roles\":[\"my-backend-role\"],\"attributes\":{},\"password\":\"\"}", new Header[0]); - Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode()); + Assert.assertEquals(expectedStatus, response.getStatusCode()); + + response = rh.executePutRequest(PLUGINS_PREFIX + "/api/internalusers/ok1", + "{\"backend_roles\":[\"my-backend-role\"],\"attributes\":{},\"password\":\"bla\"}", + new Header[0]); + Assert.assertEquals(expectedStatus, response.getStatusCode()); + } + private void verifyCanCreatePasswords() throws Exception { + addUserWithPassword("ok1", "a%A123456789", HttpStatus.SC_CREATED); + addUserWithPassword("ok2", "$aA123456789", HttpStatus.SC_CREATED); + addUserWithPassword("ok3", "$Aa123456789", HttpStatus.SC_CREATED); + addUserWithPassword("ok4", "$1aAAAAAAAAA", HttpStatus.SC_CREATED); + addUserWithPassword("ok4", "$1aAAAAAAAAC", HttpStatus.SC_OK); + HttpResponse response = rh.executePatchRequest( + PLUGINS_PREFIX + "/api/internalusers", + "[{ \"op\": \"add\", \"path\": \"/ok4\", \"value\": {\"password\": \"$1aAAAAAAAAB\", \"backend_roles\": [\"vulcan\"] } }]", + new Header[0] + ); + Assert.assertEquals(response.getBody(), HttpStatus.SC_OK, response.getStatusCode()); response = rh.executePutRequest(PLUGINS_PREFIX + "/api/internalusers/ok1", "{\"backend_roles\":[\"my-backend-role\"],\"attributes\":{},\"password\":\"Admin_123\"}", new Header[0]); Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); response = rh.executePutRequest(PLUGINS_PREFIX + "/api/internalusers/ok1", "{\"backend_roles\":[\"my-backend-role\"],\"attributes\":{}}", new Header[0]); Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); - response = rh.executePutRequest(PLUGINS_PREFIX + "/api/internalusers/ok1", "{\"backend_roles\":[\"my-backend-role\"],\"attributes\":{},\"password\":\"bla\"}", - new Header[0]); - Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode()); + } + + + private void verifySimilarity(final String expectedMessage) throws Exception { + addUserWithPassword( + "some_user_name", "H3235,cc,some_User_Name", + HttpStatus.SC_BAD_REQUEST, + expectedMessage + ); + } + + @Test + public void testScoreBasedPasswordRules() throws Exception { + + Settings nodeSettings = + Settings.builder() + .put(ConfigConstants.SECURITY_RESTAPI_PASSWORD_MIN_LENGTH, 9) + .build(); + + setup(nodeSettings); + + rh.keystore = "restapi/kirk-keystore.jks"; + rh.sendAdminCertificate = true; + + // initial configuration, 6 users + HttpResponse response = rh + .executeGetRequest("_plugins/_security/api/" + CType.INTERNALUSERS.toLCString()); + Assert.assertEquals(response.getBody(), HttpStatus.SC_OK, response.getStatusCode()); + Settings settings = Settings.builder().loadFromSource(response.getBody(), XContentType.JSON).build(); + Assert.assertEquals(133, settings.size()); + + addUserWithPassword( + "admin", "password89", + HttpStatus.SC_BAD_REQUEST, + AbstractConfigurationValidator.ErrorType.WEAK_PASSWORD.getMessage() + ); + addUserWithPassword( + "admin", "A123456789", + HttpStatus.SC_BAD_REQUEST, + AbstractConfigurationValidator.ErrorType.WEAK_PASSWORD.getMessage() + ); + + addUserWithPassword( + "admin", "pas", + HttpStatus.SC_BAD_REQUEST, + "Password does not match minimum criteria" + ); + + verifySimilarity(AbstractConfigurationValidator.ErrorType.SIMILAR_PASSWORD.getMessage()); + + addUserWithPassword("some_user_name", "ASSDsadwe324wadaasdadqwe", HttpStatus.SC_CREATED); } @Test @@ -669,13 +744,13 @@ public void testUserApiWithDots() throws Exception { addUserWithPassword(".my.dotuser0", "$2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m", HttpStatus.SC_CREATED); - addUserWithPassword(".my.dot.user0", "12345678", + addUserWithPassword(".my.dot.user0", "12345678Sd", HttpStatus.SC_CREATED); addUserWithHash(".my.dotuser1", "$2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m", HttpStatus.SC_CREATED); - addUserWithPassword(".my.dot.user2", "12345678", + addUserWithPassword(".my.dot.user2", "12345678Sd", HttpStatus.SC_CREATED); } @@ -697,7 +772,7 @@ public void testUserApiNoPasswordChange() throws Exception { response = rh.executePutRequest(ENDPOINT + "/internalusers/user1", "{\"hash\":\"$2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m\",\"password\":\"\",\"backend_roles\":[\"admin\",\"rolea\"]}"); Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode()); - response = rh.executePutRequest(ENDPOINT + "/internalusers/user1", "{\"hash\":\"$2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m\",\"password\":\"Admin_123\",\"backend_roles\":[\"admin\",\"rolea\"]}"); + response = rh.executePutRequest(ENDPOINT + "/internalusers/user1", "{\"hash\":\"$2a$12$n5nubfWATfQjSYHiWtUyeOxMIxFInUHOAx8VMmGmxFNPGpaBmeB.m\",\"password\":\"Admin_123345Yq\",\"backend_roles\":[\"admin\",\"rolea\"]}"); Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); response = rh.executeGetRequest(ENDPOINT + "/internalusers/user1"); @@ -709,7 +784,7 @@ public void testUserApiNoPasswordChange() throws Exception { response = rh.executePutRequest(ENDPOINT + "/internalusers/user2", "{\"password\":\"\",\"backend_roles\":[\"admin\",\"rolex\"]}"); Assert.assertEquals(HttpStatus.SC_BAD_REQUEST, response.getStatusCode()); - response = rh.executePutRequest(ENDPOINT + "/internalusers/user2", "{\"password\":\"Admin_123\",\"backend_roles\":[\"admin\",\"rolex\"]}"); + response = rh.executePutRequest(ENDPOINT + "/internalusers/user2", "{\"password\":\"Admin_123Qerty\",\"backend_roles\":[\"admin\",\"rolex\"]}"); Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); response = rh.executeGetRequest(ENDPOINT + "/internalusers/user2"); diff --git a/src/test/java/org/opensearch/security/dlic/rest/validation/PasswordValidatorTest.java b/src/test/java/org/opensearch/security/dlic/rest/validation/PasswordValidatorTest.java new file mode 100644 index 0000000000..b5d27827b8 --- /dev/null +++ b/src/test/java/org/opensearch/security/dlic/rest/validation/PasswordValidatorTest.java @@ -0,0 +1,197 @@ +/* + * 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.dlic.rest.validation; + +import java.util.List; + +import com.google.common.collect.ImmutableList; +import org.junit.Test; + +import org.opensearch.common.settings.Settings; + +import static org.junit.Assert.assertEquals; +import static org.opensearch.security.support.ConfigConstants.SECURITY_RESTAPI_PASSWORD_MIN_LENGTH; +import static org.opensearch.security.support.ConfigConstants.SECURITY_RESTAPI_PASSWORD_SCORE_BASED_VALIDATION_STRENGTH; +import static org.opensearch.security.support.ConfigConstants.SECURITY_RESTAPI_PASSWORD_VALIDATION_REGEX; + +public class PasswordValidatorTest { + + static final List WEAK_PASSWORDS = ImmutableList.of( + "q", "5", "&", "admin", "123456", "password" + ); + + static final List FAIR_PASSWORDS = ImmutableList.of( + "p@$$word@dmin", "qwertyuiop@[", + "zxcvbnm,./_", "asdfghjkl;:]", "20300101", + "pandapandapandapandapandapandapandapandapandaa", + "appleappleappleappleappleappleappleappleapplea", + "aelppaaelppaaelppaaelppaaelppaaelppaaelppaaelppa" + ); + + static final List GOOD_PASSWORDS = ImmutableList.of( + "xsw234rfvb", "yaq123edc", "cde345tgbn", "yaqwedcvb", + "Tr0ub4dour&3", "qwER43@!" + ); + + static final List STRONG_PASSWORDS = ImmutableList.of( + "YWert,H90", "Admincc,H90", "Hadmin,120" + ); + + static final List VERY_STRONG_PASSWORDS = ImmutableList.of( + "AeTq($%u-44c_j9NJB45a#2#JP7sH", "IB7~EOw!51gug+7s#+%A9P1O/w8f", + "1v_f%7JvS8w!_t398+ON-CObI#v0", "8lFmfc0!w)&iU9DM6~4_w)D)Y44J" + ); + + static final List SIMILAR_PASSWORDS = ImmutableList.of( + "some_user_name,H2344cc", "H3235,Some_User_Name,cc", + "H3235,cc,some_User_Name", "H3235,SOME_User_Name,cc", + "H3235,eman_resu_emos,cc" + ); + + public void verifyWeakPasswords(final PasswordValidator passwordValidator, + final AbstractConfigurationValidator.ErrorType expectedValidationResult) { + for (final String password : WEAK_PASSWORDS) + assertEquals( + password, + expectedValidationResult, + passwordValidator.validate("some_user_name", password) + ); + + } + + public void verifyFairPasswords(final PasswordValidator passwordValidator, + final AbstractConfigurationValidator.ErrorType expectedValidationResult) { + for (final String password : FAIR_PASSWORDS) + assertEquals( + password, + expectedValidationResult, + passwordValidator.validate("some_user_name", password) + ); + + } + + public void verifyGoodPasswords(final PasswordValidator passwordValidator, + final AbstractConfigurationValidator.ErrorType expectedValidationResult) { + for (final String password : GOOD_PASSWORDS) + assertEquals( + password, + expectedValidationResult, + passwordValidator.validate("some_user_name", password) + ); + + } + + public void verifyStrongPasswords(final PasswordValidator passwordValidator, + final AbstractConfigurationValidator.ErrorType expectedValidationResult) { + for (final String password : STRONG_PASSWORDS) + assertEquals( + password, + expectedValidationResult, + passwordValidator.validate("some_user_name", password) + ); + + } + + public void verifyVeryStrongPasswords(final PasswordValidator passwordValidator, + final AbstractConfigurationValidator.ErrorType expectedValidationResult) { + for (final String password : VERY_STRONG_PASSWORDS) + assertEquals( + password, + expectedValidationResult, + passwordValidator.validate("some_user_name", password) + ); + + } + + public void verifySimilarPasswords(final PasswordValidator passwordValidator) { + for (final String password : SIMILAR_PASSWORDS) + assertEquals( + password, + AbstractConfigurationValidator.ErrorType.SIMILAR_PASSWORD, + passwordValidator.validate("some_user_name", password) + ); + + } + + @Test + public void testRegExpBasedValidation() { + final PasswordValidator passwordValidator = + PasswordValidator.of( + Settings.builder() + .put( + SECURITY_RESTAPI_PASSWORD_VALIDATION_REGEX, + "(?=.*[A-Z])(?=.*[^a-zA-Z\\\\d])(?=.*[0-9])(?=.*[a-z]).{8,}") + .build() + ); + verifyWeakPasswords(passwordValidator, AbstractConfigurationValidator.ErrorType.INVALID_PASSWORD); + verifyFairPasswords(passwordValidator, AbstractConfigurationValidator.ErrorType.INVALID_PASSWORD); + for (final String password : GOOD_PASSWORDS.subList(0, GOOD_PASSWORDS.size() - 2)) + assertEquals( + password, + AbstractConfigurationValidator.ErrorType.INVALID_PASSWORD, + passwordValidator.validate("some_user_name", password) + ); + for (final String password: GOOD_PASSWORDS.subList(GOOD_PASSWORDS.size() - 2, GOOD_PASSWORDS.size())) + assertEquals( + password, + AbstractConfigurationValidator.ErrorType.WEAK_PASSWORD, + passwordValidator.validate("some_user_name", password) + ); + verifyStrongPasswords(passwordValidator, AbstractConfigurationValidator.ErrorType.NONE); + verifyVeryStrongPasswords(passwordValidator, AbstractConfigurationValidator.ErrorType.NONE); + verifySimilarPasswords(passwordValidator); + } + + @Test + public void testMinLength() { + final PasswordValidator passwordValidator = + PasswordValidator.of( + Settings.builder() + .put(SECURITY_RESTAPI_PASSWORD_MIN_LENGTH, 15) + .build() + ); + for (final String password: STRONG_PASSWORDS) { + assertEquals( + AbstractConfigurationValidator.ErrorType.INVALID_PASSWORD, + passwordValidator.validate(password, "some_user_name") + ); + } + + } + + @Test + public void testScoreBasedValidation() { + PasswordValidator passwordValidator = PasswordValidator.of(Settings.EMPTY); + verifyWeakPasswords(passwordValidator, AbstractConfigurationValidator.ErrorType.WEAK_PASSWORD); + verifyFairPasswords(passwordValidator, AbstractConfigurationValidator.ErrorType.WEAK_PASSWORD); + verifyGoodPasswords(passwordValidator, AbstractConfigurationValidator.ErrorType.WEAK_PASSWORD); + verifyStrongPasswords(passwordValidator, AbstractConfigurationValidator.ErrorType.NONE); + verifyVeryStrongPasswords(passwordValidator, AbstractConfigurationValidator.ErrorType.NONE); + verifySimilarPasswords(passwordValidator); + + passwordValidator = + PasswordValidator.of( + Settings.builder() + .put( + SECURITY_RESTAPI_PASSWORD_SCORE_BASED_VALIDATION_STRENGTH, + PasswordValidator.ScoreStrength.FAIR.name() + ).build()); + + verifyWeakPasswords(passwordValidator, AbstractConfigurationValidator.ErrorType.WEAK_PASSWORD); + verifyFairPasswords(passwordValidator, AbstractConfigurationValidator.ErrorType.NONE); + verifyGoodPasswords(passwordValidator, AbstractConfigurationValidator.ErrorType.NONE); + verifyStrongPasswords(passwordValidator, AbstractConfigurationValidator.ErrorType.NONE); + verifyVeryStrongPasswords(passwordValidator, AbstractConfigurationValidator.ErrorType.NONE); + verifySimilarPasswords(passwordValidator); + } + +}