From d511b5f320c565118fcdbb52d96c4133212906fc Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Thu, 9 Jan 2025 21:51:32 -0500 Subject: [PATCH] Change share_with data structure and rename to SharableResourceExtension Signed-off-by: Craig Perkins --- .../SampleExtensionPluginTests.java | 2 +- .../SampleExtensionPlugin.java | 4 +-- ...ch.security.spi.SharableResourceExtension} | 0 .../SampleExtensionPluginIT.java | 2 +- .../security/spi/ResourceSharingInfo.java | 33 ++++++++++++------- ...on.java => SharableResourceExtension.java} | 2 +- .../security/OpenSearchSecurityPlugin.java | 6 ++-- .../rest/api/SecurityApiDependencies.java | 8 ++--- .../dlic/rest/api/SecurityRestApiActions.java | 4 +-- .../security/rest/resource/ShareWith.java | 20 +++++------ .../rest/resource/ShareWithRestAction.java | 10 +++--- .../resource/ShareWithTransportAction.java | 9 ++--- 12 files changed, 54 insertions(+), 46 deletions(-) rename sample-extension-plugin/src/main/resources/META-INF/services/{org.opensearch.security.spi.ResourceSharingExtension => org.opensearch.security.spi.SharableResourceExtension} (100%) rename spi/src/main/java/org/opensearch/security/spi/{ResourceSharingExtension.java => SharableResourceExtension.java} (91%) diff --git a/sample-extension-plugin/src/integrationTest/java/org/opensearch/security/sampleextension/SampleExtensionPluginTests.java b/sample-extension-plugin/src/integrationTest/java/org/opensearch/security/sampleextension/SampleExtensionPluginTests.java index e261604679..c0338e508a 100644 --- a/sample-extension-plugin/src/integrationTest/java/org/opensearch/security/sampleextension/SampleExtensionPluginTests.java +++ b/sample-extension-plugin/src/integrationTest/java/org/opensearch/security/sampleextension/SampleExtensionPluginTests.java @@ -106,7 +106,7 @@ public void testCreateAndUpdateOwnSampleResource() throws Exception { } try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) { - String shareWithPayload = "{\"share_with\":{\"allowed_actions\": [\"unlimited\"], \"users\": [\"" + String shareWithPayload = "{\"share_with\":{\"action_group\": \"unlimited\", \"users\": [\"" + SHARED_WITH_USER.getName() + "\"], \"backend_roles\": []}}"; HttpResponse shareWithResponse = client.putJson( diff --git a/sample-extension-plugin/src/main/java/org/opensearch/security/sampleextension/SampleExtensionPlugin.java b/sample-extension-plugin/src/main/java/org/opensearch/security/sampleextension/SampleExtensionPlugin.java index 580a8c06bc..3271f1d7ea 100644 --- a/sample-extension-plugin/src/main/java/org/opensearch/security/sampleextension/SampleExtensionPlugin.java +++ b/sample-extension-plugin/src/main/java/org/opensearch/security/sampleextension/SampleExtensionPlugin.java @@ -50,7 +50,7 @@ import org.opensearch.security.sampleextension.actions.update.UpdateSampleResourceAction; import org.opensearch.security.sampleextension.actions.update.UpdateSampleResourceRestAction; import org.opensearch.security.sampleextension.actions.update.UpdateSampleResourceTransportAction; -import org.opensearch.security.spi.ResourceSharingExtension; +import org.opensearch.security.spi.SharableResourceExtension; import org.opensearch.threadpool.ThreadPool; import org.opensearch.watcher.ResourceWatcherService; @@ -60,7 +60,7 @@ * It use ".sample_extension_resources" index to manage its resources, and exposes a REST API * */ -public class SampleExtensionPlugin extends Plugin implements ActionPlugin, SystemIndexPlugin, ResourceSharingExtension { +public class SampleExtensionPlugin extends Plugin implements ActionPlugin, SystemIndexPlugin, SharableResourceExtension { private static final Logger log = LogManager.getLogger(SampleExtensionPlugin.class); public static final String RESOURCE_INDEX_NAME = ".sample_extension_resources"; diff --git a/sample-extension-plugin/src/main/resources/META-INF/services/org.opensearch.security.spi.ResourceSharingExtension b/sample-extension-plugin/src/main/resources/META-INF/services/org.opensearch.security.spi.SharableResourceExtension similarity index 100% rename from sample-extension-plugin/src/main/resources/META-INF/services/org.opensearch.security.spi.ResourceSharingExtension rename to sample-extension-plugin/src/main/resources/META-INF/services/org.opensearch.security.spi.SharableResourceExtension diff --git a/sample-extension-plugin/src/test/java/org/opensearch/security/sampleextension/SampleExtensionPluginIT.java b/sample-extension-plugin/src/test/java/org/opensearch/security/sampleextension/SampleExtensionPluginIT.java index 0dfaf1cd35..c6105b9075 100644 --- a/sample-extension-plugin/src/test/java/org/opensearch/security/sampleextension/SampleExtensionPluginIT.java +++ b/sample-extension-plugin/src/test/java/org/opensearch/security/sampleextension/SampleExtensionPluginIT.java @@ -168,7 +168,7 @@ public void testCreateSampleResource() throws IOException, InterruptedException Map updateSharingResponse = updateSharing( resourceId, - "{\"share_with\":{\"users\": [\"admin\"], \"backend_roles\": [], \"allowed_actions\": [\"*\"]}}", + "{\"share_with\":{\"users\": [\"admin\"], \"backend_roles\": [], \"action_group\": \"unlimited\"}}", Optional.of(Tuple.tuple("testuser", strongPassword)) ); System.out.println("updateSharingResponse: " + updateSharingResponse); diff --git a/spi/src/main/java/org/opensearch/security/spi/ResourceSharingInfo.java b/spi/src/main/java/org/opensearch/security/spi/ResourceSharingInfo.java index 04f4fb4078..866d1a9ece 100644 --- a/spi/src/main/java/org/opensearch/security/spi/ResourceSharingInfo.java +++ b/spi/src/main/java/org/opensearch/security/spi/ResourceSharingInfo.java @@ -2,7 +2,8 @@ import java.io.IOException; import java.util.Collections; -import java.util.List; +import java.util.HashMap; +import java.util.Map; import org.opensearch.core.ParseField; import org.opensearch.core.xcontent.ConstructingObjectParser; @@ -15,14 +16,14 @@ */ public class ResourceSharingInfo { private final ResourceUser resourceUser; - private final List shareWith; + private final Map shareWith; public ResourceSharingInfo(ResourceUser resourceUser) { this.resourceUser = resourceUser; - this.shareWith = Collections.emptyList(); + this.shareWith = Collections.emptyMap(); } - public ResourceSharingInfo(ResourceUser resourceUser, List shareWith) { + public ResourceSharingInfo(ResourceUser resourceUser, Map shareWith) { this.resourceUser = resourceUser; this.shareWith = shareWith; } @@ -31,7 +32,7 @@ public ResourceUser getResourceUser() { return resourceUser; } - public List getShareWith() { + public Map getShareWith() { return shareWith; } @@ -39,7 +40,7 @@ public List getShareWith() { private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>( "resource_sharing_info", true, - a -> new ResourceSharingInfo((ResourceUser) a[0], (List) a[1]) + a -> new ResourceSharingInfo((ResourceUser) a[0], (Map) a[1]) ); static { @@ -48,11 +49,21 @@ public List getShareWith() { (p, c) -> ResourceUser.fromXContent(p), new ParseField("resource_user") ); - PARSER.declareObjectArray( - ConstructingObjectParser.optionalConstructorArg(), - (p, c) -> ShareWith.fromXContent(p), - new ParseField("share_with") - ); + PARSER.declareObject(ConstructingObjectParser.optionalConstructorArg(), (p, c) -> { + Map shareWithMap = new HashMap<>(); + String fieldName; + while ((fieldName = p.currentName()) != null) { + if (p.nextToken() == XContentParser.Token.START_OBJECT) { + shareWithMap.put(fieldName, ShareWith.fromXContent(p)); + } + } + return shareWithMap; + }, new ParseField("share_with")); + // PARSER.declareObjectArray( + // ConstructingObjectParser.optionalConstructorArg(), + // (p, c) -> ShareWith.fromXContent(p), + // new ParseField("share_with") + // ); } public static ResourceSharingInfo parse(XContentParser parser) throws IOException { diff --git a/spi/src/main/java/org/opensearch/security/spi/ResourceSharingExtension.java b/spi/src/main/java/org/opensearch/security/spi/SharableResourceExtension.java similarity index 91% rename from spi/src/main/java/org/opensearch/security/spi/ResourceSharingExtension.java rename to spi/src/main/java/org/opensearch/security/spi/SharableResourceExtension.java index cddc9bfc7e..b9a1bf3d9f 100644 --- a/spi/src/main/java/org/opensearch/security/spi/ResourceSharingExtension.java +++ b/spi/src/main/java/org/opensearch/security/spi/SharableResourceExtension.java @@ -11,7 +11,7 @@ /** * SPI of security. */ -public interface ResourceSharingExtension { +public interface SharableResourceExtension { /** * @return resource type string. */ diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java index dd80854cbe..b14976c49c 100644 --- a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java +++ b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java @@ -188,7 +188,7 @@ import org.opensearch.security.securityconf.impl.CType; import org.opensearch.security.setting.OpensearchDynamicSetting; import org.opensearch.security.setting.TransportPassiveAuthSetting; -import org.opensearch.security.spi.ResourceSharingExtension; +import org.opensearch.security.spi.SharableResourceExtension; import org.opensearch.security.ssl.ExternalSecurityKeyStore; import org.opensearch.security.ssl.OpenSearchSecureSettingsFactory; import org.opensearch.security.ssl.OpenSearchSecuritySSLPlugin; @@ -281,7 +281,7 @@ public final class OpenSearchSecurityPlugin extends OpenSearchSecuritySSLPlugin private volatile DlsFlsBaseContext dlsFlsBaseContext; private final Set sharableResourceIndices = new HashSet<>(); // CS-SUPPRESS-SINGLE: RegexpSingleline SPI Extensions are unrelated to OpenSearch extensions - private final List resourceSharingExtensions = new ArrayList<>(); + private final List resourceSharingExtensions = new ArrayList<>(); // CS-ENFORCE-SINGLE public static boolean isActionTraceEnabled() { @@ -2200,7 +2200,7 @@ public Optional getSecureSettingFactory(Settings settings @Override public void loadExtensions(ExtensiblePlugin.ExtensionLoader loader) { System.out.println("loadExtensions"); - for (ResourceSharingExtension extension : loader.loadExtensions(ResourceSharingExtension.class)) { + for (SharableResourceExtension extension : loader.loadExtensions(SharableResourceExtension.class)) { String resourceIndexName = extension.getResourceIndex(); System.out.println("localClient: " + localClient); this.sharableResourceIndices.add(resourceIndexName); diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/SecurityApiDependencies.java b/src/main/java/org/opensearch/security/dlic/rest/api/SecurityApiDependencies.java index 7d3be960b3..49b3949c9b 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/api/SecurityApiDependencies.java +++ b/src/main/java/org/opensearch/security/dlic/rest/api/SecurityApiDependencies.java @@ -18,7 +18,7 @@ import org.opensearch.security.configuration.AdminDNs; import org.opensearch.security.configuration.ConfigurationRepository; import org.opensearch.security.privileges.PrivilegesEvaluator; -import org.opensearch.security.spi.ResourceSharingExtension; +import org.opensearch.security.spi.SharableResourceExtension; import org.opensearch.security.support.ConfigConstants; public class SecurityApiDependencies { @@ -28,7 +28,7 @@ public class SecurityApiDependencies { private final RestApiAdminPrivilegesEvaluator restApiAdminPrivilegesEvaluator; private final AuditLog auditLog; private final Settings settings; - private final List resourceSharingExtensions; + private final List resourceSharingExtensions; private final PrivilegesEvaluator privilegesEvaluator; @@ -40,7 +40,7 @@ public SecurityApiDependencies( final RestApiAdminPrivilegesEvaluator restApiAdminPrivilegesEvaluator, final AuditLog auditLog, final Settings settings, - final List resourceSharingExtensions + final List resourceSharingExtensions ) { this.adminDNs = adminDNs; this.configurationRepository = configurationRepository; @@ -72,7 +72,7 @@ public RestApiAdminPrivilegesEvaluator restApiAdminPrivilegesEvaluator() { return restApiAdminPrivilegesEvaluator; } - public List resourceSharingExtensions() { + public List resourceSharingExtensions() { return resourceSharingExtensions; } diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/SecurityRestApiActions.java b/src/main/java/org/opensearch/security/dlic/rest/api/SecurityRestApiActions.java index d8c5402f81..c15bacb50e 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/api/SecurityRestApiActions.java +++ b/src/main/java/org/opensearch/security/dlic/rest/api/SecurityRestApiActions.java @@ -26,7 +26,7 @@ import org.opensearch.security.hasher.PasswordHasher; import org.opensearch.security.privileges.PrivilegesEvaluator; import org.opensearch.security.rest.resource.ShareWithRestAction; -import org.opensearch.security.spi.ResourceSharingExtension; +import org.opensearch.security.spi.SharableResourceExtension; import org.opensearch.security.ssl.SslSettingsManager; import org.opensearch.security.ssl.transport.PrincipalExtractor; import org.opensearch.security.user.UserService; @@ -52,7 +52,7 @@ public static Collection getHandler( final UserService userService, final boolean certificatesReloadEnabled, final PasswordHasher passwordHasher, - final List resourceSharingExtensions + final List resourceSharingExtensions ) { final var securityApiDependencies = new SecurityApiDependencies( adminDns, diff --git a/src/main/java/org/opensearch/security/rest/resource/ShareWith.java b/src/main/java/org/opensearch/security/rest/resource/ShareWith.java index 09bd66dbb5..3782e5aaeb 100644 --- a/src/main/java/org/opensearch/security/rest/resource/ShareWith.java +++ b/src/main/java/org/opensearch/security/rest/resource/ShareWith.java @@ -19,27 +19,27 @@ public class ShareWith implements NamedWriteable, ToXContentFragment { - public final static ShareWith PRIVATE = new ShareWith(List.of("unlimited"), List.of(), List.of()); - public final static ShareWith PUBLIC = new ShareWith(List.of("unlimited"), List.of("*"), List.of("*")); + public final static ShareWith PRIVATE = new ShareWith("unlimited", List.of(), List.of()); + public final static ShareWith PUBLIC = new ShareWith("unlimited", List.of("*"), List.of("*")); - private final List allowedActions; + private final String actionGroup; private final List users; private final List backendRoles; - public ShareWith(List allowedActions, List users, List backendRoles) { - this.allowedActions = allowedActions; + public ShareWith(String actionGroup, List users, List backendRoles) { + this.actionGroup = actionGroup; this.users = users; this.backendRoles = backendRoles; } public ShareWith(StreamInput in) throws IOException { - this.allowedActions = in.readStringList(); + this.actionGroup = in.readString(); this.users = in.readStringList(); this.backendRoles = in.readStringList(); } - public List getAllowedActions() { - return allowedActions; + public String getActionGroup() { + return actionGroup; } public List getUsers() { @@ -53,7 +53,7 @@ public List getBackendRoles() { @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { return builder.startObject() - .field("allowedActions", allowedActions) + .field("action_group", actionGroup) .field("users", users) .field("backend_roles", backendRoles) .endObject(); @@ -66,7 +66,7 @@ public String getWriteableName() { @Override public void writeTo(StreamOutput out) throws IOException { - out.writeStringCollection(allowedActions); + out.writeString(actionGroup); out.writeStringCollection(users); out.writeStringCollection(backendRoles); } diff --git a/src/main/java/org/opensearch/security/rest/resource/ShareWithRestAction.java b/src/main/java/org/opensearch/security/rest/resource/ShareWithRestAction.java index f25502a3c8..929208ce73 100644 --- a/src/main/java/org/opensearch/security/rest/resource/ShareWithRestAction.java +++ b/src/main/java/org/opensearch/security/rest/resource/ShareWithRestAction.java @@ -20,7 +20,7 @@ import org.opensearch.rest.BaseRestHandler; import org.opensearch.rest.RestRequest; import org.opensearch.rest.action.RestToXContentListener; -import org.opensearch.security.spi.ResourceSharingExtension; +import org.opensearch.security.spi.SharableResourceExtension; import static org.opensearch.rest.RestRequest.Method.PUT; import static org.opensearch.security.dlic.rest.support.Utils.addRoutesPrefix; @@ -29,9 +29,9 @@ public class ShareWithRestAction extends BaseRestHandler { private final Map resourceTypeToIndexMap = new HashMap<>(); - public ShareWithRestAction(final List resourceSharingExtensions) { - if (resourceSharingExtensions != null) { - for (ResourceSharingExtension resourceSharingExtension : resourceSharingExtensions) { + public ShareWithRestAction(final List sharableResourceExtensions) { + if (sharableResourceExtensions != null) { + for (SharableResourceExtension resourceSharingExtension : sharableResourceExtensions) { resourceTypeToIndexMap.put(resourceSharingExtension.getResourceType(), resourceSharingExtension.getResourceIndex()); } } @@ -71,7 +71,7 @@ public RestChannelConsumer prepareRequest(RestRequest request, NodeClient client Map shareWithMap = (Map) source.get("share_with"); ShareWith shareWith = new ShareWith( - (List) shareWithMap.get("allowed_actions"), + (String) shareWithMap.get("action_group"), (List) shareWithMap.get("users"), (List) shareWithMap.get("backend_roles") ); diff --git a/src/main/java/org/opensearch/security/rest/resource/ShareWithTransportAction.java b/src/main/java/org/opensearch/security/rest/resource/ShareWithTransportAction.java index 84f8baa082..2873a99afc 100644 --- a/src/main/java/org/opensearch/security/rest/resource/ShareWithTransportAction.java +++ b/src/main/java/org/opensearch/security/rest/resource/ShareWithTransportAction.java @@ -35,8 +35,6 @@ public class ShareWithTransportAction extends HandledTransportAction { private static final Logger log = LogManager.getLogger(ShareWithTransportAction.class); - public static final String RESOURCE_SHARING_INDEX = ".resource-sharing"; - private final TransportService transportService; private final Client nodeClient; @@ -64,15 +62,14 @@ public void onResponse(GetResponse getResponse) { XContentBuilder builder = XContentFactory.jsonBuilder(); builder.startObject(); { - builder.startArray("share_with"); + builder.startObject("share_with"); { - builder.startObject(); - builder.field("allowed_actions", request.getShareWith().getAllowedActions()); + builder.startObject(request.getShareWith().getActionGroup()); builder.field("users", request.getShareWith().getUsers()); builder.field("backend_roles", request.getShareWith().getBackendRoles()); builder.endObject(); } - builder.endArray(); + builder.endObject(); } builder.endObject(); updateRequest.doc(builder);