Added multi-tenancy support. #3729
Conversation
Issue: apache#1089 To achieve multi-tenancy: 1. set "ENABLE_MULTI_TENANCY = True" in superset_config file. 2. add column tenant_id String(256) in the tables or views in which you want multi-tenancy. 3. if you are adding the multi-tenancy in existing project then make sure that ab_user table have the column tenant_id else alter the table. 4. if you want to enable multi-tenancy with CUSTOM_SECURITY_MANAGER, then your custom security manager class should be a subclass of MultiTenantSecurityManager class. Added the documentation for multi-tenancy. Fixed few typing errors. Also remove tenant_id from user view. Fixes few test cases and role update api to support the custom user model.
|
The coverage is decreased because multi-tenancy featured is used based on configuration. By default, multi-tenancy is not enabled, so test cases don't hit the newly added lines for multitenancy. |
|
How do you know it works without tests? |
|
I have tested it locally by setting "ENABLE_MULTI_TENANCY = True" in superset_config file and making slices from the datasource which have the tenant_id as one of its columns. If datasource does not have the column tenant_id, then tenant filter is not applied for slices made from this datasource. |
|
I tried doing this , it doesn't work , anyone was able to do this successfully ? Idea is that , this should solve the issue of dynamic data level filtering on user login. |
|
For us, it is working. Can you please tell us what steps you have taken to achieve this(please mention whether you are doing it in the existing project or new one). |
|
I used the file which you have changed to achieve this (init.py , security.py...etc) and used in my latest version of superset . All the views are broken with this error get_sqla_query() got an unexpected keyword argument 'form_data' . I believe somehow I need to download all the files from your commit tree , but am unable to do that . Is there a way to get all the files in your commit tree ? |
|
this was built on top of a deprecated endpoint (update_role). So, you would probably need to adapt it to the current superset version. |
|
Thanks, @neilsoncarlos for pointing out the new changes. I will soon do the required changes. and update the pull request. |
|
@thakur00mayank I'm interested in multi-tenancy, you need help on making the changes? |
|
if you follow the steps above you will end up with a: File "C:\dev\superset\lib\site-packages\superset_init_.py", line 19, in Did anyone else try to implement this? |
There was a problem hiding this comment.
I think we need a database migration that adds tenant_id to all relevant tables (happy to discuss if you disagree). We probably want to do this using a MultiTenantModelMixin simple class that adds the field and perhaps a is_user_tenant method and/or other utility method. There may be a way to force a filter based on tenant_id in the query method (call super and force-apply a tenant_id filter in the SQLAlchemy method itself).
I think it's good for the database structure to be homogenous across all Superset installs as opposed to creating columns manually / conditionally. Personally I'd rather have an unused tenant_id column than conditions around whether the column exists or not.
I think this solution is incomplete where we'll need a new TenantAdmin role that allows tenants to manage their own rights limited their scope. I think there might also be a need to not expose the Superset database itself in SQL Lab itself. Sounds like we need to de-couple the examples database connection from the main database itself.
| from flask_appbuilder import Model | ||
| from flask_appbuilder.security.views import UserDBModelView | ||
| from flask_babel import lazy_gettext | ||
|
|
There was a problem hiding this comment.
NIT: our linter / PEP8 likes 2 empty lines before method and class definition. To lint locally, run flake8 superset/ from the root of the repo
| # This will add multi tenant support in user model | ||
| class MultiTenantSecurityManager(SecurityManager): | ||
| user_model = MultiTenantUser | ||
| userdbmodelview = MultiTenantUserDBModelView |
|
Disregard the comment about overriding |
|
Hello there, is there a reason that this feature has not been merged to master or is there someone else working on refactoring and making this better? Just keen to know, so that we can work on it and contribute from our end if any one has a use case. Alternatively, keen on not reinventing the wheel if someone has found a workaround for this. Thanks! |
Same here, we are reviewing Superset and multi-tenancy is a must have for us.. Thanks in advance. |
|
I'm interested in a multi-tenant setup as well. Happy to help if there are details on what work is remaining here. |
|
Any updates on this, I'm interested in this as well. |
|
We don't think this is the right approach, and it turns out making Superset multitenant with strong security/isolation guarantees isn't easy. This would require a SIP (Superset Improvement Proposal) with a detailed plan. Also to be discussed is whether we'd want to do schema-level multitenancy or row-level. Row-level seems like the right approach, but would have to be baked deeply in the app. That's hard to do as an afterthought, and especially given the fact that FAB is not tenant aware. Making FAB tenant-aware may be part of the solution. |
|
if you follow the steps above you will end up with a: File "C:\dev\superset\lib\site-packages\superset_init_.py", line 19, in I get the same, is anyone find the solution for this error |
Issue: #1089
To achieve multi-tenancy:
make sure that ab_user table have the column tenant_id else alter the table.
then your custom security manager class should be a subclass of MultiTenantSecurityManager class.
Added the documentation for multi-tenancy.
Fixed few typing errors. Also remove tenant_id from user view.
Fixes few test cases and role update api to support the custom user model.