[GitHub] timifasubaa commented on a change in pull request #4409: Add permissions decider for delegating access controls.

2018-02-14 Thread GitBox
timifasubaa commented on a change in pull request #4409: Add permissions 
decider for delegating access controls.
URL: 
https://github.com/apache/incubator-superset/pull/4409#discussion_r168007018
 
 

 ##
 File path: superset/config.py
 ##
 @@ -364,6 +364,24 @@ class CeleryConfig(object):
 # Interval between consecutive polls when using Hive Engine
 HIVE_POLL_INTERVAL = 5
 
+
+# System to handle delegated data access. Implement both is_allowed_access() 
and
+# is_eligible_datasource() to delegate access controls.
+class PermsDecider:
 
 Review comment:
   But None is falsey and so it will be easy to mix up `access denied` with `I 
don't know enough to decide for this datasource (from this db)`


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


With regards,
Apache Git Services


[GitHub] timifasubaa commented on a change in pull request #4409: Add permissions decider for delegating access controls.

2018-02-14 Thread GitBox
timifasubaa commented on a change in pull request #4409: Add permissions 
decider for delegating access controls.
URL: 
https://github.com/apache/incubator-superset/pull/4409#discussion_r168307620
 
 

 ##
 File path: superset/viz.py
 ##
 @@ -307,7 +307,12 @@ def get_df_payload(self, query_obj=None):
 cached_dttm = datetime.utcnow().isoformat().split('.')[0]
 if cache_key and cache and not self.force:
 cache_value = cache.get(cache_key)
-if cache_value:
+perms_decider = config.get('DATA_PERMS_DECIDER')
+perms_decider_disapproves = (
 
 Review comment:
   In our case, we don't want to provide info from the cache based on superset 
permissions. Rather from an outside system. I moved the check from viz.py to 
explore_json. and set force=true when the perms_decider denied the person 
access. 


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


With regards,
Apache Git Services


[GitHub] timifasubaa commented on a change in pull request #4409: Add permissions decider for delegating access controls.

2018-02-13 Thread GitBox
timifasubaa commented on a change in pull request #4409: Add permissions 
decider for delegating access controls.
URL: 
https://github.com/apache/incubator-superset/pull/4409#discussion_r168007018
 
 

 ##
 File path: superset/config.py
 ##
 @@ -364,6 +364,24 @@ class CeleryConfig(object):
 # Interval between consecutive polls when using Hive Engine
 HIVE_POLL_INTERVAL = 5
 
+
+# System to handle delegated data access. Implement both is_allowed_access() 
and
+# is_eligible_datasource() to delegate access controls.
+class PermsDecider:
 
 Review comment:
   But None is falsey and so it will be easy to mix up `access denied` from `I 
don't know enough to decide for this datasource (from this db)`


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


With regards,
Apache Git Services


[GitHub] timifasubaa commented on a change in pull request #4409: Add permissions decider for delegating access controls.

2018-02-13 Thread GitBox
timifasubaa commented on a change in pull request #4409: Add permissions 
decider for delegating access controls.
URL: 
https://github.com/apache/incubator-superset/pull/4409#discussion_r168007018
 
 

 ##
 File path: superset/config.py
 ##
 @@ -364,6 +364,24 @@ class CeleryConfig(object):
 # Interval between consecutive polls when using Hive Engine
 HIVE_POLL_INTERVAL = 5
 
+
+# System to handle delegated data access. Implement both is_allowed_access() 
and
+# is_eligible_datasource() to delegate access controls.
+class PermsDecider:
 
 Review comment:
   But None is falsey and so it will be easy to mix up `access denied` from `I 
don't know enough to decide for this case`


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


With regards,
Apache Git Services


[GitHub] timifasubaa commented on a change in pull request #4409: Add permissions decider for delegating access controls.

2018-02-13 Thread GitBox
timifasubaa commented on a change in pull request #4409: Add permissions 
decider for delegating access controls.
URL: 
https://github.com/apache/incubator-superset/pull/4409#discussion_r167971247
 
 

 ##
 File path: superset/config.py
 ##
 @@ -364,6 +364,24 @@ class CeleryConfig(object):
 # Interval between consecutive polls when using Hive Engine
 HIVE_POLL_INTERVAL = 5
 
+
+# System to handle delegated data access. Implement both is_allowed_access() 
and
+# is_eligible_datasource() to delegate access controls.
+class PermsDecider:
 
 Review comment:
   Agree that it should be a static class. 
   The concern is that if it is defined outside of config.py, there's no way to 
delegate access decisions to a system outside of superset. 
   
   Since perms_decider might only apply to a subset of databases ot 
datasources, If we use has_datasource_access, it's not clear what to return 
when the datasource in question is out of the jurisdiction of the 
perms_decider. 


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


With regards,
Apache Git Services