iop
/
superset
4
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

chore(security): Renaming access methods (#10031) 

Co-authored-by: John Bodley <john.bodley@airbnb.com>
This commit is contained in:
John Bodley 2020-06-11 13:12:23 -07:00 committed by GitHub
parent 54c6ddbdb7
commit 9532bff48f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
14 changed files with 76 additions and 85 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
UPDATING.md
View File

@ -23,6 +23,9 @@ assists people when migrating to a new version.
## Next
* [10031](https://github.com/apache/incubator-superset/pull/10030): Renames the following public security manager methods: `can_access_datasource` to `can_access_table`, `all_datasource_access` to `can_access_all_datasources`, `all_database_access` to `can_access_all_databases`, `database_access` to `can_access_database`, `schema_access` to `can_access_schema`, and
`datasource_access` to `can_access_datasource`. Regrettably it is not viable to provide aliases for the deprecated methods as this would result in a name clash. Finally the `can_access_table` (previously `can_access_database`) method signature has changed, i.e., the optional `schema` argument no longer exists.
* [10030](https://github.com/apache/incubator-superset/pull/10030): Renames the public security manager `schemas_accessible_by_user` method to `get_schemas_accessible_by_user`.
* [9786](https://github.com/apache/incubator-superset/pull/9786): with the upgrade of `werkzeug` from version `0.16.0` to `1.0.1`, the `werkzeug.contrib.cache` module has been moved to a standalone package [cachelib](https://pypi.org/project/cachelib/). For example, to import the `RedisCache` class, please use the following import: `from cachelib.redis import RedisCache`.

2
superset/charts/filters.py
View File

@ -45,7 +45,7 @@ class ChartNameOrDescriptionFilter(
class ChartFilter(BaseFilter): # pylint: disable=too-few-public-methods
def apply(self, query: Query, value: Any) -> Query:
if security_manager.all_datasource_access():
if security_manager.can_access_all_datasources():
return query
perms = security_manager.user_view_menu_names("datasource_access")
schema_perms = security_manager.user_view_menu_names("schema_access")

3
superset/dashboards/filters.py
View File

@ -62,7 +62,6 @@ class DashboardFilter(BaseFilter): # pylint: disable=too-few-public-methods
datasource_perms = security_manager.user_view_menu_names("datasource_access")
schema_perms = security_manager.user_view_menu_names("schema_access")
all_datasource_access = security_manager.all_datasource_access()
published_dash_query = (
db.session.query(Dashboard.id)
.join(Dashboard.slices)
@ -72,7 +71,7 @@ class DashboardFilter(BaseFilter): # pylint: disable=too-few-public-methods
or_(
Slice.perm.in_(datasource_perms),
Slice.schema_perm.in_(schema_perms),
all_datasource_access,
security_manager.can_access_all_datasources(),
),
)
)

70
superset/security/manager.py
View File

@ -188,15 +188,14 @@ class SupersetSecurityManager(SecurityManager):
def can_access(self, permission_name: str, view_name: str) -> bool:
"""
Return True if the user can access the FAB permission/view, False
otherwise.
Return True if the user can access the FAB permission/view, False otherwise.
Note this method adds protection from has_access failing from missing
permission/view entries.
:param permission_name: The FAB permission name
:param view_name: The FAB view-menu name
:returns: Whether the use can access the FAB permission/view
:returns: Whether the user can access the FAB permission/view
"""
user = g.user
@ -206,13 +205,14 @@ class SupersetSecurityManager(SecurityManager):
def can_access_all_queries(self) -> bool:
"""
Return True if the user can access all queries, False otherwise.
Return True if the user can access all SQL Lab queries, False otherwise.
:returns: Whether the user can access all queries
"""
return self.can_access("all_query_access", "all_query_access")
def all_datasource_access(self) -> bool:
def can_access_all_datasources(self) -> bool:
"""
Return True if the user can access all Superset datasources, False otherwise.
@ -221,7 +221,7 @@ class SupersetSecurityManager(SecurityManager):
return self.can_access("all_datasource_access", "all_datasource_access")
def all_database_access(self) -> bool:
def can_access_all_databases(self) -> bool:
"""
Return True if the user can access all Superset databases, False otherwise.
@ -230,7 +230,7 @@ class SupersetSecurityManager(SecurityManager):
return self.can_access("all_database_access", "all_database_access")
def database_access(self, database: "Database") -> bool:
def can_access_database(self, database: "Database") -> bool:
"""
Return True if the user can access the Superset database, False otherwise.
@ -238,39 +238,39 @@ class SupersetSecurityManager(SecurityManager):
:returns: Whether the user can access the Superset database
"""
return (
self.all_datasource_access()
or self.all_database_access()
self.can_access_all_datasources()
or self.can_access_all_databases()
or self.can_access("database_access", database.perm)
)
def schema_access(self, datasource: "BaseDatasource") -> bool:
def can_access_schema(self, datasource: "BaseDatasource") -> bool:
"""
Return True if the user can access the schema associated with the Superset
datasource, False otherwise.
Note for Druid datasources the database and schema are akin to the Druid cluster
and datasource name prefix, i.e., [schema.]datasource, respectively.
and datasource name prefix respectively, i.e., [schema.]datasource.
:param datasource: The Superset datasource
:returns: Whether the user can access the datasource's schema
"""
schema_perm = datasource.schema_perm or ""
return (
self.all_datasource_access()
or self.database_access(datasource.database)
or self.can_access("schema_access", schema_perm)
self.can_access_all_datasources()
or self.can_access_database(datasource.database)
or self.can_access("schema_access", datasource.schema_perm or "")
)
def datasource_access(self, datasource: "BaseDatasource") -> bool:
def can_access_datasource(self, datasource: "BaseDatasource") -> bool:
"""
Return True if the user can access the Superset datasource, False otherwise.
:param datasource: The Superset datasource
:returns: Whether the use can access the Superset datasource
:returns: Whether the user can access the Superset datasource
"""
perm = datasource.perm or ""
return self.schema_access(datasource) or self.can_access(
"datasource_access", perm
return self.can_access_schema(datasource) or self.can_access(
"datasource_access", datasource.perm or ""
)
def get_datasource_access_error_msg(self, datasource: "BaseDatasource") -> str:
@ -356,30 +356,27 @@ class SupersetSecurityManager(SecurityManager):
return conf.get("PERMISSION_INSTRUCTIONS_LINK")
def can_access_datasource(
self, database: "Database", table: "Table", schema: Optional[str] = None
) -> bool:
def can_access_table(self, database: "Database", table: "Table") -> bool:
"""
Return True if the user can access the SQL table, False otherwise.
:param database: The SQL database
:param table: The SQL table
:param schema: The fallback SQL schema if not present in the table
:returns: Whether the use can access the SQL table
:returns: Whether the user can access the SQL table
"""
from superset import db
from superset.connectors.sqla.models import SqlaTable
if self.database_access(database) or self.all_datasource_access():
if self.can_access_database(database):
return True
schema_perm = self.get_schema_perm(database, schema=table.schema or schema)
schema_perm = self.get_schema_perm(database, schema=table.schema)
if schema_perm and self.can_access("schema_access", schema_perm):
return True
datasources = SqlaTable.query_datasources_by_name(
db.session, database, table.table, schema=table.schema or schema
db.session, database, table.table, schema=table.schema
)
for datasource in datasources:
if self.can_access("datasource_access", datasource.perm):
@ -397,12 +394,15 @@ class SupersetSecurityManager(SecurityManager):
:param schema: The SQL database schema
:returns: The rejected tables
"""
query = sql_parse.ParsedQuery(sql)
from superset.sql_parse import Table
return {
table
for table in query.tables
if not self.can_access_datasource(database, table, schema)
for table in sql_parse.ParsedQuery(sql).tables
if not self.can_access_table(
database, Table(table.table, table.schema or schema)
)
}
def get_public_role(self) -> Optional[Any]: # Optional[self.role_model]
@ -463,9 +463,7 @@ class SupersetSecurityManager(SecurityManager):
from superset import db
from superset.connectors.sqla.models import SqlaTable
if hierarchical and (
self.database_access(database) or self.all_datasource_access()
):
if hierarchical and self.can_access_database(database):
return schemas
# schema_access
@ -507,7 +505,7 @@ class SupersetSecurityManager(SecurityManager):
from superset import db
if self.database_access(database) or self.all_datasource_access():
if self.can_access_database(database):
return datasource_names
if schema:
@ -866,7 +864,7 @@ class SupersetSecurityManager(SecurityManager):
:raises SupersetSecurityException: If the user does not have permission
"""
if not self.datasource_access(datasource):
if not self.can_access_datasource(datasource):
raise SupersetSecurityException(
self.get_datasource_access_error_object(datasource),
)

2
superset/views/base.py
View File

@ -433,7 +433,7 @@ class DeleteMixin: # pylint: disable=too-few-public-methods
class DatasourceFilter(BaseFilter): # pylint: disable=too-few-public-methods
def apply(self, query: Query, value: Any) -> Query:
if security_manager.all_datasource_access():
if security_manager.can_access_all_datasources():
return query
datasource_perms = security_manager.user_view_menu_names("datasource_access")
schema_perms = security_manager.user_view_menu_names("schema_access")

2
superset/views/chart/filters.py
View File

@ -25,7 +25,7 @@ from superset.views.base import BaseFilter
class SliceFilter(BaseFilter): # pylint: disable=too-few-public-methods
def apply(self, query: Query, value: Any) -> Query:
if security_manager.all_datasource_access():
if security_manager.can_access_all_datasources():
return query
perms = security_manager.user_view_menu_names("datasource_access")
schema_perms = security_manager.user_view_menu_names("schema_access")

22
superset/views/core.py
View File

@ -488,7 +488,7 @@ class Superset(BaseSupersetView):
has_access = all(
(
datasource and security_manager.datasource_access(datasource)
datasource and security_manager.can_access_datasource(datasource)
for datasource in datasources
)
)
@ -520,7 +520,7 @@ class Superset(BaseSupersetView):
datasource = ConnectorRegistry.get_datasource(
r.datasource_type, r.datasource_id, session
)
if not datasource or security_manager.datasource_access(datasource):
if not datasource or security_manager.can_access_datasource(datasource):
# datasource does not exist anymore
session.delete(r)
session.commit()
@ -560,7 +560,7 @@ class Superset(BaseSupersetView):
return json_error_response(ACCESS_REQUEST_MISSING_ERR)
# check if you can approve
if security_manager.all_datasource_access() or check_ownership(
if security_manager.can_access_all_datasources() or check_ownership(
datasource, raise_if_false=False
):
# can by done by admin only
@ -868,7 +868,7 @@ class Superset(BaseSupersetView):
return redirect(error_redirect)
if config["ENABLE_ACCESS_REQUEST"] and (
not security_manager.datasource_access(datasource)
not security_manager.can_access_datasource(datasource)
):
flash(
__(security_manager.get_datasource_access_error_msg(datasource)),
@ -1874,7 +1874,9 @@ class Superset(BaseSupersetView):
if config["ENABLE_ACCESS_REQUEST"]:
for datasource in datasources:
if datasource and not security_manager.datasource_access(datasource):
if datasource and not security_manager.can_access_datasource(
datasource
):
flash(
__(
security_manager.get_datasource_access_error_msg(datasource)
@ -2137,10 +2139,7 @@ class Superset(BaseSupersetView):
return json_error_response("Not found", 404)
schema = utils.parse_js_uri_path_item(schema, eval_undefined=True)
table_name = utils.parse_js_uri_path_item(table_name) # type: ignore
# Check that the user can access the datasource
if not self.appbuilder.sm.can_access_datasource(
database, Table(table_name, schema), schema
):
if not self.appbuilder.sm.can_access_table(database, Table(table_name, schema)):
stats_logger.incr(
f"deprecated.{self.__class__.__name__}.select_star.permission_denied"
)
@ -2895,10 +2894,7 @@ class Superset(BaseSupersetView):
database = db.session.query(models.Database).filter_by(id=db_id).one()
try:
schemas_allowed = database.get_schema_access_for_csv_upload()
if (
security_manager.database_access(database)
or security_manager.all_datasource_access()
):
if security_manager.can_access_database(database):
return self.json_response(schemas_allowed)
# the list schemas_allowed should not be empty here
# and the list schemas_allowed_processed returned from security_manager

3
superset/views/dashboard/filters.py
View File

@ -47,7 +47,6 @@ class DashboardFilter(BaseFilter): # pylint: disable=too-few-public-methods
datasource_perms = security_manager.user_view_menu_names("datasource_access")
schema_perms = security_manager.user_view_menu_names("schema_access")
all_datasource_access = security_manager.all_datasource_access()
published_dash_query = (
db.session.query(Dashboard.id)
.join(Dashboard.slices)
@ -57,7 +56,7 @@ class DashboardFilter(BaseFilter): # pylint: disable=too-few-public-methods
or_(
Slice.perm.in_(datasource_perms),
Slice.schema_perm.in_(schema_perms),
all_datasource_access,
security_manager.can_access_all_datasources(),
),
)
)

5
superset/views/database/decorators.py
View File

@ -50,9 +50,8 @@ def check_datasource_access(f: Callable[..., Any]) -> Callable[..., Any]:
f"database_not_found_{self.__class__.__name__}.select_star"
)
return self.response_404()
# Check that the user can access the datasource
if not self.appbuilder.sm.can_access_datasource(
database, Table(table_name_parsed, schema_name_parsed), schema_name_parsed
if not self.appbuilder.sm.can_access_table(
database, Table(table_name_parsed, schema_name_parsed),
):
self.stats_logger.incr(
f"permisssion_denied_{self.__class__.__name__}.select_star"

2
superset/views/database/filters.py
View File

@ -32,7 +32,7 @@ class DatabaseFilter(BaseFilter):
}
def apply(self, query: Query, value: Any) -> Query:
if security_manager.all_database_access():
if security_manager.can_access_all_databases():
return query
database_perms = security_manager.user_view_menu_names("database_access")
# TODO(bogdan): consider adding datasource access here as well.

5
superset/views/database/forms.py
View File

@ -70,10 +70,7 @@ class CsvToDatabaseForm(DynamicForm):
b) if database supports schema
user is able to upload to schema in schemas_allowed_for_csv_upload
"""
if (
security_manager.database_access(database)
or security_manager.all_datasource_access()
):
if security_manager.can_access_database(database):
return True
schemas = database.get_schema_access_for_csv_upload()
if schemas and security_manager.get_schemas_accessible_by_user(

5
superset/views/database/validators.py
View File

@ -50,7 +50,4 @@ def schema_allows_csv_upload(database: Database, schema: Optional[str]) -> bool:
schemas = database.get_schema_access_for_csv_upload()
if schemas:
return schema in schemas
return (
security_manager.database_access(database)
or security_manager.all_datasource_access()
)
return security_manager.can_access_database(database)

13
tests/core_tests.py
View File

@ -966,15 +966,18 @@ class CoreTests(SupersetTestCase):
@mock.patch(
"superset.security.SupersetSecurityManager.get_schemas_accessible_by_user"
)
@mock.patch("superset.security.SupersetSecurityManager.database_access")
@mock.patch("superset.security.SupersetSecurityManager.all_datasource_access")
@mock.patch("superset.security.SupersetSecurityManager.can_access_database")
@mock.patch("superset.security.SupersetSecurityManager.can_access_all_datasources")
def test_schemas_access_for_csv_upload_endpoint(
self, mock_all_datasource_access, mock_database_access, mock_schemas_accessible
self,
mock_can_access_all_datasources,
mock_can_access_database,
mock_schemas_accessible,
):
self.login(username="admin")
dbobj = self.create_fake_db()
mock_all_datasource_access.return_value = False
mock_database_access.return_value = False
mock_can_access_all_datasources.return_value = False
mock_can_access_database.return_value = False
mock_schemas_accessible.return_value = ["this_schema_is_allowed_too"]
data = self.get_json_resp(
url="/superset/schemas_access_for_csv_upload?db_id={db_id}".format(

24
tests/security_tests.py
View File

@ -774,45 +774,45 @@ class SecurityManagerTests(SupersetTestCase):
Testing the Security Manager.
"""
@patch("superset.security.SupersetSecurityManager.datasource_access")
def test_assert_datasource_permission(self, mock_datasource_access):
@patch("superset.security.SupersetSecurityManager.can_access_datasource")
def test_assert_datasource_permission(self, mock_can_access_datasource):
datasource = self.get_datasource_mock()
# Datasource with the "datasource_access" permission.
mock_datasource_access.return_value = True
mock_can_access_datasource.return_value = True
security_manager.assert_datasource_permission(datasource)
# Datasource without the "datasource_access" permission.
mock_datasource_access.return_value = False
mock_can_access_datasource.return_value = False
with self.assertRaises(SupersetSecurityException):
security_manager.assert_datasource_permission(datasource)
@patch("superset.security.SupersetSecurityManager.datasource_access")
def test_assert_query_context_permission(self, mock_datasource_access):
@patch("superset.security.SupersetSecurityManager.can_access_datasource")
def test_assert_query_context_permission(self, mock_can_access_datasource):
query_context = Mock()
query_context.datasource = self.get_datasource_mock()
# Query context with the "datasource_access" permission.
mock_datasource_access.return_value = True
mock_can_access_datasource.return_value = True
security_manager.assert_query_context_permission(query_context)
# Query context without the "datasource_access" permission.
mock_datasource_access.return_value = False
mock_can_access_datasource.return_value = False
with self.assertRaises(SupersetSecurityException):
security_manager.assert_query_context_permission(query_context)
@patch("superset.security.SupersetSecurityManager.datasource_access")
def test_assert_viz_permission(self, mock_datasource_access):
@patch("superset.security.SupersetSecurityManager.can_access_datasource")
def test_assert_viz_permission(self, mock_can_access_datasource):
test_viz = viz.TableViz(self.get_datasource_mock(), form_data={})
# Visualization with the "datasource_access" permission.
mock_datasource_access.return_value = True
mock_can_access_datasource.return_value = True
security_manager.assert_viz_permission(test_viz)
# Visualization without the "datasource_access" permission.
mock_datasource_access.return_value = False
mock_can_access_datasource.return_value = False
with self.assertRaises(SupersetSecurityException):
security_manager.assert_viz_permission(test_viz)