[Question] How to support extensions parity for user information

[peternied]

How to support extensions parity for user information

This issue is meant to decide how information user should be provided to extensions based on exiting behavior for plugins

Background

Many existing plugins reference common-utils User object. For plugins that implement security checks on resources these are the limits of the existing security information that is exposed.

sequenceDiagram
    title How Plugin access to User Context
    actor User
    box white OpenSearch Process
       participant RestController
       participant SecurityPlugin
       participant Plugin as Plugin
    end

    User->>+RestController: Send search request
    RestController->>+SecurityPlugin: Authenticate user
    SecurityPlugin-->>-RestController: Store user object in thread context
    RestController->>+Plugin: Invoke plugin method
    Plugin->>+RestController: Read user from thread context
    RestController-->>-Plugin: Return user object
    Plugin-->>User: Return search results

Loading

Example user object represented as JSON

{
    "name": "UserNameOrUserEmailAddress"
    "backend_roles": ["BackendRoleFromLDAP", "BackendRoleFromSAML"]
    "roles": ["security_rest_api_access", "alerting_full_access"]
    "custom_attribute_names": ["attr1", "attr2"]
    "user_requested_tenant": "private"
}

Problem

User context information is stored within the thread context. Extensions should never have access to the thread context and there must be a way to reason decisions about user with existing data.

NOTE: Exposing backend_roles / roles directly to an extension should not be done to prevent tight coupling and information disclosure, instead extensions should use make request to a checkPermissions API with an action name & resources. This path is being built for parity to let plugins have a clear migration path to extensions.

Option 0: Don't provide user information

In this option extensions do not have support for existing scenarios where reasoning is done about the user identity or resource checks.

For each plugin -> extension migration the plugin would need to find any dependencies on User data and remove them. This might require modification or deprecation of features

Pros:

• Prevents exposing user information to extensions

Cons

• Feature deprecation or modification might be substantial effort
• Regression in functionality for plugin users

Option 1: Include user information on all requests to extensions

In this option, changes would be made to the RestSendToExtensionAction, the user information is collected from the existing thread context and provided to the extension as part of the payload of the ExtensionRestRequest. Extension would be able to retrieve this information as an object and use it for new / existing scenarios.

sequenceDiagram
    actor User
    box white OpenSearch Process
       participant RestController
       participant SecurityPlugin
       participant RestSendToExtensionAction
    end
    box white Separate Host
       participant Extension
    end

    User->>+RestController: Send search request
    RestController->>+SecurityPlugin: Authenticate user
    SecurityPlugin-->>-RestController: Return user context
    RestController->>+RestSendToExtensionAction: Invoke method with user context
    RestSendToExtensionAction->>+Extension: REST API Forward user object to extension
    Extension->>+RestSendToExtensionAction: Process request with user object
    RestSendToExtensionAction-->>-RestController: Return search results
    RestController-->>User: Return search results

Loading

Pros:

• Feature parity for existing plugins scenarios

Cons:

• Perpetuates information disclosure
• Perpetuates coupling of extensions with existing permissions constructs

Option 2: Build new security features to support exist plugin scenarios

In this option for all scenarios different scenarios that user information is used new features / API would be build to support individual requirements. For Anomaly Detection detector ownership check, an object that represented the identity of the caller, but encrypted would be provided. This object would be comparable as before if a user making a request != the representative identity but would not expose the username/email etc. There would need to be many additional scenarios supported.

Pros:

• Opportunity for deploying security best practices
• Scenarios supported would cover a variety of future extensions needs
• Slow time to market

Cons:

• Considerable investment for each plugin and each scenario associated with them
• Requires changes in security plugin for new APIs
• Requires changes in all plugin to extension migration efforts

Option 3: Include user information on requests to extensions restricted with 'LEGACY' scope [Recommended]

In this option we would use the approach from Option 1, but the information would not be included by default, the field would be annotated with a deprecation message. Only if the extension was permitted to a legacy_user_context scope would the information be included and it would be removed from the extensions SDK in a future release.

Pros:

• Supports existing scenarios
• Clearly marks extensions that do not follow best practices

Cons:

• Introduces functionality with intention of deprecation

[peternied]

@cwperks What do you think?

[DarshitChanpura]

I like the approach of using 'Legacy' annotation. It would define the intent correctly and would also solve the question we are addressing here. Option 3 it is from my end.

[cwperks]

@peternied Re-capping a discussion from earlier today about what claims to include in a token for an extension and what not to include. I'd like to add context with OpenSearch Dashboards and the claims that are provided in the token the dashboards receives vs. the information it additionally gets from the server by calling /_plugins/_security/authinfo after successful login. See details about the endpoint at https://github.com/opensearch-project/security/blob/main/src/main/java/org/opensearch/security/rest/SecurityInfoAction.java#L103-L124

When Dashboards is configured with SAML or OIDC, the user is asked to authenticate with the external IdP once and upon successful authentication is redirected back to OSD with a JWT that is used to authenticate and authorize subsequent requests for as long as the session remains valid.

1. For SAML - Security takes takes the Authentication Response from the SAML Auth Server and issues a JWT
2. For OIDC - Security receives a token from the external identity provider

In both cases, the token has a minimum of 2 claims, but most likely has at least 4. Those are:

1. sub or subject_key - this is the username
2. roles or roles_key - Think of this as the authorizations of the user (or could be thought of as token authorizations)
3. nbf - Not before to indicate the beginning of the window of token validity
4. exp - Expiry to indicate the end of the window of token validity

From what I gather of the security-dashboard-plugin code, these claims are never read or used by the frontend. Instead the frontend relies on the authinfo endpoint to get information about the user.

The claims are useful to the backend and the JWT Auth backend uses the roles in the token to evaluate privileges of the caller.

[peternied]

Updated the diagrams with process/host boundaries - thanks @DarshitChanpura for figuring that out!

[peternied]

@cwperks @scrawfor99 @RyanL1997 @davidlago What do you think? I'm feeling good about Option 3: Include user information on requests to extensions restricted with 'LEGACY' scope [Recommended]

[stephen-crawford]

Hi @peternied, I am in full support of option 3 myself. Thank you for putting together the thorough review.

[cwperks]

@peternied I support moving forward with option 3 with the initial goalpost being that AD backend is being converted from a plugin to an extension and needing to be compatible with plugin paradigms today like protecting resources with user's backend roles. This will be protected behind a configuration that is off by default and must be explicitly turned on to allow.

[RyanL1997]

Hi @peternied , thank you for putting all these together. I'm voting to option3.

[peternied]

We are going with Option 3: Include user information on requests to extensions restricted with 'LEGACY' scope [Recommended]. Implementation details to follow in the associated meta issues.

[saratvemulapalli]

Thanks @peternied for writing this up.
I agree with Option 3, Im on the same page as @cwperks to start small with AD (whatever it needs).

Sounds like we should define a generic Transport Message which encapsulates User identity and use it as a base Request for all extension messages.
Couple of concerns to address:

• It makes sense and works for Rest API requests, we should also handle transport API calls which needs identity information. For example: Job Scheduler picks up a job from JS metadata index and calls AD Transport API to run the Job. AD transport API needs to validate the user permissions before it runs a detector.
• With Plugins, putting in User information in threadContext didnt have problems of "who" plumbed it in, because its within OpenSearch process(same JVM) which is secure. In Extensions, as these are transport calls the claims should be signed and extensions should have a way to verify that security plugin did really plumb this information. Without which extensions wouldn't really know if the user identity information is real.

Also, Plugins dont have a way today to get User identity on Rest APIs today which is why they write transport APIs to access User information. As part of this work, could we expose generic interfaces to access this information at Rest layer for both Plugins/Extensions ?