[Feature Request] Explicitly declare what transport actions a plugin will perform and prompt on plugin install

cwperks commented 1 month ago

Is your feature request related to a problem? Please describe

This issue is coming from a conversation from here

In some cases, plugins need to perform transport actions and have a guarantee that the operation will succeed. When the security plugin is installed, transport actions will be authorized as the authenticated user[1]. In order to perform transport actions outside of the authenticated user context, a plugin will temporarily stash the ThreadContext and perform actions like interacting with a system index. In https://github.com/opensearch-project/OpenSearch/pull/14630 and https://github.com/opensearch-project/security/pull/4665, a general method was added for system index access where a plugin is assigned a pluginSubject that they can use to directly replace instances of try (ThreadContext.StoredContext ctx = threadContext.stashContext()) { ... } calls with pluginSubject.runAs(() -> { ...}). The advantage of the latter is that it allows the security plugin to enforce that plugins only interact with their own system indices and not system indices registered by other plugins.

In https://github.com/opensearch-project/opensearch-plugins/issues/238, it was pointed out that there may be instances other than system index access, that a plugin wants to perform an operation outside of an authenticated user context. I am opening this issue to discuss how plugins can declare what these actions will be and to prompt a cluster administrator at installation time to accept that a plugin will perform the requested actions.

The 2 instances pointed out are:

In the security plugin, its possible to use an OpenSearch index for audit logging (internal_opensearch). In this case, security needs a guarantee that it can write to the index even though it is not a system index.
@sohami pointed out another use case where node stats action needs to be called outside of authenticated user context

Describe the solution you'd like

I would like something analogous to plugin-security.policy that can define what transport actions a plugin will perform. Minimally, it needs to be separated into 2 sections: 1) Cluster Actions and 2) Index Actions. Index Actions will need index patterns associated with the actions.

One strategy is to define the actions in code and load a plugin at installation time to get its requested permissions. A POC for this strategy was done in this PR: https://github.com/opensearch-project/OpenSearch/pull/15778

Another strategy could be to have a yml file on the same level as plugin-security.policy and define the requested permissions in the yml file.

Related component

Plugins

Describe alternatives you've considered

Status quo where plugins can perform any transport actions within a try (ThreadContext.StoredContext ctx = threadContext.stashContext()) { ... } block.

Additional context

[1] When handling an API Request the security plugin will authenticate the request before delegating to the original handler. When an API Request is authenticated the security plugin will hydrate the threadContext with info about the authenticated user.

cwperks commented 1 month ago

In the case of the audit log index of the security plugin, the name of the audit log index could change based on cluster settings.

plugins.security.audit.config.index: 'security-auditlog-'YYYY.MM.dd <- default value

The settings may not be fully available at installation time.

reta commented 1 month ago

Another strategy could be to have a yml file on the same level as plugin-security.policy and define the requested permissions in the yml file.

I believe the permissions should be declarative and non-dynamic (which programmatic implementation brings with it), plugin-permissions.policy or plugin-permissions.yml would be a reasonable approach to me.

cwperks commented 1 month ago

@reta One of the challenges that I faced with the security plugin, for the audit log as an opensearch index use-case, is that the final index that security needs permission to is only known after the settings are read.

My solution around this problem was to show the default value on the installation prompt and leave a message:

Plugin requests permission to perform the following transport actions. Any index pattern that appears below is a default value and may change depending on plugin settings.

Does that sound reasonable to you?

Section of the installation prompt around requesting explicit permission to perform cluster actions and index actions:

...

Plugin requests permission to perform the following transport actions. Any index
pattern that appears below is a default value and may change depending on plugin settings.

Cluster Actions
---------------

* cluster:monitor/health

Index Actions
-------------

Index Pattern: security-auditlog-*

* indices:data/write/index

Continue with installation? [y/N]

reta commented 1 month ago

@reta One of the challenges that I faced with the security plugin, for the audit log as an opensearch index use-case, is that the final index that security needs permission to is only known after the settings are read.

Thanks @cwperks , so could we refer to the setting name instead?

cwperks commented 1 month ago

Yes, that's a good idea. Maybe something like:

Index Actions
-------------

Index Pattern: Setting[key=plugins.security.audit.config.index, default_value='security-auditlog-'YYYY.MM.dd]

dblock commented 1 month ago

[Catch All Triage - 1, 2, 3, 4]

opensearch-project / OpenSearch