From 54c6ddbdb722fb73bf60f7c5a7a83d82523a90b9 Mon Sep 17 00:00:00 2001 From: John Bodley <4567245+john-bodley@users.noreply.github.com> Date: Thu, 11 Jun 2020 11:29:43 -0700 Subject: [PATCH] chore(security): Renaming schemas_accessible_by_user (#10030) Co-authored-by: John Bodley --- UPDATING.md | 2 ++ superset/security/manager.py | 4 ++-- superset/views/core.py | 4 ++-- superset/views/database/forms.py | 2 +- tests/core_tests.py | 4 +++- tests/security_tests.py | 8 ++++---- 6 files changed, 14 insertions(+), 10 deletions(-) diff --git a/UPDATING.md b/UPDATING.md index 60f29ebc7..c0179303f 100644 --- a/UPDATING.md +++ b/UPDATING.md @@ -23,6 +23,8 @@ assists people when migrating to a new version. ## Next +* [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`. * [9572](https://github.com/apache/incubator-superset/pull/9572): a change which by defau;t means that the Jinja `current_user_id`, `current_username`, and `url_param` context calls no longer need to be wrapped via `cache_key_wrapper` in order to be included in the cache key. The `cache_key_wrapper` function should only be required for Jinja add-ons. diff --git a/superset/security/manager.py b/superset/security/manager.py index 293342a83..85ce12e64 100644 --- a/superset/security/manager.py +++ b/superset/security/manager.py @@ -448,11 +448,11 @@ class SupersetSecurityManager(SecurityManager): return set([s.name for s in view_menu_names]) return set() - def schemas_accessible_by_user( + def get_schemas_accessible_by_user( self, database: "Database", schemas: List[str], hierarchical: bool = True ) -> List[str]: """ - Return the sorted list of SQL schemas accessible by the user. + Return the list of SQL schemas accessible by the user. :param database: The SQL database :param schemas: The list of eligible SQL schemas diff --git a/superset/views/core.py b/superset/views/core.py index 6df7d346d..db147f02a 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -1128,7 +1128,7 @@ class Superset(BaseSupersetView): cache_timeout=database.schema_cache_timeout, force=force_refresh.lower() == "true", ) - schemas = security_manager.schemas_accessible_by_user(database, schemas) + schemas = security_manager.get_schemas_accessible_by_user(database, schemas) else: schemas = [] @@ -2905,7 +2905,7 @@ class Superset(BaseSupersetView): # should not be empty either, # otherwise the database should have been filtered out # in CsvToDatabaseForm - schemas_allowed_processed = security_manager.schemas_accessible_by_user( + schemas_allowed_processed = security_manager.get_schemas_accessible_by_user( database, schemas_allowed, False ) return self.json_response(schemas_allowed_processed) diff --git a/superset/views/database/forms.py b/superset/views/database/forms.py index 2fb62907e..68b391330 100644 --- a/superset/views/database/forms.py +++ b/superset/views/database/forms.py @@ -76,7 +76,7 @@ class CsvToDatabaseForm(DynamicForm): ): return True schemas = database.get_schema_access_for_csv_upload() - if schemas and security_manager.schemas_accessible_by_user( + if schemas and security_manager.get_schemas_accessible_by_user( database, schemas, False ): return True diff --git a/tests/core_tests.py b/tests/core_tests.py index 6631da8b1..e7b4ace16 100644 --- a/tests/core_tests.py +++ b/tests/core_tests.py @@ -963,7 +963,9 @@ class CoreTests(SupersetTestCase): "The datasource associated with this chart no longer exists", ) - @mock.patch("superset.security.SupersetSecurityManager.schemas_accessible_by_user") + @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") def test_schemas_access_for_csv_upload_endpoint( diff --git a/tests/security_tests.py b/tests/security_tests.py index 476b67019..63e15c999 100644 --- a/tests/security_tests.py +++ b/tests/security_tests.py @@ -412,7 +412,7 @@ class RolePermissionTests(SupersetTestCase): mock_g.user = security_manager.find_user("admin") with self.client.application.test_request_context(): database = get_example_database() - schemas = security_manager.schemas_accessible_by_user( + schemas = security_manager.get_schemas_accessible_by_user( database, ["1", "2", "3"] ) self.assertEquals(schemas, ["1", "2", "3"]) # no changes @@ -424,7 +424,7 @@ class RolePermissionTests(SupersetTestCase): mock_g.user = security_manager.find_user("gamma") with self.client.application.test_request_context(): database = get_example_database() - schemas = security_manager.schemas_accessible_by_user( + schemas = security_manager.get_schemas_accessible_by_user( database, ["1", "2", "3"] ) # temp_schema is not passed in the params @@ -437,7 +437,7 @@ class RolePermissionTests(SupersetTestCase): mock_g.user = security_manager.find_user("gamma") with self.client.application.test_request_context(): database = get_example_database() - schemas = security_manager.schemas_accessible_by_user( + schemas = security_manager.get_schemas_accessible_by_user( database, ["temp_schema", "2", "3"] ) self.assertEquals(schemas, ["temp_schema"]) @@ -449,7 +449,7 @@ class RolePermissionTests(SupersetTestCase): mock_g.user = security_manager.find_user("gamma") with self.client.application.test_request_context(): database = get_example_database() - schemas = security_manager.schemas_accessible_by_user( + schemas = security_manager.get_schemas_accessible_by_user( database, ["temp_schema", "2", "3"] ) self.assertEquals(schemas, ["temp_schema", "2"])