[Feature]Introduces Resource Sharing and Access Control

[DarshitChanpura]

companion PR: opensearch-project/security#4746

Description

This PR introduces a new capability (with @opensearch-experimental flag) to enable access-control and sharing of resources. This PR introduces:

1. Interfaces to be extended by security plugin for concrete implementation, and to be used by plugins when authorizing the requested resources.
2. Adds a Default implementation when security plugin is disabled.

At present, plugins have implemented in-house authorization mechanisms to control access to their resources. This framework enables capability to have a centralized resource-authorization framework.

This feature adds authorization flow on-top-of existing authorization scheme. Meaning, a user X declaring a resource RX shares with user Y. user Y must have access to the index defining the resource.

Please review feature proposal here that discusses the problem-statement and design approach. opensearch-project/security#4500

Plugins will leverage the APIs introduced in this to check user access to resources.
Documentation update on the project website will be added once the feature is in.

A future update to this design will introduce a new request type called ResourceRequest which will then be utilized in security plugin to invoke a new authorization flow that will run in-parallel with Transport authorization.

Related Issues

• Related to [Proposal] Resource Permissions and Sharing security#4500

Check List

• Functionality includes testing.
• API changes companion pull request created, if applicable.
• Public documentation issue/PR created, if applicable.

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.

[DarshitChanpura]

The Jenkins gradle build [1] passed but the gradle check task here has failed :/

[1]- https://build.ci.opensearch.org/job/gradle-check/51651/

[github-actions]

❌ Gradle check result for 5e6b8ff: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[DarshitChanpura]

@reta @jainankitk @nibix Would you mind giving this a review? I'm targeting 2.19 to get this change shipped.

[reta]

@reta @jainankitk @nibix Would you mind giving this a review? I'm targeting 2.19 to get this change shipped.

@DarshitChanpura I am off for the next few days, had time to glance through the change very quickly. The one thing that stood to me right away is disconnect from the identity management (IdentityPlugin & Subject), solely focusing on presence of the user, it does not seem right.

[github-actions]

✅ Gradle check result for 5e6b8ff: SUCCESS

[cwperks]

@reta @jainankitk @nibix Would you mind giving this a review? I'm targeting 2.19 to get this change shipped.

@DarshitChanpura I am off for the next few days, had time to glance through the change very quickly. The one thing that stood to me right away is disconnect from the identity management (IdentityPlugin & Subject), solely focusing on presence of the user, it does not seem right.

@DarshitChanpura I agree with @reta here. The notion of user, roles or backend roles should not be introduced into the core. I know we had some back-and-forth on the RFC about the extensibility model and I really do think the SPI model is appropriate here, with the caveat that there would either need to be the introduction in the core of optionalExtendedPlugins or make extendedPlugins themselves optional.

I created a spike on the idea here: cwperks/security#26

[DarshitChanpura]

@cwperks @reta Thank you for reviews. I have removed the notion of users.

[github-actions]

❌ Gradle check result for ce9d5ec: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

✅ Gradle check result for 7a868cb: SUCCESS

[github-actions]

✅ Gradle check result for 6fb5055: SUCCESS

[DarshitChanpura]

@froh @andrross @dbwiddis Would love to get your feedback on this PR as well.

[github-actions]

✅ Gradle check result for 3dfc16e: SUCCESS

[cwperks]

What is a scope exactly? Is it the same as security's notion of an action group or something different? How is it defined and why is it a String?

If resourceIndex is passed in here then is it assumed that resources are stored in an index? Should this be abstracted to not necessarily assume that resources are stored in an index and leave the implementation up to the plugin that provides access control for resources?

[cwperks]

Would this always be able to computed synchronously? If it can't be synchronous then does this need to accept an ActionListener?

[cwperks]

What is the purpose of this method here? If a user wants to make a resource private then the resource sharing record would still exist, but any shared with entry would be emptied and it entry would keep track of the resource user (owner of the resource)

IMO this should be left up to the IndexOperationListened on postDelete. If an entry in the resourceIndex is deleted then we can also delete its corresponding entry in the .resource-sharing index.

[cwperks]

wdyt about implementing this and the method below as a REST API in the plugin that implements resource access control?

i.e. Security plugin could have an API

PUT /resource/{resource_type}/{id}/share_with
{
    "resource_id": "...",
    "resource_type": "...", // resource_type is 1 <-> 1 with resource index, but is a human-readable type
    "share_with": { 
         "users": ["userB"]
    }
}

Does this need to be part of the interface?

[github-actions]

✅ Gradle check result for 532d13a: SUCCESS

[cwperks]

When thinking about the role of this plugin, I was thinking that its job is to initialize separate Resource(Sharing)Service per resource type and the role of the service would be the method below this one. i.e. resourceService.hasBeenSharedWith(String resourceId) - the resourceService is specific to the type of resource and would already have info necessary required to do the lookup.

These resourceServices would be assigned back to the ResourcePlugins which can be used to determine if a resource has been shared with the currently authenticated user.

When it comes to re-usable generic actions to get a resource or search for resources, we could consider adding a library that plugins can add a dependency on that has the re-usable generic actions. The ResourcePlugins could need to supply a ResourceParser so that the generic actions would know how to parse entries from the resource index. The jobParser from the job-scheduler has a model for generic parsing: https://github.com/opensearch-project/job-scheduler/blob/main/spi/src/main/java/org/opensearch/jobscheduler/spi/JobSchedulerExtension.java#L30-L33

[cwperks]

@reta @jainankitk @nibix Would you mind giving this a review? I'm targeting 2.19 to get this change shipped.

@DarshitChanpura I am off for the next few days, had time to glance through the change very quickly. The one thing that stood to me right away is disconnect from the identity management (IdentityPlugin & Subject), solely focusing on presence of the user, it does not seem right.

@DarshitChanpura I agree with @reta here. The notion of user, roles or backend roles should not be introduced into the core. I know we had some back-and-forth on the RFC about the extensibility model and I really do think the SPI model is appropriate here, with the caveat that there would either need to be the introduction in the core of optionalExtendedPlugins or make extendedPlugins themselves optional.

I created a spike on the idea here: cwperks/security#26

I have been thinking about the different approaches (SPI vs new extension points) over the last couple of days and I think this approach makes sense, but I would suggest some changes to the interfaces. I think the extension point approach is sensible because it allows other plugins (not only the default security plugin) to define the ResourceAccessControl interface whereas a SPI would make it so that only the security plugin could provide an implementation.

These are some ideas I had about the interface and wanted to get your thoughts. See example here.

1. ResourceAccessControlPlugin

public interface ResourceAccessControlPlugin {
    void assignResourceSharingService(SharableResourceType resourceType);
}

2. ResourcePlugin

public interface ResourcePlugin {
    List<SharableResourceType> getResourceTypes();
}

and the associated interfaces SharableResourceType, ResourceSharingService and SharableResource

a. SharableResourceType

public interface SharableResourceType {
    /**
     * Type of the resource. This must be unique across all registered resource types.
     * @return a string containing the type of the resource
     */
    String getResourceType();

    /**
     * The index where resource metadata is stored
     * @return the name of the index where resource metadata is stored
     */
    String getResourceIndex();

    /**
     * This method is called when initializing ResourcePlugins to assign an instance
     * of {@link ResourceSharingService} to the resource type.
     */
    void assignResourceSharingService(ResourceSharingService resourceSharingService);

    /**
     * @return returns a parser for this resource
     */
    default ResourceParser<? extends SharableResource> getResourceParser() {
        return null;
    };
}

b. ResourceSharingService

public interface ResourceSharingService {

    /**
     * Returns the resource type this service is responsible for.
     *
     * @return The resource type. Will match {@link SharableResourceType#getResourceType()} for the respective
     * sharable resource type
     */
    String getResourceType();

    /**
     * Checks if the resource is shared with the current requester.
     *
     * @param resourceId The resource id
     * @param shareListener The listener to be called when the check is complete
     */
    void isSharedWithCurrentRequester(String resourceId, ActionListener<Boolean> shareListener);
}

c. SharableResource

/**
 * Interface for a generic Sharable Resource. Resources are entities created by plugins that are typically
 * stored in system indices. Access Control is provided by the ResourceAccessControlPlugin.
 */
public interface SharableResource extends NamedWriteable, ToXContentObject {

    /**
     * @return resource name.
     */
    String getName();

    /**
     * @return resource last update time.
     */
    Instant getLastUpdateTime();
}

[reta]

I have been thinking about the different approaches (SPI vs new extension points) over the last couple of days and I think this approach makes sense, but I would suggest some changes to the interfaces

Thanks @cwperks , the APIs refinement is on the right track I believe, but I have larger concerns, @DarshitChanpura this is not the first attempt to bring the notion of "resources" to core. There is probably a need to introduce some abstractions around that so the plugin authors should not reinvent the wheel, I got that, but, at least per this change:

• the core has no understanding what the "resource" is
• as such, the core has no any centralized / coordinated logic to manage access to "resources"
• it merely serves as a proxy / mediator, without having any control over the "resources"

In my opinion, as it stands now, I don't see why it has to be part of the core. It seems to me it could be a part of standalone library instead (so plugin authors could integrated it and reuse the building blocks but fully manage the access aspects, the core could provide the identity (requestor) piece.

If we want to make it a part of the core, we need to design the unified "resources" access semantics, not only some abstract notions and largely disconnected API.