Filter table list based on the user permissions. (#1769)

* Filter table list based on the user permissions.

* Fix tests

superset/views.py

@@ -79,6 +79,11 @@ def datasource_access_by_name(
         if (self.database_access(database) or
                 self.all_datasource_access()):
             return True
+
+        schema_perm = utils.get_schema_perm(database, schema)
+        if schema and utils.can_access(sm, 'schema_access', schema_perm):
+            return True
+
         datasources = SourceRegistry.query_datasources_by_name(
             db.session, database, datasource_name, schema=schema)
         for datasource in datasources:
@@ -214,10 +219,7 @@ class SupersetFilter(BaseFilter):
     """
 
     def get_user_roles(self):
-        attr = '__get_user_roles'
-        if not hasattr(self, attr):
-            setattr(self, attr, get_user_roles())
-        return getattr(self, attr)
+        return get_user_roles()
 
     def get_all_permissions(self):
         """Returns a set of tuples with the perm name and view menu name"""
@@ -253,21 +255,12 @@ def has_all_datasource_access(self):
             self.has_perm('all_datasource_access', 'all_datasource_access'))
 
 
-class DatabaseFilter(SupersetFilter):
-    def apply(self, query, func):  # noqa
-        if (
-                self.has_role('Admin') or
-                self.has_perm('all_database_access', 'all_database_access')):
-            return query
-        perms = self.get_view_menus('database_access')
-        return query.filter(self.model.perm.in_(perms))
-
-
 class DatasourceFilter(SupersetFilter):
     def apply(self, query, func):  # noqa
         if self.has_all_datasource_access():
             return query
         perms = self.get_view_menus('datasource_access')
+        # TODO(bogdan): add `schema_access` support here
         return query.filter(self.model.perm.in_(perms))
 
 
@@ -276,6 +269,7 @@ def apply(self, query, func):  # noqa
         if self.has_all_datasource_access():
             return query
         perms = self.get_view_menus('datasource_access')
+        # TODO(bogdan): add `schema_access` support here
         return query.filter(self.model.perm.in_(perms))
 
 
@@ -288,6 +282,7 @@ def apply(self, query, func):  # noqa
             return query
         Slice = models.Slice  # noqa
         Dash = models.Dashboard  # noqa
+        # TODO(bogdan): add `schema_access` support here
         datasource_perms = self.get_view_menus('datasource_access')
         slice_ids_qry = (
             db.session
@@ -304,6 +299,7 @@ def apply(self, query, func):  # noqa
         )
         return query
 
+
 def validate_json(form, field):  # noqa
     try:
         json.loads(field.data)
@@ -622,7 +618,6 @@ def pre_update(self, db):
 
 
 class DatabaseAsync(DatabaseView):
-    base_filters = [['id', DatabaseFilter, lambda: []]]
     list_columns = [
         'id', 'database_name',
         'expose_in_sqllab', 'allow_ctas', 'force_ctas_schema',
@@ -1695,10 +1690,11 @@ def tables(self, db_id, schema):
             .filter_by(id=db_id)
             .one()
         )
-        payload = {
-            'tables': database.all_table_names(schema),
-            'views': database.all_view_names(schema),
-        }
+        tables = [t for t in database.all_table_names(schema) if
+                  self.datasource_access_by_name(database, t, schema=schema)]
+        views = [v for v in database.all_table_names(schema) if
+                 self.datasource_access_by_name(database, v, schema=schema)]
+        payload = {'tables': tables, 'views': views}
         return Response(
             json.dumps(payload), mimetype="application/json")
 
@@ -2397,7 +2393,7 @@ def table_accessible(database, full_table_name, schema_name=None):
             t for t in superset_query.tables if not
             table_accessible(mydb, t, schema_name=schema)]
         if rejected_tables:
-            json_error_response(
+            return json_error_response(
                 get_datasource_access_error_msg('{}'.format(rejected_tables)))
         session.commit()
 

tests/base_tests.py

@@ -11,7 +11,7 @@
 
 from flask_appbuilder.security.sqla import models as ab_models
 
-from superset import app, cli, db, models, appbuilder, sm
+from superset import app, cli, db, models, appbuilder, security, sm
 from superset.security import sync_role_definitions
 
 os.environ['SUPERSET_CONFIG'] = 'tests.superset_test_config'
@@ -43,6 +43,11 @@ def __init__(self, *args, **kwargs):
         gamma_sqllab = sm.add_role("gamma_sqllab")
         for perm in sm.find_role('Gamma').permissions:
             sm.add_permission_role(gamma_sqllab, perm)
+        db_perm = self.get_main_database(sm.get_session).perm
+        security.merge_perm(sm, 'database_access', db_perm)
+        db_pvm = sm.find_permission_view_menu(
+            view_menu_name=db_perm, permission_name='database_access')
+        gamma_sqllab.permissions.append(db_pvm)
         for perm in sm.find_role('sql_lab').permissions:
             sm.add_permission_role(gamma_sqllab, perm)
 
@@ -73,6 +78,7 @@ def __init__(self, *args, **kwargs):
                 'alpha', 'alpha', 'user', '[email protected]',
                 appbuilder.sm.find_role('Alpha'),
                 password='general')
+        sm.get_session.commit()
 
         # create druid cluster and druid datasources
         session = db.session

tests/sqllab_tests.py

@@ -9,7 +9,7 @@
 import unittest
 
 from flask_appbuilder.security.sqla import models as ab_models
-from superset import db, models, utils, appbuilder, sm
+from superset import db, models, utils, appbuilder, security, sm
 from .base_tests import SupersetTestCase
 
 
@@ -18,6 +18,8 @@ class SqlLabTests(SupersetTestCase):
 
     def __init__(self, *args, **kwargs):
         super(SqlLabTests, self).__init__(*args, **kwargs)
+        gamma_sqllab = appbuilder.sm.find_role('gamma_sqllab')
+        security.merge_perm(sm, 'database_access', self.get_main_database(db.session).perm)
 
     def run_some_queries(self):
         self.logout()
@@ -93,15 +95,15 @@ def test_queries_endpoint(self):
         self.assertEquals(2, len(data))
 
         # Run 2 more queries
-        self.run_sql("SELECT * FROM ab_user1", client_id='client_id_4')
-        self.run_sql("SELECT * FROM ab_user2", client_id='client_id_5')
+        self.run_sql("SELECT * FROM ab_user LIMIT 1", client_id='client_id_4')
+        self.run_sql("SELECT * FROM ab_user LIMIT 2", client_id='client_id_5')
         self.login('admin')
         data = self.get_json_resp('/superset/queries/0')
         self.assertEquals(4, len(data))
 
         now = datetime.now() + timedelta(days=1)
         query = db.session.query(models.Query).filter_by(
-            sql='SELECT * FROM ab_user1').first()
+            sql='SELECT * FROM ab_user LIMIT 1').first()
         query.changed_on = now
         db.session.commit()
 
@@ -140,7 +142,8 @@ def test_search_query_on_user(self):
         self.assertEquals(set([user.id]), user_ids)
 
         user = appbuilder.sm.find_user('gamma_sqllab')
-        resp = self.get_resp('/superset/search_queries?user_id={}'.format(user.id))
+        resp = self.get_resp(
+            '/superset/search_queries?user_id={}'.format(user.id))
         data = json.loads(resp)
         self.assertEquals(1, len(data))
         self.assertEquals(list(data.values())[0]['userId'] , user.id)