[Resource Access Control] [Part1] Introduces SPI for resource access control #5185

DarshitChanpura · 2025-03-17T19:57:20Z

#5016 is being broken down into smaller pieces. This is part 1.

Description

Introduces an SPI to implement Resource Access Control in OpenSearch. This will allow plugins to declare itself as Resource Plugins to leverage the access control methods to be provided by Security plugin.

Category - New feature
Why these changes are required?
- At present, plugins have implemented in-house authorization mechanisms for a lack of centralized framework. This PR introduces a centralized way to offload resource permissions check to security plugin.

Issues Resolved

Related to #4500

Testing

automated tests

Check List

~~- [ ] New functionality includes testing~~
~~- [ ] New functionality has been documented~~
~- [ ] New Roles/Permissions have a corresponding security dashboards plugin PR
~~- [ ] API changes companion pull request created~~

Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

codecov · 2025-03-17T21:06:47Z

Codecov Report

Attention: Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.

Project coverage is 71.67%. Comparing base (dae9f6c) to head (eaa92b0).

Files with missing lines	Patch %	Lines
.../opensearch/security/OpenSearchSecurityPlugin.java	66.66%	1 Missing ⚠️

Additional details and impacted files

@@                       Coverage Diff                        @@
##           feature/resource-permissions    #5185      +/-   ##
================================================================
- Coverage                         71.67%   71.67%   -0.01%     
================================================================
  Files                               337      337              
  Lines                             22789    22790       +1     
  Branches                           3606     3606              
================================================================
  Hits                              16334    16334              
- Misses                             4651     4655       +4     
+ Partials                           1804     1801       -3

Files with missing lines	Coverage Δ
...g/opensearch/security/support/ConfigConstants.java	`95.23% <ø> (ø)`
.../opensearch/security/OpenSearchSecurityPlugin.java	`83.72% <66.66%> (-0.12%)`	⬇️

... and 8 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

spi/src/main/java/org/opensearch/security/spi/resources/Resource.java

cwperks · 2025-03-20T15:49:14Z

spi/README.md

+resourceSharingClient.verifyResourceAccess(
+    "resource-123",
+    "resource_index",
+    scopes,


I really think we should not try to duplicate existing mechanisms in the security plugin and use action groups for this.

When it comes to sharable resources there are 2 questions that can be asked for the current user:

Is this resource visible to the current user

Can the current user perform a given action on this resource?

For question 1, its visible if the current user is the owner or if the resource has been shared with them through some attribute

For question 2, the security plugin needs to be the one to determine whether the current user is authorized for the action being performed..similar to index actions. I know we discussed resource authorization in a future iteration, but I think we should introduce the constructs now that will be necessary for resource authorization.

Scopes are different from action groups.

Scopes are in context of the resource plugin vs action groups are known by security plugin. Scopes will be defined and used by the plugin and other plugins may not be aware of these since they are defined at plugin level.

The actions are possibly 1 of 3 : Read, Update (includes Share and Revoke) and Delete. These can be controlled by plugin with scopes. The only difference is that, at present, scopes are just semantic names unlike action groups which have actions associated with them.

cwperks · 2025-03-20T16:06:40Z

spi/src/main/java/org/opensearch/security/spi/resources/ResourceSharingExtension.java

+     * The parser for the resource, which will be used by security plugin to parse the resource
+     * @return the parser for the resource
+     */
+    ResourceParser<? extends Resource> getResourceParser();


One thing to consider is having a method on this interface to assign a service to this extension point that contains methods to interrogate access for this specific type of sharable resource. Could be an alternative to having a client and instead include everything in the SPI.

I think client is a better approach as it separates functionalities. SPI is purely intended for providing interfaces that plugins must implement. Client provides a class with APIs that plugin can utilize to implement Access Control.

Signed-off-by: Darshit Chanpura <[email protected]>

.gitignore

derek-ho · 2025-03-20T18:27:00Z

spi/README.md

+- Navigate to your plugin's `src/main/resources` folder.
+- Locate or create the `META-INF/services` directory.
+- Inside `META-INF/services`, create a file named:
+  ```
+  org.opensearch.security.spi.resources.ResourceSharingExtension
+  ```
+- Edit the file and add a **single line** containing the **fully qualified class name** of your plugin implementation.
+  Example:
+  ```
+  org.opensearch.sample.SampleResourcePlugin
+  ```
+  > This step ensures that OpenSearch **dynamically loads your plugin** as a resource-sharing extension.
+
+---


Nit: remove bullets and make this consistent look + feel with the rest of the readme

This should be bullets since these are steps, others only have 1 sentence in them.

spi/src/main/java/org/opensearch/security/spi/resources/ResourceAccessScope.java

Signed-off-by: Darshit Chanpura <[email protected]>

nibix

Thanks for this interesting PR @DarshitChanpura !

Like in #5194, I have added a couple of comments and questions. As I went through the code linearly, I also commented both on conceptual things and code level things (which I mostly marked with Nit:). I would recommend that you first have a look on conceptual things ... we can talk about the Nit stuff later.

nibix · 2025-03-21T05:42:05Z

spi/README.md

+A **singleton accessor** should be created to manage the `ResourceSharingNodeClient`.
+Example:
+```java
+public class ResourceSharingClientAccessor {


Could you explain the purpose of this ResourceSharingClientAccessor ? It is not used any more in this readme.

it will be used to access the ResourceSharingClient. Similar pattern is followed in other repos. This accessor will allow a singleton instantiation of the client (introduced in #5194) to access JAVA APIs introduced.

I've updated the section below this one in this README on declaration, and will push the change shortly.

...src/main/java/org/opensearch/security/spi/resources/exceptions/ResourceSharingException.java

nibix · 2025-03-21T05:47:17Z

spi/src/main/java/org/opensearch/security/spi/resources/sharing/Creator.java

+ * @opensearch.experimental
+ */
+public enum Creator {
+    USER("user");


What are possible future other creators?

Ideally none (except ROLES and BACKEND_ROLES). But i've left it open for now.

nibix · 2025-03-21T05:49:12Z

spi/src/main/java/org/opensearch/security/spi/resources/sharing/Recipient.java

+public enum Recipient {
+    USERS("users"),
+    ROLES("roles"),
+    BACKEND_ROLES("backend_roles");


I know that some plugins use backend roles for sharing. However, this might lead to sub-optimal situations, as users might have different backend roles depending on the auth system they are using. If we need to support BACKEND_ROLES, it should be marked IMHO as deprecated with the suggestion to use ROLES instead.

BACKEND_ROLES are added to support transition from filterByBackendRoles authz mechanism to this. This will allow migration to be easier. Maybe we deprecate this in next iteration of this feature?

nibix · 2025-03-21T05:50:05Z

spi/src/main/java/org/opensearch/security/spi/resources/sharing/RecipientType.java

+/**
+ * This class determines a type of recipient a resource can be shared with.
+ * An example type would be a user or a role.
+ * This class is used to determine the type of recipient a resource can be shared with.
+ *
+ * @opensearch.experimental
+ */
+public record RecipientType(String type) {


What's the relation between Recipient and RecipientType?

This is a form of abstraction implemented by security plugin. This was originally intended for the design which would've brought this change in core. Ideal values are USERS, ROLES, BACKEND_ROLES, and these are registered via ResourceAccessHandler introduced in Part 2.

Recipient enum provides a list of available types (USERS, ROLES and BACKEND_ROLES). And once initialized in ResourceAccessHandler these will be stored in the registry to then be evaluation when parsing XContent in SharedWithScope.java.

Similar to Creator above, this was added to allow addition of other types of recipients. I contemplated whether I should make this a subclass of RecipientTypeRegistry.

nibix · 2025-03-21T05:53:42Z

spi/src/main/java/org/opensearch/security/spi/resources/sharing/ResourceSharing.java

+ * @see org.opensearch.security.spi.resources.sharing.CreatedBy
+ * @see org.opensearch.security.spi.resources.sharing.ShareWith
+ */
+public class ResourceSharing implements ToXContentFragment, NamedWriteable {


Nit: To make the class name a noun, I'd recommend to name it ResourceSharingConfiguration.

isn't it already a noun (noun-phrase)? It is intended to represent a single ResourceSharing document (similar to Roles or RolesMapping)

nibix · 2025-03-21T05:56:56Z

spi/src/test/java/org/opensearch/security/spi/resources/ShareWithTests.java

+                MatcherAssert.assertThat(
+                    recipients.get(RecipientTypeRegistry.fromValue(DefaultRecipientType.USERS.getName())).size(),
+                    is(2)
+                );


Nit: In tests, matching on the size of the collection makes test debugging always more difficult when a test fails. You will always have to start a debugger and see what the elements might be.

Thus, I'd recommend to assert on the equality of the collection.

wouldn't this be okay since this a unit test and the value is not expected to change unless the data that was fed in changed (line 105)?

nibix · 2025-03-21T07:14:47Z

spi/src/main/java/org/opensearch/security/spi/resources/ResourceAccessScope.java

+
+/**
+ * This interface defines the two basic access scopes for resource-access. Plugins can decide whether to use these.
+ * Each plugin must implement their own scopes and manage them.


Just for me to understand better: Could you give more examples for possible/thinkable scopes?

I would let DC chime in bc I don't think there's 100% alignment on the implementation of "scopes", but there is an alignment conceptually. A "scope" is an enumerable list of actions..akin to an action group. Mentally, I've been thinking that many of the default roles (fe alerting roles) could be translated directly.

scopes matter for authorization. In the current security model, access is determined by the collection of cluster actions across all mapped roles. With fine-grained resource permissions, the owner of the resource gets to specify the access level when sharing.

In a mental model of a "searchable photo album" plugin, I create a photo album and share it with you and specify your level of access when sharing. For example, I could give you access to view or access to view/add/remove photos or I could give you full access which could entail deleting the album. The main difference is that the resource owner determines the access level when sharing, not whichever cluster actions you have from your mapped roles.

@DarshitChanpura can you also elaborate more on what you had in mind and update any requisite docs accordingly?

#5185 (comment)

I've added proposal for possible path forward for scopes. Lets vote and finalize there.

nibix · 2025-03-21T07:39:49Z

spi/src/main/java/org/opensearch/security/spi/resources/ResourceAccessScope.java

+        throw new IllegalArgumentException("Unknown value: " + value);
+    }
+
+    String value();


If I see it correctly, the scope is used as an attribute name in the resource sharing document. This puts some restrictions on it (e.g. it cannot contain .). I'd recommend to document and verify these.

These scopes are declared and controlled by plugin. But before we proceed with this discussion let's finalize the path forward for scopes here: #5185 (comment)

nibix · 2025-03-21T09:57:01Z

spi/src/main/java/org/opensearch/security/spi/resources/sharing/SharedWithScope.java

+/**
+ * This class represents the scope at which a resource is shared with for a particular scope.
+ * Example:
+ * "read_only": {
+ * "users": [],
+ * "roles": [],
+ * "backend_roles": []
+ * }
+ * where "users", "roles" and "backend_roles" are the recipient entities
+ *
+ * @opensearch.experimental
+ */
+public class SharedWithScope implements ToXContentFragment, NamedWriteable {


It took me a while to understand the example. It might be helpful to point out in the comment that read_only is the scope.

#5185 (comment)

let's finalize the path forward for scopes here.

derek-ho · 2025-03-21T18:14:34Z

spi/src/main/java/org/opensearch/security/spi/resources/sharing/ShareWith.java

+ * "share_with": {
+ * "read_only": {
+ * "users": [],
+ * "roles": [],
+ * "backend_roles": []
+ * },
+ * "read_write": {
+ * "users": [],
+ * "roles": [],
+ * "backend_roles": []
+ * }


Did we finalize the path forward w.r.t. whether we even have scopes in p0, or do we simply have something like "full_access" as a placeholder that doesn't do anything? CC @cwperks

#5185 (comment)

DarshitChanpura · 2025-03-21T20:05:32Z

Proposal on path forward for scopes:

To add to the comment here,

Scopes can be thought of as placeholders for action groups. The question here is whether we should include Scopes in this iteration of the feature, at all. From what I see there are 3 paths forward:

Path 1: No Scopes

{
	"share_with": {
		"users": [...],
		"roles": [...],
		"backend_roles": [...]
	}
}

With this structure there is no concept of "scope". The document will be visible on 3 levels: Private, Restricted and Public. Here is what a sample resource-sharing document will look like in each case:

Private

share_with record can be empty or not be present at all.

{
	"resource_id": "id",
	"source_idx": "resource-index",
	"created_by": {
		"user": "x"
	},
	"share_with": { }
}

Restricted

share_with record will contain access information about individual entities.

{
	"resource_id": "id",
	"source_idx": "resource-index",
	"created_by": {
		"user": "x"
	},
	"share_with": {
		"users": [...],
		"roles": [...],
		"backend_roles": [...]
	}
}

Public:

share_with record will contain access information about individual entities.

{
	"resource_id": "id",
	"source_idx": "resource-index",
	"created_by": {
		"user": "x"
	},
	"share_with": {
		"users": ["*"],
		"roles": ["*"],
		"backend_roles": ["*"]
	}
}

Pros:

No confusion around scopes

Cons:

No access control of any specific type. It relies on APIs to be defined clearly and users to be mapped correctly.
Requires another plugin migration when we introduce Resource Authorization Framework as a standalone authorization mechanism

Path 2: Placeholder for Scopes

{
	"share_with": {
		"*" : {
			"users": [...],
			"roles": [...],
			"backend_roles": [...]
		}
	}
}

With this structure we introduce a default scope "" which acts as a place holder for action-groups to be declared when a standalone Resource Authorization framework is in place. "" allows all actions to be matched.

Private

* record inside share_with can be empty or not be present at all

{
	"resource_id": "id",
	"source_idx": "resource-index",
	"created_by": {
		"user": "x"
	},
	"share_with": { 
		"*" : { }
	} 
}

Restricted

* record inside share_with record will contain access information about individual entities.

{
	"resource_id": "id",
	"source_idx": "resource-index",
	"created_by": {
		"user": "x"
	},
	"share_with": { 
		"*": {
			"users": [...],
			"roles": [...],
			"backend_roles": [...]
		}
	}
}

Public:

* record inside share_with record will contain access information about individual entities.

{
	"resource_id": "id",
	"source_idx": "resource-index",
	"created_by": {
		"user": "x"
	},
	"share_with": { 
		"*": {
			"users": ["*"],
			"roles": ["*"],
			"backend_roles": ["*"]
		}
	}
}

Pros:

No confusion around scopes.
Will not require migration since future sharing info can be added on top this ("*" still retains its meaning of all actions being allowed, but an additional read_only action-group can be added to allow read-only access)

Cons:

Any action can be allowed since it is "*".

Path 3: Continue with scopes

This approach allows defining individual access type over each resource. It doesn't rely completely on the APIs. It can be in future expanded to be mapped to action groups when resource authorization framework is implemented.

Example, AD plugin can, at present, define a scope named ad_read_only and in future an action-group can be added by the name of ad_read_only which contains a set of actions to be utilized by Resource Authorization framework.

Here is what an example detector sharing record may look like:

{
	"resource_id": "detector1",
	"source_idx": ".opendistro-anomaly-detectors",
	"created_by": {
		"user": "darshit"
	},
	"share_with": { 
		"ad_read_only" : {
			"users": [...],
			"roles": [...],
			"backend_roles": [...]
		},
		"ad_full_access" : {
			"users": [...],
			"roles": [...],
			"backend_roles": [...]
		}
		
	}
}

or if the detector is intended to be publicly accessible:

{
	"resource_id": "detector1",
	"source_idx": ".opendistro-anomaly-detectors",
	"created_by": {
		"user": "darshit"
	},
	"share_with": { 
		"*" : {
			"users": ["*"],
			"roles": ["*"],
			"backend_roles": ["*"]
		}
	}
}

Pros:

No future migration

Cons:

Complex to understand at first.
Requires plugin to pass scopes, when asking for access verification. What scopes should current user's access be matched against.

@nibix @cwperks @derek-ho Please review this proposal so we can finalize the path-forward for scopes.

I lean towards Path 2 as it is good place between no scopes and complete scopes, and it unblocks this PR. It would require some re-work in following Part 2 and 3 tho.

DarshitChanpura · 2025-03-21T20:18:55Z

Proposal on path forward for scopes:
@nibix @cwperks @derek-ho Please review this proposal so we can finalize the path-forward for scopes.

I personally am leaning towards Path 2 as it is good place between no scopes and complete scopes.

Path 3 puts us in a good state for future, but I don't want that to be a blocker for this PR. All changes are labelled as @opensearch.experimental so that gives us room to modify things later.

Signed-off-by: Darshit Chanpura <[email protected]>

nibix · 2025-03-22T08:51:20Z

@DarshitChanpura

Proposal on path forward for scopes:

Thank you for the proposals!

Before I am going into the proposals, I'd like to go one step back and try to get a better picture of the situation. Sorry if the following question may sound dumb, but I am possibly missing some details at the moment.

So, my question would be: What's the actual reason we are now discussing these options about the path forward with scopes?

Is it because there is disagreement about the scopes?
Is it because the scopes are not sufficiently/thoroughly defined yet?
Or something else?

I think an answer to this question could help me to contribute better to this discussion.

DarshitChanpura changed the title ~~[Resource Access Control] Introduces SPI for resource access control~~ [Resource Access Control] [Part1] Introduces SPI for resource access control Mar 17, 2025

DarshitChanpura mentioned this pull request Mar 17, 2025

[Resource Access Control] [Part2] Introduces a client for Resource Access Control and adds concrete implementation via common package #5186

Closed

4 tasks

DarshitChanpura force-pushed the resource-sharing-spi-only branch 2 times, most recently from d076506 to ccc5022 Compare March 17, 2025 21:21

DarshitChanpura changed the base branch from feature/resource-permissions to main March 17, 2025 21:23

DarshitChanpura changed the base branch from main to feature/resource-permissions March 17, 2025 21:23

DarshitChanpura mentioned this pull request Mar 17, 2025

[Resource Access Control] [Part3] Introduces a sample plugin to demonstrate usage of Resource Access Control feature #5187

Draft

3 tasks

DarshitChanpura marked this pull request as ready for review March 17, 2025 22:42

DarshitChanpura requested review from cwperks, derek-ho, nibix, peternied, RyanL1997, reta and willyborankin as code owners March 17, 2025 22:42

This was referenced Mar 17, 2025

[Feature] Introduces Centralized Resource Access Control and Sharing #5016

Closed

[Resource Access Control] [Cleanup] Removes redundant files between common and root package #5188

Closed

DarshitChanpura added the resource-permissions Label to track all items related to resource permissions label Mar 18, 2025

cwperks reviewed Mar 19, 2025

View reviewed changes

spi/src/main/java/org/opensearch/security/spi/resources/Resource.java Outdated Show resolved Hide resolved

cwperks reviewed Mar 19, 2025

View reviewed changes

spi/src/main/java/org/opensearch/security/spi/resources/Resource.java Outdated Show resolved Hide resolved

cwperks reviewed Mar 20, 2025

View reviewed changes

DarshitChanpura mentioned this pull request Mar 20, 2025

[Resource Access Control] [Part2] Introduces a client for Resource Access Control and adds concrete implementation for resource access control #5194

Open

4 tasks

DarshitChanpura added 2 commits March 20, 2025 14:19

Introduces resource sharing and access control SPI

77d92f7

Signed-off-by: Darshit Chanpura <[email protected]>

Renames couple of SPI classes

3379974

Signed-off-by: Darshit Chanpura <[email protected]>

DarshitChanpura force-pushed the resource-sharing-spi-only branch from 9833c7e to 3379974 Compare March 20, 2025 18:19

derek-ho reviewed Mar 20, 2025

View reviewed changes

.gitignore Show resolved Hide resolved

derek-ho reviewed Mar 20, 2025

View reviewed changes

spi/src/main/java/org/opensearch/security/spi/resources/ResourceAccessScope.java Outdated Show resolved Hide resolved

DarshitChanpura added 2 commits March 20, 2025 16:22

Updates scope name

e179968

Signed-off-by: Darshit Chanpura <[email protected]>

Fixes CI

63c4a27

Signed-off-by: Darshit Chanpura <[email protected]>

DarshitChanpura force-pushed the resource-sharing-spi-only branch from cb45e7e to 63c4a27 Compare March 20, 2025 20:37

Fixes test

2fd2a8b

Signed-off-by: Darshit Chanpura <[email protected]>

nibix reviewed Mar 21, 2025

View reviewed changes

derek-ho reviewed Mar 21, 2025

View reviewed changes

Adds new exception types and updated README.md

eaa92b0

Signed-off-by: Darshit Chanpura <[email protected]>

DarshitChanpura force-pushed the resource-sharing-spi-only branch from fa10c26 to eaa92b0 Compare March 21, 2025 22:17

[Resource Access Control] [Part1] Introduces SPI for resource access control #5185

Are you sure you want to change the base?

[Resource Access Control] [Part1] Introduces SPI for resource access control #5185

Conversation

DarshitChanpura commented Mar 17, 2025 • edited Loading

Description

Issues Resolved

Testing

Check List

codecov bot commented Mar 17, 2025 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nibix left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DarshitChanpura Mar 21, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cwperks Mar 21, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DarshitChanpura commented Mar 21, 2025 • edited Loading

Path 1: No Scopes

Path 2: Placeholder for Scopes

Path 3: Continue with scopes

DarshitChanpura commented Mar 21, 2025

nibix commented Mar 22, 2025 • edited Loading

DarshitChanpura commented Mar 17, 2025 •

edited

Loading

codecov bot commented Mar 17, 2025 •

edited

Loading

DarshitChanpura Mar 21, 2025 •

edited

Loading

cwperks Mar 21, 2025 •

edited

Loading

DarshitChanpura commented Mar 21, 2025 •

edited

Loading

nibix commented Mar 22, 2025 •

edited

Loading