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

Allow extensions to register roles #2595

Open
cwperks opened this issue Mar 28, 2023 · 5 comments
Open

Allow extensions to register roles #2595

cwperks opened this issue Mar 28, 2023 · 5 comments
Labels
triaged Issues labeled as 'Triaged' have been reviewed and are deemed actionable.

Comments

@cwperks
Copy link
Member

cwperks commented Mar 28, 2023

Plugins can register roles by adding new roles to roles.yml. The roles are added to the cluster on bootstrap of a fresh cluster installation, by running securityadmin.sh or by using the API to create the roles.

Extensions need a way to register roles with a list of actions and have the roles sourced to the opensearch security index on extension installation.

An example of this for the HelloWorld extension could look like:

# Roles for Hello World extension, roles that extensions would like to add should be ultimately be defined by the extension
extension_hw_greet:
  reserved: true
  cluster_permissions:
    - 'hw:greet'

extension_hw_full:
  reserved: true
  cluster_permissions:
    - 'hw:greet'
    - 'hw:greet_with_adjective'
    - 'hw:great_with_name'
    - 'hw:goodbye'

Inside of extensions/extensions.yml there should be a setting for a cluster admin to configure to allow the registration of roles.

i.e.

extensions:
  - name: hello-world
    uniqueId: hw
    hostAddress: '127.0.0.1'
    port: '4532'
    version: '1.0'
    opensearchVersion: '3.0.0'
    minimumCompatibleVersion: '3.0.0'
    roles:
      - extension_hw_greet:
           reserved: true
           cluster_permissions:
             - 'hw:greet'
      
      - extension_hw_full:
           reserved: true
           cluster_permissions:
             - 'hw:greet'
             - 'hw:greet_with_adjective'
             - 'hw:great_with_name'
             - 'hw:goodbye'
@cwperks cwperks converted this from a draft issue Mar 28, 2023
@github-actions github-actions bot added the untriaged Require the attention of the repository maintainers and may need to be prioritized label Mar 28, 2023
@cwperks cwperks changed the title Allow extensions to dynamically register roles Allow extensions to register roles Apr 3, 2023
@stephen-crawford stephen-crawford added triaged Issues labeled as 'Triaged' have been reviewed and are deemed actionable. and removed untriaged Require the attention of the repository maintainers and may need to be prioritized labels Apr 3, 2023
@stephen-crawford
Copy link
Contributor

[Triage] This is part of the Extensions project.

samuelcostae added a commit to samuelcostae/security that referenced this issue May 4, 2023
…the extension (/extensions/register)

Signed-off-by: scosta <[email protected]>
samuelcostae added a commit to samuelcostae/security that referenced this issue May 4, 2023
samuelcostae added a commit to samuelcostae/security that referenced this issue May 5, 2023
samuelcostae added a commit to samuelcostae/security that referenced this issue May 5, 2023
samuelcostae added a commit to samuelcostae/security that referenced this issue May 8, 2023
samuelcostae added a commit to samuelcostae/security that referenced this issue May 8, 2023
samuelcostae added a commit to samuelcostae/security that referenced this issue May 8, 2023
samuelcostae added a commit to samuelcostae/security that referenced this issue May 9, 2023
@peternied
Copy link
Member

By putting roles into the extension configuring this tightly couples the security plugin's permissions model to extensions. This seems like it will have significant drawbacks for compatibility going forward. We should be careful what goes into the extension configuration from an administrators perspective. I'd recommend index high on intuitive and self-explanatory.

I suggest we do not engage on this work until its needed, as much as it isn't ideal living with the roles being hardcoded into the security plugin has worked to date. We will need to figure out how to engage with permissions especially in a way that allows non-security plugins to function with extensions ecosystem. This is outside the critical path of the work that needs to be delivered for extensions

@davidlago
Copy link

+1

Based on a few open issues and PRs it seems like we're getting to the part of the Extensions project where some of these abstractions start to leak in favor of backwards compatibility. Though necessary to support the current plugin model, I agree with @peternied that we should defer these decisions as much as possible if they are not on the critical path to having an extension working end to end leveraging the security plugin constructs. This will feel awkward for sure, but it is a temporary state... we'll pounce at the 3.0 opportunity :)

@cwperks
Copy link
Member Author

cwperks commented May 9, 2023

@peternied I'm ok with deprioritizing, but the current model of accepting a PR for every plugin/extension that wants to define roles to map to a user is unsustainable imagining a future with many extensions in a catalog. I believe its possible to create a generic hook where an IdentityPlugin can extend the settings inside extensions/extensions.yml to define settings that it needs and then hook into the extension bootstrap process to react to the settings being read in extensions.yml and act accordingly. In the case of the security plugin that would mean sourcing reserved roles that do not yet exist into the roles section of the security index.

@davidlago davidlago moved this to In Progress in Security for Extensions May 10, 2023
@davidlago davidlago moved this from In Progress to Todo in Security for Extensions May 11, 2023
@stephen-crawford
Copy link
Contributor

stephen-crawford commented Jun 1, 2023

@davidlago, what do you think about moving back to Backlog (Extensions)? I have a couple concerns about the registration process for new extensions that I think this issue would help address. @cwperks mentioned it above, but to restate: I am not sure how well the current model will scale.

It will work for the immediate future, but once we get more and more custom extensions (the goal), the process of registering the new roles with a PR to the Security repo will quickly become untenable. I could also see some concerns arising around a custom configuration where someone adds roles to the Security plugin and then uninstalls the extension but the roles still remain.

Because we cannot guarantee that a custom role will not have generic privileges, you could end up having "ghost roles" where a user continues to have escalated generic permissions even if a given extension no longer exists. By adding the feature Craig mentions here, we can avoid this since the bootstrap process would dynamically add the roles each spin-up.

I don't think this feature is P0 for Extensions but I do think it is something required P1-P2.

I also think that Sam and Maciej would do an excellent job tackling this should either decide to take it on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triaged Issues labeled as 'Triaged' have been reviewed and are deemed actionable.
Projects
Status: Todo
Development

No branches or pull requests

4 participants