diff --git a/superset/databases/api.py b/superset/databases/api.py index a4c8f79a6..a0689f4f9 100644 --- a/superset/databases/api.py +++ b/superset/databases/api.py @@ -41,6 +41,7 @@ from superset.databases.commands.exceptions import ( DatabaseInvalidError, DatabaseNotFoundError, DatabaseUpdateFailedError, + InvalidParametersError, ) from superset.databases.commands.export import ExportDatabasesCommand from superset.databases.commands.importers.dispatcher import ImportDatabasesCommand @@ -65,7 +66,8 @@ from superset.databases.schemas import ( ) from superset.databases.utils import get_table_metadata from superset.db_engine_specs import get_available_engine_specs -from superset.exceptions import InvalidPayloadFormatError, InvalidPayloadSchemaError +from superset.errors import ErrorLevel, SupersetError, SupersetErrorType +from superset.exceptions import InvalidPayloadFormatError from superset.extensions import security_manager from superset.models.core import Database from superset.typing import FlaskResponse @@ -1003,7 +1005,16 @@ class DatabaseRestApi(BaseSupersetModelRestApi): try: payload = DatabaseValidateParametersSchema().load(request.json) except ValidationError as error: - raise InvalidPayloadSchemaError(error) + errors = [ + SupersetError( + message="\n".join(messages), + error_type=SupersetErrorType.INVALID_PAYLOAD_SCHEMA_ERROR, + level=ErrorLevel.ERROR, + extra={"invalid": [attribute]}, + ) + for attribute, messages in error.messages.items() + ] + raise InvalidParametersError(errors) command = ValidateDatabaseParametersCommand(g.user, payload) command.run() diff --git a/superset/databases/schemas.py b/superset/databases/schemas.py index 1df799c5f..2b56791ac 100644 --- a/superset/databases/schemas.py +++ b/superset/databases/schemas.py @@ -16,7 +16,7 @@ # under the License. import inspect import json -from typing import Any, Dict +from typing import Any, Dict, Optional, Type from flask import current_app from flask_babel import lazy_gettext as _ @@ -27,7 +27,7 @@ from sqlalchemy import MetaData from sqlalchemy.engine.url import make_url from sqlalchemy.exc import ArgumentError -from superset.db_engine_specs import get_engine_specs +from superset.db_engine_specs import BaseEngineSpec, get_engine_specs from superset.exceptions import CertificateException, SupersetSecurityException from superset.models.core import ConfigurationMethod, PASSWORD_MASK from superset.security.analytics_db_safety import check_sqlalchemy_uri @@ -253,28 +253,11 @@ class DatabaseParametersSchemaMixin: the constructed SQLAlchemy URI to be passed. """ parameters = data.pop("parameters", {}) - - # TODO (betodealmeida): remove second expression after making sure - # frontend is not passing engine inside parameters - engine = data.pop("engine", None) or parameters.pop("engine", None) + engine = data.pop("engine", None) configuration_method = data.get("configuration_method") if configuration_method == ConfigurationMethod.DYNAMIC_FORM: - if not engine: - raise ValidationError( - [ - _( - "An engine must be specified when passing " - "individual parameters to a database." - ) - ] - ) - engine_specs = get_engine_specs() - if engine not in engine_specs: - raise ValidationError( - [_('Engine "%(engine)s" is not a valid engine.', engine=engine,)] - ) - engine_spec = engine_specs[engine] + engine_spec = get_engine_spec(engine) if not hasattr(engine_spec, "build_sqlalchemy_uri") or not hasattr( engine_spec, "parameters_schema" @@ -304,6 +287,24 @@ class DatabaseParametersSchemaMixin: return data +def get_engine_spec(engine: Optional[str]) -> Type[BaseEngineSpec]: + if not engine: + raise ValidationError( + [ + _( + "An engine must be specified when passing " + "individual parameters to a database." + ) + ] + ) + engine_specs = get_engine_specs() + if engine not in engine_specs: + raise ValidationError( + [_('Engine "%(engine)s" is not a valid engine.', engine=engine,)] + ) + return engine_specs[engine] + + class DatabaseValidateParametersSchema(Schema): engine = fields.String(required=True, description="SQLAlchemy engine to use") parameters = fields.Dict( @@ -333,6 +334,17 @@ class DatabaseValidateParametersSchema(Schema): description=configuration_method_description, ) + @validates_schema + def validate_parameters( # pylint: disable=no-self-use + self, data: Dict[str, Any], **kwargs: Any # pylint: disable=unused-argument + ) -> None: + """ + Validate the DB engine spec specific parameters schema. + """ + # TODO (aafghahi): use a single parameter + engine_spec = get_engine_spec(data.get("engine") or data.get("backend")) + engine_spec.parameters_schema.load(data["parameters"]) # type: ignore + class DatabasePostSchema(Schema, DatabaseParametersSchemaMixin): class Meta: # pylint: disable=too-few-public-methods diff --git a/superset/db_engine_specs/bigquery.py b/superset/db_engine_specs/bigquery.py index 7320eab6d..8951c7e21 100644 --- a/superset/db_engine_specs/bigquery.py +++ b/superset/db_engine_specs/bigquery.py @@ -29,7 +29,7 @@ from typing_extensions import TypedDict from superset.databases.schemas import encrypted_field_properties, EncryptedField from superset.db_engine_specs.base import BaseEngineSpec -from superset.errors import SupersetErrorType +from superset.errors import SupersetError, SupersetErrorType from superset.exceptions import SupersetGenericDBErrorException from superset.sql_parse import Table from superset.utils import core as utils @@ -331,6 +331,12 @@ class BigQueryEngineSpec(BaseEngineSpec): message="Big Query encrypted_extra is not available.", ) + @classmethod + def validate_parameters( + cls, parameters: BigQueryParametersType # pylint: disable=unused-argument + ) -> List[SupersetError]: + return [] + @classmethod def parameters_json_schema(cls) -> Any: """ diff --git a/tests/databases/api_tests.py b/tests/databases/api_tests.py index fad93ade8..a637ee979 100644 --- a/tests/databases/api_tests.py +++ b/tests/databases/api_tests.py @@ -1674,14 +1674,11 @@ class TestDatabaseApi(SupersetTestCase): assert response == { "errors": [ { - "message": "An error happened when validating the request", + "message": "Missing data for required field.", "error_type": "INVALID_PAYLOAD_SCHEMA_ERROR", "level": "error", "extra": { - "messages": { - "engine": ["Missing data for required field."], - "foo": ["Unknown field."], - }, + "invalid": ["engine"], "issue_codes": [ { "code": 1020, @@ -1689,7 +1686,21 @@ class TestDatabaseApi(SupersetTestCase): } ], }, - } + }, + { + "message": "Unknown field.", + "error_type": "INVALID_PAYLOAD_SCHEMA_ERROR", + "level": "error", + "extra": { + "invalid": ["foo"], + "issue_codes": [ + { + "code": 1020, + "message": "Issue 1020 - The submitted payload has the incorrect schema.", + } + ], + }, + }, ] } @@ -1733,6 +1744,77 @@ class TestDatabaseApi(SupersetTestCase): ] } + @mock.patch("superset.db_engine_specs.base.is_hostname_valid") + @mock.patch("superset.db_engine_specs.base.is_port_open") + @mock.patch("superset.databases.api.ValidateDatabaseParametersCommand") + def test_validate_parameters_valid_payload( + self, ValidateDatabaseParametersCommand, is_port_open, is_hostname_valid + ): + is_hostname_valid.return_value = True + is_port_open.return_value = True + + self.login(username="admin") + url = "api/v1/database/validate_parameters" + payload = { + "engine": "postgresql", + "parameters": defaultdict(dict), + } + payload["parameters"].update( + { + "host": "localhost", + "port": 6789, + "username": "superset", + "password": "XXX", + "database": "test", + "query": {}, + } + ) + rv = self.client.post(url, json=payload) + response = json.loads(rv.data.decode("utf-8")) + + assert rv.status_code == 200 + assert response == {"message": "OK"} + + def test_validate_parameters_invalid_port(self): + self.login(username="admin") + url = "api/v1/database/validate_parameters" + payload = { + "engine": "postgresql", + "parameters": defaultdict(dict), + } + payload["parameters"].update( + { + "host": "localhost", + "port": "string", + "username": "superset", + "password": "XXX", + "database": "test", + "query": {}, + } + ) + rv = self.client.post(url, json=payload) + response = json.loads(rv.data.decode("utf-8")) + + assert rv.status_code == 422 + assert response == { + "errors": [ + { + "message": "Not a valid integer.", + "error_type": "INVALID_PAYLOAD_SCHEMA_ERROR", + "level": "error", + "extra": { + "invalid": ["port"], + "issue_codes": [ + { + "code": 1020, + "message": "Issue 1020 - The submitted payload has the incorrect schema.", + } + ], + }, + } + ] + } + @mock.patch("superset.db_engine_specs.base.is_hostname_valid") def test_validate_parameters_invalid_host(self, is_hostname_valid): is_hostname_valid.return_value = False