From 8a91e41ec4c3cf0783b6b115a8614cea7c1656a1 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Fri, 16 May 2025 14:39:28 -0400 Subject: [PATCH 01/18] Create a mechanism for plugins to declare permissions Signed-off-by: Craig Perkins --- .github/actions/create-bwc-build/action.yaml | 2 +- .github/workflows/ci.yml | 2 +- .github/workflows/plugin_install.yml | 2 +- checkstyle/checkstyle.xml | 24 ++-- sample-resource-plugin/build.gradle | 1 + .../SampleResourcePluginTestHelper.java | 14 +-- .../sample/secure/SecurePluginTests.java | 105 ++++++++++++++++++ .../sample/SampleResourcePlugin.java | 37 ++++-- .../sample/SampleSecurePluginExtension.java | 10 ++ .../rest/create/CreateResourceRestAction.java | 6 +- .../rest/delete/DeleteResourceRestAction.java | 4 +- .../rest/get/GetResourceRestAction.java | 7 +- .../RevokeResourceAccessRestAction.java | 4 +- .../rest/share/ShareResourceRestAction.java | 4 +- .../rest/create/SecurePluginAction.java | 29 +++++ .../rest/create/SecurePluginRequest.java | 57 ++++++++++ .../rest/create/SecurePluginResponse.java | 55 +++++++++ .../rest/create/SecurePluginRestAction.java | 81 ++++++++++++++ .../SecurePluginTransportAction.java | 88 +++++++++++++++ .../opensearch/sample/utils/Constants.java | 6 +- .../sample/utils/RunAsSubjectClient.java | 65 +++++++++++ ...nsearch.security.spi.SecurePluginExtension | 1 + .../src/main/resources/plugin-permissions.yml | 8 ++ settings.gradle | 2 +- .../security/spi/SecurePluginExtension.java | 23 ++++ .../privileges/ActionPrivilegesTest.java | 12 +- .../systemindex/SystemIndexDisabledTests.java | 30 ++++- .../systemindex/SystemIndexTests.java | 30 ++++- .../sampleplugin/SystemIndexPlugin1.java | 8 +- .../sampleplugin/SystemIndexPlugin2.java | 8 +- ...nsearch.security.spi.SecurePluginExtension | 2 + .../security/OpenSearchSecurityPlugin.java | 93 ++++++++++++---- .../security/filter/SecurityFilter.java | 5 +- .../ContextProvidingPluginSubject.java | 6 +- .../security/privileges/ActionPrivileges.java | 40 ++++--- .../privileges/PrivilegesEvaluator.java | 10 +- .../SystemIndexAccessEvaluator.java | 32 +++--- .../security/securityconf/impl/v7/RoleV7.java | 36 ++++++ 38 files changed, 836 insertions(+), 113 deletions(-) create mode 100644 sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/secure/SecurePluginTests.java create mode 100644 sample-resource-plugin/src/main/java/org/opensearch/sample/SampleSecurePluginExtension.java create mode 100644 sample-resource-plugin/src/main/java/org/opensearch/sample/secure/actions/rest/create/SecurePluginAction.java create mode 100644 sample-resource-plugin/src/main/java/org/opensearch/sample/secure/actions/rest/create/SecurePluginRequest.java create mode 100644 sample-resource-plugin/src/main/java/org/opensearch/sample/secure/actions/rest/create/SecurePluginResponse.java create mode 100644 sample-resource-plugin/src/main/java/org/opensearch/sample/secure/actions/rest/create/SecurePluginRestAction.java create mode 100644 sample-resource-plugin/src/main/java/org/opensearch/sample/secure/actions/transport/SecurePluginTransportAction.java create mode 100644 sample-resource-plugin/src/main/java/org/opensearch/sample/utils/RunAsSubjectClient.java create mode 100644 sample-resource-plugin/src/main/resources/META-INF/services/org.opensearch.security.spi.SecurePluginExtension create mode 100644 sample-resource-plugin/src/main/resources/plugin-permissions.yml create mode 100644 spi/src/main/java/org/opensearch/security/spi/SecurePluginExtension.java create mode 100644 src/integrationTest/resources/META-INF/services/org.opensearch.security.spi.SecurePluginExtension diff --git a/.github/actions/create-bwc-build/action.yaml b/.github/actions/create-bwc-build/action.yaml index 8960849333..0f9e373b16 100644 --- a/.github/actions/create-bwc-build/action.yaml +++ b/.github/actions/create-bwc-build/action.yaml @@ -42,7 +42,7 @@ runs: uses: gradle/gradle-build-action@v2 with: cache-disabled: true - arguments: assemble + arguments: :assemble build-root-directory: ${{ inputs.plugin-branch }} - id: get-opensearch-version diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 56114de73a..8a9a9bbf56 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -447,7 +447,7 @@ jobs: - uses: github/codeql-action/init@v3 with: languages: java - - run: ./gradlew clean assemble + - run: ./gradlew clean :assemble - uses: github/codeql-action/analyze@v3 build-artifact-names: diff --git a/.github/workflows/plugin_install.yml b/.github/workflows/plugin_install.yml index da3e240a68..1f28dec592 100644 --- a/.github/workflows/plugin_install.yml +++ b/.github/workflows/plugin_install.yml @@ -33,7 +33,7 @@ jobs: uses: gradle/gradle-build-action@v3 with: cache-disabled: true - arguments: assemble + arguments: :assemble # Move and rename the plugin for installation - name: Move and rename the plugin for installation diff --git a/checkstyle/checkstyle.xml b/checkstyle/checkstyle.xml index a9c1a8f765..7fe4a703de 100644 --- a/checkstyle/checkstyle.xml +++ b/checkstyle/checkstyle.xml @@ -205,12 +205,12 @@ - - - - - - + + + + + + @@ -228,12 +228,12 @@ - - - - - - + + + + + + diff --git a/sample-resource-plugin/build.gradle b/sample-resource-plugin/build.gradle index 1b3670fa7d..ceb5a464fd 100644 --- a/sample-resource-plugin/build.gradle +++ b/sample-resource-plugin/build.gradle @@ -86,6 +86,7 @@ sourceSets { srcDir file('src/integrationTest/java') compileClasspath += sourceSets.main.output runtimeClasspath += sourceSets.main.output + // TODO How to ensure resource are also on the classpath? } resources { srcDir file('src/integrationTest/resources') diff --git a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/SampleResourcePluginTestHelper.java b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/SampleResourcePluginTestHelper.java index fddaa94940..9f08dcfa17 100644 --- a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/SampleResourcePluginTestHelper.java +++ b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/SampleResourcePluginTestHelper.java @@ -10,7 +10,7 @@ import org.opensearch.test.framework.TestSecurityConfig; -import static org.opensearch.sample.utils.Constants.SAMPLE_RESOURCE_PLUGIN_PREFIX; +import static org.opensearch.sample.utils.Constants.SAMPLE_PLUGIN_PREFIX; /** * Abstract class for sample resource plugin tests. Provides common constants and utility methods for testing. This class is not intended to be @@ -35,12 +35,12 @@ public abstract class SampleResourcePluginTestHelper { ) ); - protected static final String SAMPLE_RESOURCE_CREATE_ENDPOINT = SAMPLE_RESOURCE_PLUGIN_PREFIX + "/create"; - protected static final String SAMPLE_RESOURCE_GET_ENDPOINT = SAMPLE_RESOURCE_PLUGIN_PREFIX + "/get"; - protected static final String SAMPLE_RESOURCE_UPDATE_ENDPOINT = SAMPLE_RESOURCE_PLUGIN_PREFIX + "/update"; - protected static final String SAMPLE_RESOURCE_DELETE_ENDPOINT = SAMPLE_RESOURCE_PLUGIN_PREFIX + "/delete"; - protected static final String SAMPLE_RESOURCE_SHARE_ENDPOINT = SAMPLE_RESOURCE_PLUGIN_PREFIX + "/share"; - protected static final String SAMPLE_RESOURCE_REVOKE_ENDPOINT = SAMPLE_RESOURCE_PLUGIN_PREFIX + "/revoke"; + protected static final String SAMPLE_RESOURCE_CREATE_ENDPOINT = SAMPLE_PLUGIN_PREFIX + "/create"; + protected static final String SAMPLE_RESOURCE_GET_ENDPOINT = SAMPLE_PLUGIN_PREFIX + "/get"; + protected static final String SAMPLE_RESOURCE_UPDATE_ENDPOINT = SAMPLE_PLUGIN_PREFIX + "/update"; + protected static final String SAMPLE_RESOURCE_DELETE_ENDPOINT = SAMPLE_PLUGIN_PREFIX + "/delete"; + protected static final String SAMPLE_RESOURCE_SHARE_ENDPOINT = SAMPLE_PLUGIN_PREFIX + "/share"; + protected static final String SAMPLE_RESOURCE_REVOKE_ENDPOINT = SAMPLE_PLUGIN_PREFIX + "/revoke"; protected static String shareWithPayload(String user) { return """ 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..8eb55b73bd --- /dev/null +++ b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/secure/SecurePluginTests.java @@ -0,0 +1,105 @@ +/* + * 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_PLUGIN_PREFIX; +import static org.opensearch.security.support.ConfigConstants.SECURITY_RESTAPI_ROLES_ENABLED; +import static org.opensearch.security.support.ConfigConstants.SECURITY_SYSTEM_INDICES_ENABLED_KEY; +import static org.opensearch.test.framework.TestSecurityConfig.Role.ALL_ACCESS; +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_RESTAPI_ROLES_ENABLED, + List.of("user_" + USER_ADMIN.getName() + "__" + ALL_ACCESS.getName()), + SECURITY_SYSTEM_INDICES_ENABLED_KEY, + true + ) + ) + .build(); + + @Test + public void testRunClusterHealthWithPluginSubject() { + try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) { + HttpResponse response = client.postJson(SAMPLE_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_PLUGIN_PREFIX + "/run_action", """ + { + "action": "indices:admin/create", + "index": "test-index" + } + """); + + System.out.println("body: " + response.getBody()); + + assertThat(response.getStatusCode(), equalTo(RestStatus.OK.getStatus())); + } + } +} 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..2a3eba7069 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,13 +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; - public SampleResourcePlugin(final Settings settings) { - isResourceSharingEnabled = settings.getAsBoolean(OPENSEARCH_RESOURCE_SHARING_ENABLED, OPENSEARCH_RESOURCE_SHARING_ENABLED_DEFAULT); - } + private RunAsSubjectClient pluginClient; + + public SampleResourcePlugin() {} @Override public Collection createComponents( @@ -90,7 +95,8 @@ public Collection createComponents( IndexNameExpressionResolver indexNameExpressionResolver, Supplier repositoriesServiceSupplier ) { - return Collections.emptyList(); + this.pluginClient = new RunAsSubjectClient(client); + return List.of(pluginClient); } @Override @@ -103,10 +109,15 @@ public List getRestHandlers( IndexNameExpressionResolver indexNameExpressionResolver, Supplier nodesInCluster ) { + boolean isResourceSharingEnabled = settings.getAsBoolean( + OPENSEARCH_RESOURCE_SHARING_ENABLED, + OPENSEARCH_RESOURCE_SHARING_ENABLED_DEFAULT + ); List handlers = new ArrayList<>(); handlers.add(new CreateResourceRestAction()); handlers.add(new GetResourceRestAction()); handlers.add(new DeleteResourceRestAction()); + handlers.add(new SecurePluginRestAction()); if (isResourceSharingEnabled) { handlers.add(new ShareResourceRestAction()); @@ -122,10 +133,9 @@ public List getRestHandlers( actions.add(new ActionHandler<>(GetResourceAction.INSTANCE, GetResourceTransportAction.class)); actions.add(new ActionHandler<>(UpdateResourceAction.INSTANCE, UpdateResourceTransportAction.class)); actions.add(new ActionHandler<>(DeleteResourceAction.INSTANCE, DeleteResourceTransportAction.class)); - if (isResourceSharingEnabled) { - actions.add(new ActionHandler<>(ShareResourceAction.INSTANCE, ShareResourceTransportAction.class)); - actions.add(new ActionHandler<>(RevokeResourceAccessAction.INSTANCE, RevokeResourceAccessTransportAction.class)); - } + 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 +144,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/SampleSecurePluginExtension.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/SampleSecurePluginExtension.java new file mode 100644 index 0000000000..14cfdf9418 --- /dev/null +++ b/sample-resource-plugin/src/main/java/org/opensearch/sample/SampleSecurePluginExtension.java @@ -0,0 +1,10 @@ +package org.opensearch.sample; + +import org.opensearch.security.spi.SecurePluginExtension; + +public class SampleSecurePluginExtension implements SecurePluginExtension { + @Override + public String getPluginCanonicalClassname() { + return SampleResourcePlugin.class.getCanonicalName(); + } +} diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/create/CreateResourceRestAction.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/create/CreateResourceRestAction.java index db8a153404..973a275e2d 100644 --- a/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/create/CreateResourceRestAction.java +++ b/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/create/CreateResourceRestAction.java @@ -21,7 +21,7 @@ import static org.opensearch.rest.RestRequest.Method.POST; import static org.opensearch.rest.RestRequest.Method.PUT; -import static org.opensearch.sample.utils.Constants.SAMPLE_RESOURCE_PLUGIN_API_PREFIX; +import static org.opensearch.sample.utils.Constants.SAMPLE_PLUGIN_API_PREFIX; /** * Rest Action to create a Sample Resource. Registers Create and Update REST APIs. @@ -33,8 +33,8 @@ public CreateResourceRestAction() {} @Override public List routes() { return List.of( - new Route(PUT, SAMPLE_RESOURCE_PLUGIN_API_PREFIX + "/create"), - new Route(POST, SAMPLE_RESOURCE_PLUGIN_API_PREFIX + "/update/{resource_id}") + new Route(PUT, SAMPLE_PLUGIN_API_PREFIX + "/create"), + new Route(POST, SAMPLE_PLUGIN_API_PREFIX + "/update/{resource_id}") ); } diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/delete/DeleteResourceRestAction.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/delete/DeleteResourceRestAction.java index 32dec08084..4012241bb7 100644 --- a/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/delete/DeleteResourceRestAction.java +++ b/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/delete/DeleteResourceRestAction.java @@ -18,7 +18,7 @@ import static java.util.Collections.singletonList; import static org.opensearch.rest.RestRequest.Method.DELETE; -import static org.opensearch.sample.utils.Constants.SAMPLE_RESOURCE_PLUGIN_API_PREFIX; +import static org.opensearch.sample.utils.Constants.SAMPLE_PLUGIN_API_PREFIX; /** * Rest Action to delete a Sample Resource. @@ -29,7 +29,7 @@ public DeleteResourceRestAction() {} @Override public List routes() { - return singletonList(new Route(DELETE, SAMPLE_RESOURCE_PLUGIN_API_PREFIX + "/delete/{resource_id}")); + return singletonList(new Route(DELETE, SAMPLE_PLUGIN_API_PREFIX + "/delete/{resource_id}")); } @Override diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/get/GetResourceRestAction.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/get/GetResourceRestAction.java index f534543fde..838bc9c7d8 100644 --- a/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/get/GetResourceRestAction.java +++ b/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/get/GetResourceRestAction.java @@ -16,7 +16,7 @@ import org.opensearch.transport.client.node.NodeClient; import static org.opensearch.rest.RestRequest.Method.GET; -import static org.opensearch.sample.utils.Constants.SAMPLE_RESOURCE_PLUGIN_API_PREFIX; +import static org.opensearch.sample.utils.Constants.SAMPLE_PLUGIN_API_PREFIX; /** * Rest action to get a sample resource @@ -27,10 +27,7 @@ public GetResourceRestAction() {} @Override public List routes() { - return List.of( - new Route(GET, SAMPLE_RESOURCE_PLUGIN_API_PREFIX + "/get/{resource_id}"), - new Route(GET, SAMPLE_RESOURCE_PLUGIN_API_PREFIX + "/get") - ); + return List.of(new Route(GET, SAMPLE_PLUGIN_API_PREFIX + "/get/{resource_id}"), new Route(GET, SAMPLE_PLUGIN_API_PREFIX + "/get")); } @Override diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/revoke/RevokeResourceAccessRestAction.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/revoke/RevokeResourceAccessRestAction.java index a25914548d..acdcc4af8a 100644 --- a/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/revoke/RevokeResourceAccessRestAction.java +++ b/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/revoke/RevokeResourceAccessRestAction.java @@ -26,7 +26,7 @@ import static java.util.Collections.singletonList; import static org.opensearch.rest.RestRequest.Method.POST; -import static org.opensearch.sample.utils.Constants.SAMPLE_RESOURCE_PLUGIN_API_PREFIX; +import static org.opensearch.sample.utils.Constants.SAMPLE_PLUGIN_API_PREFIX; /** * Rest Action to revoke sample resource access @@ -37,7 +37,7 @@ public RevokeResourceAccessRestAction() {} @Override public List routes() { - return singletonList(new Route(POST, SAMPLE_RESOURCE_PLUGIN_API_PREFIX + "/revoke/{resource_id}")); + return singletonList(new Route(POST, SAMPLE_PLUGIN_API_PREFIX + "/revoke/{resource_id}")); } @Override diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/share/ShareResourceRestAction.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/share/ShareResourceRestAction.java index 6793ff6801..484eb7636f 100644 --- a/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/share/ShareResourceRestAction.java +++ b/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/share/ShareResourceRestAction.java @@ -27,7 +27,7 @@ import static java.util.Collections.singletonList; import static org.opensearch.rest.RestRequest.Method.POST; -import static org.opensearch.sample.utils.Constants.SAMPLE_RESOURCE_PLUGIN_API_PREFIX; +import static org.opensearch.sample.utils.Constants.SAMPLE_PLUGIN_API_PREFIX; /** * Rest Action to share a resource @@ -38,7 +38,7 @@ public ShareResourceRestAction() {} @Override public List routes() { - return singletonList(new Route(POST, SAMPLE_RESOURCE_PLUGIN_API_PREFIX + "/share/{resource_id}")); + return singletonList(new Route(POST, SAMPLE_PLUGIN_API_PREFIX + "/share/{resource_id}")); } @Override 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..e2816eb8a0 --- /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_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_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..83c0824bff 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_API_PREFIX = "/" + SAMPLE_RESOURCE_PLUGIN_PREFIX; + public static final String SAMPLE_PLUGIN_PREFIX = "_plugins/sample_plugin"; + public static final String SAMPLE_PLUGIN_API_PREFIX = "/" + SAMPLE_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..149843eb54 --- /dev/null +++ b/sample-resource-plugin/src/main/java/org/opensearch/sample/utils/RunAsSubjectClient.java @@ -0,0 +1,65 @@ +/* + * 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 (Exception e) { + throw new RuntimeException(e); + } + } +} diff --git a/sample-resource-plugin/src/main/resources/META-INF/services/org.opensearch.security.spi.SecurePluginExtension b/sample-resource-plugin/src/main/resources/META-INF/services/org.opensearch.security.spi.SecurePluginExtension new file mode 100644 index 0000000000..3bed0cfe0a --- /dev/null +++ b/sample-resource-plugin/src/main/resources/META-INF/services/org.opensearch.security.spi.SecurePluginExtension @@ -0,0 +1 @@ +org.opensearch.sample.SampleSecurePluginExtension \ No newline at end of file diff --git a/sample-resource-plugin/src/main/resources/plugin-permissions.yml b/sample-resource-plugin/src/main/resources/plugin-permissions.yml new file mode 100644 index 0000000000..15bf9e6537 --- /dev/null +++ b/sample-resource-plugin/src/main/resources/plugin-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/spi/src/main/java/org/opensearch/security/spi/SecurePluginExtension.java b/spi/src/main/java/org/opensearch/security/spi/SecurePluginExtension.java new file mode 100644 index 0000000000..eadb689738 --- /dev/null +++ b/spi/src/main/java/org/opensearch/security/spi/SecurePluginExtension.java @@ -0,0 +1,23 @@ +package org.opensearch.security.spi; + +public interface SecurePluginExtension { + + /** + * This method returns a brief description of this plugin's use-case for + * @return A description of the use-case + */ + default String getDescription() { + return """ + Plugin that requires additional privileges to operate independently in addition to direct system index access. + + The permissions this plugin requests are located in the plugin-permissions.yml file of the plugin. + """; + }; + + /** + * This method returns the canonical class name of the plugin implementing the SecurePluginExtension interface. + * This is used to ensure that the plugin's implementation is loaded and initialized correctly. + * @return Canonical class name of the plugin + */ + String getPluginCanonicalClassname(); +} diff --git a/src/integrationTest/java/org/opensearch/security/privileges/ActionPrivilegesTest.java b/src/integrationTest/java/org/opensearch/security/privileges/ActionPrivilegesTest.java index 8618e27306..a7537839e2 100644 --- a/src/integrationTest/java/org/opensearch/security/privileges/ActionPrivilegesTest.java +++ b/src/integrationTest/java/org/opensearch/security/privileges/ActionPrivilegesTest.java @@ -72,6 +72,16 @@ ActionPrivilegesTest.Misc.class, ActionPrivilegesTest.StatefulIndexPrivilegesHeapSize.class }) public class ActionPrivilegesTest { + // TODO Create unlimited role statically here + private static final RoleV7 UNLIMITED_ROLE = new RoleV7(); + private static final RoleV7.Index UNLIMITED_INDEX = new RoleV7.Index(); + static { + UNLIMITED_INDEX.setIndex_patterns(List.of("*")); + UNLIMITED_INDEX.setAllowed_actions(List.of("*")); + UNLIMITED_ROLE.setCluster_permissions(List.of("*")); + UNLIMITED_ROLE.setIndex_permissions(List.of(UNLIMITED_INDEX)); + } + public static class ClusterPrivileges { @Test public void wellKnown() throws Exception { @@ -135,7 +145,7 @@ public void wildcardByUsername() throws Exception { FlattenedActionGroups.EMPTY, null, Settings.EMPTY, - Map.of("plugin:org.opensearch.sample.SamplePlugin", Set.of("*")) + Map.of("plugin:org.opensearch.sample.SamplePlugin", UNLIMITED_ROLE) ); assertThat( diff --git a/src/integrationTest/java/org/opensearch/security/systemindex/SystemIndexDisabledTests.java b/src/integrationTest/java/org/opensearch/security/systemindex/SystemIndexDisabledTests.java index 109e5228cf..2a0b1f2b5c 100644 --- a/src/integrationTest/java/org/opensearch/security/systemindex/SystemIndexDisabledTests.java +++ b/src/integrationTest/java/org/opensearch/security/systemindex/SystemIndexDisabledTests.java @@ -18,7 +18,10 @@ import org.junit.Test; import org.junit.runner.RunWith; +import org.opensearch.Version; import org.opensearch.core.rest.RestStatus; +import org.opensearch.plugins.PluginInfo; +import org.opensearch.security.OpenSearchSecurityPlugin; import org.opensearch.security.systemindex.sampleplugin.SystemIndexPlugin1; import org.opensearch.security.systemindex.sampleplugin.SystemIndexPlugin2; import org.opensearch.test.framework.TestSecurityConfig.AuthcDomain; @@ -46,7 +49,32 @@ public class SystemIndexDisabledTests { .anonymousAuth(false) .authc(AUTHC_DOMAIN) .users(USER_ADMIN) - .plugin(SystemIndexPlugin1.class, SystemIndexPlugin2.class) + .plugin( + new PluginInfo( + SystemIndexPlugin1.class.getName(), + "classpath plugin", + "NA", + Version.CURRENT, + "1.8", + SystemIndexPlugin1.class.getName(), + null, + List.of(OpenSearchSecurityPlugin.class.getName()), + false + ) + ) + .plugin( + new PluginInfo( + SystemIndexPlugin2.class.getName(), + "classpath plugin", + "NA", + Version.CURRENT, + "1.8", + SystemIndexPlugin2.class.getName(), + null, + List.of(OpenSearchSecurityPlugin.class.getName()), + false + ) + ) .nodeSettings( Map.of( SECURITY_RESTAPI_ROLES_ENABLED, diff --git a/src/integrationTest/java/org/opensearch/security/systemindex/SystemIndexTests.java b/src/integrationTest/java/org/opensearch/security/systemindex/SystemIndexTests.java index e8fdd9d7d4..1882b58f8d 100644 --- a/src/integrationTest/java/org/opensearch/security/systemindex/SystemIndexTests.java +++ b/src/integrationTest/java/org/opensearch/security/systemindex/SystemIndexTests.java @@ -19,7 +19,10 @@ import org.junit.Test; import org.junit.runner.RunWith; +import org.opensearch.Version; import org.opensearch.core.rest.RestStatus; +import org.opensearch.plugins.PluginInfo; +import org.opensearch.security.OpenSearchSecurityPlugin; import org.opensearch.security.systemindex.sampleplugin.SystemIndexPlugin1; import org.opensearch.security.systemindex.sampleplugin.SystemIndexPlugin2; import org.opensearch.test.framework.TestSecurityConfig.AuthcDomain; @@ -50,7 +53,32 @@ public class SystemIndexTests { .anonymousAuth(false) .authc(AUTHC_DOMAIN) .users(USER_ADMIN) - .plugin(SystemIndexPlugin1.class, SystemIndexPlugin2.class) + .plugin( + new PluginInfo( + SystemIndexPlugin1.class.getName(), + "classpath plugin", + "NA", + Version.CURRENT, + "1.8", + SystemIndexPlugin1.class.getName(), + null, + List.of(OpenSearchSecurityPlugin.class.getName()), + false + ) + ) + .plugin( + new PluginInfo( + SystemIndexPlugin2.class.getName(), + "classpath plugin", + "NA", + Version.CURRENT, + "1.8", + SystemIndexPlugin2.class.getName(), + null, + List.of(OpenSearchSecurityPlugin.class.getName()), + false + ) + ) .nodeSettings( Map.of( SECURITY_RESTAPI_ROLES_ENABLED, diff --git a/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/SystemIndexPlugin1.java b/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/SystemIndexPlugin1.java index 785995d7a2..ff1fc138f6 100644 --- a/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/SystemIndexPlugin1.java +++ b/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/SystemIndexPlugin1.java @@ -38,11 +38,12 @@ import org.opensearch.rest.RestController; import org.opensearch.rest.RestHandler; import org.opensearch.script.ScriptService; +import org.opensearch.security.spi.SecurePluginExtension; import org.opensearch.threadpool.ThreadPool; import org.opensearch.transport.client.Client; import org.opensearch.watcher.ResourceWatcherService; -public class SystemIndexPlugin1 extends Plugin implements SystemIndexPlugin, IdentityAwarePlugin { +public class SystemIndexPlugin1 extends Plugin implements SystemIndexPlugin, IdentityAwarePlugin, SecurePluginExtension { public static final String SYSTEM_INDEX_1 = ".system-index1"; private RunAsSubjectClient pluginClient; @@ -109,4 +110,9 @@ public void assignSubject(PluginSubject pluginSystemSubject) { this.pluginClient.setSubject(pluginSystemSubject); } } + + @Override + public String getPluginCanonicalClassname() { + return SystemIndexPlugin1.class.getCanonicalName(); + } } diff --git a/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/SystemIndexPlugin2.java b/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/SystemIndexPlugin2.java index 75358c7daa..ba123624fd 100644 --- a/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/SystemIndexPlugin2.java +++ b/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/SystemIndexPlugin2.java @@ -26,11 +26,12 @@ import org.opensearch.plugins.SystemIndexPlugin; import org.opensearch.repositories.RepositoriesService; import org.opensearch.script.ScriptService; +import org.opensearch.security.spi.SecurePluginExtension; import org.opensearch.threadpool.ThreadPool; import org.opensearch.transport.client.Client; import org.opensearch.watcher.ResourceWatcherService; -public class SystemIndexPlugin2 extends Plugin implements SystemIndexPlugin { +public class SystemIndexPlugin2 extends Plugin implements SystemIndexPlugin, SecurePluginExtension { public static final String SYSTEM_INDEX_2 = ".system-index2"; private Client client; @@ -58,4 +59,9 @@ public Collection getSystemIndexDescriptors(Settings sett final SystemIndexDescriptor systemIndexDescriptor = new SystemIndexDescriptor(SYSTEM_INDEX_2, "System index 2"); return Collections.singletonList(systemIndexDescriptor); } + + @Override + public String getPluginCanonicalClassname() { + return SystemIndexPlugin2.class.getCanonicalName(); + } } diff --git a/src/integrationTest/resources/META-INF/services/org.opensearch.security.spi.SecurePluginExtension b/src/integrationTest/resources/META-INF/services/org.opensearch.security.spi.SecurePluginExtension new file mode 100644 index 0000000000..d88a6f1d0f --- /dev/null +++ b/src/integrationTest/resources/META-INF/services/org.opensearch.security.spi.SecurePluginExtension @@ -0,0 +1,2 @@ +org.opensearch.security.systemindex.sampleplugin.SystemIndexPlugin1 +org.opensearch.security.systemindex.sampleplugin.SystemIndexPlugin2 \ No newline at end of file diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java index 6eb4268f41..205e39dc7c 100644 --- a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java +++ b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java @@ -29,6 +29,11 @@ // CS-SUPPRESS-SINGLE: RegexpSingleline Extensions manager used to allow/disallow TLS connections to extensions import java.io.IOException; +import java.io.InputStream; +import java.io.InputStreamReader; +import java.io.Reader; +import java.net.URL; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.LinkOption; import java.nio.file.Path; @@ -60,6 +65,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.Lists; +import com.fasterxml.jackson.databind.JsonNode; import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -189,8 +195,10 @@ 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.SecurePluginExtension; import org.opensearch.security.spi.resources.FeatureConfigConstants; import org.opensearch.security.spi.resources.ResourceProvider; import org.opensearch.security.spi.resources.ResourceSharingExtension; @@ -233,6 +241,7 @@ import static org.opensearch.security.dlic.rest.api.RestApiAdminPrivilegesEvaluator.ENDPOINTS_WITH_PERMISSIONS; import static org.opensearch.security.dlic.rest.api.RestApiAdminPrivilegesEvaluator.SECURITY_CONFIG_UPDATE; +import static org.opensearch.security.identity.ContextProvidingPluginSubject.getPluginPrincipalName; import static org.opensearch.security.privileges.dlsfls.FieldMasking.Config.BLAKE2B_LEGACY_DEFAULT; import static org.opensearch.security.setting.DeprecatedSettings.checkForDeprecatedSetting; import static org.opensearch.security.support.ConfigConstants.OPENDISTRO_SECURITY_AUTHENTICATED_USER; @@ -250,8 +259,8 @@ public final class OpenSearchSecurityPlugin extends OpenSearchSecuritySSLPlugin MapperPlugin, IdentityPlugin, // CS-SUPPRESS-SINGLE: RegexpSingleline get Extensions Settings - ExtensionAwarePlugin, - ExtensiblePlugin + ExtensiblePlugin, + ExtensionAwarePlugin // CS-ENFORCE-SINGLE { @@ -288,6 +297,7 @@ public final class OpenSearchSecurityPlugin extends OpenSearchSecuritySSLPlugin private volatile OpensearchDynamicSetting transportPassiveAuthSetting; private volatile PasswordHasher passwordHasher; private volatile DlsFlsBaseContext dlsFlsBaseContext; + private volatile Map pluginToRoleMap; private ResourceSharingIndexHandler rsIndexHandler; private final ResourcePluginInfo resourcePluginInfo = new ResourcePluginInfo(); @@ -1069,7 +1079,6 @@ public Collection createComponents( IndexNameExpressionResolver indexNameExpressionResolver, Supplier repositoriesServiceSupplier ) { - SSLConfig.registerClusterSettingsChangeListener(clusterService.getClusterSettings()); if (SSLConfig.isSslOnlyMode()) { return super.createComponents( @@ -2255,10 +2264,12 @@ public SecurityTokenManager getTokenManager() { @Override public PluginSubject getPluginSubject(Plugin plugin) { - Set clusterActions = new HashSet<>(); - clusterActions.add(BulkAction.NAME); PluginSubject subject = new ContextProvidingPluginSubject(threadPool, settings, plugin); - sf.updatePluginToClusterActions(subject.getPrincipal().getName(), clusterActions); + String pluginPrincipal = subject.getPrincipal().getName(); + if (pluginToRoleMap != null && pluginToRoleMap.containsKey(pluginPrincipal)) { + sf.updatePluginToPermissions(pluginPrincipal, pluginToRoleMap.get(pluginPrincipal)); + + } return subject; } @@ -2275,6 +2286,60 @@ public Optional getSecureSettingFactory(Settings settings ); } + // CS-SUPPRESS-SINGLE: RegexpSingleline SPI Extensions are unrelated to OpenSearch extensions + @Override + public void loadExtensions(ExtensiblePlugin.ExtensionLoader loader) { + System.out.println("loadExtensions"); + if (settings != null + && settings.getAsBoolean( + FeatureConfigConstants.OPENSEARCH_RESOURCE_SHARING_ENABLED, + FeatureConfigConstants.OPENSEARCH_RESOURCE_SHARING_ENABLED_DEFAULT + )) { + // load all resource-sharing extensions + Set resourceSharingExtensions = new HashSet<>(loader.loadExtensions(ResourceSharingExtension.class)); + resourcePluginInfo.setResourceSharingExtensions(resourceSharingExtensions); + } + + for (SecurePluginExtension extension : loader.loadExtensions(SecurePluginExtension.class)) { + System.out.println("extension: " + extension.getPluginCanonicalClassname()); + try { + // TODO Parse plugin-permissions.yml file (available as resource on classpath) into RoleV7 object + /** + * Example yml + * + * cluster_permissions: + * - "cluster:monitor/health" + * index_permissions: + * - index_patterns: + * - "security-auditlog*" + * allowed_actions: + * - "indices:data/write/index*" + */ + URL resource = extension.getClass().getClassLoader().getResource("plugin-permissions.yml"); + if (pluginToRoleMap == null) { + pluginToRoleMap = new HashMap<>(); + } + RoleV7 pluginPermissions; + if (resource == null) { + log.warn("plugin-permissions.yml not found on classpath"); + pluginPermissions = new RoleV7(); + pluginPermissions.setCluster_permissions(new ArrayList<>()); + } else { + try (InputStream in = resource.openStream(); Reader yamlReader = new InputStreamReader(in, StandardCharsets.UTF_8)) { + JsonNode roleJson = DefaultObjectMapper.YAML_MAPPER.readTree(yamlReader); + System.out.println("pluginPermissions: " + roleJson); + pluginPermissions = RoleV7.fromJsonNode(roleJson); + } + } + pluginPermissions.getCluster_permissions().add(BulkAction.NAME); + pluginToRoleMap.put(getPluginPrincipalName(extension.getPluginCanonicalClassname()), pluginPermissions); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + } + // CS-ENFORCE-SINGLE + @SuppressWarnings("removal") private void tryAddSecurityProvider() { final SecurityManager sm = System.getSecurityManager(); @@ -2299,22 +2364,6 @@ private void tryAddSecurityProvider() { }); } - // CS-SUPPRESS-SINGLE: RegexpSingleline get Resource Sharing Extensions - @Override - public void loadExtensions(ExtensiblePlugin.ExtensionLoader loader) { - - if (settings != null - && settings.getAsBoolean( - FeatureConfigConstants.OPENSEARCH_RESOURCE_SHARING_ENABLED, - FeatureConfigConstants.OPENSEARCH_RESOURCE_SHARING_ENABLED_DEFAULT - )) { - // load all resource-sharing extensions - Set resourceSharingExtensions = new HashSet<>(loader.loadExtensions(ResourceSharingExtension.class)); - resourcePluginInfo.setResourceSharingExtensions(resourceSharingExtensions); - } - } - // CS-ENFORCE-SINGLE - public static class GuiceHolder implements LifecycleComponent { private static RepositoriesService repositoriesService; diff --git a/src/main/java/org/opensearch/security/filter/SecurityFilter.java b/src/main/java/org/opensearch/security/filter/SecurityFilter.java index 2e0b61289f..2934b21f0d 100644 --- a/src/main/java/org/opensearch/security/filter/SecurityFilter.java +++ b/src/main/java/org/opensearch/security/filter/SecurityFilter.java @@ -86,6 +86,7 @@ import org.opensearch.security.privileges.PrivilegesEvaluator; import org.opensearch.security.privileges.PrivilegesEvaluatorResponse; import org.opensearch.security.resolver.IndexResolverReplacer; +import org.opensearch.security.securityconf.impl.v7.RoleV7; import org.opensearch.security.support.Base64Helper; import org.opensearch.security.support.ConfigConstants; import org.opensearch.security.support.HeaderHelper; @@ -518,8 +519,8 @@ private boolean checkImmutableIndices(Object request, ActionListener listener) { return false; } - public void updatePluginToClusterActions(String pluginIdentifier, Set clusterActions) { - evalp.updatePluginToClusterActions(pluginIdentifier, clusterActions); + public void updatePluginToPermissions(String pluginIdentifier, RoleV7 pluginPermissions) { + evalp.updatePluginToPermissions(pluginIdentifier, pluginPermissions); } private boolean isRequestIndexImmutable(Object request) { 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/ActionPrivileges.java b/src/main/java/org/opensearch/security/privileges/ActionPrivileges.java index f095d31b2c..cdde7f7019 100644 --- a/src/main/java/org/opensearch/security/privileges/ActionPrivileges.java +++ b/src/main/java/org/opensearch/security/privileges/ActionPrivileges.java @@ -14,6 +14,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -91,10 +92,10 @@ public ActionPrivileges( ImmutableSet wellKnownClusterActions, ImmutableSet wellKnownIndexActions, ImmutableSet explicitlyRequiredIndexActions, - Map> pluginToClusterActions + Map pluginToRole ) { - this.cluster = new ClusterPrivileges(roles, actionGroups, wellKnownClusterActions, pluginToClusterActions); - this.index = new IndexPrivileges(roles, actionGroups, wellKnownIndexActions, explicitlyRequiredIndexActions); + this.cluster = new ClusterPrivileges(roles, actionGroups, wellKnownClusterActions, pluginToRole); + this.index = new IndexPrivileges(roles, actionGroups, wellKnownIndexActions, explicitlyRequiredIndexActions, pluginToRole); this.roles = roles; this.actionGroups = actionGroups; this.wellKnownClusterActions = wellKnownClusterActions; @@ -126,7 +127,7 @@ public ActionPrivileges( FlattenedActionGroups actionGroups, Supplier> indexMetadataSupplier, Settings settings, - Map> pluginToClusterActions + Map pluginToRole ) { this( roles, @@ -136,7 +137,7 @@ public ActionPrivileges( WellKnownActions.CLUSTER_ACTIONS, WellKnownActions.INDEX_ACTIONS, WellKnownActions.EXPLICITLY_REQUIRED_INDEX_ACTIONS, - pluginToClusterActions + pluginToRole ); } @@ -334,7 +335,7 @@ static class ClusterPrivileges { SecurityDynamicConfiguration roles, FlattenedActionGroups actionGroups, ImmutableSet wellKnownClusterActions, - Map> pluginToClusterActions + Map pluginToRole ) { DeduplicatingCompactSubSetBuilder roleSetBuilder = new DeduplicatingCompactSubSetBuilder<>( roles.getCEntries().keySet() @@ -392,9 +393,9 @@ static class ClusterPrivileges { } } - if (pluginToClusterActions != null) { - for (String pluginIdentifier : pluginToClusterActions.keySet()) { - Set clusterActions = pluginToClusterActions.get(pluginIdentifier); + if (pluginToRole != null) { + for (String pluginIdentifier : pluginToRole.keySet()) { + List clusterActions = pluginToRole.get(pluginIdentifier).getCluster_permissions(); WildcardMatcher matcher = WildcardMatcher.from(clusterActions); usersToActionMatcher.put(pluginIdentifier, matcher); } @@ -603,18 +604,22 @@ static class IndexPrivileges { SecurityDynamicConfiguration roles, FlattenedActionGroups actionGroups, ImmutableSet wellKnownIndexActions, - ImmutableSet explicitlyRequiredIndexActions + ImmutableSet explicitlyRequiredIndexActions, + Map pluginToRole ) { - DeduplicatingCompactSubSetBuilder roleSetBuilder = new DeduplicatingCompactSubSetBuilder<>( - roles.getCEntries().keySet() - ); Map> rolesToActionToIndexPattern = new HashMap<>(); Map> rolesToActionPatternToIndexPattern = new HashMap<>(); Map> actionToRolesWithWildcardIndexPrivileges = new HashMap<>(); Map> rolesToExplicitActionToIndexPattern = new HashMap<>(); - for (Map.Entry entry : roles.getCEntries().entrySet()) { + Map permissionEntries = new HashMap<>(); + permissionEntries.putAll(roles.getCEntries()); + permissionEntries.putAll(pluginToRole); + + DeduplicatingCompactSubSetBuilder roleSetBuilder = new DeduplicatingCompactSubSetBuilder<>(permissionEntries.keySet()); + + for (Map.Entry entry : permissionEntries.entrySet()) { try { String roleName = entry.getKey(); RoleV7 role = entry.getValue(); @@ -754,8 +759,13 @@ PrivilegesEvaluatorResponse providesPrivilege( Map indexMetadata ) { List exceptions = new ArrayList<>(); + Set rolesToCheck = context.getMappedRoles(); + if (context.getUser().isPluginUser()) { + rolesToCheck = new HashSet<>(rolesToCheck); + rolesToCheck.add(context.getUser().getName()); + } - for (String role : context.getMappedRoles()) { + for (String role : rolesToCheck) { ImmutableMap actionToIndexPattern = this.rolesToActionToIndexPattern.get(role); if (actionToIndexPattern != null) { diff --git a/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java b/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java index 76e0d83434..42b540e7a2 100644 --- a/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java +++ b/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java @@ -155,7 +155,7 @@ public class PrivilegesEvaluator { private DynamicConfigModel dcm; private final NamedXContentRegistry namedXContentRegistry; private final Settings settings; - private final Map> pluginToClusterActions; + private final Map pluginToRole; private final AtomicReference actionPrivileges = new AtomicReference<>(); public PrivilegesEvaluator( @@ -179,7 +179,7 @@ public PrivilegesEvaluator( this.threadContext = threadContext; this.privilegesInterceptor = privilegesInterceptor; - this.pluginToClusterActions = new HashMap<>(); + this.pluginToRole = new HashMap<>(); this.clusterStateSupplier = clusterStateSupplier; this.settings = settings; @@ -238,7 +238,7 @@ void updateConfiguration( flattenedActionGroups, () -> clusterStateSupplier.get().metadata().getIndicesLookup(), settings, - pluginToClusterActions + pluginToRole ); Metadata metadata = clusterStateSupplier.get().metadata(); actionPrivileges.updateStatefulIndexPrivileges(metadata.getIndicesLookup(), metadata.version()); @@ -848,7 +848,7 @@ private List toString(List aliases) { return Collections.unmodifiableList(ret); } - public void updatePluginToClusterActions(String pluginIdentifier, Set clusterActions) { - pluginToClusterActions.put(pluginIdentifier, clusterActions); + public void updatePluginToPermissions(String pluginIdentifier, RoleV7 pluginPermissions) { + pluginToRole.put(pluginIdentifier, pluginPermissions); } } 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 2b2da40927..62973fc29b 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 @@ -27,10 +27,14 @@ package org.opensearch.security.securityconf.impl.v7; +import java.util.ArrayList; import java.util.Collections; +import java.util.Iterator; import java.util.List; import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.node.ArrayNode; import org.opensearch.security.securityconf.Hideable; import org.opensearch.security.securityconf.StaticDefinable; @@ -50,6 +54,38 @@ public RoleV7() { } + public static RoleV7 fromJsonNode(JsonNode node) { + RoleV7 role = new RoleV7(); + System.out.println("pretty print: " + node.toPrettyString()); + ArrayNode clusterPermsArray = node.withArray("cluster_permissions"); + List clusterPermissions = new ArrayList<>(clusterPermsArray.size()); + for (JsonNode elt : clusterPermsArray) { + clusterPermissions.add(elt.asText()); + } + role.cluster_permissions = clusterPermissions; + role.index_permissions = new ArrayList<>(); + if (node.get("index_permissions") != null) { + for (Iterator it = node.get("index_permissions").elements(); it.hasNext();) { + JsonNode indexNode = it.next(); + Index indexPerm = new Index(); + ArrayNode actionsArray = indexNode.withArray("allowed_actions"); + List allowedActions = new ArrayList<>(actionsArray.size()); + for (JsonNode elt : actionsArray) { + allowedActions.add(elt.asText()); + } + indexPerm.allowed_actions = allowedActions; + ArrayNode indexPatternsArray = indexNode.withArray("index_patterns"); + List indexPatterns = new ArrayList<>(indexPatternsArray.size()); + for (JsonNode elt : indexPatternsArray) { + indexPatterns.add(elt.asText()); + } + indexPerm.index_patterns = indexPatterns; + role.index_permissions.add(indexPerm); + } + } + return role; + } + public static class Index { private List index_patterns = Collections.emptyList(); From 7dbf73fa0c131071e44911c39505531735a2ea81 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Fri, 16 May 2025 14:49:35 -0400 Subject: [PATCH 02/18] Remove sysouts used for debugging Signed-off-by: Craig Perkins --- .github/actions/create-bwc-build/action.yaml | 2 +- .github/workflows/ci.yml | 2 +- .github/workflows/plugin_install.yml | 2 +- checkstyle/checkstyle.xml | 24 +++++++++---------- sample-resource-plugin/build.gradle | 1 - .../sample/secure/SecurePluginTests.java | 2 -- .../sampleplugin/SystemIndexPlugin1.java | 2 ++ .../sampleplugin/SystemIndexPlugin2.java | 5 ++-- .../security/OpenSearchSecurityPlugin.java | 15 ------------ .../security/securityconf/impl/v7/RoleV7.java | 1 - 10 files changed, 19 insertions(+), 37 deletions(-) diff --git a/.github/actions/create-bwc-build/action.yaml b/.github/actions/create-bwc-build/action.yaml index 0f9e373b16..8960849333 100644 --- a/.github/actions/create-bwc-build/action.yaml +++ b/.github/actions/create-bwc-build/action.yaml @@ -42,7 +42,7 @@ runs: uses: gradle/gradle-build-action@v2 with: cache-disabled: true - arguments: :assemble + arguments: assemble build-root-directory: ${{ inputs.plugin-branch }} - id: get-opensearch-version diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8a9a9bbf56..56114de73a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -447,7 +447,7 @@ jobs: - uses: github/codeql-action/init@v3 with: languages: java - - run: ./gradlew clean :assemble + - run: ./gradlew clean assemble - uses: github/codeql-action/analyze@v3 build-artifact-names: diff --git a/.github/workflows/plugin_install.yml b/.github/workflows/plugin_install.yml index 1f28dec592..da3e240a68 100644 --- a/.github/workflows/plugin_install.yml +++ b/.github/workflows/plugin_install.yml @@ -33,7 +33,7 @@ jobs: uses: gradle/gradle-build-action@v3 with: cache-disabled: true - arguments: :assemble + arguments: assemble # Move and rename the plugin for installation - name: Move and rename the plugin for installation diff --git a/checkstyle/checkstyle.xml b/checkstyle/checkstyle.xml index 7fe4a703de..a9c1a8f765 100644 --- a/checkstyle/checkstyle.xml +++ b/checkstyle/checkstyle.xml @@ -205,12 +205,12 @@ - - - - - - + + + + + + @@ -228,12 +228,12 @@ - - - - - - + + + + + + diff --git a/sample-resource-plugin/build.gradle b/sample-resource-plugin/build.gradle index ceb5a464fd..1b3670fa7d 100644 --- a/sample-resource-plugin/build.gradle +++ b/sample-resource-plugin/build.gradle @@ -86,7 +86,6 @@ sourceSets { srcDir file('src/integrationTest/java') compileClasspath += sourceSets.main.output runtimeClasspath += sourceSets.main.output - // TODO How to ensure resource are also on the classpath? } resources { srcDir file('src/integrationTest/resources') 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 index 8eb55b73bd..be9611176e 100644 --- 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 @@ -97,8 +97,6 @@ public void testRunCreateIndexWithPluginSubject() { } """); - System.out.println("body: " + response.getBody()); - assertThat(response.getStatusCode(), equalTo(RestStatus.OK.getStatus())); } } diff --git a/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/SystemIndexPlugin1.java b/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/SystemIndexPlugin1.java index ff1fc138f6..a60558cd45 100644 --- a/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/SystemIndexPlugin1.java +++ b/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/SystemIndexPlugin1.java @@ -10,6 +10,7 @@ package org.opensearch.security.systemindex.sampleplugin; +// CS-SUPPRESS-SINGLE: RegexpSingleline SPI Extensions are unrelated to OpenSearch extensions import java.util.Arrays; import java.util.Collection; import java.util.Collections; @@ -116,3 +117,4 @@ public String getPluginCanonicalClassname() { return SystemIndexPlugin1.class.getCanonicalName(); } } +// CS-ENFORCE-SINGLE diff --git a/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/SystemIndexPlugin2.java b/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/SystemIndexPlugin2.java index ba123624fd..916eae22d4 100644 --- a/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/SystemIndexPlugin2.java +++ b/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/SystemIndexPlugin2.java @@ -10,6 +10,7 @@ package org.opensearch.security.systemindex.sampleplugin; +// CS-SUPPRESS-SINGLE: RegexpSingleline SPI Extensions are unrelated to OpenSearch extensions import java.util.Collection; import java.util.Collections; import java.util.function.Supplier; @@ -34,8 +35,6 @@ public class SystemIndexPlugin2 extends Plugin implements SystemIndexPlugin, SecurePluginExtension { public static final String SYSTEM_INDEX_2 = ".system-index2"; - private Client client; - @Override public Collection createComponents( Client client, @@ -50,7 +49,6 @@ public Collection createComponents( IndexNameExpressionResolver indexNameExpressionResolver, Supplier repositoriesServiceSupplier ) { - this.client = client; return Collections.emptyList(); } @@ -65,3 +63,4 @@ public String getPluginCanonicalClassname() { return SystemIndexPlugin2.class.getCanonicalName(); } } +// CS-ENFORCE-SINGLE diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java index 205e39dc7c..4c913c4cf9 100644 --- a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java +++ b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java @@ -2289,7 +2289,6 @@ public Optional getSecureSettingFactory(Settings settings // CS-SUPPRESS-SINGLE: RegexpSingleline SPI Extensions are unrelated to OpenSearch extensions @Override public void loadExtensions(ExtensiblePlugin.ExtensionLoader loader) { - System.out.println("loadExtensions"); if (settings != null && settings.getAsBoolean( FeatureConfigConstants.OPENSEARCH_RESOURCE_SHARING_ENABLED, @@ -2301,20 +2300,7 @@ public void loadExtensions(ExtensiblePlugin.ExtensionLoader loader) { } for (SecurePluginExtension extension : loader.loadExtensions(SecurePluginExtension.class)) { - System.out.println("extension: " + extension.getPluginCanonicalClassname()); try { - // TODO Parse plugin-permissions.yml file (available as resource on classpath) into RoleV7 object - /** - * Example yml - * - * cluster_permissions: - * - "cluster:monitor/health" - * index_permissions: - * - index_patterns: - * - "security-auditlog*" - * allowed_actions: - * - "indices:data/write/index*" - */ URL resource = extension.getClass().getClassLoader().getResource("plugin-permissions.yml"); if (pluginToRoleMap == null) { pluginToRoleMap = new HashMap<>(); @@ -2327,7 +2313,6 @@ public void loadExtensions(ExtensiblePlugin.ExtensionLoader loader) { } else { try (InputStream in = resource.openStream(); Reader yamlReader = new InputStreamReader(in, StandardCharsets.UTF_8)) { JsonNode roleJson = DefaultObjectMapper.YAML_MAPPER.readTree(yamlReader); - System.out.println("pluginPermissions: " + roleJson); pluginPermissions = RoleV7.fromJsonNode(roleJson); } } 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 62973fc29b..8da62fafb8 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 @@ -56,7 +56,6 @@ public RoleV7() { public static RoleV7 fromJsonNode(JsonNode node) { RoleV7 role = new RoleV7(); - System.out.println("pretty print: " + node.toPrettyString()); ArrayNode clusterPermsArray = node.withArray("cluster_permissions"); List clusterPermissions = new ArrayList<>(clusterPermsArray.size()); for (JsonNode elt : clusterPermsArray) { From 8ff152519187ab5f8b2aa763182714c8866aece4 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Fri, 16 May 2025 15:05:54 -0400 Subject: [PATCH 03/18] Add to CHANGELOG Signed-off-by: Craig Perkins --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6fc1099c1e..5259926a41 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - [Resource Permissions] Introduces Centralized Resource Access Control Framework ([#5281](https://github.com/opensearch-project/security/pull/5281)) - Github workflow for changelog verification ([#5318](https://github.com/opensearch-project/security/pull/5318)) - Register cluster settings listener for `plugins.security.cache.ttl_minutes` ([#5324](https://github.com/opensearch-project/security/pull/5324)) +- 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 - Use extendedPlugins in integrationTest framework for sample resource plugin testing ([#5322](https://github.com/opensearch-project/security/pull/5322)) From 7f35d02357c5a937993c12370ffd34afacac48a6 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Fri, 16 May 2025 15:29:49 -0400 Subject: [PATCH 04/18] Remove SecurePluginExtension Signed-off-by: Craig Perkins --- .../sample/SampleSecurePluginExtension.java | 10 ---- ...nsearch.security.spi.SecurePluginExtension | 1 - .../security/spi/SecurePluginExtension.java | 23 --------- .../sampleplugin/SystemIndexPlugin1.java | 10 +--- .../sampleplugin/SystemIndexPlugin2.java | 10 +--- ...nsearch.security.spi.SecurePluginExtension | 2 - .../security/OpenSearchSecurityPlugin.java | 47 +++++++------------ 7 files changed, 19 insertions(+), 84 deletions(-) delete mode 100644 sample-resource-plugin/src/main/java/org/opensearch/sample/SampleSecurePluginExtension.java delete mode 100644 sample-resource-plugin/src/main/resources/META-INF/services/org.opensearch.security.spi.SecurePluginExtension delete mode 100644 spi/src/main/java/org/opensearch/security/spi/SecurePluginExtension.java delete mode 100644 src/integrationTest/resources/META-INF/services/org.opensearch.security.spi.SecurePluginExtension diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/SampleSecurePluginExtension.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/SampleSecurePluginExtension.java deleted file mode 100644 index 14cfdf9418..0000000000 --- a/sample-resource-plugin/src/main/java/org/opensearch/sample/SampleSecurePluginExtension.java +++ /dev/null @@ -1,10 +0,0 @@ -package org.opensearch.sample; - -import org.opensearch.security.spi.SecurePluginExtension; - -public class SampleSecurePluginExtension implements SecurePluginExtension { - @Override - public String getPluginCanonicalClassname() { - return SampleResourcePlugin.class.getCanonicalName(); - } -} diff --git a/sample-resource-plugin/src/main/resources/META-INF/services/org.opensearch.security.spi.SecurePluginExtension b/sample-resource-plugin/src/main/resources/META-INF/services/org.opensearch.security.spi.SecurePluginExtension deleted file mode 100644 index 3bed0cfe0a..0000000000 --- a/sample-resource-plugin/src/main/resources/META-INF/services/org.opensearch.security.spi.SecurePluginExtension +++ /dev/null @@ -1 +0,0 @@ -org.opensearch.sample.SampleSecurePluginExtension \ No newline at end of file diff --git a/spi/src/main/java/org/opensearch/security/spi/SecurePluginExtension.java b/spi/src/main/java/org/opensearch/security/spi/SecurePluginExtension.java deleted file mode 100644 index eadb689738..0000000000 --- a/spi/src/main/java/org/opensearch/security/spi/SecurePluginExtension.java +++ /dev/null @@ -1,23 +0,0 @@ -package org.opensearch.security.spi; - -public interface SecurePluginExtension { - - /** - * This method returns a brief description of this plugin's use-case for - * @return A description of the use-case - */ - default String getDescription() { - return """ - Plugin that requires additional privileges to operate independently in addition to direct system index access. - - The permissions this plugin requests are located in the plugin-permissions.yml file of the plugin. - """; - }; - - /** - * This method returns the canonical class name of the plugin implementing the SecurePluginExtension interface. - * This is used to ensure that the plugin's implementation is loaded and initialized correctly. - * @return Canonical class name of the plugin - */ - String getPluginCanonicalClassname(); -} diff --git a/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/SystemIndexPlugin1.java b/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/SystemIndexPlugin1.java index a60558cd45..785995d7a2 100644 --- a/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/SystemIndexPlugin1.java +++ b/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/SystemIndexPlugin1.java @@ -10,7 +10,6 @@ package org.opensearch.security.systemindex.sampleplugin; -// CS-SUPPRESS-SINGLE: RegexpSingleline SPI Extensions are unrelated to OpenSearch extensions import java.util.Arrays; import java.util.Collection; import java.util.Collections; @@ -39,12 +38,11 @@ import org.opensearch.rest.RestController; import org.opensearch.rest.RestHandler; import org.opensearch.script.ScriptService; -import org.opensearch.security.spi.SecurePluginExtension; import org.opensearch.threadpool.ThreadPool; import org.opensearch.transport.client.Client; import org.opensearch.watcher.ResourceWatcherService; -public class SystemIndexPlugin1 extends Plugin implements SystemIndexPlugin, IdentityAwarePlugin, SecurePluginExtension { +public class SystemIndexPlugin1 extends Plugin implements SystemIndexPlugin, IdentityAwarePlugin { public static final String SYSTEM_INDEX_1 = ".system-index1"; private RunAsSubjectClient pluginClient; @@ -111,10 +109,4 @@ public void assignSubject(PluginSubject pluginSystemSubject) { this.pluginClient.setSubject(pluginSystemSubject); } } - - @Override - public String getPluginCanonicalClassname() { - return SystemIndexPlugin1.class.getCanonicalName(); - } } -// CS-ENFORCE-SINGLE diff --git a/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/SystemIndexPlugin2.java b/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/SystemIndexPlugin2.java index 916eae22d4..6cd30b02d6 100644 --- a/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/SystemIndexPlugin2.java +++ b/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/SystemIndexPlugin2.java @@ -10,7 +10,6 @@ package org.opensearch.security.systemindex.sampleplugin; -// CS-SUPPRESS-SINGLE: RegexpSingleline SPI Extensions are unrelated to OpenSearch extensions import java.util.Collection; import java.util.Collections; import java.util.function.Supplier; @@ -27,12 +26,11 @@ import org.opensearch.plugins.SystemIndexPlugin; import org.opensearch.repositories.RepositoriesService; import org.opensearch.script.ScriptService; -import org.opensearch.security.spi.SecurePluginExtension; import org.opensearch.threadpool.ThreadPool; import org.opensearch.transport.client.Client; import org.opensearch.watcher.ResourceWatcherService; -public class SystemIndexPlugin2 extends Plugin implements SystemIndexPlugin, SecurePluginExtension { +public class SystemIndexPlugin2 extends Plugin implements SystemIndexPlugin { public static final String SYSTEM_INDEX_2 = ".system-index2"; @Override @@ -57,10 +55,4 @@ public Collection getSystemIndexDescriptors(Settings sett final SystemIndexDescriptor systemIndexDescriptor = new SystemIndexDescriptor(SYSTEM_INDEX_2, "System index 2"); return Collections.singletonList(systemIndexDescriptor); } - - @Override - public String getPluginCanonicalClassname() { - return SystemIndexPlugin2.class.getCanonicalName(); - } } -// CS-ENFORCE-SINGLE diff --git a/src/integrationTest/resources/META-INF/services/org.opensearch.security.spi.SecurePluginExtension b/src/integrationTest/resources/META-INF/services/org.opensearch.security.spi.SecurePluginExtension deleted file mode 100644 index d88a6f1d0f..0000000000 --- a/src/integrationTest/resources/META-INF/services/org.opensearch.security.spi.SecurePluginExtension +++ /dev/null @@ -1,2 +0,0 @@ -org.opensearch.security.systemindex.sampleplugin.SystemIndexPlugin1 -org.opensearch.security.systemindex.sampleplugin.SystemIndexPlugin2 \ No newline at end of file diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java index 4c913c4cf9..78c7e4477e 100644 --- a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java +++ b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java @@ -198,7 +198,6 @@ 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.SecurePluginExtension; import org.opensearch.security.spi.resources.FeatureConfigConstants; import org.opensearch.security.spi.resources.ResourceProvider; import org.opensearch.security.spi.resources.ResourceSharingExtension; @@ -241,7 +240,6 @@ import static org.opensearch.security.dlic.rest.api.RestApiAdminPrivilegesEvaluator.ENDPOINTS_WITH_PERMISSIONS; import static org.opensearch.security.dlic.rest.api.RestApiAdminPrivilegesEvaluator.SECURITY_CONFIG_UPDATE; -import static org.opensearch.security.identity.ContextProvidingPluginSubject.getPluginPrincipalName; import static org.opensearch.security.privileges.dlsfls.FieldMasking.Config.BLAKE2B_LEGACY_DEFAULT; import static org.opensearch.security.setting.DeprecatedSettings.checkForDeprecatedSetting; import static org.opensearch.security.support.ConfigConstants.OPENDISTRO_SECURITY_AUTHENTICATED_USER; @@ -297,7 +295,6 @@ public final class OpenSearchSecurityPlugin extends OpenSearchSecuritySSLPlugin private volatile OpensearchDynamicSetting transportPassiveAuthSetting; private volatile PasswordHasher passwordHasher; private volatile DlsFlsBaseContext dlsFlsBaseContext; - private volatile Map pluginToRoleMap; private ResourceSharingIndexHandler rsIndexHandler; private final ResourcePluginInfo resourcePluginInfo = new ResourcePluginInfo(); @@ -2266,9 +2263,23 @@ public SecurityTokenManager getTokenManager() { public PluginSubject getPluginSubject(Plugin plugin) { PluginSubject subject = new ContextProvidingPluginSubject(threadPool, settings, plugin); String pluginPrincipal = subject.getPrincipal().getName(); - if (pluginToRoleMap != null && pluginToRoleMap.containsKey(pluginPrincipal)) { - sf.updatePluginToPermissions(pluginPrincipal, pluginToRoleMap.get(pluginPrincipal)); - + try { + URL resource = plugin.getClass().getClassLoader().getResource("plugin-permissions.yml"); + RoleV7 pluginPermissions; + if (resource == null) { + log.warn("plugin-permissions.yml not found on classpath"); + pluginPermissions = new RoleV7(); + pluginPermissions.setCluster_permissions(new ArrayList<>()); + } else { + try (InputStream in = resource.openStream(); Reader yamlReader = new InputStreamReader(in, StandardCharsets.UTF_8)) { + JsonNode roleJson = DefaultObjectMapper.YAML_MAPPER.readTree(yamlReader); + pluginPermissions = RoleV7.fromJsonNode(roleJson); + } + } + pluginPermissions.getCluster_permissions().add(BulkAction.NAME); + sf.updatePluginToPermissions(pluginPrincipal, pluginPermissions); + } catch (IOException e) { + throw new RuntimeException(e); } return subject; } @@ -2298,30 +2309,6 @@ public void loadExtensions(ExtensiblePlugin.ExtensionLoader loader) { Set resourceSharingExtensions = new HashSet<>(loader.loadExtensions(ResourceSharingExtension.class)); resourcePluginInfo.setResourceSharingExtensions(resourceSharingExtensions); } - - for (SecurePluginExtension extension : loader.loadExtensions(SecurePluginExtension.class)) { - try { - URL resource = extension.getClass().getClassLoader().getResource("plugin-permissions.yml"); - if (pluginToRoleMap == null) { - pluginToRoleMap = new HashMap<>(); - } - RoleV7 pluginPermissions; - if (resource == null) { - log.warn("plugin-permissions.yml not found on classpath"); - pluginPermissions = new RoleV7(); - pluginPermissions.setCluster_permissions(new ArrayList<>()); - } else { - try (InputStream in = resource.openStream(); Reader yamlReader = new InputStreamReader(in, StandardCharsets.UTF_8)) { - JsonNode roleJson = DefaultObjectMapper.YAML_MAPPER.readTree(yamlReader); - pluginPermissions = RoleV7.fromJsonNode(roleJson); - } - } - pluginPermissions.getCluster_permissions().add(BulkAction.NAME); - pluginToRoleMap.put(getPluginPrincipalName(extension.getPluginCanonicalClassname()), pluginPermissions); - } catch (IOException e) { - throw new RuntimeException(e); - } - } } // CS-ENFORCE-SINGLE From 06e9340e8f652d60bd4d2bfa30fb05b1e967d8f8 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Fri, 16 May 2025 15:33:23 -0400 Subject: [PATCH 05/18] undo renaming to minimize changes Signed-off-by: Craig Perkins --- .../sample/SampleResourcePluginTestHelper.java | 14 +++++++------- .../sample/secure/SecurePluginTests.java | 6 +++--- .../rest/create/CreateResourceRestAction.java | 6 +++--- .../rest/delete/DeleteResourceRestAction.java | 4 ++-- .../actions/rest/get/GetResourceRestAction.java | 7 +++++-- .../revoke/RevokeResourceAccessRestAction.java | 4 ++-- .../rest/share/ShareResourceRestAction.java | 4 ++-- .../rest/create/SecurePluginRestAction.java | 4 ++-- .../org/opensearch/sample/utils/Constants.java | 4 ++-- 9 files changed, 28 insertions(+), 25 deletions(-) diff --git a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/SampleResourcePluginTestHelper.java b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/SampleResourcePluginTestHelper.java index 9f08dcfa17..fddaa94940 100644 --- a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/SampleResourcePluginTestHelper.java +++ b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/SampleResourcePluginTestHelper.java @@ -10,7 +10,7 @@ import org.opensearch.test.framework.TestSecurityConfig; -import static org.opensearch.sample.utils.Constants.SAMPLE_PLUGIN_PREFIX; +import static org.opensearch.sample.utils.Constants.SAMPLE_RESOURCE_PLUGIN_PREFIX; /** * Abstract class for sample resource plugin tests. Provides common constants and utility methods for testing. This class is not intended to be @@ -35,12 +35,12 @@ public abstract class SampleResourcePluginTestHelper { ) ); - protected static final String SAMPLE_RESOURCE_CREATE_ENDPOINT = SAMPLE_PLUGIN_PREFIX + "/create"; - protected static final String SAMPLE_RESOURCE_GET_ENDPOINT = SAMPLE_PLUGIN_PREFIX + "/get"; - protected static final String SAMPLE_RESOURCE_UPDATE_ENDPOINT = SAMPLE_PLUGIN_PREFIX + "/update"; - protected static final String SAMPLE_RESOURCE_DELETE_ENDPOINT = SAMPLE_PLUGIN_PREFIX + "/delete"; - protected static final String SAMPLE_RESOURCE_SHARE_ENDPOINT = SAMPLE_PLUGIN_PREFIX + "/share"; - protected static final String SAMPLE_RESOURCE_REVOKE_ENDPOINT = SAMPLE_PLUGIN_PREFIX + "/revoke"; + protected static final String SAMPLE_RESOURCE_CREATE_ENDPOINT = SAMPLE_RESOURCE_PLUGIN_PREFIX + "/create"; + protected static final String SAMPLE_RESOURCE_GET_ENDPOINT = SAMPLE_RESOURCE_PLUGIN_PREFIX + "/get"; + protected static final String SAMPLE_RESOURCE_UPDATE_ENDPOINT = SAMPLE_RESOURCE_PLUGIN_PREFIX + "/update"; + protected static final String SAMPLE_RESOURCE_DELETE_ENDPOINT = SAMPLE_RESOURCE_PLUGIN_PREFIX + "/delete"; + protected static final String SAMPLE_RESOURCE_SHARE_ENDPOINT = SAMPLE_RESOURCE_PLUGIN_PREFIX + "/share"; + protected static final String SAMPLE_RESOURCE_REVOKE_ENDPOINT = SAMPLE_RESOURCE_PLUGIN_PREFIX + "/revoke"; protected static String shareWithPayload(String user) { return """ 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 index be9611176e..0aabca307e 100644 --- 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 @@ -32,7 +32,7 @@ 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_PLUGIN_PREFIX; +import static org.opensearch.sample.utils.Constants.SAMPLE_RESOURCE_PLUGIN_PREFIX; import static org.opensearch.security.support.ConfigConstants.SECURITY_RESTAPI_ROLES_ENABLED; import static org.opensearch.security.support.ConfigConstants.SECURITY_SYSTEM_INDICES_ENABLED_KEY; import static org.opensearch.test.framework.TestSecurityConfig.Role.ALL_ACCESS; @@ -76,7 +76,7 @@ public class SecurePluginTests { @Test public void testRunClusterHealthWithPluginSubject() { try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) { - HttpResponse response = client.postJson(SAMPLE_PLUGIN_PREFIX + "/run_action", """ + HttpResponse response = client.postJson(SAMPLE_RESOURCE_PLUGIN_PREFIX + "/run_action", """ { "action": "cluster:monitor/health" } @@ -90,7 +90,7 @@ public void testRunClusterHealthWithPluginSubject() { @Test public void testRunCreateIndexWithPluginSubject() { try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) { - HttpResponse response = client.postJson(SAMPLE_PLUGIN_PREFIX + "/run_action", """ + HttpResponse response = client.postJson(SAMPLE_RESOURCE_PLUGIN_PREFIX + "/run_action", """ { "action": "indices:admin/create", "index": "test-index" diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/create/CreateResourceRestAction.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/create/CreateResourceRestAction.java index 973a275e2d..db8a153404 100644 --- a/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/create/CreateResourceRestAction.java +++ b/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/create/CreateResourceRestAction.java @@ -21,7 +21,7 @@ import static org.opensearch.rest.RestRequest.Method.POST; import static org.opensearch.rest.RestRequest.Method.PUT; -import static org.opensearch.sample.utils.Constants.SAMPLE_PLUGIN_API_PREFIX; +import static org.opensearch.sample.utils.Constants.SAMPLE_RESOURCE_PLUGIN_API_PREFIX; /** * Rest Action to create a Sample Resource. Registers Create and Update REST APIs. @@ -33,8 +33,8 @@ public CreateResourceRestAction() {} @Override public List routes() { return List.of( - new Route(PUT, SAMPLE_PLUGIN_API_PREFIX + "/create"), - new Route(POST, SAMPLE_PLUGIN_API_PREFIX + "/update/{resource_id}") + new Route(PUT, SAMPLE_RESOURCE_PLUGIN_API_PREFIX + "/create"), + new Route(POST, SAMPLE_RESOURCE_PLUGIN_API_PREFIX + "/update/{resource_id}") ); } diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/delete/DeleteResourceRestAction.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/delete/DeleteResourceRestAction.java index 4012241bb7..32dec08084 100644 --- a/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/delete/DeleteResourceRestAction.java +++ b/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/delete/DeleteResourceRestAction.java @@ -18,7 +18,7 @@ import static java.util.Collections.singletonList; import static org.opensearch.rest.RestRequest.Method.DELETE; -import static org.opensearch.sample.utils.Constants.SAMPLE_PLUGIN_API_PREFIX; +import static org.opensearch.sample.utils.Constants.SAMPLE_RESOURCE_PLUGIN_API_PREFIX; /** * Rest Action to delete a Sample Resource. @@ -29,7 +29,7 @@ public DeleteResourceRestAction() {} @Override public List routes() { - return singletonList(new Route(DELETE, SAMPLE_PLUGIN_API_PREFIX + "/delete/{resource_id}")); + return singletonList(new Route(DELETE, SAMPLE_RESOURCE_PLUGIN_API_PREFIX + "/delete/{resource_id}")); } @Override diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/get/GetResourceRestAction.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/get/GetResourceRestAction.java index 838bc9c7d8..f534543fde 100644 --- a/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/get/GetResourceRestAction.java +++ b/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/get/GetResourceRestAction.java @@ -16,7 +16,7 @@ import org.opensearch.transport.client.node.NodeClient; import static org.opensearch.rest.RestRequest.Method.GET; -import static org.opensearch.sample.utils.Constants.SAMPLE_PLUGIN_API_PREFIX; +import static org.opensearch.sample.utils.Constants.SAMPLE_RESOURCE_PLUGIN_API_PREFIX; /** * Rest action to get a sample resource @@ -27,7 +27,10 @@ public GetResourceRestAction() {} @Override public List routes() { - return List.of(new Route(GET, SAMPLE_PLUGIN_API_PREFIX + "/get/{resource_id}"), new Route(GET, SAMPLE_PLUGIN_API_PREFIX + "/get")); + return List.of( + new Route(GET, SAMPLE_RESOURCE_PLUGIN_API_PREFIX + "/get/{resource_id}"), + new Route(GET, SAMPLE_RESOURCE_PLUGIN_API_PREFIX + "/get") + ); } @Override diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/revoke/RevokeResourceAccessRestAction.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/revoke/RevokeResourceAccessRestAction.java index acdcc4af8a..a25914548d 100644 --- a/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/revoke/RevokeResourceAccessRestAction.java +++ b/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/revoke/RevokeResourceAccessRestAction.java @@ -26,7 +26,7 @@ import static java.util.Collections.singletonList; import static org.opensearch.rest.RestRequest.Method.POST; -import static org.opensearch.sample.utils.Constants.SAMPLE_PLUGIN_API_PREFIX; +import static org.opensearch.sample.utils.Constants.SAMPLE_RESOURCE_PLUGIN_API_PREFIX; /** * Rest Action to revoke sample resource access @@ -37,7 +37,7 @@ public RevokeResourceAccessRestAction() {} @Override public List routes() { - return singletonList(new Route(POST, SAMPLE_PLUGIN_API_PREFIX + "/revoke/{resource_id}")); + return singletonList(new Route(POST, SAMPLE_RESOURCE_PLUGIN_API_PREFIX + "/revoke/{resource_id}")); } @Override diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/share/ShareResourceRestAction.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/share/ShareResourceRestAction.java index 484eb7636f..6793ff6801 100644 --- a/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/share/ShareResourceRestAction.java +++ b/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/share/ShareResourceRestAction.java @@ -27,7 +27,7 @@ import static java.util.Collections.singletonList; import static org.opensearch.rest.RestRequest.Method.POST; -import static org.opensearch.sample.utils.Constants.SAMPLE_PLUGIN_API_PREFIX; +import static org.opensearch.sample.utils.Constants.SAMPLE_RESOURCE_PLUGIN_API_PREFIX; /** * Rest Action to share a resource @@ -38,7 +38,7 @@ public ShareResourceRestAction() {} @Override public List routes() { - return singletonList(new Route(POST, SAMPLE_PLUGIN_API_PREFIX + "/share/{resource_id}")); + return singletonList(new Route(POST, SAMPLE_RESOURCE_PLUGIN_API_PREFIX + "/share/{resource_id}")); } @Override 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 index e2816eb8a0..2905f4e41c 100644 --- 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 @@ -19,7 +19,7 @@ import org.opensearch.transport.client.node.NodeClient; import static org.opensearch.rest.RestRequest.Method.POST; -import static org.opensearch.sample.utils.Constants.SAMPLE_PLUGIN_API_PREFIX; +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 @@ -45,7 +45,7 @@ public SecurePluginRestAction() {} @Override public List routes() { - return List.of(new Route(POST, SAMPLE_PLUGIN_API_PREFIX + "/run_action")); + return List.of(new Route(POST, SAMPLE_RESOURCE_PLUGIN_API_PREFIX + "/run_action")); } @Override 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 83c0824bff..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 @@ -14,6 +14,6 @@ public class Constants { public static final String RESOURCE_INDEX_NAME = ".sample_resource"; - public static final String SAMPLE_PLUGIN_PREFIX = "_plugins/sample_plugin"; - public static final String SAMPLE_PLUGIN_API_PREFIX = "/" + SAMPLE_PLUGIN_PREFIX; + public static final String SAMPLE_RESOURCE_PLUGIN_PREFIX = "_plugins/sample_plugin"; + public static final String SAMPLE_RESOURCE_PLUGIN_API_PREFIX = "/" + SAMPLE_RESOURCE_PLUGIN_PREFIX; } From 031cfac5dceef4cf77eb600be723800d22efe047 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Fri, 16 May 2025 15:36:35 -0400 Subject: [PATCH 06/18] Further minimize changes Signed-off-by: Craig Perkins --- .../opensearch/sample/SampleResourcePlugin.java | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) 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 2a3eba7069..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 @@ -76,10 +76,13 @@ */ 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() {} + public SampleResourcePlugin(final Settings settings) { + isResourceSharingEnabled = settings.getAsBoolean(OPENSEARCH_RESOURCE_SHARING_ENABLED, OPENSEARCH_RESOURCE_SHARING_ENABLED_DEFAULT); + } @Override public Collection createComponents( @@ -109,10 +112,6 @@ public List getRestHandlers( IndexNameExpressionResolver indexNameExpressionResolver, Supplier nodesInCluster ) { - boolean isResourceSharingEnabled = settings.getAsBoolean( - OPENSEARCH_RESOURCE_SHARING_ENABLED, - OPENSEARCH_RESOURCE_SHARING_ENABLED_DEFAULT - ); List handlers = new ArrayList<>(); handlers.add(new CreateResourceRestAction()); handlers.add(new GetResourceRestAction()); @@ -133,8 +132,10 @@ public List getRestHandlers( actions.add(new ActionHandler<>(GetResourceAction.INSTANCE, GetResourceTransportAction.class)); actions.add(new ActionHandler<>(UpdateResourceAction.INSTANCE, UpdateResourceTransportAction.class)); actions.add(new ActionHandler<>(DeleteResourceAction.INSTANCE, DeleteResourceTransportAction.class)); - actions.add(new ActionHandler<>(ShareResourceAction.INSTANCE, ShareResourceTransportAction.class)); - actions.add(new ActionHandler<>(RevokeResourceAccessAction.INSTANCE, RevokeResourceAccessTransportAction.class)); + if (isResourceSharingEnabled) { + 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; } From d5f5b234e07bc9886fb20126152ccfe48ad09153 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Fri, 16 May 2025 15:41:13 -0400 Subject: [PATCH 07/18] Remove unnecessary changes Signed-off-by: Craig Perkins --- .../systemindex/SystemIndexDisabledTests.java | 30 +--------------- .../systemindex/SystemIndexTests.java | 30 +--------------- .../sampleplugin/SystemIndexPlugin2.java | 3 ++ .../security/OpenSearchSecurityPlugin.java | 35 ++++++++++--------- 4 files changed, 23 insertions(+), 75 deletions(-) diff --git a/src/integrationTest/java/org/opensearch/security/systemindex/SystemIndexDisabledTests.java b/src/integrationTest/java/org/opensearch/security/systemindex/SystemIndexDisabledTests.java index 2a0b1f2b5c..109e5228cf 100644 --- a/src/integrationTest/java/org/opensearch/security/systemindex/SystemIndexDisabledTests.java +++ b/src/integrationTest/java/org/opensearch/security/systemindex/SystemIndexDisabledTests.java @@ -18,10 +18,7 @@ import org.junit.Test; import org.junit.runner.RunWith; -import org.opensearch.Version; import org.opensearch.core.rest.RestStatus; -import org.opensearch.plugins.PluginInfo; -import org.opensearch.security.OpenSearchSecurityPlugin; import org.opensearch.security.systemindex.sampleplugin.SystemIndexPlugin1; import org.opensearch.security.systemindex.sampleplugin.SystemIndexPlugin2; import org.opensearch.test.framework.TestSecurityConfig.AuthcDomain; @@ -49,32 +46,7 @@ public class SystemIndexDisabledTests { .anonymousAuth(false) .authc(AUTHC_DOMAIN) .users(USER_ADMIN) - .plugin( - new PluginInfo( - SystemIndexPlugin1.class.getName(), - "classpath plugin", - "NA", - Version.CURRENT, - "1.8", - SystemIndexPlugin1.class.getName(), - null, - List.of(OpenSearchSecurityPlugin.class.getName()), - false - ) - ) - .plugin( - new PluginInfo( - SystemIndexPlugin2.class.getName(), - "classpath plugin", - "NA", - Version.CURRENT, - "1.8", - SystemIndexPlugin2.class.getName(), - null, - List.of(OpenSearchSecurityPlugin.class.getName()), - false - ) - ) + .plugin(SystemIndexPlugin1.class, SystemIndexPlugin2.class) .nodeSettings( Map.of( SECURITY_RESTAPI_ROLES_ENABLED, diff --git a/src/integrationTest/java/org/opensearch/security/systemindex/SystemIndexTests.java b/src/integrationTest/java/org/opensearch/security/systemindex/SystemIndexTests.java index 1882b58f8d..e8fdd9d7d4 100644 --- a/src/integrationTest/java/org/opensearch/security/systemindex/SystemIndexTests.java +++ b/src/integrationTest/java/org/opensearch/security/systemindex/SystemIndexTests.java @@ -19,10 +19,7 @@ import org.junit.Test; import org.junit.runner.RunWith; -import org.opensearch.Version; import org.opensearch.core.rest.RestStatus; -import org.opensearch.plugins.PluginInfo; -import org.opensearch.security.OpenSearchSecurityPlugin; import org.opensearch.security.systemindex.sampleplugin.SystemIndexPlugin1; import org.opensearch.security.systemindex.sampleplugin.SystemIndexPlugin2; import org.opensearch.test.framework.TestSecurityConfig.AuthcDomain; @@ -53,32 +50,7 @@ public class SystemIndexTests { .anonymousAuth(false) .authc(AUTHC_DOMAIN) .users(USER_ADMIN) - .plugin( - new PluginInfo( - SystemIndexPlugin1.class.getName(), - "classpath plugin", - "NA", - Version.CURRENT, - "1.8", - SystemIndexPlugin1.class.getName(), - null, - List.of(OpenSearchSecurityPlugin.class.getName()), - false - ) - ) - .plugin( - new PluginInfo( - SystemIndexPlugin2.class.getName(), - "classpath plugin", - "NA", - Version.CURRENT, - "1.8", - SystemIndexPlugin2.class.getName(), - null, - List.of(OpenSearchSecurityPlugin.class.getName()), - false - ) - ) + .plugin(SystemIndexPlugin1.class, SystemIndexPlugin2.class) .nodeSettings( Map.of( SECURITY_RESTAPI_ROLES_ENABLED, diff --git a/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/SystemIndexPlugin2.java b/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/SystemIndexPlugin2.java index 6cd30b02d6..75358c7daa 100644 --- a/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/SystemIndexPlugin2.java +++ b/src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/SystemIndexPlugin2.java @@ -33,6 +33,8 @@ public class SystemIndexPlugin2 extends Plugin implements SystemIndexPlugin { public static final String SYSTEM_INDEX_2 = ".system-index2"; + private Client client; + @Override public Collection createComponents( Client client, @@ -47,6 +49,7 @@ public Collection createComponents( IndexNameExpressionResolver indexNameExpressionResolver, Supplier repositoriesServiceSupplier ) { + this.client = client; return Collections.emptyList(); } diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java index 78c7e4477e..13abfdafb5 100644 --- a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java +++ b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java @@ -257,8 +257,8 @@ public final class OpenSearchSecurityPlugin extends OpenSearchSecuritySSLPlugin MapperPlugin, IdentityPlugin, // CS-SUPPRESS-SINGLE: RegexpSingleline get Extensions Settings - ExtensiblePlugin, - ExtensionAwarePlugin + ExtensionAwarePlugin, + ExtensiblePlugin // CS-ENFORCE-SINGLE { @@ -2297,21 +2297,6 @@ public Optional getSecureSettingFactory(Settings settings ); } - // CS-SUPPRESS-SINGLE: RegexpSingleline SPI Extensions are unrelated to OpenSearch extensions - @Override - public void loadExtensions(ExtensiblePlugin.ExtensionLoader loader) { - if (settings != null - && settings.getAsBoolean( - FeatureConfigConstants.OPENSEARCH_RESOURCE_SHARING_ENABLED, - FeatureConfigConstants.OPENSEARCH_RESOURCE_SHARING_ENABLED_DEFAULT - )) { - // load all resource-sharing extensions - Set resourceSharingExtensions = new HashSet<>(loader.loadExtensions(ResourceSharingExtension.class)); - resourcePluginInfo.setResourceSharingExtensions(resourceSharingExtensions); - } - } - // CS-ENFORCE-SINGLE - @SuppressWarnings("removal") private void tryAddSecurityProvider() { final SecurityManager sm = System.getSecurityManager(); @@ -2336,6 +2321,22 @@ private void tryAddSecurityProvider() { }); } + // CS-SUPPRESS-SINGLE: RegexpSingleline get Resource Sharing Extensions + @Override + public void loadExtensions(ExtensiblePlugin.ExtensionLoader loader) { + + if (settings != null + && settings.getAsBoolean( + FeatureConfigConstants.OPENSEARCH_RESOURCE_SHARING_ENABLED, + FeatureConfigConstants.OPENSEARCH_RESOURCE_SHARING_ENABLED_DEFAULT + )) { + // load all resource-sharing extensions + Set resourceSharingExtensions = new HashSet<>(loader.loadExtensions(ResourceSharingExtension.class)); + resourcePluginInfo.setResourceSharingExtensions(resourceSharingExtensions); + } + } + // CS-ENFORCE-SINGLE + public static class GuiceHolder implements LifecycleComponent { private static RepositoriesService repositoriesService; From 9e7ed97f122452f29fc7ab42ff53cbc276d340dd Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Fri, 16 May 2025 17:10:57 -0400 Subject: [PATCH 08/18] Handle security disabled Signed-off-by: Craig Perkins --- .../security/OpenSearchSecurityPlugin.java | 40 +++++++++++-------- 1 file changed, 23 insertions(+), 17 deletions(-) diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java index 13abfdafb5..0c8091b1cf 100644 --- a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java +++ b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java @@ -173,6 +173,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.ActionPrivileges; import org.opensearch.security.privileges.PrivilegesEvaluationException; @@ -2261,25 +2262,30 @@ public SecurityTokenManager getTokenManager() { @Override public PluginSubject getPluginSubject(Plugin plugin) { - PluginSubject subject = new ContextProvidingPluginSubject(threadPool, settings, plugin); - String pluginPrincipal = subject.getPrincipal().getName(); - try { - URL resource = plugin.getClass().getClassLoader().getResource("plugin-permissions.yml"); - RoleV7 pluginPermissions; - if (resource == null) { - log.warn("plugin-permissions.yml not found on classpath"); - pluginPermissions = new RoleV7(); - pluginPermissions.setCluster_permissions(new ArrayList<>()); - } else { - try (InputStream in = resource.openStream(); Reader yamlReader = new InputStreamReader(in, StandardCharsets.UTF_8)) { - JsonNode roleJson = DefaultObjectMapper.YAML_MAPPER.readTree(yamlReader); - pluginPermissions = RoleV7.fromJsonNode(roleJson); + PluginSubject subject; + if (!client && !disabled && !SSLConfig.isSslOnlyMode()) { + subject = new ContextProvidingPluginSubject(threadPool, settings, plugin); + String pluginPrincipal = subject.getPrincipal().getName(); + try { + URL resource = plugin.getClass().getClassLoader().getResource("plugin-permissions.yml"); + RoleV7 pluginPermissions; + if (resource == null) { + log.warn("plugin-permissions.yml not found on classpath"); + pluginPermissions = new RoleV7(); + pluginPermissions.setCluster_permissions(new ArrayList<>()); + } else { + try (InputStream in = resource.openStream(); Reader yamlReader = new InputStreamReader(in, StandardCharsets.UTF_8)) { + JsonNode roleJson = DefaultObjectMapper.YAML_MAPPER.readTree(yamlReader); + pluginPermissions = RoleV7.fromJsonNode(roleJson); + } } + pluginPermissions.getCluster_permissions().add(BulkAction.NAME); + sf.updatePluginToPermissions(pluginPrincipal, pluginPermissions); + } catch (IOException e) { + throw new RuntimeException(e); } - pluginPermissions.getCluster_permissions().add(BulkAction.NAME); - sf.updatePluginToPermissions(pluginPrincipal, pluginPermissions); - } catch (IOException e) { - throw new RuntimeException(e); + } else { + subject = new NoopPluginSubject(threadPool); } return subject; } From 12bc531e041b3b0e20264f759ba96c44c2e191bc Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Tue, 20 May 2025 14:39:39 -0400 Subject: [PATCH 09/18] Add more unit tests Signed-off-by: Craig Perkins --- .../security/OpenSearchSecurityPlugin.java | 32 ++--- .../security/securityconf/impl/v7/RoleV7.java | 76 ++++++++---- .../securityconf/impl/v7/RoleV7Test.java | 116 ++++++++++++++++++ src/test/resources/test-role-cluster-only.yml | 2 + src/test/resources/test-role-empty.yml | 1 + src/test/resources/test-role-invalid.yml | 1 + src/test/resources/test-role.yml | 10 ++ 7 files changed, 193 insertions(+), 45 deletions(-) create mode 100644 src/test/java/org/opensearch/security/securityconf/impl/v7/RoleV7Test.java create mode 100644 src/test/resources/test-role-cluster-only.yml create mode 100644 src/test/resources/test-role-empty.yml create mode 100644 src/test/resources/test-role-invalid.yml create mode 100644 src/test/resources/test-role.yml diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java index 0c8091b1cf..86ea9b5717 100644 --- a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java +++ b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java @@ -29,11 +29,7 @@ // CS-SUPPRESS-SINGLE: RegexpSingleline Extensions manager used to allow/disallow TLS connections to extensions import java.io.IOException; -import java.io.InputStream; -import java.io.InputStreamReader; -import java.io.Reader; import java.net.URL; -import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.LinkOption; import java.nio.file.Path; @@ -65,7 +61,6 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.Lists; -import com.fasterxml.jackson.databind.JsonNode; import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -2266,24 +2261,17 @@ public PluginSubject getPluginSubject(Plugin plugin) { if (!client && !disabled && !SSLConfig.isSslOnlyMode()) { subject = new ContextProvidingPluginSubject(threadPool, settings, plugin); String pluginPrincipal = subject.getPrincipal().getName(); - try { - URL resource = plugin.getClass().getClassLoader().getResource("plugin-permissions.yml"); - RoleV7 pluginPermissions; - if (resource == null) { - log.warn("plugin-permissions.yml not found on classpath"); - pluginPermissions = new RoleV7(); - pluginPermissions.setCluster_permissions(new ArrayList<>()); - } else { - try (InputStream in = resource.openStream(); Reader yamlReader = new InputStreamReader(in, StandardCharsets.UTF_8)) { - JsonNode roleJson = DefaultObjectMapper.YAML_MAPPER.readTree(yamlReader); - pluginPermissions = RoleV7.fromJsonNode(roleJson); - } - } - pluginPermissions.getCluster_permissions().add(BulkAction.NAME); - sf.updatePluginToPermissions(pluginPrincipal, pluginPermissions); - } catch (IOException e) { - throw new RuntimeException(e); + URL resource = plugin.getClass().getClassLoader().getResource("plugin-permissions.yml"); + RoleV7 pluginPermissions; + if (resource == null) { + log.warn("plugin-permissions.yml not found on classpath"); + pluginPermissions = new RoleV7(); + pluginPermissions.setCluster_permissions(new ArrayList<>()); + } else { + pluginPermissions = RoleV7.fromYmlFile(resource); } + pluginPermissions.getCluster_permissions().add(BulkAction.NAME); + sf.updatePluginToPermissions(pluginPrincipal, pluginPermissions); } else { subject = new NoopPluginSubject(threadPool); } 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 8da62fafb8..5b68ca60b0 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 @@ -27,15 +27,24 @@ 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.net.URL; +import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Collections; import java.util.Iterator; import java.util.List; +import java.util.Set; import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.node.ArrayNode; +import org.opensearch.OpenSearchSecurityException; +import org.opensearch.security.DefaultObjectMapper; import org.opensearch.security.securityconf.Hideable; import org.opensearch.security.securityconf.StaticDefinable; @@ -54,34 +63,55 @@ public RoleV7() { } - public static RoleV7 fromJsonNode(JsonNode node) { + public static RoleV7 fromYmlFile(URL pluginPermissionsFile) { RoleV7 role = new RoleV7(); - ArrayNode clusterPermsArray = node.withArray("cluster_permissions"); - List clusterPermissions = new ArrayList<>(clusterPermsArray.size()); - for (JsonNode elt : clusterPermsArray) { - clusterPermissions.add(elt.asText()); - } - role.cluster_permissions = clusterPermissions; - role.index_permissions = new ArrayList<>(); - if (node.get("index_permissions") != null) { - for (Iterator it = node.get("index_permissions").elements(); it.hasNext();) { - JsonNode indexNode = it.next(); - Index indexPerm = new Index(); - ArrayNode actionsArray = indexNode.withArray("allowed_actions"); - List allowedActions = new ArrayList<>(actionsArray.size()); - for (JsonNode elt : actionsArray) { - allowedActions.add(elt.asText()); + try (InputStream in = pluginPermissionsFile.openStream(); Reader yamlReader = new InputStreamReader(in, StandardCharsets.UTF_8)) { + JsonNode node = DefaultObjectMapper.YAML_MAPPER.readTree(yamlReader); + if (node.isEmpty()) { + return role; + } + Set validKeys = Set.of("cluster_permissions", "index_permissions"); + Iterator fieldNames = node.fieldNames(); + while (fieldNames.hasNext()) { + String fieldName = fieldNames.next(); + if (!validKeys.contains(fieldName)) { + throw new OpenSearchSecurityException( + "Invalid configuration: unexpected key '" + + fieldName + + "'. Only 'cluster_permissions' and 'index_permissions' are allowed." + ); } - indexPerm.allowed_actions = allowedActions; - ArrayNode indexPatternsArray = indexNode.withArray("index_patterns"); - List indexPatterns = new ArrayList<>(indexPatternsArray.size()); - for (JsonNode elt : indexPatternsArray) { - indexPatterns.add(elt.asText()); + } + ArrayNode clusterPermsArray = node.withArray("cluster_permissions"); + List clusterPermissions = new ArrayList<>(clusterPermsArray.size()); + for (JsonNode elt : clusterPermsArray) { + clusterPermissions.add(elt.asText()); + } + role.cluster_permissions = clusterPermissions; + role.index_permissions = new ArrayList<>(); + if (node.get("index_permissions") != null) { + for (Iterator it = node.get("index_permissions").elements(); it.hasNext();) { + JsonNode indexNode = it.next(); + Index indexPerm = new Index(); + ArrayNode actionsArray = indexNode.withArray("allowed_actions"); + List allowedActions = new ArrayList<>(actionsArray.size()); + for (JsonNode elt : actionsArray) { + allowedActions.add(elt.asText()); + } + indexPerm.allowed_actions = allowedActions; + ArrayNode indexPatternsArray = indexNode.withArray("index_patterns"); + List indexPatterns = new ArrayList<>(indexPatternsArray.size()); + for (JsonNode elt : indexPatternsArray) { + indexPatterns.add(elt.asText()); + } + indexPerm.index_patterns = indexPatterns; + role.index_permissions.add(indexPerm); } - indexPerm.index_patterns = indexPatterns; - role.index_permissions.add(indexPerm); } + } catch (IOException e) { + throw new RuntimeException(e); } + return role; } 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..d1ef9a0b65 --- /dev/null +++ b/src/test/java/org/opensearch/security/securityconf/impl/v7/RoleV7Test.java @@ -0,0 +1,116 @@ +/* + * 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 org.junit.Before; +import org.junit.Test; + +import static org.junit.Assert.*; + +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.fromYmlFile(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.fromYmlFile(emptyYamlUrl); + + // Then + assertNotNull(role); + assertTrue(role.getCluster_permissions().isEmpty()); + assertTrue(role.getIndex_permissions().isEmpty()); + } + + @Test(expected = Exception.class) + public void testFromYmlFileWithInvalidYaml() throws Exception { + // Given an invalid YAML file + URL invalidYamlUrl = RoleV7Test.class.getResource("/test-role-invalid.yml"); + + // When/Then + RoleV7.fromYmlFile(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.fromYmlFile(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..8b13789179 --- /dev/null +++ b/src/test/resources/test-role-empty.yml @@ -0,0 +1 @@ + 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" From 13f54f99ac0fb160f93ae7dce5920f87710d546b Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Tue, 20 May 2025 15:24:16 -0400 Subject: [PATCH 10/18] Handle empty file Signed-off-by: Craig Perkins --- check-permissions-order.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/check-permissions-order.js b/check-permissions-order.js index 934694fc73..b36990303d 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; From 182e9379cea9007101ec7bddf1250ddd2be1803b Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Tue, 20 May 2025 15:30:06 -0400 Subject: [PATCH 11/18] Fix typo in silent Signed-off-by: Craig Perkins --- check-permissions-order.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/check-permissions-order.js b/check-permissions-order.js index b36990303d..6b18a98ec8 100644 --- a/check-permissions-order.js +++ b/check-permissions-order.js @@ -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.`) } } From a5f90fedda0d12d701eb9b6a6a8abafab28f846e Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Tue, 20 May 2025 15:50:32 -0400 Subject: [PATCH 12/18] Fix import Signed-off-by: Craig Perkins --- .../security/securityconf/impl/v7/RoleV7Test.java | 8 ++++++-- src/test/resources/test-role-empty.yml | 3 ++- 2 files changed, 8 insertions(+), 3 deletions(-) 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 index d1ef9a0b65..b8a062e9fa 100644 --- a/src/test/java/org/opensearch/security/securityconf/impl/v7/RoleV7Test.java +++ b/src/test/java/org/opensearch/security/securityconf/impl/v7/RoleV7Test.java @@ -16,7 +16,11 @@ import org.junit.Before; import org.junit.Test; -import static org.junit.Assert.*; +import org.opensearch.OpenSearchSecurityException; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; public class RoleV7Test { @@ -84,7 +88,7 @@ public void testFromYmlFileWithEmptyPermissions() throws Exception { assertTrue(role.getIndex_permissions().isEmpty()); } - @Test(expected = Exception.class) + @Test(expected = OpenSearchSecurityException.class) public void testFromYmlFileWithInvalidYaml() throws Exception { // Given an invalid YAML file URL invalidYamlUrl = RoleV7Test.class.getResource("/test-role-invalid.yml"); diff --git a/src/test/resources/test-role-empty.yml b/src/test/resources/test-role-empty.yml index 8b13789179..0c25139ec9 100644 --- a/src/test/resources/test-role-empty.yml +++ b/src/test/resources/test-role-empty.yml @@ -1 +1,2 @@ - +cluster_permissions: [] +index_permissions: [] From 46b4fb5f2f4a1371ce4422f53d3584c3e94db85c Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Mon, 26 May 2025 12:52:09 -0400 Subject: [PATCH 13/18] Address code review comments Signed-off-by: Craig Perkins --- .../privileges/ActionPrivilegesTest.java | 17 ++-- .../security/OpenSearchSecurityPlugin.java | 6 +- .../security/securityconf/impl/v7/RoleV7.java | 89 ++++++++----------- 3 files changed, 52 insertions(+), 60 deletions(-) diff --git a/src/integrationTest/java/org/opensearch/security/privileges/ActionPrivilegesTest.java b/src/integrationTest/java/org/opensearch/security/privileges/ActionPrivilegesTest.java index a7537839e2..4fc57efee3 100644 --- a/src/integrationTest/java/org/opensearch/security/privileges/ActionPrivilegesTest.java +++ b/src/integrationTest/java/org/opensearch/security/privileges/ActionPrivilegesTest.java @@ -73,14 +73,15 @@ ActionPrivilegesTest.StatefulIndexPrivilegesHeapSize.class }) public class ActionPrivilegesTest { // TODO Create unlimited role statically here - private static final RoleV7 UNLIMITED_ROLE = new RoleV7(); - private static final RoleV7.Index UNLIMITED_INDEX = new RoleV7.Index(); - static { - UNLIMITED_INDEX.setIndex_patterns(List.of("*")); - UNLIMITED_INDEX.setAllowed_actions(List.of("*")); - UNLIMITED_ROLE.setCluster_permissions(List.of("*")); - UNLIMITED_ROLE.setIndex_permissions(List.of(UNLIMITED_INDEX)); - } + private static final RoleV7 UNLIMITED_ROLE = RoleV7.fromYamlStringUnchecked(""" + cluster_permissions: + - "*" + index_permissions: + - index_patterns: + - "*" + allowed_actions: + - "*" + """); public static class ClusterPrivileges { @Test diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java index 86ea9b5717..512ff8560d 100644 --- a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java +++ b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java @@ -2268,7 +2268,11 @@ public PluginSubject getPluginSubject(Plugin plugin) { pluginPermissions = new RoleV7(); pluginPermissions.setCluster_permissions(new ArrayList<>()); } else { - pluginPermissions = RoleV7.fromYmlFile(resource); + try { + pluginPermissions = RoleV7.fromPluginPermissionsFile(resource); + } catch (IOException e) { + throw new OpenSearchSecurityException(e.getMessage(), e); + } } pluginPermissions.getCluster_permissions().add(BulkAction.NAME); sf.updatePluginToPermissions(pluginPrincipal, pluginPermissions); 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 5b68ca60b0..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 @@ -31,19 +31,15 @@ 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.ArrayList; import java.util.Collections; -import java.util.Iterator; import java.util.List; -import java.util.Set; import com.fasterxml.jackson.annotation.JsonProperty; -import com.fasterxml.jackson.databind.JsonNode; -import com.fasterxml.jackson.databind.node.ArrayNode; -import org.opensearch.OpenSearchSecurityException; +import org.opensearch.OpenSearchCorruptionException; import org.opensearch.security.DefaultObjectMapper; import org.opensearch.security.securityconf.Hideable; import org.opensearch.security.securityconf.StaticDefinable; @@ -63,54 +59,45 @@ public RoleV7() { } - public static RoleV7 fromYmlFile(URL pluginPermissionsFile) { - RoleV7 role = new RoleV7(); - try (InputStream in = pluginPermissionsFile.openStream(); Reader yamlReader = new InputStreamReader(in, StandardCharsets.UTF_8)) { - JsonNode node = DefaultObjectMapper.YAML_MAPPER.readTree(yamlReader); - if (node.isEmpty()) { - return role; - } - Set validKeys = Set.of("cluster_permissions", "index_permissions"); - Iterator fieldNames = node.fieldNames(); - while (fieldNames.hasNext()) { - String fieldName = fieldNames.next(); - if (!validKeys.contains(fieldName)) { - throw new OpenSearchSecurityException( - "Invalid configuration: unexpected key '" - + fieldName - + "'. Only 'cluster_permissions' and 'index_permissions' are allowed." - ); - } - } - ArrayNode clusterPermsArray = node.withArray("cluster_permissions"); - List clusterPermissions = new ArrayList<>(clusterPermsArray.size()); - for (JsonNode elt : clusterPermsArray) { - clusterPermissions.add(elt.asText()); - } - role.cluster_permissions = clusterPermissions; - role.index_permissions = new ArrayList<>(); - if (node.get("index_permissions") != null) { - for (Iterator it = node.get("index_permissions").elements(); it.hasNext();) { - JsonNode indexNode = it.next(); - Index indexPerm = new Index(); - ArrayNode actionsArray = indexNode.withArray("allowed_actions"); - List allowedActions = new ArrayList<>(actionsArray.size()); - for (JsonNode elt : actionsArray) { - allowedActions.add(elt.asText()); - } - indexPerm.allowed_actions = allowedActions; - ArrayNode indexPatternsArray = indexNode.withArray("index_patterns"); - List indexPatterns = new ArrayList<>(indexPatternsArray.size()); - for (JsonNode elt : indexPatternsArray) { - indexPatterns.add(elt.asText()); - } - indexPerm.index_patterns = indexPatterns; - role.index_permissions.add(indexPerm); - } - } + public static RoleV7 fromYamlString(String yamlString) throws IOException { + try (Reader yamlReader = new StringReader(yamlString)) { + return fromYaml(yamlReader); + } + } + + /** + * Converts any validation error exceptions into runtime exceptions. Only use when you are sure that is safe; + * useful for tests. + */ + public static RoleV7 fromYamlStringUnchecked(String 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); + } + + /** + * Does additional validations regarding limitiations of plugin permissions files. + */ + public static RoleV7 fromPluginPermissionsFile(URL pluginPermissionsFile) throws IOException { + RoleV7 role = fromYaml(pluginPermissionsFile); + + 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; } From 562e9bec0c63cf9e8fb3a83ff2dddc01f7b7bb99 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Mon, 26 May 2025 12:58:28 -0400 Subject: [PATCH 14/18] Address more comments Signed-off-by: Craig Perkins --- .../opensearch/sample/secure/SecurePluginTests.java | 11 +---------- .../security/privileges/ActionPrivileges.java | 8 +------- .../security/privileges/PrivilegesEvaluator.java | 3 +++ 3 files changed, 5 insertions(+), 17 deletions(-) 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 index 0aabca307e..34d10fcf05 100644 --- 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 @@ -33,9 +33,7 @@ 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_RESTAPI_ROLES_ENABLED; import static org.opensearch.security.support.ConfigConstants.SECURITY_SYSTEM_INDICES_ENABLED_KEY; -import static org.opensearch.test.framework.TestSecurityConfig.Role.ALL_ACCESS; import static org.opensearch.test.framework.TestSecurityConfig.User.USER_ADMIN; @RunWith(com.carrotsearch.randomizedtesting.RandomizedRunner.class) @@ -63,14 +61,7 @@ public class SecurePluginTests { false ) ) - .nodeSettings( - Map.of( - SECURITY_RESTAPI_ROLES_ENABLED, - List.of("user_" + USER_ADMIN.getName() + "__" + ALL_ACCESS.getName()), - SECURITY_SYSTEM_INDICES_ENABLED_KEY, - true - ) - ) + .nodeSettings(Map.of(SECURITY_SYSTEM_INDICES_ENABLED_KEY, true)) .build(); @Test diff --git a/src/main/java/org/opensearch/security/privileges/ActionPrivileges.java b/src/main/java/org/opensearch/security/privileges/ActionPrivileges.java index cdde7f7019..1e4dcc6ff9 100644 --- a/src/main/java/org/opensearch/security/privileges/ActionPrivileges.java +++ b/src/main/java/org/opensearch/security/privileges/ActionPrivileges.java @@ -14,7 +14,6 @@ import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; -import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -759,13 +758,8 @@ PrivilegesEvaluatorResponse providesPrivilege( Map indexMetadata ) { List exceptions = new ArrayList<>(); - Set rolesToCheck = context.getMappedRoles(); - if (context.getUser().isPluginUser()) { - rolesToCheck = new HashSet<>(rolesToCheck); - rolesToCheck.add(context.getUser().getName()); - } - for (String role : rolesToCheck) { + for (String role : context.getMappedRoles()) { ImmutableMap actionToIndexPattern = this.rolesToActionToIndexPattern.get(role); if (actionToIndexPattern != null) { diff --git a/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java b/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java index 42b540e7a2..7c2cd0f261 100644 --- a/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java +++ b/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java @@ -593,6 +593,9 @@ public PrivilegesEvaluatorResponse evaluate(PrivilegesEvaluationContext context) } public Set mapRoles(final User user, final TransportAddress caller) { + if (user.isPluginUser()) { + return Set.of(user.getName()); + } return this.configModel.mapSecurityRoles(user, caller); } From 2857354e19af0d44a8c6edbe02245425e723e451 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Tue, 27 May 2025 09:48:36 -0400 Subject: [PATCH 15/18] Fix tests Signed-off-by: Craig Perkins --- .../security/securityconf/impl/v7/RoleV7Test.java | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) 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 index b8a062e9fa..335b93e4b5 100644 --- a/src/test/java/org/opensearch/security/securityconf/impl/v7/RoleV7Test.java +++ b/src/test/java/org/opensearch/security/securityconf/impl/v7/RoleV7Test.java @@ -13,11 +13,10 @@ import java.net.URL; +import com.fasterxml.jackson.databind.exc.UnrecognizedPropertyException; import org.junit.Before; import org.junit.Test; -import org.opensearch.OpenSearchSecurityException; - import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; @@ -50,7 +49,7 @@ public void testFromYmlFileWithValidInput() throws Exception { */ // When - RoleV7 role = RoleV7.fromYmlFile(testYamlUrl); + RoleV7 role = RoleV7.fromPluginPermissionsFile(testYamlUrl); // Then assertNotNull(role); @@ -80,7 +79,7 @@ public void testFromYmlFileWithEmptyPermissions() throws Exception { URL emptyYamlUrl = RoleV7Test.class.getResource("/test-role-empty.yml"); // When - RoleV7 role = RoleV7.fromYmlFile(emptyYamlUrl); + RoleV7 role = RoleV7.fromPluginPermissionsFile(emptyYamlUrl); // Then assertNotNull(role); @@ -88,13 +87,13 @@ public void testFromYmlFileWithEmptyPermissions() throws Exception { assertTrue(role.getIndex_permissions().isEmpty()); } - @Test(expected = OpenSearchSecurityException.class) + @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.fromYmlFile(invalidYamlUrl); // Should throw an exception + RoleV7.fromPluginPermissionsFile(invalidYamlUrl); // Should throw an exception } @Test @@ -109,7 +108,7 @@ public void testFromYmlFileWithMissingIndexPermissions() throws Exception { URL clusterOnlyYamlUrl = RoleV7Test.class.getResource("/test-role-cluster-only.yml"); // When - RoleV7 role = RoleV7.fromYmlFile(clusterOnlyYamlUrl); + RoleV7 role = RoleV7.fromPluginPermissionsFile(clusterOnlyYamlUrl); // Then assertNotNull(role); From 1bbc0e3c7beb441d290fab9b59a0d0ea0e8b1902 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Thu, 12 Jun 2025 10:56:45 -0400 Subject: [PATCH 16/18] Address review feedback Signed-off-by: Craig Perkins --- .../java/org/opensearch/security/OpenSearchSecurityPlugin.java | 2 +- .../opensearch/security/privileges/PrivilegesEvaluator.java | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java index 1cd5e86e80..c1f9d81220 100644 --- a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java +++ b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java @@ -2271,7 +2271,7 @@ public PluginSubject getPluginSubject(Plugin plugin) { URL resource = plugin.getClass().getClassLoader().getResource("plugin-permissions.yml"); RoleV7 pluginPermissions; if (resource == null) { - log.warn("plugin-permissions.yml not found on classpath"); + log.info("plugin-permissions.yml not found on classpath for plugin {}, using empty permissions", pluginPrincipal); pluginPermissions = new RoleV7(); pluginPermissions.setCluster_permissions(new ArrayList<>()); } else { diff --git a/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java b/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java index 0ff396fddc..af5c3c3124 100644 --- a/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java +++ b/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java @@ -629,9 +629,6 @@ public PrivilegesEvaluatorResponse evaluate(PrivilegesEvaluationContext context) } public Set mapRoles(final User user, final TransportAddress caller) { - if (user.isPluginUser()) { - return Set.of(user.getName()); - } return this.configModel.mapSecurityRoles(user, caller); } From 3edcdd8dfb0ad8440b7b6724dba8f332e26b9e4a Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Tue, 17 Jun 2025 15:26:41 -0400 Subject: [PATCH 17/18] Rename to plugin-additional-permissions.yml Signed-off-by: Craig Perkins --- .../org/opensearch/sample/utils/RunAsSubjectClient.java | 2 ++ ...n-permissions.yml => plugin-additional-permissions.yml} | 0 .../systemindex/sampleplugin/RunAsSubjectClient.java | 2 ++ .../org/opensearch/security/OpenSearchSecurityPlugin.java | 7 +++++-- 4 files changed, 9 insertions(+), 2 deletions(-) rename sample-resource-plugin/src/main/resources/{plugin-permissions.yml => plugin-additional-permissions.yml} (100%) 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 index 149843eb54..2239f7c0a0 100644 --- 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 @@ -58,6 +58,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/sample-resource-plugin/src/main/resources/plugin-permissions.yml b/sample-resource-plugin/src/main/resources/plugin-additional-permissions.yml similarity index 100% rename from sample-resource-plugin/src/main/resources/plugin-permissions.yml rename to sample-resource-plugin/src/main/resources/plugin-additional-permissions.yml 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 c1f9d81220..c6d0dc52e7 100644 --- a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java +++ b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java @@ -2268,10 +2268,13 @@ public PluginSubject getPluginSubject(Plugin plugin) { if (!client && !disabled && !SSLConfig.isSslOnlyMode()) { subject = new ContextProvidingPluginSubject(threadPool, settings, plugin); String pluginPrincipal = subject.getPrincipal().getName(); - URL resource = plugin.getClass().getClassLoader().getResource("plugin-permissions.yml"); + URL resource = plugin.getClass().getClassLoader().getResource("plugin-additional-permissions.yml"); RoleV7 pluginPermissions; if (resource == null) { - log.info("plugin-permissions.yml not found on classpath for plugin {}, using empty permissions", pluginPrincipal); + 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 { From c677d0b06c125ab7ee09099cc8438296d67799c6 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Tue, 17 Jun 2025 17:45:57 -0400 Subject: [PATCH 18/18] Assert that index is created Signed-off-by: Craig Perkins --- .../java/org/opensearch/sample/secure/SecurePluginTests.java | 2 ++ 1 file changed, 2 insertions(+) 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 index 34d10fcf05..c823ca0caa 100644 --- 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 @@ -89,6 +89,8 @@ public void testRunCreateIndexWithPluginSubject() { """); assertThat(response.getStatusCode(), equalTo(RestStatus.OK.getStatus())); + HttpResponse catIndicesResponse = client.get("_cat/indices"); + assertThat(catIndicesResponse.getBody(), containsString("test-index")); } } }