Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modify ActionRequest in core to allow for resource_names to be passed in the request #14751

Closed
DarshitChanpura opened this issue Jul 15, 2024 · 3 comments
Assignees

Comments

@DarshitChanpura
Copy link
Member

DarshitChanpura commented Jul 15, 2024

META issue: opensearch-project/security#4560

Description

This base class is extended by every plugin’s implementation of transport request calls. Hence, the idea is to add a standard class property List resources. In addition, add resource: as a valid transport action prefix here.

This will then be implemented by plugins to populate this property when handling an actionRequest after which SecurityFilter.java will intercept this call to perform privilege evaluation.

Implementation

Coming from comment: opensearch-project/security#4500 (comment), we will proceed with approach a, which introduces a new getter for resources, there-by allowing request handlers in plugin to populate the resource names before sending the request to Transport layer. Here is the pseudo-code (thank you @nibix):

public abstract class ActionRequest extends TransportRequest {

    public ActionRequest() {
        super();
    }

    public ActionRequest(StreamInput in) throws IOException {
        super(in);
    }
 
    /**
      * NEW NEW NEW .. to be overridden by whoever wants to
      */
    public List<String> getResources() {
       return Collections.emptyList();
    }

    public abstract ActionRequestValidationException validate();

    public boolean getShouldStoreResult() {
        return false;
    }

    @Override
    public void writeTo(StreamOutput out) throws IOException {
        super.writeTo(out);
    }
}
@cwperks
Copy link
Member

cwperks commented Jul 15, 2024

[Triage] Thank you for filing this issue @DarshitChanpura going to transfer this issue to the OpenSearch core repository.

@cwperks cwperks transferred this issue from opensearch-project/security Jul 15, 2024
@nibix
Copy link

nibix commented Jul 16, 2024

I mentioned it just briefly in opensearch-project/security#4500 (comment), but thought it might be worth pointing out more explicitly:

Instead of directly extending ActionRequest, one could also think about introducing a new interface which actions need to implement when they want to provide resource information.

That would be consistent to the style so far. Examples for this are:

This helps to keep the ActionRequest class terse.

It could look similar to this:

interface PluginResourceRequest {
   List<String> getResources();
}

@peternied
Copy link
Member

[Triage - attendees 1 2 3]
@DarshitChanpura Thanks for creating this issue; however, it isn't being accepted due to not having a clear problem that is trying to be solved.

If we had a model for defining addresses of resources, those could be useful for identifying and permission items, but it isn't clear if this issue would follow those needs. A list of untyped strings seems like it would create future issue without other infrastructure built up around it.

Please feel free to open a new issue after addressing the reason.

@DarshitChanpura DarshitChanpura self-assigned this Aug 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants