This is an automated email from the ASF dual-hosted git repository.
michellet pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-superset.git
The following commit(s) were added to refs/heads/master by this push:
new 51068f0 Adding permission for can_only_access_owned_queries (#7234)
51068f0 is described below
commit 51068f007efb1362cf59a54f7d3909a526bd24c4
Author: michellethomas <[email protected]>
AuthorDate: Wed Apr 17 16:11:11 2019 -0700
Adding permission for can_only_access_owned_queries (#7234)
* Adding permission for can_only_access_owned_queries
* Fixing lint adding typing to variable
* Adding test for queryview and enabling /queryview/api/read
* Fixing issues with python typing
---
superset/models/sql_lab.py | 8 ++++
superset/security.py | 13 ++++++
superset/views/core.py | 22 +++++++---
superset/views/sql_lab.py | 32 ++++++++++++--
tests/sqllab_tests.py | 105 ++++++++++++++++++++++++++++++++++++++++++++-
5 files changed, 170 insertions(+), 10 deletions(-)
diff --git a/superset/models/sql_lab.py b/superset/models/sql_lab.py
index 93eae2f..2d23a64 100644
--- a/superset/models/sql_lab.py
+++ b/superset/models/sql_lab.py
@@ -138,6 +138,14 @@ class Query(Model, ExtraJSONMixin):
tab = re.sub(r'\W+', '', tab)
return f'sqllab_{tab}_{ts}'
+ @property
+ def database_name(self):
+ return self.database.name
+
+ @property
+ def username(self):
+ return self.user.username
+
class SavedQuery(Model, AuditMixinNullable, ExtraJSONMixin):
"""ORM model for SQL query"""
diff --git a/superset/security.py b/superset/security.py
index d2208e6..e5ca963 100644
--- a/superset/security.py
+++ b/superset/security.py
@@ -92,6 +92,7 @@ class SupersetSecurityManager(SecurityManager):
'schema_access',
'datasource_access',
'metric_access',
+ 'can_only_access_owned_queries',
])
def get_schema_perm(self, database, schema):
@@ -105,6 +106,17 @@ class SupersetSecurityManager(SecurityManager):
return self.is_item_public(permission_name, view_name)
return self._has_view_access(user, permission_name, view_name)
+ def can_only_access_owned_queries(self) -> bool:
+ """
+ can_access check for custom can_only_access_owned_queries permissions.
+
+ :returns: True if current user can access custom permissions
+ """
+ return self.can_access(
+ 'can_only_access_owned_queries',
+ 'can_only_access_owned_queries',
+ )
+
def all_datasource_access(self):
return self.can_access('all_datasource_access',
'all_datasource_access')
@@ -268,6 +280,7 @@ class SupersetSecurityManager(SecurityManager):
# Global perms
self.merge_perm('all_datasource_access', 'all_datasource_access')
self.merge_perm('all_database_access', 'all_database_access')
+ self.merge_perm('can_only_access_owned_queries',
'can_only_access_owned_queries')
def create_missing_perms(self):
"""Creates missing perms for datasources, schemas and metrics"""
diff --git a/superset/views/core.py b/superset/views/core.py
index 832c89a..d910923 100755
--- a/superset/views/core.py
+++ b/superset/views/core.py
@@ -22,6 +22,7 @@ import os
import re
import time
import traceback
+from typing import List # noqa: F401
from urllib import parse
from flask import (
@@ -82,7 +83,7 @@ ACCESS_REQUEST_MISSING_ERR = __(
'The access requests seem to have been deleted')
USER_MISSING_ERR = __('The user seems to have been deleted')
-FORM_DATA_KEY_BLACKLIST = []
+FORM_DATA_KEY_BLACKLIST: List[str] = []
if not config.get('ENABLE_JAVASCRIPT_CONTROLS'):
FORM_DATA_KEY_BLACKLIST = [
'js_tooltip',
@@ -2759,10 +2760,21 @@ class Superset(BaseSupersetView):
@has_access
@expose('/search_queries')
@log_this
- def search_queries(self):
- """Search for queries."""
+ def search_queries(self) -> Response:
+ """
+ Search for previously run sqllab queries. Used for Sqllab Query Search
+ page /superset/sqllab#search.
+
+ Custom permission can_only_search_queries_owned restricts queries
+ to only queries run by current user.
+
+ :returns: Response with list of sql query dicts
+ """
query = db.session.query(Query)
- search_user_id = request.args.get('user_id')
+ if security_manager.can_only_access_owned_queries():
+ search_user_id = g.user.get_user_id()
+ else:
+ search_user_id = request.args.get('user_id')
database_id = request.args.get('database_id')
search_text = request.args.get('search_text')
status = request.args.get('status')
@@ -2771,7 +2783,7 @@ class Superset(BaseSupersetView):
to_time = request.args.get('to')
if search_user_id:
- # Filter on db Id
+ # Filter on user_id
query = query.filter(Query.user_id == search_user_id)
if database_id:
diff --git a/superset/views/sql_lab.py b/superset/views/sql_lab.py
index 0c8e87f..adbdd46 100644
--- a/superset/views/sql_lab.py
+++ b/superset/views/sql_lab.py
@@ -15,16 +15,38 @@
# specific language governing permissions and limitations
# under the License.
# pylint: disable=C,R,W
+from typing import Callable
+
from flask import g, redirect
from flask_appbuilder import expose
from flask_appbuilder.models.sqla.interface import SQLAInterface
from flask_appbuilder.security.decorators import has_access
from flask_babel import gettext as __
from flask_babel import lazy_gettext as _
+from flask_sqlalchemy import BaseQuery
-from superset import appbuilder
+from superset import appbuilder, security_manager
from superset.models.sql_lab import Query, SavedQuery
-from .base import BaseSupersetView, DeleteMixin, SupersetModelView
+from .base import BaseSupersetView, DeleteMixin, SupersetFilter,
SupersetModelView
+
+
+class QueryFilter(SupersetFilter):
+ def apply(
+ self,
+ query: BaseQuery,
+ func: Callable) -> BaseQuery:
+ """
+ Filter queries to only those owned by current user if
+ can_only_access_owned_queries permission is set.
+
+ :returns: query
+ """
+ if security_manager.can_only_access_owned_queries():
+ query = (
+ query
+ .filter(Query.user_id == g.user.get_user_id())
+ )
+ return query
class QueryView(SupersetModelView):
@@ -35,10 +57,12 @@ class QueryView(SupersetModelView):
add_title = _('Add Query')
edit_title = _('Edit Query')
- list_columns = ['user', 'database', 'status', 'start_time', 'end_time']
+ list_columns = ['username', 'database_name', 'status', 'start_time',
'end_time']
+ base_filters = [['id', QueryFilter, lambda: []]]
label_columns = {
'user': _('User'),
- 'database': _('Database'),
+ 'username': _('User'),
+ 'database_name': _('Database'),
'status': _('Status'),
'start_time': _('Start Time'),
'end_time': _('End Time'),
diff --git a/tests/sqllab_tests.py b/tests/sqllab_tests.py
index d54601c..5fe9ef1 100644
--- a/tests/sqllab_tests.py
+++ b/tests/sqllab_tests.py
@@ -53,9 +53,10 @@ class SqlLabTests(SupersetTestCase):
self.logout()
def tearDown(self):
+ self.logout()
db.session.query(Query).delete()
db.session.commit()
- self.logout()
+ db.session.close()
def test_sql_json(self):
self.login('admin')
@@ -223,6 +224,46 @@ class SqlLabTests(SupersetTestCase):
data = json.loads(resp)
self.assertEquals(2, len(data))
+ def test_search_query_with_owner_only_perms(self) -> None:
+ """
+ Test a search query with can_only_access_owned_queries perm added to
+ Admin and make sure only Admin queries show up.
+ """
+ session = db.session
+
+ # Add can_only_access_owned_queries perm to Admin user
+ owned_queries_view = security_manager.find_permission_view_menu(
+ 'can_only_access_owned_queries',
+ 'can_only_access_owned_queries',
+ )
+ security_manager.add_permission_role(
+ security_manager.find_role('Admin'),
+ owned_queries_view,
+ )
+ session.commit()
+
+ # Test search_queries for Admin user
+ self.run_some_queries()
+ self.login('admin')
+
+ user_id = security_manager.find_user('admin').id
+ data = self.get_json_resp('/superset/search_queries')
+ self.assertEquals(2, len(data))
+ user_ids = {k['userId'] for k in data}
+ self.assertEquals(set([user_id]), user_ids)
+
+ # Remove can_only_access_owned_queries from Admin
+ owned_queries_view = security_manager.find_permission_view_menu(
+ 'can_only_access_owned_queries',
+ 'can_only_access_owned_queries',
+ )
+ security_manager.del_permission_role(
+ security_manager.find_role('Admin'),
+ owned_queries_view,
+ )
+
+ session.commit()
+
def test_alias_duplicate(self):
self.run_sql(
'SELECT username as col, id as col, username FROM ab_user',
@@ -309,6 +350,68 @@ class SqlLabTests(SupersetTestCase):
query_limit=test_limit)
self.assertEquals(len(data['data']), test_limit)
+ def test_queryview_filter(self) -> None:
+ """
+ Test queryview api without can_only_access_owned_queries perm added to
+ Admin and make sure all queries show up.
+ """
+ self.run_some_queries()
+ self.login(username='admin')
+
+ url = '/queryview/api/read'
+ data = self.get_json_resp(url)
+ admin = security_manager.find_user('admin')
+ gamma_sqllab = security_manager.find_user('gamma_sqllab')
+ self.assertEquals(3, len(data['result']))
+ user_queries = [
+ result.get('username') for result in data['result']
+ ]
+ assert admin.username in user_queries
+ assert gamma_sqllab.username in user_queries
+
+ def test_queryview_filter_owner_only(self) -> None:
+ """
+ Test queryview api with can_only_access_owned_queries perm added to
+ Admin and make sure only Admin queries show up.
+ """
+ session = db.session
+
+ # Add can_only_access_owned_queries perm to Admin user
+ owned_queries_view = security_manager.find_permission_view_menu(
+ 'can_only_access_owned_queries',
+ 'can_only_access_owned_queries',
+ )
+ security_manager.add_permission_role(
+ security_manager.find_role('Admin'),
+ owned_queries_view,
+ )
+ session.commit()
+
+ # Test search_queries for Admin user
+ self.run_some_queries()
+ self.login('admin')
+
+ url = '/queryview/api/read'
+ data = self.get_json_resp(url)
+ admin = security_manager.find_user('admin')
+ self.assertEquals(2, len(data['result']))
+ all_admin_user_queries = all([
+ result.get('username') == admin.username for result in
data['result']
+ ])
+ assert all_admin_user_queries is True
+
+ # Remove can_only_access_owned_queries from Admin
+ owned_queries_view = security_manager.find_permission_view_menu(
+ 'can_only_access_owned_queries',
+ 'can_only_access_owned_queries',
+ )
+ security_manager.del_permission_role(
+ security_manager.find_role('Admin'),
+ owned_queries_view,
+ )
+
+ session.commit()
+
if __name__ == '__main__':
unittest.main()
