From 358a4ecedd13a20b3491ca9f536d773d87b6ca65 Mon Sep 17 00:00:00 2001 From: Daniel Vaz Gaspar Date: Mon, 7 Nov 2022 08:55:15 +0000 Subject: [PATCH] fix: deprecate approve and request_access endpoint (#22022) Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com> --- UPDATING.md | 1 + superset/views/core.py | 16 ++++++++++++++-- tests/integration_tests/access_tests.py | 6 +++--- 3 files changed, 18 insertions(+), 5 deletions(-) diff --git a/UPDATING.md b/UPDATING.md index ddda9e51f616..ec3a15a157aa 100644 --- a/UPDATING.md +++ b/UPDATING.md @@ -24,6 +24,7 @@ assists people when migrating to a new version. ## Next +- [22022](https://github.com/apache/superset/pull/22022): HTTP API endpoints `/superset/approve` and `/superset/request_access` have been deprecated and their HTTP methods were changed from GET to POST - [21895](https://github.com/apache/superset/pull/21895): Markdown components had their security increased by adhering to the same sanitization process enforced by Github. This means that some HTML elements found in markdowns are not allowed anymore due to the security risks they impose. If you're deploying Superset in a trusted environment and wish to use some of the blocked elements, then you can use the HTML_SANITIZATION_SCHEMA_EXTENSIONS configuration to extend the default sanitization schema. There's also the option to disable HTML sanitization using the HTML_SANITIZATION configuration but we do not recommend this approach because of the security risks. Given the provided configurations, we don't view the improved sanitization as a breaking change but as a security patch. - [20606](https://github.com/apache/superset/pull/20606): When user clicks on chart title or "Edit chart" button in Dashboard page, Explore opens in the same tab. Clicking while holding cmd/ctrl opens Explore in a new tab. To bring back the old behaviour (always opening Explore in a new tab), flip feature flag `DASHBOARD_EDIT_CHART_IN_NEW_TAB` to `True`. - [20799](https://github.com/apache/superset/pull/20799): Presto and Trino engine will now display tracking URL for running queries in SQL Lab. If for some reason you don't want to show the tracking URL (for example, when your data warehouse hasn't enable access for to Presto or Trino UI), update `TRACKING_URL_TRANSFORMER` in `config.py` to return `None`. diff --git a/superset/views/core.py b/superset/views/core.py index 371a632f2953..353671ef01e4 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -277,8 +277,14 @@ def override_role_permissions(self) -> FlaskResponse: @has_access @event_logger.log_this - @expose("/request_access/") + @expose("/request_access/", methods=["POST"]) def request_access(self) -> FlaskResponse: + logger.warning( + "%s.approve " + "This API endpoint is deprecated and will be removed in version 3.0.0", + self.__class__.__name__, + ) + datasources = set() dashboard_id = request.args.get("dashboard_id") if dashboard_id: @@ -320,7 +326,7 @@ def request_access(self) -> FlaskResponse: @has_access @event_logger.log_this - @expose("/approve") + @expose("/approve", methods=["POST"]) def approve(self) -> FlaskResponse: # pylint: disable=too-many-locals,no-self-use def clean_fulfilled_requests(session: Session) -> None: for dar in session.query(DAR).all(): @@ -332,6 +338,12 @@ def clean_fulfilled_requests(session: Session) -> None: session.delete(dar) session.commit() + logger.warning( + "%s.approve " + "This API endpoint is deprecated and will be removed in version 3.0.0", + self.__class__.__name__, + ) + datasource_type = request.args["datasource_type"] datasource_id = request.args["datasource_id"] created_by_username = request.args.get("created_by") diff --git a/tests/integration_tests/access_tests.py b/tests/integration_tests/access_tests.py index ae8b39a8d289..38fd10524019 100644 --- a/tests/integration_tests/access_tests.py +++ b/tests/integration_tests/access_tests.py @@ -270,7 +270,7 @@ def test_clean_requests_after_alpha_grant(self): session.commit() access_requests = self.get_access_requests("gamma", "table", ds_1_id) self.assertTrue(access_requests) - self.client.get( + self.client.post( EXTEND_ROLE_REQUEST.format("table", ds_1_id, "gamma2", TEST_ROLE_2) ) access_requests = self.get_access_requests("gamma", "table", ds_1_id) @@ -309,7 +309,7 @@ def test_clean_requests_after_db_grant(self): access_requests = self.get_access_requests("gamma", "table", ds_1_id) self.assertTrue(access_requests) # gamma2 request gets fulfilled - self.client.get( + self.client.post( EXTEND_ROLE_REQUEST.format("table", ds_1_id, "gamma2", TEST_ROLE_2) ) access_requests = self.get_access_requests("gamma", "table", ds_1_id) @@ -353,7 +353,7 @@ def test_clean_requests_after_schema_grant(self): gamma_user.roles.append(security_manager.find_role(SCHEMA_ACCESS_ROLE)) session.commit() # gamma2 request gets fulfilled - self.client.get( + self.client.post( EXTEND_ROLE_REQUEST.format("table", ds_1_id, "gamma2", TEST_ROLE_2) ) access_requests = self.get_access_requests("gamma", "table", ds_1_id)