[GitHub] [incubator-superset] altef commented on issue #8699: [SIP-29] Add support for row-level security

[GitBox <gi...@apache.org>]

altef commented on issue #8699: [SIP-29] Add support for row-level security
URL: https://github.com/apache/incubator-superset/pull/8699#issuecomment-560004957
 
 
   Hey - thanks for looking it over!  I wanted to point that out in the docs for clarity, 
   and viewed it as more of something the user should be aware of than as an issue.  I can definitely 
   see a certain use-case where it may not be ideal, but am of the mindset that it's better
   to show no data (which can be easily detected and fixed), should the user introduce a 
   conflict in the filters, than to be overly permissive and allow a user to see data they otherwise
   shouldn't be able to see.  Should _that_ occur, it may not be readily apparent.
   
   I think everything you've mentioned can be mostly accomplished as-is, by taking a slightly 
   different approach to the role/filter composition.
   
   1.  Filters clauses can contain OR.  You could simply make a filter with the 
   clause `dept_id = 'Finance' OR dept_id = 'Risk'` and assign it to a particular role.  Obviously 
   this is not ideal if you want to assign a user to more than one department by assigning them 
   two roles.  What I'm not sure of is how much this needs to come up.  If someone has access to 
   both the `Finance` and the `Risk`, are they really in each department, or are they some 
   higher-up who should have greater access? The current method, I hope, seems analogous to superset's **Restricting access to a subset of data sources** recommendation.
   
   Which leads me to realize that there was a bug in my initial implementation - filters are now 
   wrapped in parentheses in case one has an OR in it.
   
   2.  To reference the **Restricting access to a subset of data sources** again, it's recommended 
   to assign a limited user to the gamma role, and to make a role giving them access to the 
   datasource in question.  In the case of creating a default filter, you could just assign a 
   filter to that role.  However, it is not exactly what you have in mind, since it wouldn't 
   be _overwritten_ by additional filters.
   
   
   My worry is that we'd be increasing complexity, in the code, as well as in ease-of-understanding on the 
   user's part, for minimal gains.  However, it could be that I'm taking the wrong approach here.  
   Is this something others would find useful?
   
   
   
   I would like to eventually add (probably as a separate SIP?) arbitrary user attributes that 
   can then be referenced in the row level security filters.  But I'd want to talk to someone 
   about that, as I have many questions in that regard.
   

----------------------------------------------------------------
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