You are viewing a plain text version of this content. The canonical link for it is here.

Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2020/04/14 16:26:44 UTC

[GitHub] [incubator-superset] axelet opened a new issue #9532: Row Level Security filter wildcard for all tables and multiple table filters

axelet opened a new issue #9532: Row Level Security filter wildcard for all tables and multiple table filters
URL: https://github.com/apache/incubator-superset/issues/9532

### **Case**
As we work closely with Superset and the new **Row Level Security** feature we soon came to the situation in which we have pretty much the same filters for multiple tables. And the amount of the filters is growing constantly. So, it becomes difficult to manage all these filters in the current RowLevelSecurityView.

Also, there is a security case when the user have a filter constraint on a certain table and an additional sqllab_view table is being created. So the user will be able to see unauthorised data. And the 1st solution below will solve this one too.

### **Suggestion**
As now we have only per table filters we propose to implement some kind of all table filters and multiple table filters.

### **Possible solutions**
1. We can make the **table_id** column of the row_level_security_filters table **nullable**. And **null** will stand for asterisk (apply the filter to all tables). The solution currently fits all own requirements.
2. As a development of the 1st idea we can introduce a new table called, for example, **rls_filter_tables**. It will represent the relation between **rls_filter_id** and **table_id** pretty much the same as we have for role-filter relations in the **rls_filter_roles**.
```
id | table_id | rls_filter_id
1 | 5 | 1
2 | 6 | 1
```

### **Outcomes**
This way we can re-use existing filters for other tables and preserve space and readability on the RowLevelSecurityView page.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] villebro commented on issue #9532: Row Level Security filter wildcard for all tables and multiple table filters

Posted by GitBox <gi...@apache.org>.

villebro commented on issue #9532: Row Level Security filter wildcard for all tables and multiple table filters
URL: https://github.com/apache/incubator-superset/issues/9532#issuecomment-613619153
 
 
   Thanks for the proposal @axelet ! please let me digest this a bit and get back in a day or two.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] axelet edited a comment on issue #9532: Row Level Security filter wildcard for all tables and multiple table filters

Posted by GitBox <gi...@apache.org>.

axelet edited a comment on issue #9532: Row Level Security filter wildcard for all tables and multiple table filters
URL: https://github.com/apache/incubator-superset/issues/9532#issuecomment-613545195
 
 
   @altef @villebro @justin-barton @mistercrunch Hey guys, please express your thoughts on the ideas if any :slightly_smiling_face:

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] axelet commented on issue #9532: Row Level Security filter wildcard for all tables and multiple table filters

Posted by GitBox <gi...@apache.org>.

axelet commented on issue #9532: Row Level Security filter wildcard for all tables and multiple table filters
URL: https://github.com/apache/incubator-superset/issues/9532#issuecomment-613545195
 
 
   @altef @villebro @justin-barton @mistercrunch Hey guys, please express your thought on the ideas if any :slightly_smiling_face:

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] axelet commented on issue #9532: Row Level Security filter wildcard for all tables and multiple table filters

Posted by GitBox <gi...@apache.org>.

axelet commented on issue #9532:
URL: https://github.com/apache/incubator-superset/issues/9532#issuecomment-616641336


   Hey @villebro , did you have a chance to look at these ones? I can try to implement if they are all right


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] stale[bot] commented on issue #9532: Row Level Security filter wildcard for all tables and multiple table filters

Posted by GitBox <gi...@apache.org>.

stale[bot] commented on issue #9532:
URL: https://github.com/apache/incubator-superset/issues/9532#issuecomment-653895550


   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.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] villebro commented on issue #9532: Row Level Security filter wildcard for all tables and multiple table filters

Posted by GitBox <gi...@apache.org>.

villebro commented on issue #9532:
URL: https://github.com/apache/incubator-superset/issues/9532#issuecomment-616661137


   Hey @axelet thanks for the ping! Will put a post-it on my desk to remember to review asap!


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] stale[bot] closed issue #9532: Row Level Security filter wildcard for all tables and multiple table filters

Posted by GitBox <gi...@apache.org>.

stale[bot] closed issue #9532:
URL: https://github.com/apache/incubator-superset/issues/9532


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [incubator-superset] axelet commented on issue #9532: Row Level Security filter wildcard for all tables and multiple table filters

Posted by GitBox <gi...@apache.org>.

axelet commented on issue #9532:
URL: https://github.com/apache/incubator-superset/issues/9532#issuecomment-624671462

@villebro
As for now I posted a version (https://github.com/apache/incubator-superset/pull/9751) where we have the same filters for different tables grouped together in one filter with multiple tables (Solution 2 described above, except that it doesn't support any wildcard logic). I also added a test to ensure that it works. The original behaviour is not changed for now. However, this doesn't cover the security case I described before.

As for your question about column not present in the table we can handle it by checking and filtering all clauses came from **_get_sqla_row_level_filters()** (if I got you correctly). We need them to have the filters specific columns, so we can check them in **SqlaTable.get_sqla_query()** and apply only appropriate ones. We have the **cols** dict with col_names, let's check the clauses to have the col_names. Or can we leave it as a user responsibility?

For expr_qry and aliases I agree it can be circumvented as long as a potential admin grants the SQL Lab access to users. I'm not sure if it's possible without Sql Lab, could you provide any cases? So, I assume it could be done for users without SQL Lab rights (if only admins can create views). Please, correct me if I'm wrong. So, there is nothing we can do here if not introducing some wildcards for tables or schemas.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-superset] villebro commented on issue #9532: Row Level Security filter wildcard for all tables and multiple table filters

Posted by GitBox <gi...@apache.org>.

villebro commented on issue #9532:
URL: https://github.com/apache/incubator-superset/issues/9532#issuecomment-619996182


   @axelet Have you tested making these changes locally as a POC? While in principle this makes sense, it would be interesting to see if any problems arise with this change. Also, this does require all tables to conform to a specific column naming convention, so there would need to be logic in place to handle cases where a filter's columns are not present in the table. Also, this could potentially be circumvented by introducing aliases in a SQL query, and injecting filters into the subquery (`expr_qry`) might be problematic, so I'm not sure if these filters can be enforced in SQL Lab as easily as in regular tables.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org