You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by dp...@apache.org on 2020/02/05 08:58:31 UTC
[incubator-superset] branch master updated: [query] deprecate
can_only_access_owned_queries (#9046)
This is an automated email from the ASF dual-hosted git repository.
dpgaspar 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 916d184 [query] deprecate can_only_access_owned_queries (#9046)
916d184 is described below
commit 916d184076f39c83c7a9f96de7ab8653955d5238
Author: Daniel Vaz Gaspar <da...@gmail.com>
AuthorDate: Wed Feb 5 08:58:12 2020 +0000
[query] deprecate can_only_access_owned_queries (#9046)
---
UPDATING.md | 5 +++
superset/security/manager.py | 17 ++++-----
superset/views/core.py | 11 ++++--
superset/views/sql_lab.py | 6 ++--
tests/sqllab_tests.py | 84 +++++++++++++++++++-------------------------
5 files changed, 58 insertions(+), 65 deletions(-)
diff --git a/UPDATING.md b/UPDATING.md
index 0076259..8f8c8c7 100644
--- a/UPDATING.md
+++ b/UPDATING.md
@@ -22,6 +22,11 @@ This file documents any backwards-incompatible changes in Superset and
assists people when migrating to a new version.
## Next
+* [9046](https://github.com/apache/incubator-superset/pull/9046): Replaces `can_only_access_owned_queries` by
+`all_query_access` favoring a white list approach. Since a new permission is introduced use `superset init`
+to create and associate it by default to the `Admin` role. Note that, by default, all non `Admin` users will
+not be able to access queries they do not own.
+
* [8901](https://github.com/apache/incubator-superset/pull/8901): The datasource's update
timestamp has been added to the query object's cache key to ensure updates to
datasources are always reflected in associated query results. As a consequence all
diff --git a/superset/security/manager.py b/superset/security/manager.py
index 4a3394d..12cf776 100644
--- a/superset/security/manager.py
+++ b/superset/security/manager.py
@@ -128,6 +128,7 @@ class SupersetSecurityManager(SecurityManager):
"can_override_role_permissions",
"can_approve",
"can_update_role",
+ "all_query_access",
}
READ_ONLY_PERMISSION = {"can_show", "can_list"}
@@ -143,7 +144,6 @@ class SupersetSecurityManager(SecurityManager):
"schema_access",
"datasource_access",
"metric_access",
- "can_only_access_owned_queries",
}
ACCESSIBLE_PERMS = {"can_userinfo"}
@@ -188,15 +188,13 @@ 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:
+ def can_access_all_queries(self) -> bool:
"""
- Return True if the user can only access owned queries, False otherwise.
+ Return True if the user can access all queries, False otherwise.
- :returns: Whether the use can only access owned queries
+ :returns: Whether the user can access all queries
"""
- return self.can_access(
- "can_only_access_owned_queries", "can_only_access_owned_queries"
- )
+ return self.can_access("all_query_access", "all_query_access")
def all_datasource_access(self) -> bool:
"""
@@ -549,12 +547,9 @@ class SupersetSecurityManager(SecurityManager):
"""
Create custom FAB permissions.
"""
-
self.add_permission_view_menu("all_datasource_access", "all_datasource_access")
self.add_permission_view_menu("all_database_access", "all_database_access")
- self.add_permission_view_menu(
- "can_only_access_owned_queries", "can_only_access_owned_queries"
- )
+ self.add_permission_view_menu("all_query_access", "all_query_access")
def create_missing_perms(self) -> None:
"""
diff --git a/superset/views/core.py b/superset/views/core.py
index bab004e..ffc9eea 100755
--- a/superset/views/core.py
+++ b/superset/views/core.py
@@ -2502,10 +2502,15 @@ class Superset(BaseSupersetView):
:returns: Response with list of sql query dicts
"""
query = db.session.query(Query)
- if security_manager.can_only_access_owned_queries():
- search_user_id = g.user.get_user_id()
- else:
+ if security_manager.can_access_all_queries():
search_user_id = request.args.get("user_id")
+ elif (
+ request.args.get("user_id") is not None
+ and request.args.get("user_id") != g.user.get_user_id()
+ ):
+ return Response(status=403, mimetype="application/json")
+ else:
+ search_user_id = g.user.get_user_id()
database_id = request.args.get("database_id")
search_text = request.args.get("search_text")
status = request.args.get("status")
diff --git a/superset/views/sql_lab.py b/superset/views/sql_lab.py
index 5414c15..3646965 100644
--- a/superset/views/sql_lab.py
+++ b/superset/views/sql_lab.py
@@ -41,12 +41,12 @@ from .base import (
class QueryFilter(BaseFilter): # pylint: disable=too-few-public-methods
def apply(self, query: BaseQuery, value: Callable) -> BaseQuery:
"""
- Filter queries to only those owned by current user if
- can_only_access_owned_queries permission is set.
+ Filter queries to only those owned by current user. If
+ can_access_all_queries permission is set a user can list all queries
:returns: query
"""
- if security_manager.can_only_access_owned_queries():
+ if not security_manager.can_access_all_queries():
query = query.filter(Query.user_id == g.user.get_user_id())
return query
diff --git a/tests/sqllab_tests.py b/tests/sqllab_tests.py
index 05566d7..be452c5 100644
--- a/tests/sqllab_tests.py
+++ b/tests/sqllab_tests.py
@@ -223,42 +223,21 @@ class SqlLabTests(SupersetTestCase):
data = json.loads(resp)
self.assertEqual(2, len(data))
- def test_search_query_with_owner_only_perms(self) -> None:
+ def test_search_query_only_owned(self) -> None:
"""
- Test a search query with can_only_access_owned_queries perm added to
- Admin and make sure only Admin queries show up.
+ Test a search query with a user that does not have can_access_all_queries.
"""
- 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
+ # Test search_queries for Alpha user
self.run_some_queries()
- self.login("admin")
+ self.login("gamma_sqllab")
- user_id = security_manager.find_user("admin").id
+ user_id = security_manager.find_user("gamma_sqllab").id
data = self.get_json_resp("/superset/search_queries")
- self.assertEqual(2, len(data))
+
+ self.assertEqual(1, len(data))
user_ids = {k["userId"] for k in data}
self.assertEqual(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 name as col, gender as col FROM birth_names LIMIT 10",
@@ -356,45 +335,54 @@ class SqlLabTests(SupersetTestCase):
assert admin.username in user_queries
assert gamma_sqllab.username in user_queries
- def test_queryview_filter_owner_only(self) -> None:
+ def test_queryview_can_access_all_queries(self) -> None:
"""
- Test queryview api with can_only_access_owned_queries perm added to
- Admin and make sure only Admin queries show up.
+ Test queryview api with can_access_all_queries perm added to
+ gamma and make sure all 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"
+ # Add all_query_access perm to Gamma user
+ all_queries_view = security_manager.find_permission_view_menu(
+ "all_query_access", "all_query_access"
)
+
security_manager.add_permission_role(
- security_manager.find_role("Admin"), owned_queries_view
+ security_manager.find_role("gamma_sqllab"), all_queries_view
)
session.commit()
# Test search_queries for Admin user
self.run_some_queries()
- self.login("admin")
-
+ self.login("gamma_sqllab")
url = "/queryview/api/read"
data = self.get_json_resp(url)
- admin = security_manager.find_user("admin")
- self.assertEqual(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
+ self.assertEqual(3, len(data["result"]))
- # 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"
+ # Remove all_query_access from gamma sqllab
+ all_queries_view = security_manager.find_permission_view_menu(
+ "all_query_access", "all_query_access"
)
security_manager.del_permission_role(
- security_manager.find_role("Admin"), owned_queries_view
+ security_manager.find_role("gamma_sqllab"), all_queries_view
)
session.commit()
+ def test_queryview_admin_can_access_all_queries(self) -> None:
+ """
+ Test queryview api with all_query_access perm added to
+ Admin and make sure only Admin queries show up. This is the default
+ """
+ # 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.assertEqual(3, len(data["result"]))
+
def test_api_database(self):
self.login("admin")
self.create_fake_db()
@@ -407,7 +395,7 @@ class SqlLabTests(SupersetTestCase):
"page": 0,
"page_size": -1,
}
- url = "api/v1/database/?{}={}".format("q", prison.dumps(arguments))
+ url = f"api/v1/database/?q={prison.dumps(arguments)}"
self.assertEqual(
{"examples", "fake_db_100"},
{r.get("database_name") for r in self.get_json_resp(url)["result"]},