Incubator-superset: [SIP-29] Add support for row-level security

[justin-barton]

[SIP-29] Proposal to add support for row-level security

Motivation

Many BI applications, particularly in multi-tenancy scenarios, require support for row-level security. That is, the ability to show different slices of a table to users based on some user attribute.

This feature has been requested in Superset several times, including:
https://github.com/apache/incubator-superset/issues/660
https://github.com/apache/incubator-superset/issues/5128
https://github.com/apache/incubator-superset/issues/7887
https://github.com/apache/incubator-superset/issues/8023

Proposed Change

Primarily, this feature would require two sub-features:

1. The ability to add user attributes
2. The ability to filter queries based on these attributes

The proposed solution would be to add an 'attributes' property to roles that would essentially be a user-defined key-value store:

and to make those role attributes available as values for Query Filters:

New or Changed Public Interfaces

• New 'attributes' property added to roles
• Changes to Query Filters to allow access to attribute values (e.g by macros like WHERE column={{role.attributes.key}}
• Minor UI changes associated with the above

New dependencies

N/A

Migration Plan and Compatibility

TBD

Rejected Alternatives

Pre-processing data into multiple tables or views:

• This solution does not scale well, and requires all attribute values to be known a priori
• Requires duplicate dashboards and charts

Reverse proxy

• Issues with identifying the user initiating the query

[issue-label-bot[bot]]

Issue-Label Bot is automatically applying the label #enhancement to this issue, with a confidence of 0.98. Please mark this comment with :thumbsup: or :thumbsdown: to give our bot feedback!

Links: app homepage, dashboard and code for this bot.

[axuew]

I also need this function very much. At present, I solve this problem by developing custom data permissions through jinja template

[toop]

There are security problems in this plan: Use query filters to allow access to attribute values ,The current login user can delete the filter conditions in the query. If you do not give explore permission, the current user will lack the explore interaction function.

[thunter009]

I also need this function very much. At present, I solve this problem by developing custom data permissions through jinja template

Can you elaborate on your approach?

[altef]

I've been looking into this with an eye toward implementing it, using a slightly different approach:

Proposed Change

1. Add a new model to describe row level security filters, which references a Table and a Role. So when adding a row level security filter, you specify a particular Role and Table.

2. Add the filters in to the query when applicable. I've modified the query function here to add any relevant to the WHERE clause.

3. Create a UI for row level security filters. I haven't started on this yet, in case I'm missing something and this method proves ill-advised.

New or Changed Public Interfaces

• New row level security filters interface

New dependencies

N/A

Migration Plan and Compatibility

TBD

It all seems to be working as expected at this point. Does anyone foresee any obvious issues with this method?

[justin-barton]

I've been looking into this with an eye toward implementing it, using a slightly different approach:

Proposed Change

1. Add a new model to describe row level security filters, which references a Table and a Role. So when adding a row level security filter, you specify a particular Role and Table.
2. Add the filters in to the query when applicable. I've modified the query function here to add any relevant to the WHERE clause.
3. Create a UI for row level security filters. I haven't started on this yet, in case I'm missing something and this method proves ill-advised.

New or Changed Public Interfaces

• New row level security filters interface

New dependencies

N/A

Migration Plan and Compatibility

TBD

It all seems to be working as expected at this point. Does anyone foresee any obvious issues with this method?

This sounds ideal. @toop any concerns with this revised approach?

[axuew]

@thunter009 I basically follow @altef and define a model to store the roles and data permissions of the account. For some roles, the data permissions are not controlled, while the normal permissions control the data of the query according to the roles and data permissions.You end up adding this custom permission control to the front of the filter, and then interacting with the data you see on the screen.
However, direct control through tables is not currently supported, only through sqllab

[axuew]

@altef :I think just adding a component to the diagram (as shown in the initial screenshot of @justin-barton ) that is controlled by permissions is a good solution to the user interaction and security issues.
:+1:

[altef]

Proposed solution

1. Row level security filters added to the Security menu:
   

2. Row level security list
   

3. Row level security interface
   

That allows for the management of the RowLevelSecurityFilters model. Additionally, for convenience, I've added it as a related view for tables.

After logging in with a user assigned to that role, I can still supply additional filters:

The generated SQL includes the additional filters AND the clause supplied by the row level security filter(s).

Everything seems to be working as expected on my end, and I'd be happy to provide a pull request if no one has any objections to this implementation?

[axuew]

@altef How do you resolve different client_ids for the same role in your current solution?

[altef]

@axuew Different clients would just have different Roles assigned to them. Each client can potentially have any number of users, and all would be assigned the role for that client.
However, since the clause can be whatever you want injected into the query's WHERE, it should be fairly flexible and you can handle things however suits you.

[axuew]

@altef Your solution is good enough so far, but I recommend that you use the expression client_id={g.u.ser.id.dp} in a model that stores different ids for users to resolve the complexity of permission configuration.

[altef]

@axuew I'm sorry, I don't think I follow. Could you rephrase?

In our case this method seems suitable because the field we check against can vary by table and is often a string. So the filter's clause would often be something like advertiser_name IN ("foo", "bar"), for roles who should have access to advertisers foo and bar.

[toop]

I've been looking into this with an eye toward implementing it, using a slightly different approach:

Proposed Change

1. Add a new model to describe row level security filters, which references a Table and a Role. So when adding a row level security filter, you specify a particular Role and Table.
2. Add the filters in to the query when applicable. I've modified the query function here to add any relevant to the WHERE clause.
3. Create a UI for row level security filters. I haven't started on this yet, in case I'm missing something and this method proves ill-advised.

New or Changed Public Interfaces

• New row level security filters interface

New dependencies

N/A

Migration Plan and Compatibility

TBD
It all seems to be working as expected at this point. Does anyone foresee any obvious issues with this method?

This sounds ideal. @toop any concerns with this revised approach?

Agree to this revised ！This good ideal ！！！
Some proposals ：
1、Dynamic multi conditional support, such as：“department = {user}.department，client_id =4”
2、Can support the datasources`s tables
3、Can add user attributes
4、FAB's empowerment view is too ugly and does not support 18in, improve FAB's authority view

Thanks！！！
@altef @justin-barton

[altef]

Essentially, we're giving each role access to a subset of the table. So we could have one role: Company A, who has the clause company_id=12
And another role: Last 30 days, who has the clause date_field > DATE_SUB(NOW(), INTERVAL 30 DAY)

And for a specific user of Company A, if they should only be able to see the last 30 days worth of data, apply both roles.

Since a row level security filter's clause is arbitrary, it can also support multiple conditions: client_id = 6 AND advertiser="foo", etc. You can throw whatever you want in there to define the subset of the table you want the role in question to have access to.

[toop]

We are looking forward to your revised ！Thank you very much. @altef

[axuew]

@altef Your plan does solve all my problems and is flexible.However, if you configure one role for each user, it will be difficult to maintain because of the large number of users and roles. Therefore, I suggest to optimize the configuration for the user level after you merge the code

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. For admin, please label this issue .pinned to prevent stale bot from closing the issue.

[altef]

Update: I've created a pull request which adds this feature; working through the process of getting it accepted. I think we're close. Currently awaiting another review.

[i2cute]

hi, have this released?

[altef]

@i2cute it's been merged in, i'm not sure how that'll be reflected in the release schedule though
(Apparently it's in 0.36.0rc1, according to durchgedreht)

[AleVzT]

Proposed solution

1. Row level security filters added to the Security menu:
   
2. Row level security list
   
3. Row level security interface
   

That allows for the management of the RowLevelSecurityFilters model. Additionally, for convenience, I've added it as a related view for tables.

After logging in with a user assigned to that role, I can still supply additional filters:

The generated SQL includes the additional filters AND the clause supplied by the row level security filter(s).

Everything seems to be working as expected on my end, and I'd be happy to provide a pull request if no one has any objections to this implementation?

How can I add the filter_fila option in the security menu. I use version 0.27

[amitmiran137]

use ENABLE_ROW_LEVEL_SECURITY = True in you superset_config.py file
@AleVzT

[AleVzT]

utilizar ENABLE_ROW_LEVEL_SECURITY = Trueen su archivo superset_config.py
@AleVzT

the superset_config.py file does not exist in version 0.27. config.py is called but the ENABLE_ROW_LEVEL_SECURITY statement is missing

[AleVzT]

@altef @amitNielsen @justin-barton

[amitmiran137]

The row level security feature only exist from version 0.36
And you also cherry-pick it with the following fix:
important role bug fix

Otherwise feature will just not work

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. For admin, please label this issue .pinned to prevent stale bot from closing the issue.

[xcoder123]

Hi,

I'm running the latest 0.36 version

But when I enable the ENABLE_ROW_LEVEL_SECURITY

Nothing appears in menu. With logger I can see the enable flag is set to true

I thought maybe I need to set permissions for my gropup, similar to list LogModelView, maybe there is list RowLevelSecurityView.

It doesn't seem to be the case

[altef]

Hi @xcoder123 - does your user have the Admin role?

[xcoder123]

HI @altef

Yes indeed, my user is part of admin group.

What is interesting it doesn't show up in the menu:

One more thing I want to quickly note, this might relate to the issue.

I'm running the superset in docker, with pyodbc with MSSQL support.
In addition I have implemented the JWT token authorization that has been flowing around the git issue tracker.

Where I have my custom security manager. Though switching it off, doesn't seem to help.

Thank You

[altef]

That is weird! As far as I know these are the two sections that determine if it appears:

https://github.com/apache/incubator-superset/blob/9d757403be58cba16386f4a55e4c7194abc6881c/superset/app.py#L260-L268

https://github.com/apache/incubator-superset/blob/9d757403be58cba16386f4a55e4c7194abc6881c/superset/security/manager.py#L129-L140

And both of those seem to be true in your case

[xcoder123]

Hi @altef

I managed to find what was wrong. It was misconfigured docker-compose file from our side.

For some reason superset-init and superset-worker was commented out by me.

Thank you for quick response.

[altef]

That is a relief - thanks for letting me know!

[redmonki]

@altef is the feature only enabled for queries executed from "charts"? I am not seeing the row level filters applied on the queries run from SQL Lab.

[mistercrunch]

We should clarify the docs on that topic if it's not clear there.

[altef]

It applies the restriction in connectors/sqla/models.py:get_sqla_query(), so unless SQL Lab uses that (I haven't used SQL Lab yet, but it seems unlikely) it probably doesn't.
How does SQL Lab work? Restricting arbitrary SQL would at the very least require parsing of that SQL so as to appropriately limit any subqueries and joins, in addition to the base table.

[vrnvikas]

@altef - I am facing a similar issue where I added the flag for enabling ROW_LEVEL in superset config but still I am not able to see this option enabled in UI. I tried it in charts also no luck there also. I tried this in 0.36 plus 0.37rc4 not able to get this working.

[altef]

Hi @vrnvikas - just to be clear, did you use ROW_LEVEL or ENABLE_ROW_LEVEL_SECURITY?

[vrnvikas]

Hi @vrnvikas - just to be clear, did you use ROW_LEVEL or ENABLE_ROW_LEVEL_SECURITY?

I used "ENABLE_ROW_LEVEL_SECURITY" only.

[altef]

I've heard a few people recently with similar issues^1,2. It seems like running superset init has solved it for them!

[vrnvikas]

I've heard a few people recently with similar issues1,2. It seems like running superset init has solved it for them!

thanks @altef running superset init solved the issue.

[Nj-kol]

Yes, by default ENABLE_ROW_LEVEL_SECURITY is False, if you change this, you must do superset init. This is because this feature creates some backend Database artifacts