diff --git a/CHANGELOG.md b/CHANGELOG.md index 9215f0874d..f7c9fa2253 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ## [Unreleased 3.x] ### Added - Introduced new experimental versioned security configuration management feature ([#5357] (https://github.com/opensearch-project/security/pull/5357)) - +- Create a mechanism for plugins to explicitly declare actions they need to perform with their assigned PluginSubject ([#5341](https://github.com/opensearch-project/security/pull/5341)) ### Changed diff --git a/check-permissions-order.js b/check-permissions-order.js index 934694fc73..6b18a98ec8 100644 --- a/check-permissions-order.js +++ b/check-permissions-order.js @@ -15,7 +15,7 @@ const yaml = require('yaml') function checkPermissionsOrder(file, fix = false) { const contents = fs.readFileSync(file, 'utf8') const doc = yaml.parseDocument(contents, { keepCstNodes: true }) - const roles = doc.contents.items + const roles = doc.contents?.items || [] let requiresChanges = false roles.forEach(role => { const itemsFromRole = role?.value?.items; @@ -73,14 +73,14 @@ if (args.length === 0) { } const filePath = args[0] const fix = args.includes('--fix') -const slient = args.includes('--slient') +const silent = args.includes('--silent') if (checkPermissionsOrder(filePath, fix)) { if (fix) { - if (!slient) { console.log(`${filePath} has been updated.`) } + if (!silent) { console.log(`${filePath} has been updated.`) } } else { - if (!slient) { console.error(`Error: ${filePath} requires changes.`) } + if (!silent) { console.error(`Error: ${filePath} requires changes.`) } process.exit(1) } } else { - if (!slient) { console.log(`${filePath} is up-to-date.`) } + if (!silent) { console.log(`${filePath} is up-to-date.`) } } diff --git a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/secure/SecurePluginTests.java b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/secure/SecurePluginTests.java new file mode 100644 index 0000000000..c823ca0caa --- /dev/null +++ b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/secure/SecurePluginTests.java @@ -0,0 +1,96 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + * + */ +package org.opensearch.sample.secure; + +import java.util.List; +import java.util.Map; + +import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope; +import org.junit.ClassRule; +import org.junit.Test; +import org.junit.runner.RunWith; + +import org.opensearch.Version; +import org.opensearch.core.rest.RestStatus; +import org.opensearch.painless.PainlessModulePlugin; +import org.opensearch.plugins.PluginInfo; +import org.opensearch.sample.SampleResourcePlugin; +import org.opensearch.security.OpenSearchSecurityPlugin; +import org.opensearch.test.framework.TestSecurityConfig.AuthcDomain; +import org.opensearch.test.framework.cluster.ClusterManager; +import org.opensearch.test.framework.cluster.LocalCluster; +import org.opensearch.test.framework.cluster.TestRestClient; +import org.opensearch.test.framework.cluster.TestRestClient.HttpResponse; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.equalTo; +import static org.opensearch.sample.utils.Constants.SAMPLE_RESOURCE_PLUGIN_PREFIX; +import static org.opensearch.security.support.ConfigConstants.SECURITY_SYSTEM_INDICES_ENABLED_KEY; +import static org.opensearch.test.framework.TestSecurityConfig.User.USER_ADMIN; + +@RunWith(com.carrotsearch.randomizedtesting.RandomizedRunner.class) +@ThreadLeakScope(ThreadLeakScope.Scope.NONE) +public class SecurePluginTests { + + public static final AuthcDomain AUTHC_DOMAIN = new AuthcDomain("basic", 0).httpAuthenticatorWithChallenge("basic").backend("internal"); + + @ClassRule + public static final LocalCluster cluster = new LocalCluster.Builder().clusterManager(ClusterManager.SINGLENODE) + .anonymousAuth(false) + .authc(AUTHC_DOMAIN) + .users(USER_ADMIN) + .plugin(PainlessModulePlugin.class) + .plugin( + new PluginInfo( + SampleResourcePlugin.class.getName(), + "classpath plugin", + "NA", + Version.CURRENT, + "1.8", + SampleResourcePlugin.class.getName(), + null, + List.of(OpenSearchSecurityPlugin.class.getName()), + false + ) + ) + .nodeSettings(Map.of(SECURITY_SYSTEM_INDICES_ENABLED_KEY, true)) + .build(); + + @Test + public void testRunClusterHealthWithPluginSubject() { + try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) { + HttpResponse response = client.postJson(SAMPLE_RESOURCE_PLUGIN_PREFIX + "/run_action", """ + { + "action": "cluster:monitor/health" + } + """); + + assertThat(response.getStatusCode(), equalTo(RestStatus.OK.getStatus())); + assertThat(response.getBody(), containsString("number_of_nodes")); + } + } + + @Test + public void testRunCreateIndexWithPluginSubject() { + try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) { + HttpResponse response = client.postJson(SAMPLE_RESOURCE_PLUGIN_PREFIX + "/run_action", """ + { + "action": "indices:admin/create", + "index": "test-index" + } + """); + + assertThat(response.getStatusCode(), equalTo(RestStatus.OK.getStatus())); + HttpResponse catIndicesResponse = client.get("_cat/indices"); + assertThat(catIndicesResponse.getBody(), containsString("test-index")); + } + } +} diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/SampleResourcePlugin.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/SampleResourcePlugin.java index 338f8b9acf..b3d5f73eac 100644 --- a/sample-resource-plugin/src/main/java/org/opensearch/sample/SampleResourcePlugin.java +++ b/sample-resource-plugin/src/main/java/org/opensearch/sample/SampleResourcePlugin.java @@ -30,8 +30,10 @@ import org.opensearch.core.xcontent.NamedXContentRegistry; import org.opensearch.env.Environment; import org.opensearch.env.NodeEnvironment; +import org.opensearch.identity.PluginSubject; import org.opensearch.indices.SystemIndexDescriptor; import org.opensearch.plugins.ActionPlugin; +import org.opensearch.plugins.IdentityAwarePlugin; import org.opensearch.plugins.Plugin; import org.opensearch.plugins.SystemIndexPlugin; import org.opensearch.repositories.RepositoriesService; @@ -54,6 +56,10 @@ import org.opensearch.sample.resource.actions.transport.RevokeResourceAccessTransportAction; import org.opensearch.sample.resource.actions.transport.ShareResourceTransportAction; import org.opensearch.sample.resource.actions.transport.UpdateResourceTransportAction; +import org.opensearch.sample.secure.actions.rest.create.SecurePluginAction; +import org.opensearch.sample.secure.actions.rest.create.SecurePluginRestAction; +import org.opensearch.sample.secure.actions.transport.SecurePluginTransportAction; +import org.opensearch.sample.utils.RunAsSubjectClient; import org.opensearch.script.ScriptService; import org.opensearch.threadpool.ThreadPool; import org.opensearch.transport.client.Client; @@ -68,10 +74,12 @@ * It uses ".sample_resource_sharing_plugin" index to manage its resources, and exposes few REST APIs that manage CRUD operations on sample resources. * */ -public class SampleResourcePlugin extends Plugin implements ActionPlugin, SystemIndexPlugin { +public class SampleResourcePlugin extends Plugin implements ActionPlugin, SystemIndexPlugin, IdentityAwarePlugin { private static final Logger log = LogManager.getLogger(SampleResourcePlugin.class); private boolean isResourceSharingEnabled = false; + private RunAsSubjectClient pluginClient; + public SampleResourcePlugin(final Settings settings) { isResourceSharingEnabled = settings.getAsBoolean(OPENSEARCH_RESOURCE_SHARING_ENABLED, OPENSEARCH_RESOURCE_SHARING_ENABLED_DEFAULT); } @@ -90,7 +98,8 @@ public Collection createComponents( IndexNameExpressionResolver indexNameExpressionResolver, Supplier repositoriesServiceSupplier ) { - return Collections.emptyList(); + this.pluginClient = new RunAsSubjectClient(client); + return List.of(pluginClient); } @Override @@ -107,6 +116,7 @@ public List getRestHandlers( handlers.add(new CreateResourceRestAction()); handlers.add(new GetResourceRestAction()); handlers.add(new DeleteResourceRestAction()); + handlers.add(new SecurePluginRestAction()); if (isResourceSharingEnabled) { handlers.add(new ShareResourceRestAction()); @@ -126,6 +136,7 @@ public List getRestHandlers( actions.add(new ActionHandler<>(ShareResourceAction.INSTANCE, ShareResourceTransportAction.class)); actions.add(new ActionHandler<>(RevokeResourceAccessAction.INSTANCE, RevokeResourceAccessTransportAction.class)); } + actions.add(new ActionHandler<>(SecurePluginAction.INSTANCE, SecurePluginTransportAction.class)); return actions; } @@ -134,4 +145,11 @@ public Collection getSystemIndexDescriptors(Settings sett final SystemIndexDescriptor systemIndexDescriptor = new SystemIndexDescriptor(RESOURCE_INDEX_NAME, "Sample index with resources"); return Collections.singletonList(systemIndexDescriptor); } + + @Override + public void assignSubject(PluginSubject pluginSubject) { + if (this.pluginClient != null) { + this.pluginClient.setSubject(pluginSubject); + } + } } diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/secure/actions/rest/create/SecurePluginAction.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/secure/actions/rest/create/SecurePluginAction.java new file mode 100644 index 0000000000..2745e04c17 --- /dev/null +++ b/sample-resource-plugin/src/main/java/org/opensearch/sample/secure/actions/rest/create/SecurePluginAction.java @@ -0,0 +1,29 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.sample.secure.actions.rest.create; + +import org.opensearch.action.ActionType; + +/** + * Action for testing running actions with PluginSubject + */ +public class SecurePluginAction extends ActionType { + /** + * Secure plugin action instance + */ + public static final SecurePluginAction INSTANCE = new SecurePluginAction(); + /** + * Secure plugin action name + */ + public static final String NAME = "cluster:admin/sample-resource-plugin/run-actions"; + + private SecurePluginAction() { + super(NAME, SecurePluginResponse::new); + } +} diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/secure/actions/rest/create/SecurePluginRequest.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/secure/actions/rest/create/SecurePluginRequest.java new file mode 100644 index 0000000000..723d9df4fc --- /dev/null +++ b/sample-resource-plugin/src/main/java/org/opensearch/sample/secure/actions/rest/create/SecurePluginRequest.java @@ -0,0 +1,57 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.sample.secure.actions.rest.create; + +import java.io.IOException; + +import org.opensearch.action.ActionRequest; +import org.opensearch.action.ActionRequestValidationException; +import org.opensearch.core.common.io.stream.StreamInput; +import org.opensearch.core.common.io.stream.StreamOutput; + +/** + * Request object for SecurePluginAction transport action + */ +public class SecurePluginRequest extends ActionRequest { + + private final String action; + private final String index; + + /** + * Default constructor + */ + public SecurePluginRequest(String action, String index) { + this.action = action; + this.index = index; + } + + public SecurePluginRequest(StreamInput in) throws IOException { + this.action = in.readString(); + this.index = in.readString(); + } + + @Override + public void writeTo(final StreamOutput out) throws IOException { + out.writeString(action); + out.writeString(index); + } + + @Override + public ActionRequestValidationException validate() { + return null; + } + + public String getAction() { + return this.action; + } + + public String getIndex() { + return this.index; + } +} diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/secure/actions/rest/create/SecurePluginResponse.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/secure/actions/rest/create/SecurePluginResponse.java new file mode 100644 index 0000000000..f288292b32 --- /dev/null +++ b/sample-resource-plugin/src/main/java/org/opensearch/sample/secure/actions/rest/create/SecurePluginResponse.java @@ -0,0 +1,55 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.sample.secure.actions.rest.create; + +import java.io.IOException; + +import org.opensearch.core.action.ActionResponse; +import org.opensearch.core.common.io.stream.StreamInput; +import org.opensearch.core.common.io.stream.StreamOutput; +import org.opensearch.core.xcontent.ToXContentObject; +import org.opensearch.core.xcontent.XContentBuilder; + +/** + * Response to a CreateSampleResourceRequest + */ +public class SecurePluginResponse extends ActionResponse implements ToXContentObject { + private final String message; + + /** + * Default constructor + * + * @param message The message + */ + public SecurePluginResponse(String message) { + this.message = message; + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + out.writeString(message); + } + + /** + * Constructor with StreamInput + * + * @param in the stream input + */ + public SecurePluginResponse(final StreamInput in) throws IOException { + message = in.readString(); + } + + @Override + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + builder.startObject(); + builder.field("message", message); + builder.endObject(); + return builder; + } +} diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/secure/actions/rest/create/SecurePluginRestAction.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/secure/actions/rest/create/SecurePluginRestAction.java new file mode 100644 index 0000000000..2905f4e41c --- /dev/null +++ b/sample-resource-plugin/src/main/java/org/opensearch/sample/secure/actions/rest/create/SecurePluginRestAction.java @@ -0,0 +1,81 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.sample.secure.actions.rest.create; + +import java.io.IOException; +import java.util.List; +import java.util.Map; + +import org.opensearch.core.xcontent.XContentParser; +import org.opensearch.rest.BaseRestHandler; +import org.opensearch.rest.RestRequest; +import org.opensearch.rest.action.RestToXContentListener; +import org.opensearch.transport.client.node.NodeClient; + +import static org.opensearch.rest.RestRequest.Method.POST; +import static org.opensearch.sample.utils.Constants.SAMPLE_RESOURCE_PLUGIN_API_PREFIX; + +/** + * Rest action to trigger the sample plugin to run actions using its assigned PluginSubject + * + * Example payloads + * + * Cluster action: + * + * { + * "action": "cluster:monitor/health" + * } + * + * Index action: + * + * { + * "action": "indices:admin/create", + * "indices": "test-index" + * } + */ +public class SecurePluginRestAction extends BaseRestHandler { + + public SecurePluginRestAction() {} + + @Override + public List routes() { + return List.of(new Route(POST, SAMPLE_RESOURCE_PLUGIN_API_PREFIX + "/run_action")); + } + + @Override + public String getName() { + return "run_secure_plugin_test_action"; + } + + @Override + protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) throws IOException { + Map source; + try (XContentParser parser = request.contentParser()) { + source = parser.map(); + } + + switch (request.method()) { + case POST: + return runAction(source, client); + default: + throw new IllegalArgumentException("Illegal method: " + request.method()); + } + } + + private RestChannelConsumer runAction(Map source, NodeClient client) { + String action = (String) source.get("action"); + String index = source.containsKey("index") ? (String) source.get("index") : null; + final SecurePluginRequest createSampleResourceRequest = new SecurePluginRequest(action, index); + return channel -> client.executeLocally( + SecurePluginAction.INSTANCE, + createSampleResourceRequest, + new RestToXContentListener<>(channel) + ); + } +} diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/secure/actions/transport/SecurePluginTransportAction.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/secure/actions/transport/SecurePluginTransportAction.java new file mode 100644 index 0000000000..02f896d5a7 --- /dev/null +++ b/sample-resource-plugin/src/main/java/org/opensearch/sample/secure/actions/transport/SecurePluginTransportAction.java @@ -0,0 +1,88 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.sample.secure.actions.transport; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + +import org.opensearch.action.admin.cluster.health.ClusterHealthAction; +import org.opensearch.action.admin.cluster.health.ClusterHealthRequest; +import org.opensearch.action.admin.indices.create.CreateIndexAction; +import org.opensearch.action.admin.indices.create.CreateIndexRequest; +import org.opensearch.action.support.ActionFilters; +import org.opensearch.action.support.HandledTransportAction; +import org.opensearch.common.inject.Inject; +import org.opensearch.common.xcontent.XContentFactory; +import org.opensearch.core.action.ActionListener; +import org.opensearch.core.xcontent.ToXContent; +import org.opensearch.sample.secure.actions.rest.create.SecurePluginAction; +import org.opensearch.sample.secure.actions.rest.create.SecurePluginRequest; +import org.opensearch.sample.secure.actions.rest.create.SecurePluginResponse; +import org.opensearch.sample.utils.RunAsSubjectClient; +import org.opensearch.tasks.Task; +import org.opensearch.transport.TransportService; +import org.opensearch.transport.client.Client; + +/** + * Transport action for creating a new resource. + */ +public class SecurePluginTransportAction extends HandledTransportAction { + private static final Logger log = LogManager.getLogger(SecurePluginTransportAction.class); + + // TODO Get RunAsClient + + private final Client pluginClient; + + @Inject + public SecurePluginTransportAction(TransportService transportService, ActionFilters actionFilters, RunAsSubjectClient pluginClient) { + super(SecurePluginAction.NAME, transportService, actionFilters, SecurePluginRequest::new); + this.pluginClient = pluginClient; + } + + @Override + protected void doExecute(Task task, SecurePluginRequest request, ActionListener listener) { + runAction(request, listener); + } + + private void runAction(SecurePluginRequest request, ActionListener listener) { + String action = request.getAction(); + if (ClusterHealthAction.NAME.equals(action)) { + pluginClient.execute( + ClusterHealthAction.INSTANCE, + new ClusterHealthRequest(), + ActionListener.wrap( + clusterHealthResponse -> listener.onResponse( + new SecurePluginResponse( + String.valueOf(clusterHealthResponse.toXContent(XContentFactory.jsonBuilder(), ToXContent.EMPTY_PARAMS)) + ) + ), + listener::onFailure + ) + ); + return; + } else if (CreateIndexAction.NAME.equals(action)) { + String index = request.getIndex(); + pluginClient.execute( + CreateIndexAction.INSTANCE, + new CreateIndexRequest(index), + ActionListener.wrap( + createIndexResponse -> listener.onResponse( + new SecurePluginResponse( + String.valueOf(createIndexResponse.toXContent(XContentFactory.jsonBuilder(), ToXContent.EMPTY_PARAMS)) + ) + ), + listener::onFailure + ) + ); + return; + } + + listener.onResponse(new SecurePluginResponse("Unrecognized action: " + action)); + } +} diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/utils/Constants.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/utils/Constants.java index 8cccb7e178..8b3c61ede7 100644 --- a/sample-resource-plugin/src/main/java/org/opensearch/sample/utils/Constants.java +++ b/sample-resource-plugin/src/main/java/org/opensearch/sample/utils/Constants.java @@ -12,8 +12,8 @@ * Constants for Sample Resource Sharing Plugin */ public class Constants { - public static final String RESOURCE_INDEX_NAME = ".sample_resource_sharing_plugin"; + public static final String RESOURCE_INDEX_NAME = ".sample_resource"; - public static final String SAMPLE_RESOURCE_PLUGIN_PREFIX = "_plugins/sample_resource_sharing"; + public static final String SAMPLE_RESOURCE_PLUGIN_PREFIX = "_plugins/sample_plugin"; public static final String SAMPLE_RESOURCE_PLUGIN_API_PREFIX = "/" + SAMPLE_RESOURCE_PLUGIN_PREFIX; } diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/utils/RunAsSubjectClient.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/utils/RunAsSubjectClient.java new file mode 100644 index 0000000000..2239f7c0a0 --- /dev/null +++ b/sample-resource-plugin/src/main/java/org/opensearch/sample/utils/RunAsSubjectClient.java @@ -0,0 +1,67 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ +package org.opensearch.sample.utils; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + +import org.opensearch.action.ActionRequest; +import org.opensearch.action.ActionType; +import org.opensearch.common.util.concurrent.ThreadContext; +import org.opensearch.core.action.ActionListener; +import org.opensearch.core.action.ActionResponse; +import org.opensearch.identity.Subject; +import org.opensearch.transport.client.Client; +import org.opensearch.transport.client.FilterClient; + +/** + * Implementation of client that will run transport actions in a stashed context and inject the name of the provided + * subject into the context. + */ +public class RunAsSubjectClient extends FilterClient { + + private static final Logger logger = LogManager.getLogger(RunAsSubjectClient.class); + + private Subject subject; + + public RunAsSubjectClient(Client delegate) { + super(delegate); + } + + public RunAsSubjectClient(Client delegate, Subject subject) { + super(delegate); + this.subject = subject; + } + + public void setSubject(Subject subject) { + this.subject = subject; + } + + @Override + protected void doExecute( + ActionType action, + Request request, + ActionListener listener + ) { + if (subject == null) { + throw new IllegalStateException("RunAsSubjectClient is not initialized."); + } + try (ThreadContext.StoredContext ctx = threadPool().getThreadContext().newStoredContext(false)) { + subject.runAs(() -> { + logger.info("Running transport action with subject: {}", subject.getPrincipal().getName()); + super.doExecute(action, request, ActionListener.runBefore(listener, ctx::restore)); + return null; + }); + } catch (RuntimeException e) { + throw e; + } catch (Exception e) { + throw new RuntimeException(e); + } + } +} diff --git a/sample-resource-plugin/src/main/resources/plugin-additional-permissions.yml b/sample-resource-plugin/src/main/resources/plugin-additional-permissions.yml new file mode 100644 index 0000000000..15bf9e6537 --- /dev/null +++ b/sample-resource-plugin/src/main/resources/plugin-additional-permissions.yml @@ -0,0 +1,8 @@ +cluster_permissions: + - "cluster:monitor/health" +index_permissions: + - index_patterns: + - "test-index*" + allowed_actions: + - "indices:data/write/index*" + - "indices:admin/create" diff --git a/settings.gradle b/settings.gradle index 19b259b700..dd8f066cbe 100644 --- a/settings.gradle +++ b/settings.gradle @@ -7,7 +7,7 @@ rootProject.name = 'opensearch-security' include "spi" -project(":spi").name = "opensearch-security-spi" +project(":spi").name = rootProject.name + "-spi" include "sample-resource-plugin" project(":sample-resource-plugin").name = "opensearch-sample-resource-plugin" diff --git a/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/RunAsSubjectClient.java b/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/RunAsSubjectClient.java index 9242dbca5e..cca3b57830 100644 --- a/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/RunAsSubjectClient.java +++ b/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/RunAsSubjectClient.java @@ -55,6 +55,8 @@ protected void super.doExecute(action, request, ActionListener.runBefore(listener, ctx::restore)); return null; }); + } catch (RuntimeException e) { + throw e; } catch (Exception e) { throw new RuntimeException(e); } diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java index 92c6cba56d..1f60e342d3 100644 --- a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java +++ b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java @@ -29,6 +29,7 @@ // CS-SUPPRESS-SINGLE: RegexpSingleline Extensions manager used to allow/disallow TLS connections to extensions import java.io.IOException; +import java.net.URL; import java.nio.file.Files; import java.nio.file.LinkOption; import java.nio.file.Path; @@ -167,6 +168,7 @@ import org.opensearch.security.http.NonSslHttpServerTransport; import org.opensearch.security.http.XFFResolver; import org.opensearch.security.identity.ContextProvidingPluginSubject; +import org.opensearch.security.identity.NoopPluginSubject; import org.opensearch.security.identity.SecurityTokenManager; import org.opensearch.security.privileges.PrivilegesEvaluationException; import org.opensearch.security.privileges.PrivilegesEvaluator; @@ -188,6 +190,7 @@ import org.opensearch.security.rest.TenantInfoAction; import org.opensearch.security.securityconf.DynamicConfigFactory; import org.opensearch.security.securityconf.impl.CType; +import org.opensearch.security.securityconf.impl.v7.RoleV7; import org.opensearch.security.setting.OpensearchDynamicSetting; import org.opensearch.security.setting.TransportPassiveAuthSetting; import org.opensearch.security.spi.resources.FeatureConfigConstants; @@ -1066,7 +1069,6 @@ public Collection createComponents( IndexNameExpressionResolver indexNameExpressionResolver, Supplier repositoriesServiceSupplier ) { - SSLConfig.registerClusterSettingsChangeListener(clusterService.getClusterSettings()); if (SSLConfig.isSslOnlyMode()) { return super.createComponents( @@ -2290,10 +2292,31 @@ public SecurityTokenManager getTokenManager() { @Override public PluginSubject getPluginSubject(Plugin plugin) { - Set clusterActions = new HashSet<>(); - clusterActions.add(BulkAction.NAME); - PluginSubject subject = new ContextProvidingPluginSubject(threadPool, settings, plugin); - evaluator.updatePluginToClusterActions(subject.getPrincipal().getName(), clusterActions); + PluginSubject subject; + if (!client && !disabled && !SSLConfig.isSslOnlyMode()) { + subject = new ContextProvidingPluginSubject(threadPool, settings, plugin); + String pluginPrincipal = subject.getPrincipal().getName(); + URL resource = plugin.getClass().getClassLoader().getResource("plugin-additional-permissions.yml"); + RoleV7 pluginPermissions; + if (resource == null) { + log.info( + "plugin-additional-permissions.yml not found on classpath for plugin {}, using empty permissions", + pluginPrincipal + ); + pluginPermissions = new RoleV7(); + pluginPermissions.setCluster_permissions(new ArrayList<>()); + } else { + try { + pluginPermissions = RoleV7.fromPluginPermissionsFile(resource); + } catch (IOException e) { + throw new OpenSearchSecurityException(e.getMessage(), e); + } + } + pluginPermissions.getCluster_permissions().add(BulkAction.NAME); + evaluator.updatePluginToActionPrivileges(pluginPrincipal, pluginPermissions); + } else { + subject = new NoopPluginSubject(threadPool); + } return subject; } diff --git a/src/main/java/org/opensearch/security/identity/ContextProvidingPluginSubject.java b/src/main/java/org/opensearch/security/identity/ContextProvidingPluginSubject.java index ab6dddceba..f2e7449b2f 100644 --- a/src/main/java/org/opensearch/security/identity/ContextProvidingPluginSubject.java +++ b/src/main/java/org/opensearch/security/identity/ContextProvidingPluginSubject.java @@ -26,10 +26,14 @@ public class ContextProvidingPluginSubject implements PluginSubject { private final NamedPrincipal pluginPrincipal; private final User pluginUser; + public static String getPluginPrincipalName(String canonicalClassName) { + return "plugin:" + canonicalClassName; + } + public ContextProvidingPluginSubject(ThreadPool threadPool, Settings settings, Plugin plugin) { super(); this.threadPool = threadPool; - String principal = "plugin:" + plugin.getClass().getCanonicalName(); + String principal = getPluginPrincipalName(plugin.getClass().getCanonicalName()); this.pluginPrincipal = new NamedPrincipal(principal); // Convention for plugin username. Prefixed with 'plugin:'. ':' is forbidden from usernames, so this // guarantees that a user with this username cannot be created by other means. diff --git a/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java b/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java index 133cce391f..af5c3c3124 100644 --- a/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java +++ b/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java @@ -879,9 +879,7 @@ private List toString(List aliases) { return Collections.unmodifiableList(ret); } - public void updatePluginToClusterActions(String pluginIdentifier, Set clusterActions) { - RoleV7 pluginPermissions = new RoleV7(); - pluginPermissions.setCluster_permissions(ImmutableList.copyOf(clusterActions)); - this.pluginIdToActionPrivileges.put(pluginIdentifier, new SubjectBasedActionPrivileges(pluginPermissions, this.staticActionGroups)); + public void updatePluginToActionPrivileges(String pluginIdentifier, RoleV7 pluginPermissions) { + pluginIdToActionPrivileges.put(pluginIdentifier, new SubjectBasedActionPrivileges(pluginPermissions, staticActionGroups)); } } diff --git a/src/main/java/org/opensearch/security/privileges/SystemIndexAccessEvaluator.java b/src/main/java/org/opensearch/security/privileges/SystemIndexAccessEvaluator.java index ba03ff2f25..6b0d1f3c25 100644 --- a/src/main/java/org/opensearch/security/privileges/SystemIndexAccessEvaluator.java +++ b/src/main/java/org/opensearch/security/privileges/SystemIndexAccessEvaluator.java @@ -305,28 +305,34 @@ private void evaluateSystemIndicesAccess( // cluster actions will return true for requestedResolved.isLocalAll() // the following section should only be run for index actions if (this.isSystemIndexEnabled && user.isPluginUser() && !requestedResolved.isLocalAll()) { - Set matchingSystemIndices = SystemIndexRegistry.matchesPluginSystemIndexPattern( + Set matchingPluginIndices = SystemIndexRegistry.matchesPluginSystemIndexPattern( user.getName().replace("plugin:", ""), requestedResolved.getAllIndices() ); - if (requestedResolved.getAllIndices().equals(matchingSystemIndices)) { + if (requestedResolved.getAllIndices().equals(matchingPluginIndices)) { // plugin is authorized to perform any actions on its own registered system indices presponse.allowed = true; presponse.markComplete(); + return; } else { - if (log.isInfoEnabled()) { - log.info( - "Plugin {} can only perform {} on it's own registered System Indices. System indices from request that match plugin's registered system indices: {}", - user.getName(), - action, - matchingSystemIndices - ); + Set matchingSystemIndices = SystemIndexRegistry.matchesSystemIndexPattern(requestedResolved.getAllIndices()); + matchingSystemIndices.removeAll(matchingPluginIndices); + // See if request matches other system indices not belong to the plugin + if (!matchingSystemIndices.isEmpty()) { + if (log.isInfoEnabled()) { + log.info( + "Plugin {} can only perform {} on it's own registered System Indices. System indices from request that match plugin's registered system indices: {}", + user.getName(), + action, + matchingPluginIndices + ); + } + presponse.allowed = false; + presponse.getMissingPrivileges(); + presponse.markComplete(); + return; } - presponse.allowed = false; - presponse.getMissingPrivileges(); - presponse.markComplete(); } - return; } if (isActionAllowed(action)) { diff --git a/src/main/java/org/opensearch/security/securityconf/impl/v7/RoleV7.java b/src/main/java/org/opensearch/security/securityconf/impl/v7/RoleV7.java index 064207c5f8..8859b4586e 100644 --- a/src/main/java/org/opensearch/security/securityconf/impl/v7/RoleV7.java +++ b/src/main/java/org/opensearch/security/securityconf/impl/v7/RoleV7.java @@ -28,19 +28,37 @@ package org.opensearch.security.securityconf.impl.v7; import java.io.IOException; +import java.io.InputStream; +import java.io.InputStreamReader; import java.io.Reader; import java.io.StringReader; +import java.net.URL; +import java.nio.charset.StandardCharsets; import java.util.Collections; import java.util.List; import com.fasterxml.jackson.annotation.JsonProperty; +import org.opensearch.OpenSearchCorruptionException; import org.opensearch.security.DefaultObjectMapper; import org.opensearch.security.securityconf.Hideable; import org.opensearch.security.securityconf.StaticDefinable; public class RoleV7 implements Hideable, StaticDefinable { + private boolean reserved; + private boolean hidden; + @JsonProperty(value = "static") + private boolean _static; + private String description; + private List cluster_permissions = Collections.emptyList(); + private List index_permissions = Collections.emptyList(); + private List tenant_permissions = Collections.emptyList(); + + public RoleV7() { + + } + public static RoleV7 fromYamlString(String yamlString) throws IOException { try (Reader yamlReader = new StringReader(yamlString)) { return fromYaml(yamlReader); @@ -52,28 +70,36 @@ public static RoleV7 fromYamlString(String yamlString) throws IOException { * useful for tests. */ public static RoleV7 fromYamlStringUnchecked(String yamlString) { - try { - return fromYamlString(yamlString); + try (Reader yamlReader = new StringReader(yamlString)) { + return fromYaml(yamlReader); } catch (IOException e) { throw new RuntimeException(e); } } + public static RoleV7 fromYaml(URL yamlFile) throws IOException { + try (InputStream in = yamlFile.openStream(); Reader yamlReader = new InputStreamReader(in, StandardCharsets.UTF_8)) { + return fromYaml(yamlReader); + } + } + public static RoleV7 fromYaml(Reader yamlReader) throws IOException { return DefaultObjectMapper.YAML_MAPPER.readValue(yamlReader, RoleV7.class); } - private boolean reserved; - private boolean hidden; - @JsonProperty(value = "static") - private boolean _static; - private String description; - private List cluster_permissions = Collections.emptyList(); - private List index_permissions = Collections.emptyList(); - private List tenant_permissions = Collections.emptyList(); + /** + * Does additional validations regarding limitiations of plugin permissions files. + */ + public static RoleV7 fromPluginPermissionsFile(URL pluginPermissionsFile) throws IOException { + RoleV7 role = fromYaml(pluginPermissionsFile); - public RoleV7() { + if (role.tenant_permissions != null && !role.tenant_permissions.isEmpty()) { + throw new OpenSearchCorruptionException( + "Unsupported key tenant_permissions. Only 'cluster_permissions' and 'index_permissions' are allowed." + ); + } + return role; } public static class Index { diff --git a/src/test/java/org/opensearch/security/securityconf/impl/v7/RoleV7Test.java b/src/test/java/org/opensearch/security/securityconf/impl/v7/RoleV7Test.java new file mode 100644 index 0000000000..335b93e4b5 --- /dev/null +++ b/src/test/java/org/opensearch/security/securityconf/impl/v7/RoleV7Test.java @@ -0,0 +1,119 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + * + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + +package org.opensearch.security.securityconf.impl.v7; + +import java.net.URL; + +import com.fasterxml.jackson.databind.exc.UnrecognizedPropertyException; +import org.junit.Before; +import org.junit.Test; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; + +public class RoleV7Test { + + private URL testYamlUrl; + + @Before + public void setUp() throws Exception { + // Create a test YAML file in resources + testYamlUrl = RoleV7Test.class.getResource("/test-role.yml"); + } + + @Test + public void testFromYmlFileWithValidInput() throws Exception { + // Given a valid YAML file with both cluster and index permissions + // test-role.yml content: + /* + cluster_permissions: + - "cluster:admin/opensearch/monitor" + - "cluster:admin/opensearch/upgrade" + index_permissions: + - index_patterns: + - "test-*" + - "index-*" + allowed_actions: + - "read" + - "write" + */ + + // When + RoleV7 role = RoleV7.fromPluginPermissionsFile(testYamlUrl); + + // Then + assertNotNull(role); + assertEquals(2, role.getCluster_permissions().size()); + assertTrue(role.getCluster_permissions().contains("cluster:monitor/health")); + assertTrue(role.getCluster_permissions().contains("cluster:monitor/shards")); + + assertEquals(1, role.getIndex_permissions().size()); + RoleV7.Index indexPerm = role.getIndex_permissions().get(0); + assertEquals(2, indexPerm.getIndex_patterns().size()); + assertTrue(indexPerm.getIndex_patterns().contains("test-*")); + assertTrue(indexPerm.getIndex_patterns().contains("index-*")); + assertEquals(2, indexPerm.getAllowed_actions().size()); + assertTrue(indexPerm.getAllowed_actions().contains("read")); + assertTrue(indexPerm.getAllowed_actions().contains("write")); + } + + @Test + public void testFromYmlFileWithEmptyPermissions() throws Exception { + // Given a YAML file with empty permissions + // test-role-empty.yml content: + /* + cluster_permissions: [] + index_permissions: [] + */ + + URL emptyYamlUrl = RoleV7Test.class.getResource("/test-role-empty.yml"); + + // When + RoleV7 role = RoleV7.fromPluginPermissionsFile(emptyYamlUrl); + + // Then + assertNotNull(role); + assertTrue(role.getCluster_permissions().isEmpty()); + assertTrue(role.getIndex_permissions().isEmpty()); + } + + @Test(expected = UnrecognizedPropertyException.class) + public void testFromYmlFileWithInvalidYaml() throws Exception { + // Given an invalid YAML file + URL invalidYamlUrl = RoleV7Test.class.getResource("/test-role-invalid.yml"); + + // When/Then + RoleV7.fromPluginPermissionsFile(invalidYamlUrl); // Should throw an exception + } + + @Test + public void testFromYmlFileWithMissingIndexPermissions() throws Exception { + // Given a YAML file with only cluster permissions + // test-role-cluster-only.yml content: + /* + cluster_permissions: + - "cluster:admin/opensearch/monitor" + */ + + URL clusterOnlyYamlUrl = RoleV7Test.class.getResource("/test-role-cluster-only.yml"); + + // When + RoleV7 role = RoleV7.fromPluginPermissionsFile(clusterOnlyYamlUrl); + + // Then + assertNotNull(role); + assertEquals(1, role.getCluster_permissions().size()); + assertTrue(role.getCluster_permissions().contains("cluster:monitor/health")); + assertTrue(role.getIndex_permissions().isEmpty()); + } +} diff --git a/src/test/resources/test-role-cluster-only.yml b/src/test/resources/test-role-cluster-only.yml new file mode 100644 index 0000000000..ed26ce732d --- /dev/null +++ b/src/test/resources/test-role-cluster-only.yml @@ -0,0 +1,2 @@ +cluster_permissions: + - "cluster:monitor/health" diff --git a/src/test/resources/test-role-empty.yml b/src/test/resources/test-role-empty.yml new file mode 100644 index 0000000000..0c25139ec9 --- /dev/null +++ b/src/test/resources/test-role-empty.yml @@ -0,0 +1,2 @@ +cluster_permissions: [] +index_permissions: [] diff --git a/src/test/resources/test-role-invalid.yml b/src/test/resources/test-role-invalid.yml new file mode 100644 index 0000000000..29958b27e2 --- /dev/null +++ b/src/test/resources/test-role-invalid.yml @@ -0,0 +1 @@ +does-not-exist: test diff --git a/src/test/resources/test-role.yml b/src/test/resources/test-role.yml new file mode 100644 index 0000000000..be38029c62 --- /dev/null +++ b/src/test/resources/test-role.yml @@ -0,0 +1,10 @@ +cluster_permissions: + - "cluster:monitor/health" + - "cluster:monitor/shards" +index_permissions: + - index_patterns: + - "test-*" + - "index-*" + allowed_actions: + - "read" + - "write"