This is an automated email from the ASF dual-hosted git repository. diegomedina24 pushed a commit to branch dm/chore/migrate-schemas-access-for-file-upload-to-v1 in repository https://gitbox.apache.org/repos/asf/superset.git
commit 8a6127301769bc6dfbeb70a50950abbacb0d7624 Author: Diego Medina <[email protected]> AuthorDate: Wed Feb 8 09:03:24 2023 -0300 chore: Migrate /superset/schemas_access_for_file_upload to v1 --- docs/static/resources/openapi.json | 199 ++++++++++++++------- superset/constants.py | 1 + superset/databases/api.py | 63 ++++++- superset/databases/schemas.py | 7 + .../form_view/database_schemas_selector.html | 5 +- superset/views/core.py | 1 + tests/integration_tests/databases/api_tests.py | 42 +++++ 7 files changed, 245 insertions(+), 73 deletions(-) diff --git a/docs/static/resources/openapi.json b/docs/static/resources/openapi.json index cc92f091e2..b5f9a62aab 100644 --- a/docs/static/resources/openapi.json +++ b/docs/static/resources/openapi.json @@ -345,7 +345,7 @@ "AnnotationLayerRestApi.get_list": { "properties": { "changed_by": { - "$ref": "#/components/schemas/AnnotationLayerRestApi.get_list.User1" + "$ref": "#/components/schemas/AnnotationLayerRestApi.get_list.User" }, "changed_on": { "format": "date-time", @@ -356,7 +356,7 @@ "readOnly": true }, "created_by": { - "$ref": "#/components/schemas/AnnotationLayerRestApi.get_list.User" + "$ref": "#/components/schemas/AnnotationLayerRestApi.get_list.User1" }, "created_on": { "format": "date-time", @@ -502,13 +502,13 @@ "AnnotationRestApi.get_list": { "properties": { "changed_by": { - "$ref": "#/components/schemas/AnnotationRestApi.get_list.User1" + "$ref": "#/components/schemas/AnnotationRestApi.get_list.User" }, "changed_on_delta_humanized": { "readOnly": true }, "created_by": { - "$ref": "#/components/schemas/AnnotationRestApi.get_list.User" + "$ref": "#/components/schemas/AnnotationRestApi.get_list.User1" }, "end_dttm": { "format": "date-time", @@ -1223,6 +1223,7 @@ "example": false }, "periods": { + "description": "Time periods (in units of `time_grain`) to predict into the future", "example": 7, "format": "int32", "type": "integer" @@ -1578,6 +1579,7 @@ "type": "string" }, "from_dttm": { + "description": "Start timestamp of time range", "format": "int32", "nullable": true, "type": "integer" @@ -1603,6 +1605,7 @@ "type": "integer" }, "stacktrace": { + "description": "Stacktrace if there was an error", "nullable": true, "type": "string" }, @@ -1620,6 +1623,7 @@ "type": "string" }, "to_dttm": { + "description": "End timestamp of time range", "format": "int32", "nullable": true, "type": "integer" @@ -1768,7 +1772,7 @@ "type": "string" }, "changed_by": { - "$ref": "#/components/schemas/ChartDataRestApi.get_list.User" + "$ref": "#/components/schemas/ChartDataRestApi.get_list.User1" }, "changed_by_name": { "readOnly": true @@ -1783,7 +1787,7 @@ "readOnly": true }, "created_by": { - "$ref": "#/components/schemas/ChartDataRestApi.get_list.User2" + "$ref": "#/components/schemas/ChartDataRestApi.get_list.User" }, "created_on_delta_humanized": { "readOnly": true @@ -1833,7 +1837,7 @@ "$ref": "#/components/schemas/ChartDataRestApi.get_list.User3" }, "owners": { - "$ref": "#/components/schemas/ChartDataRestApi.get_list.User1" + "$ref": "#/components/schemas/ChartDataRestApi.get_list.User2" }, "params": { "nullable": true, @@ -1897,6 +1901,10 @@ "maxLength": 64, "type": "string" }, + "id": { + "format": "int32", + "type": "integer" + }, "last_name": { "maxLength": 64, "type": "string" @@ -1914,23 +1922,14 @@ "maxLength": 64, "type": "string" }, - "id": { - "format": "int32", - "type": "integer" - }, "last_name": { "maxLength": 64, "type": "string" - }, - "username": { - "maxLength": 64, - "type": "string" } }, "required": [ "first_name", - "last_name", - "username" + "last_name" ], "type": "object" }, @@ -1947,11 +1946,16 @@ "last_name": { "maxLength": 64, "type": "string" + }, + "username": { + "maxLength": 64, + "type": "string" } }, "required": [ "first_name", - "last_name" + "last_name", + "username" ], "type": "object" }, @@ -2232,6 +2236,7 @@ "type": "string" }, "rolling_type_options": { + "description": "Optional options to pass to rolling method. Needed for e.g. quantile operation.", "example": {}, "type": "object" }, @@ -2560,7 +2565,7 @@ "type": "string" }, "changed_by": { - "$ref": "#/components/schemas/ChartRestApi.get_list.User" + "$ref": "#/components/schemas/ChartRestApi.get_list.User1" }, "changed_by_name": { "readOnly": true @@ -2575,7 +2580,7 @@ "readOnly": true }, "created_by": { - "$ref": "#/components/schemas/ChartRestApi.get_list.User2" + "$ref": "#/components/schemas/ChartRestApi.get_list.User" }, "created_on_delta_humanized": { "readOnly": true @@ -2625,7 +2630,7 @@ "$ref": "#/components/schemas/ChartRestApi.get_list.User3" }, "owners": { - "$ref": "#/components/schemas/ChartRestApi.get_list.User1" + "$ref": "#/components/schemas/ChartRestApi.get_list.User2" }, "params": { "nullable": true, @@ -2689,6 +2694,10 @@ "maxLength": 64, "type": "string" }, + "id": { + "format": "int32", + "type": "integer" + }, "last_name": { "maxLength": 64, "type": "string" @@ -2706,23 +2715,14 @@ "maxLength": 64, "type": "string" }, - "id": { - "format": "int32", - "type": "integer" - }, "last_name": { "maxLength": 64, "type": "string" - }, - "username": { - "maxLength": 64, - "type": "string" } }, "required": [ "first_name", - "last_name", - "username" + "last_name" ], "type": "object" }, @@ -2739,11 +2739,16 @@ "last_name": { "maxLength": 64, "type": "string" + }, + "username": { + "maxLength": 64, + "type": "string" } }, "required": [ "first_name", - "last_name" + "last_name", + "username" ], "type": "object" }, @@ -3027,13 +3032,13 @@ "CssTemplateRestApi.get_list": { "properties": { "changed_by": { - "$ref": "#/components/schemas/CssTemplateRestApi.get_list.User1" + "$ref": "#/components/schemas/CssTemplateRestApi.get_list.User" }, "changed_on_delta_humanized": { "readOnly": true }, "created_by": { - "$ref": "#/components/schemas/CssTemplateRestApi.get_list.User" + "$ref": "#/components/schemas/CssTemplateRestApi.get_list.User1" }, "created_on": { "format": "date-time", @@ -3400,7 +3405,7 @@ "type": "string" }, "changed_by": { - "$ref": "#/components/schemas/DashboardRestApi.get_list.User" + "$ref": "#/components/schemas/DashboardRestApi.get_list.User1" }, "changed_by_name": { "readOnly": true @@ -3415,7 +3420,7 @@ "readOnly": true }, "created_by": { - "$ref": "#/components/schemas/DashboardRestApi.get_list.User2" + "$ref": "#/components/schemas/DashboardRestApi.get_list.User" }, "created_on_delta_humanized": { "readOnly": true @@ -3441,7 +3446,7 @@ "type": "string" }, "owners": { - "$ref": "#/components/schemas/DashboardRestApi.get_list.User1" + "$ref": "#/components/schemas/DashboardRestApi.get_list.User2" }, "position_json": { "nullable": true, @@ -3500,25 +3505,16 @@ "last_name": { "maxLength": 64, "type": "string" - }, - "username": { - "maxLength": 64, - "type": "string" } }, "required": [ "first_name", - "last_name", - "username" + "last_name" ], "type": "object" }, "DashboardRestApi.get_list.User1": { "properties": { - "email": { - "maxLength": 64, - "type": "string" - }, "first_name": { "maxLength": 64, "type": "string" @@ -3537,7 +3533,6 @@ } }, "required": [ - "email", "first_name", "last_name", "username" @@ -3546,6 +3541,10 @@ }, "DashboardRestApi.get_list.User2": { "properties": { + "email": { + "maxLength": 64, + "type": "string" + }, "first_name": { "maxLength": 64, "type": "string" @@ -3557,11 +3556,17 @@ "last_name": { "maxLength": 64, "type": "string" + }, + "username": { + "maxLength": 64, + "type": "string" } }, "required": [ + "email", "first_name", - "last_name" + "last_name", + "username" ], "type": "object" }, @@ -4288,6 +4293,18 @@ }, "type": "object" }, + "DatabaseSchemaAccessForFileUploadResponse": { + "properties": { + "schemas": { + "description": "The list of schemas allowed for the database to upload information", + "items": { + "type": "string" + }, + "type": "array" + } + }, + "type": "object" + }, "DatabaseTablesResponse": { "properties": { "extra": { @@ -4898,7 +4915,7 @@ "type": "integer" }, "changed_by": { - "$ref": "#/components/schemas/DatasetRestApi.get.User" + "$ref": "#/components/schemas/DatasetRestApi.get.User2" }, "changed_on": { "format": "date-time", @@ -4912,7 +4929,7 @@ "$ref": "#/components/schemas/DatasetRestApi.get.TableColumn" }, "created_by": { - "$ref": "#/components/schemas/DatasetRestApi.get.User2" + "$ref": "#/components/schemas/DatasetRestApi.get.User" }, "created_on": { "format": "date-time", @@ -5230,7 +5247,7 @@ "DatasetRestApi.get_list": { "properties": { "changed_by": { - "$ref": "#/components/schemas/DatasetRestApi.get_list.User1" + "$ref": "#/components/schemas/DatasetRestApi.get_list.User" }, "changed_by_name": { "readOnly": true @@ -5273,7 +5290,7 @@ "readOnly": true }, "owners": { - "$ref": "#/components/schemas/DatasetRestApi.get_list.User" + "$ref": "#/components/schemas/DatasetRestApi.get_list.User1" }, "schema": { "maxLength": 255, @@ -5317,14 +5334,6 @@ "maxLength": 64, "type": "string" }, - "id": { - "format": "int32", - "type": "integer" - }, - "last_name": { - "maxLength": 64, - "type": "string" - }, "username": { "maxLength": 64, "type": "string" @@ -5332,7 +5341,6 @@ }, "required": [ "first_name", - "last_name", "username" ], "type": "object" @@ -5343,6 +5351,14 @@ "maxLength": 64, "type": "string" }, + "id": { + "format": "int32", + "type": "integer" + }, + "last_name": { + "maxLength": 64, + "type": "string" + }, "username": { "maxLength": 64, "type": "string" @@ -5350,6 +5366,7 @@ }, "required": [ "first_name", + "last_name", "username" ], "type": "object" @@ -6950,7 +6967,7 @@ "type": "boolean" }, "changed_by": { - "$ref": "#/components/schemas/ReportScheduleRestApi.get_list.User" + "$ref": "#/components/schemas/ReportScheduleRestApi.get_list.User1" }, "changed_on": { "format": "date-time", @@ -6966,7 +6983,7 @@ "type": "integer" }, "created_by": { - "$ref": "#/components/schemas/ReportScheduleRestApi.get_list.User2" + "$ref": "#/components/schemas/ReportScheduleRestApi.get_list.User" }, "created_on": { "format": "date-time", @@ -7016,7 +7033,7 @@ "type": "string" }, "owners": { - "$ref": "#/components/schemas/ReportScheduleRestApi.get_list.User1" + "$ref": "#/components/schemas/ReportScheduleRestApi.get_list.User2" }, "recipients": { "$ref": "#/components/schemas/ReportScheduleRestApi.get_list.ReportRecipients" @@ -7077,10 +7094,6 @@ "maxLength": 64, "type": "string" }, - "id": { - "format": "int32", - "type": "integer" - }, "last_name": { "maxLength": 64, "type": "string" @@ -7098,6 +7111,10 @@ "maxLength": 64, "type": "string" }, + "id": { + "format": "int32", + "type": "integer" + }, "last_name": { "maxLength": 64, "type": "string" @@ -15248,6 +15265,50 @@ ] } }, + "/api/v1/database/{pk}/schemas_access_for_file_upload/": { + "get": { + "parameters": [ + { + "in": "path", + "name": "pk", + "required": true, + "schema": { + "type": "integer" + } + } + ], + "responses": { + "200": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/DatabaseSchemaAccessForFileUploadResponse" + } + } + }, + "description": "The list of schemas allowed for the database to upload information" + }, + "401": { + "$ref": "#/components/responses/401" + }, + "404": { + "$ref": "#/components/responses/404" + }, + "500": { + "$ref": "#/components/responses/500" + } + }, + "security": [ + { + "jwt": [] + } + ], + "summary": "Get the list of schemas allowed for the database to upload information", + "tags": [ + "Database" + ] + } + }, "/api/v1/database/{pk}/select_star/{table_name}/": { "get": { "description": "Get database select star for table", diff --git a/superset/constants.py b/superset/constants.py index 5c1f0e36fe..0b2d6f9bba 100644 --- a/superset/constants.py +++ b/superset/constants.py @@ -143,6 +143,7 @@ MODEL_API_RW_METHOD_PERMISSION_MAP = { "delete_ssh_tunnel": "write", "get_updated_since": "read", "stop_query": "read", + "schemas_access_for_file_upload": "read", } EXTRA_FORM_DATA_APPEND_KEYS = { diff --git a/superset/databases/api.py b/superset/databases/api.py index c285198747..851fc88ead 100644 --- a/superset/databases/api.py +++ b/superset/databases/api.py @@ -65,6 +65,7 @@ from superset.databases.schemas import ( DatabasePostSchema, DatabasePutSchema, DatabaseRelatedObjectsResponse, + DatabaseSchemaAccessForFileUploadResponse, DatabaseTablesResponse, DatabaseTestConnectionSchema, DatabaseValidateParametersSchema, @@ -120,6 +121,7 @@ class DatabaseRestApi(BaseSupersetModelRestApi): "validate_parameters", "validate_sql", "delete_ssh_tunnel", + "schemas_access_for_file_upload", } resource_name = "database" class_permission_name = "Database" @@ -222,6 +224,7 @@ class DatabaseRestApi(BaseSupersetModelRestApi): openapi_spec_tag = "Database" openapi_spec_component_schemas = ( DatabaseFunctionNamesResponse, + DatabaseSchemaAccessForFileUploadResponse, DatabaseRelatedObjectsResponse, DatabaseTablesResponse, DatabaseTestConnectionSchema, @@ -813,7 +816,7 @@ class DatabaseRestApi(BaseSupersetModelRestApi): ) except NoSuchTableError: self.incr_stats("error", self.select_star.__name__) - return self.response(404, message="Table not found in the database") + return self.response(404, message="Table not found on the database") self.incr_stats("success", self.select_star.__name__) return self.response(200, result=result) @@ -1410,3 +1413,61 @@ class DatabaseRestApi(BaseSupersetModelRestApi): exc_info=True, ) return self.response_400(message=str(ex)) + + @expose("/<int:pk>/schemas_access_for_file_upload/") + @protect() + @safe + @statsd_metrics + @event_logger.log_this_with_context( + action=lambda self, *args, **kwargs: f"{self.__class__.__name__}" + f".schemas_access_for_file_upload", + log_to_statsd=False, + ) + def schemas_access_for_file_upload(self, pk: int) -> Response: + """Get the list of schemas allowed for the database to upload information + --- + get: + summary: + Get the list of schemas allowed for the database to upload information + parameters: + - in: path + name: pk + schema: + type: integer + responses: + 200: + description: The list of schemas allowed for the database to upload information + content: + application/json: + schema: + $ref: "#/components/schemas/DatabaseSchemaAccessForFileUploadResponse" + 401: + $ref: '#/components/responses/401' + 404: + $ref: '#/components/responses/404' + 500: + $ref: '#/components/responses/500' + """ + database = DatabaseDAO.find_by_id(pk) + if not database: + return self.response_404() + + try: + schemas_allowed = database.get_schema_access_for_file_upload() + if security_manager.can_access_database(database): + return self.response(200, schemas=schemas_allowed) + # the list schemas_allowed should not be empty here + # and the list schemas_allowed_processed returned from security_manager + # should not be empty either, + # otherwise the database should have been filtered out + # in CsvToDatabaseForm + schemas_allowed_processed = security_manager.get_schemas_accessible_by_user( + database, schemas_allowed, False + ) + return self.response(200, schemas=schemas_allowed_processed) + except Exception as ex: # pylint: disable=broad-except + logger.exception(ex) + return self.response_500( + "Failed to fetch schemas allowed for csv upload in this database! " + "Please contact your Superset Admin!" + ) diff --git a/superset/databases/schemas.py b/superset/databases/schemas.py index e318e41121..8ae36962aa 100644 --- a/superset/databases/schemas.py +++ b/superset/databases/schemas.py @@ -740,3 +740,10 @@ def encrypted_field_properties(self, field: Any, **_) -> Dict[str, Any]: # type if self.openapi_version.major > 2: ret["x-encrypted-extra"] = True return ret + + +class DatabaseSchemaAccessForFileUploadResponse(Schema): + schemas = fields.List( + fields.String(), + description="The list of schemas allowed for the database to upload information", + ) diff --git a/superset/templates/superset/form_view/database_schemas_selector.html b/superset/templates/superset/form_view/database_schemas_selector.html index b9efb68d7e..ac827c1330 100644 --- a/superset/templates/superset/form_view/database_schemas_selector.html +++ b/superset/templates/superset/form_view/database_schemas_selector.html @@ -32,12 +32,11 @@ under the License. function update_schemas_allowed_for_csv_upload(db_id) { $.ajax({ method: "GET", - url: "/superset/schemas_access_for_file_upload", - data: {db_id: db_id}, + url: `/api/v1/database/${db_id}/schemas_access_for_file_upload/`, dataType: 'json', contentType: "application/json; charset=utf-8" }).done(function (data) { - change_schema_field_in_formview(data) + change_schema_field_in_formview(data ? data.schemas : []) }).fail(function (error) { var errorMsg = error.responseJSON.error; alert("ERROR: " + errorMsg); diff --git a/superset/views/core.py b/superset/views/core.py index 94cabfa96c..6095ed581f 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -2734,6 +2734,7 @@ class Superset(BaseSupersetView): # pylint: disable=too-many-public-methods @has_access_api @event_logger.log_this @expose("/schemas_access_for_file_upload") + @deprecated() def schemas_access_for_file_upload(self) -> FlaskResponse: """ This method exposes an API endpoint to diff --git a/tests/integration_tests/databases/api_tests.py b/tests/integration_tests/databases/api_tests.py index f4968edae9..47ba5c3869 100644 --- a/tests/integration_tests/databases/api_tests.py +++ b/tests/integration_tests/databases/api_tests.py @@ -3171,3 +3171,45 @@ class TestDatabaseApi(SupersetTestCase): return self.assertEqual(rv.status_code, 422) self.assertIn("Kaboom!", response["errors"][0]["message"]) + + @mock.patch( + "superset.security.SupersetSecurityManager.get_schemas_accessible_by_user" + ) + @mock.patch("superset.security.SupersetSecurityManager.can_access_database") + @mock.patch("superset.security.SupersetSecurityManager.can_access_all_datasources") + def test_schemas_access_for_csv_upload_not_found_endpoint( + self, + mock_can_access_all_datasources, + mock_can_access_database, + mock_schemas_accessible, + ): + self.login(username="admin") + self.create_fake_db() + 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"] + rv = self.client.get(f"/api/v1/database/120ff/schemas_access_for_file_upload") + self.assertEqual(rv.status_code, 404) + self.delete_fake_db() + + @mock.patch( + "superset.security.SupersetSecurityManager.get_schemas_accessible_by_user" + ) + @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_can_access_all_datasources, + mock_can_access_database, + mock_schemas_accessible, + ): + self.login(username="admin") + dbobj = self.create_fake_db() + 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=f"/api/v1/database/{dbobj.id}/schemas_access_for_file_upload" + ) + assert data == {"schemas": ["this_schema_is_allowed_too"]} + self.delete_fake_db()
