From a44063b71d3b387f1ee6acdf0e40e5da91c52675 Mon Sep 17 00:00:00 2001 From: John Bodley Date: Wed, 10 Jun 2020 09:21:31 -0700 Subject: [PATCH] chore(security): Renaming schemas_accessible_by_user --- 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 60f29ebc706f8..c0179303f48fb 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 772edbbc6c06d..e32bc7e66b46a 100644 --- a/superset/security/manager.py +++ b/superset/security/manager.py @@ -448,11 +448,11 @@ def user_view_menu_names(self, permission_name: str) -> Set[str]: 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 44cade9140aa8..55d40bb1a4c20 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -1130,7 +1130,7 @@ def schemas(self, db_id: int, force_refresh: str = "false") -> FlaskResponse: 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 = [] @@ -2907,7 +2907,7 @@ def schemas_access_for_csv_upload(self) -> FlaskResponse: # 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 2fb62907e42f7..68b391330c6be 100644 --- a/superset/views/database/forms.py +++ b/superset/views/database/forms.py @@ -76,7 +76,7 @@ def at_least_one_schema_is_allowed(database: Database) -> bool: ): 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 6631da8b157fa..e7b4ace169e71 100644 --- a/tests/core_tests.py +++ b/tests/core_tests.py @@ -963,7 +963,9 @@ def test_slice_payload_no_datasource(self): "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 476b67019f15a..63e15c9998fde 100644 --- a/tests/security_tests.py +++ b/tests/security_tests.py @@ -412,7 +412,7 @@ def test_schemas_accessible_by_user_admin(self, mock_g): 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 @@ def test_schemas_accessible_by_user_schema_access(self, mock_g): 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 @@ def test_schemas_accessible_by_user_datasource_access(self, mock_g): 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 @@ def test_schemas_accessible_by_user_datasource_and_schema_access(self, mock_g): 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"])