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

[RFC] Dynamically Configurable Tenancy Feature #5853

Closed
abhivka7 opened this issue Jan 13, 2023 · 8 comments
Closed

[RFC] Dynamically Configurable Tenancy Feature #5853

abhivka7 opened this issue Jan 13, 2023 · 8 comments
Labels
enhancement Enhancement or improvement to existing feature or request RFC Issues requesting major changes

Comments

@abhivka7
Copy link

abhivka7 commented Jan 13, 2023

Is your feature request related to a problem? Please describe.
Feature request: opensearch-project/security-dashboards-plugin#1302

Dashboards security plugin offers customers the feature to have multiple tenants for each user. This feature is called multi-tenancy. Under this feature, users can have global tenant, private tenant and custom tenants. But having so many tenant options can cause confusion among users if they don't want to use multi-tenancy feature. Therefore there should be an option to enable/disable multi-tenancy and private-tenant to avoid confusion and save space on unwanted indexes.
Right now we have the option to change these options in opensearch_dashboards.yml, but that requires a restart of Dashboards env. We want to make these changes dynamic from Dashboards page.

Also whenever user logs in, dashboard security plugin uses the last used tenant from previous session to login. But there is no way to choose a default tenant whenever a user logins for the first time. There should be an option to set a Default-Tenant out of all available tenants for better user experience.

How does Multi-Tenancy work?

Dashboards stores each visualisation setting as a document. Now for each domain, there will be a common Global tenant, and each Dashboards user will have their own private tenant. Besides that, users can also create custom tenants. Each of these tenants will have a Dashboards index of their own which are uniquely named.
Each private tenant can only be accessed by the corresponding user, whereas sharing of global and custom tenants is role based controlled by Admin.
When using Dashboards , users can switch between tenants and based on selected tenants, visualisation is loaded from the corresponding index.

Describe the solution you'd like
Security Config of Security plugin already has the property: config.dynamic.kibana.multitenancy_enabled.
Wen will introduce 2 new properties:

  1. config.dynamic.kibana.private_tenant_enabled and
  2. config.dynamic.kibana.default_tenant.

We will also make changes in authInfo api to have three new properties:

  1. multi_tenancy_enabled
  2. private_tenant_enabled
  3. default_tenant

We can set these new properties using authInfo API and these will be used to enable/disable multi_tenancy and private_tenancy and set default_tenant. If multi_tenancy is disabled, we can remove the option to switch tenants from Dashboards page and set default tenant to Global tenant. If only private_tenant is disabled, we can grey-out the private_tenant option from tenant switch panel. We are also working on how the UI should look like to choose default_tenant.

cc: @varun-lodaya @prabhat-chaturvedi @devardee

@abhivka7 abhivka7 added enhancement Enhancement or improvement to existing feature or request untriaged labels Jan 13, 2023
@minalsha minalsha added RFC Issues requesting major changes and removed untriaged labels Jan 13, 2023
@abhivka7 abhivka7 changed the title [RFC] Configurable Tenancy Feature [RFC] Dynamically Configurable Tenancy Feature Feb 7, 2023
@abhivka7
Copy link
Author

abhivka7 commented Feb 7, 2023

Why should we make these features Dynamic?

As mentioned earlier, right now we can enable/disable multi-tenancy and private-tenant by changing YAML file & restarting Dashboard environment of each data node. But this has some drawbacks like:

  1. We will need to do a Dashboards env restart. This takes time and it is better if we can give our users the option to make these changes without having to restart.
  2. In case of 100+ data nodes, user will have to change YAML file of each data node either manually or have to automate the process.
  3. In case of multiple data nodes, if user forgets to update YAML file of a few nodes, then user experience will differ based on which node the request hits which is not desirable.

With our new approach, we will change tenancy properties using Patch SecurityConfig API, eg:

1. curl -X PATCH https://localhost:9200/_plugins/_security/api/securityconfig -H 'Content-Type: application/json' -H 'Accept: application/json' -d '[{"op": "add","path": "/config/dynamic/kibana/multitenancy_enabled","value": "false"}]' -k -u 'admin:admin'

We will further read this value from authinfo API in backend and use it in Dashboards plugin. These API changes will be dynamic and will only require a dashboards page refresh instead of a node restart.

@abhivka7
Copy link
Author

abhivka7 commented Feb 7, 2023

Scope of the Project:

  1. The current user experience remains same. Only admin gets access to enable/disable multi-tenancy and private tenant and set default tenant.

@abhivka7
Copy link
Author

abhivka7 commented Feb 8, 2023

Earlier we were planning to change dynamic section of SecurityConfig and use these values to implement our changes dynamically.
But since tenancy values are not specific to Security, we are now exploring the option to put these values in a separate document called TenancyConfig and use it to get values dynamically.

@davidlago
Copy link

Tagging teams/people who have put some thought into multitenancy, to make sure they lay eyes on this RFC:

@opensearch-project/security @jimishsh @shanilpa @kgcreative @cliu123 @zengyan-amazon @seraphjiang

@stephen-crawford
Copy link
Contributor

Hi @abhivka7,

I really like the direction you are going with this.

I wanted to leave a few comments for consideration as you design and implement things:

  • For the API REST endpoint and payload, you gave an example of curl -X PATCH https://localhost:9200/_plugins/_security/api/securityconfig -H 'Content-Type: application/json' -H 'Accept: application/json' -d '[{"op": "add","path": "/config/dynamic/kibana/multitenancy_enabled","value": "false"}]' -k -u 'admin:admin'
    • What are your thoughts on using some kind of resolution alias for the request? I know that it is not a huge deal, but this is certainly not something I would want to CLI request. Of course users are more likely to use the admin tool on dashboards or another tool such as postman, but I was curious is you thought about some way of resolving the request behind the scenes so that the actual REST call could be less ominous.
  • You mentioned that only admin get to set different tenant settings. I was wondering if you had considered allowing non-admin users to be able to set their default tenant in the case that they have multiple?
  • For the configuration file, I understand your point about tenancy not being unique to Security. That being said, it is part of the Security plugin since it relates to authc/authz. I think you are fine to store the configurations in the normal file but whatever you decide would be fine in my opinion. You may also be interested in the Identity work since there may be a good place for you to store configurations there.

Great work!

@peternied
Copy link
Member

@scrawfor99 Good questions, @abhivka7 could you address the comment?

@jimishs
Copy link

jimishs commented Mar 10, 2023

Thanks @scrawfor99

You mentioned that only admin get to set different tenant settings. I was wondering if you had considered allowing non-admin users to be able to set their default tenant in the case that they have multiple?

Default tenant is a configuration setting only for the initial login or if the user starts a new browser session. Once the user logs in and selects a different tenant, that tenant will be remembered for subsequent user logins.

@kgcreative
Copy link
Member

Since this feature has launched, can we close this RFC?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement or improvement to existing feature or request RFC Issues requesting major changes
Projects
None yet
Development

No branches or pull requests

7 participants