diff --git a/CHANGELOG.md b/CHANGELOG.md index 767263681..dc95022dd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - `datacontract breaking` now allows to select a `--perspective` (either `consumer` or `producer`) +- `datacontract test`: API servers send POST requests with query parameters and a JSON body, set by the `method`, `params` and `body` custom properties, and an `X-API-Key` header from `DATACONTRACT_API_HEADER_X_API_KEY` (also used by `datacontract import odata`) (#1747) +- `datacontract import openapi` imports POST operations, and writes required query parameters to the `params` custom property (#1747) + +### Security +- `datacontract api`: a posted contract's `api` server may not reach private network addresses unless the server runs with `--allow-private-network`, and is refused while `DATACONTRACT_API_HEADER_AUTHORIZATION` or `DATACONTRACT_API_HEADER_X_API_KEY` is set on the server (#1747) ### Fixed - `datacontract breaking` grades from the consumer's perspective by default (previously inconsistent mix of consumer and producer perspective) diff --git a/datacontract/api.py b/datacontract/api.py index 6d69c4949..6f96b9499 100644 --- a/datacontract/api.py +++ b/datacontract/api.py @@ -259,9 +259,10 @@ def _cli_version() -> str: untrusted input: a `quality.type: sql` rule must be a read-only query, a credential held in the server's environment is never sent to a host the contract names, a `publish_url` may only point at the Entropy Data platform or the host configured via `ENTROPY_DATA_HOST`, and `servers[].type: local` -is refused so a caller cannot read the server's own files. Set -`DATACONTRACT_CLI_API_ALLOW_LOCAL_FILES=true` if the deployment serves its own files on purpose; -the contract is then still confined to the paths it declares. +is refused so a caller cannot read the server's own files, and an `api` server's requests may not +reach a private network address. Set `DATACONTRACT_CLI_API_ALLOW_LOCAL_FILES=true` if the deployment +serves its own files on purpose; the contract is then still confined to the paths it declares. Set +`DATACONTRACT_CLI_API_ALLOW_PRIVATE_NETWORK=true` to test APIs in the server's own network. ## Connecting to data sources @@ -525,6 +526,8 @@ def _parse_filters_query(filters: str | None) -> dict[str, str] | None: # option pins it), so an environment-held SASL password can never be paired # with a posted contract safely -- host_field is None, so it is always rejected. "kafka": ("broker host", None, ("kafka_sasl_password",)), + # Nothing pins an API server's location either. + "api": ("location", None, ("api_header_authorization", "api_header_x_api_key")), } @@ -552,6 +555,7 @@ def _selected_server(body: str, server_name: str | None, config): _LOCAL_SERVER_TYPES = frozenset({"local", "duckdb"}) ALLOW_LOCAL_FILES_ENV = "DATACONTRACT_CLI_API_ALLOW_LOCAL_FILES" +ALLOW_PRIVATE_NETWORK_ENV = "DATACONTRACT_CLI_API_ALLOW_PRIVATE_NETWORK" _TRUTHY = frozenset({"1", "true", "yes", "on"}) @@ -570,6 +574,11 @@ def _local_files_allowed() -> bool: return (os.getenv(ALLOW_LOCAL_FILES_ENV) or "").strip().lower() in _TRUTHY +def _private_network_allowed() -> bool: + """Whether the operator lets a posted contract's API requests reach private network addresses.""" + return (os.getenv(ALLOW_PRIVATE_NETWORK_ENV) or "").strip().lower() in _TRUTHY + + def _reject_local_server_type(body: str, server_name: str | None, config) -> None: """Keep a posted contract from reading the server's own disk. @@ -623,20 +632,23 @@ def _reject_environment_credentials_to_untrusted_host(body: str, server_name: st return # no environment-held secret for this server type; nothing to protect if host_field is not None and resolved.option_source(host_field) == "env": return # the operator pinned the target host; the contract's host is ignored + held = [ + env_name(field, Config.model_fields[field]) for field in secret_fields if resolved.option_source(field) == "env" + ] if host_field is not None: env_var = env_name(host_field, Config.model_fields[host_field]) detail = ( - f"This server holds a {server.type} credential in its environment, so the connection " - f"{target_attr} must also come from the server's environment ({env_var}); it will not be " - f"taken from the posted data contract. Set {env_var}, or send the full set of credentials " - f"with the request instead of relying on the server's environment." + f"This server holds a credential for {server.type} servers in its environment ({', '.join(held)}), so the " + f"connection {target_attr} must also come from the server's environment ({env_var}); it will not " + f"be taken from the posted data contract. Set {env_var}, or send the full set of credentials with " + f"the request instead of relying on the server's environment." ) else: detail = ( - f"This server holds a {server.type} credential in its environment, but its {target_attr} " - f"can only come from the posted data contract, so the credential cannot be pinned to a " - f"trusted target. Send the credentials with the request instead of relying on the server's " - f"environment." + f"This server holds a credential for {server.type} servers in its environment ({', '.join(held)}), but " + f"their {target_attr} can only come from the posted data contract, so the credential cannot be pinned to " + f"a trusted target. Send the credentials as request headers instead " + f"({', '.join(name.lower().replace('_', '-') for name in held)})." ) raise HTTPException(status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, detail=detail) @@ -835,6 +847,7 @@ def test( filters=parsed_filters, untrusted_contract=untrusted_contract, allowed_variables=_allowed_contract_variables(), + allow_private_network=_private_network_allowed(), ).test() diff --git a/datacontract/command_api.py b/datacontract/command_api.py index 7f3f3de4e..b99b1e633 100644 --- a/datacontract/command_api.py +++ b/datacontract/command_api.py @@ -57,6 +57,14 @@ def api( help="Let a posted data contract read the server's own disk through servers[].type: local. Off by default.", ), ] = None, + allow_private_network: Annotated[ + bool | None, + typer.Option( + "--allow-private-network/--no-allow-private-network", + help="Let a posted data contract's API requests (servers[].type: api) reach private network addresses, " + "such as localhost or the cloud metadata endpoint. Off by default.", + ), + ] = None, debug: debug_option = None, ): """ @@ -78,12 +86,14 @@ def api( enable_debug_logging(debug) # Passed through the environment, which survives uvicorn's --reload respawn. - from datacontract.api import ALLOW_LOCAL_FILES_ENV + from datacontract.api import ALLOW_LOCAL_FILES_ENV, ALLOW_PRIVATE_NETWORK_ENV if contract_variables is not None: os.environ[CONTRACT_VARIABLES_ENV] = contract_variables if allow_local_files is not None: os.environ[ALLOW_LOCAL_FILES_ENV] = "true" if allow_local_files else "false" + if allow_private_network is not None: + os.environ[ALLOW_PRIVATE_NETWORK_ENV] = "true" if allow_private_network else "false" import uvicorn from uvicorn.config import LOGGING_CONFIG diff --git a/datacontract/command_import.py b/datacontract/command_import.py index 7249aafe4..8c45f18fd 100644 --- a/datacontract/command_import.py +++ b/datacontract/command_import.py @@ -1053,8 +1053,9 @@ def import_openapi( operation: Annotated[ Optional[str], typer.Option( - help="The GET operation to import, by operationId or path (e.g. listOrders or /orders). " - "Required when the document has more than one GET operation." + help="The GET or POST operation to import, by operationId, method and path, or a GET operation's path " + "(e.g. listOrders, 'POST /orders/search', or /orders). Required unless the document has exactly one GET " + "operation." ), ] = None, output: output_option = None, @@ -1062,7 +1063,7 @@ def import_openapi( id: id_option = None, debug: debug_option = None, ): - """Import a data contract from the response of a GET operation in an OpenAPI 3.x document.""" + """Import a data contract from the response of a GET or POST operation in an OpenAPI 3.x document.""" enable_debug_logging(debug) result = DataContract.import_from_source( config=cli_config(), format="openapi", source=source, openapi_operation=operation, owner=owner, id=id diff --git a/datacontract/config/settings.py b/datacontract/config/settings.py index 915421142..184a0e0bc 100644 --- a/datacontract/config/settings.py +++ b/datacontract/config/settings.py @@ -113,6 +113,7 @@ class Config(BaseSettings): # general api_header_authorization: SecretStr | None = None + api_header_x_api_key: SecretStr | None = None custom_quality_checks: str | None = None max_errors: int | None = None connect_timeout: int | None = None @@ -526,6 +527,9 @@ def get_datacontract_manager_host(self, required: bool = False) -> str | None: def get_api_header_authorization(self, required: bool = False) -> str | None: return self._str_option("api_header_authorization", required) + def get_api_header_x_api_key(self, required: bool = False) -> str | None: + return self._str_option("api_header_x_api_key", required) + # --- custom quality checks --- def get_custom_quality_checks(self) -> str | None: return self._str_option("custom_quality_checks") diff --git a/datacontract/config/variables.py b/datacontract/config/variables.py index 58aa5cb60..f86144c22 100644 --- a/datacontract/config/variables.py +++ b/datacontract/config/variables.py @@ -175,7 +175,8 @@ def resolve_server_variables(server: Server, variables: Optional[Mapping[str, st ``port`` may hold a reference (the schema allows a string there for that reason); a resolved all-digit port becomes an int again. Custom property - values are resolved too. The contract's server is not mutated. + values are resolved too, nested ones like an API request body included. + The contract's server is not mutated. """ updates = {} for field in Server.model_fields: @@ -192,17 +193,15 @@ def resolve_server_variables(server: Server, variables: Optional[Mapping[str, st custom_properties = [] changed = False for prop in server.customProperties: + source = f"server '{server.server}' custom property '{prop.property}'" if contains_variables(prop.value): changed = True prop = prop.model_copy( - update={ - "value": resolve_variables( - prop.value, - source=f"server '{server.server}' custom property '{prop.property}'", - variables=variables, - ) - } + update={"value": resolve_variables(prop.value, source=source, variables=variables)} ) + elif isinstance(prop.value, (dict, list)): + changed = True + prop = prop.model_copy(update={"value": resolve_runtime_variables(prop.value, source, variables)}) custom_properties.append(prop) if changed: updates["customProperties"] = custom_properties diff --git a/datacontract/data_contract.py b/datacontract/data_contract.py index 680cdb5de..28309e5f9 100644 --- a/datacontract/data_contract.py +++ b/datacontract/data_contract.py @@ -114,6 +114,7 @@ def __init__( dry_run: bool = False, untrusted_contract: bool = False, allowed_variables: list[str] | None = None, + allow_private_network: bool = False, config: "Config | dict[str, str] | None" = None, ): self._data_contract_file = data_contract_file @@ -145,6 +146,8 @@ def __init__( self._untrusted_contract = untrusted_contract # fnmatch globs of the environment variables an untrusted contract may read. self._allowed_variables = allowed_variables + # Whether an untrusted contract's API requests may reach private network addresses. + self._allow_private_network = allow_private_network self._config = Config.resolve(config) @classmethod @@ -259,6 +262,7 @@ def test(self) -> Run: config=self._config, untrusted_contract=self._untrusted_contract, allowed_variables=self._allowed_variables, + allow_private_network=self._allow_private_network, ) except DataContractException as e: diff --git a/datacontract/engines/data_contract_test.py b/datacontract/engines/data_contract_test.py index 520652c51..a4f3e3ce3 100644 --- a/datacontract/engines/data_contract_test.py +++ b/datacontract/engines/data_contract_test.py @@ -3,6 +3,7 @@ import tempfile import typing from typing import Mapping +from urllib.parse import parse_qsl, urlsplit import requests import yaml @@ -28,6 +29,7 @@ check_that_datacontract_contains_valid_server_configuration, ) from datacontract.engines.fastjsonschema.check_jsonschema import check_jsonschema +from datacontract.engines.http_api import ApiSession, PrivateAddressRefused, PublicAddressAdapter, api_headers from datacontract.engines.ibis.ibis_check_execute import build_check_stubs, execute_ibis_checks, set_result from datacontract.lint.resolve import _SafeLoaderNoTimestamp from datacontract.model.exceptions import DataContractException @@ -54,6 +56,7 @@ def execute_data_contract_test( config: Config | None = None, untrusted_contract: bool = False, allowed_variables: list[str] | None = None, + allow_private_network: bool = False, ): config = Config.resolve(config) # An untrusted contract reads only the allow-listed variables. @@ -109,18 +112,13 @@ def execute_data_contract_test( check_that_quality_ids_exist(data_contract, quality_ids, schema_name) if server.type == "api": - if dry_run: - # A dry run reads nothing, so the response that would become the - # local file is never fetched; stand in the server it would return. - server = Server( - server="api_local", - type="local", - format="json", - path="(not fetched: dry run)", - delimiter=server.delimiter, - ) - else: - server = process_api_response(run, server, config) + server = process_api_response( + run, + server, + config, + dry_run=dry_run, + public_addresses_only=untrusted_contract and not allow_private_network, + ) model_filters = resolve_row_filters(data_contract, server, run, filter, filters, schema_name) @@ -436,22 +434,79 @@ def _report_dry_run( run.log_warn("dry run: file-metadata checks for blob schemas are not included in the plan") -def process_api_response(run, server, config: Config | None = None): +def process_api_response( + run, server, config: Config | None = None, dry_run: bool = False, public_addresses_only: bool = False +): config = Config.resolve(config) + custom_properties = {prop.property: prop.value for prop in server.customProperties or []} + method = str(custom_properties.get("method", "GET")).upper() + if method not in ("GET", "POST"): + raise _api_request_error(f"Method {custom_properties['method']!r} is not supported; use GET or POST.") + if "body" in custom_properties and method != "POST": + raise _api_request_error("A body requires method POST.") + if not isinstance(custom_properties.get("params", {}), dict): + raise _api_request_error("The params custom property must map each query parameter name to its value.") + params = {} + for name, value in custom_properties.get("params", {}).items(): + values = value if isinstance(value, list) else [value] + if not all(isinstance(v, (str, int, float, bool)) for v in values): + raise _api_request_error( + f"Query parameter {name!r} must be a string, number, or boolean, or a list of them." + ) + params[name] = [str(v).lower() if isinstance(v, bool) else v for v in values] + clashes = sorted( + set(params) & {name for name, _ in parse_qsl(urlsplit(server.location).query, keep_blank_values=True)} + ) + if clashes: + raise _api_request_error( + f"Query parameter {clashes[0]!r} is set both in the location and in the params custom property." + ) + + if dry_run: + # A dry run reads nothing, so the response that would become the + # local file is never fetched; stand in the server it would return. + return Server( + server="api_local", + type="local", + format="json", + path="(not fetched: dry run)", + delimiter=server.delimiter, + ) + tmp_dir = tempfile.TemporaryDirectory(prefix="datacontract_cli_api_") atexit.register(tmp_dir.cleanup) - headers = {"Accept": "application/json, application/yaml;q=0.9, */*;q=0.8"} - if config.get_api_header_authorization() is not None: - headers["Authorization"] = config.get_api_header_authorization() + headers = {"Accept": "application/json, application/yaml;q=0.9, */*;q=0.8", **api_headers(config)} + body = custom_properties.get("body") + # a string is the JSON text as written, anything else is converted to JSON + if isinstance(body, str): + headers["Content-Type"] = "application/json" try: - response = requests.get(server.location, headers=headers) - response.raise_for_status() + with ApiSession() as session: + if public_addresses_only: + session.trust_env = False + session.mount("http://", PublicAddressAdapter()) + session.mount("https://", PublicAddressAdapter()) + response = session.request( + method, + server.location, + params=params, + headers=headers, + timeout=(config.get_connect_timeout(), None), + data=body.encode() if isinstance(body, str) else None, + json=None if isinstance(body, str) else body, + ) + response.raise_for_status() except requests.exceptions.RequestException as e: + # urllib3 wraps the refusal in its retry error + refusal = getattr(e.args[0], "reason", None) if e.args else None + error = refusal.message if isinstance(refusal, PrivateAddressRefused) else e + # the API's own explanation, such as which part of the body it rejected + answer = f" {e.response.text[:500]}" if e.response is not None and e.response.text else "" raise DataContractException( type="connection", name="API server connection error", result=ResultEnum.error, - reason=f"Failed to fetch API response from {server.location}: {e}", + reason=f"Failed to fetch API response from {server.location}: {error}{answer}", engine="datacontract-cli", ) content = response.text @@ -482,6 +537,16 @@ def process_api_response(run, server, config: Config | None = None): return new_server +def _api_request_error(reason: str) -> DataContractException: + return DataContractException( + type="general", + name="Build the API request", + result=ResultEnum.failed, + reason=reason, + engine="datacontract-cli", + ) + + def _has_blob_schemas(data_contract: OpenDataContractStandard, schema_name: str) -> bool: """Return True if the (possibly filtered) schema list contains any logicalType='blob' schema.""" if data_contract.schema_ is None: diff --git a/datacontract/engines/http_api.py b/datacontract/engines/http_api.py new file mode 100644 index 000000000..a18be3433 --- /dev/null +++ b/datacontract/engines/http_api.py @@ -0,0 +1,96 @@ +"""Requests to the HTTP APIs that `api` servers and OData services name.""" + +import ipaddress +import socket + +import requests +from requests.adapters import HTTPAdapter +from urllib3.connection import HTTPConnection, HTTPSConnection +from urllib3.connectionpool import HTTPConnectionPool, HTTPSConnectionPool +from urllib3.exceptions import NewConnectionError + +from datacontract.config import Config + + +def api_headers(config: Config) -> dict[str, str]: + """The configured credential headers.""" + headers = {} + if config.get_api_header_authorization() is not None: + headers["Authorization"] = config.get_api_header_authorization() + if config.get_api_header_x_api_key() is not None: + headers["X-API-Key"] = config.get_api_header_x_api_key() + return headers + + +class ApiSession(requests.Session): + """Keep Requests' redirect policy without introducing .netrc credentials, and drop the credential headers + on a redirect to another host.""" + + def rebuild_auth(self, prepared_request: requests.PreparedRequest, response: requests.Response) -> None: + if self.should_strip_auth(response.request.url, prepared_request.url): + prepared_request.headers.pop("Authorization", None) + prepared_request.headers.pop("X-API-Key", None) + + +class PrivateAddressRefused(NewConnectionError): + def __init__(self, conn, host: str): + self.message = ( + f"Refused to connect to {host}: a posted data contract may not reach a private network address. " + f"Start the API server with --allow-private-network to allow it." + ) + super().__init__(conn, self.message) + + +def _is_public(address: str) -> bool: + address = ipaddress.ip_address(address) + # IPv4-mapped, NAT64, IPv4-compatible and IPv4-translated addresses reach the IPv4 address in their low 32 bits. + if address.version == 6 and any( + address in ipaddress.ip_network(prefix) + for prefix in ("::ffff:0:0/96", "64:ff9b::/96", "::/96", "::ffff:0:0:0/96") + ): + address = ipaddress.IPv4Address(int(address) & 0xFFFFFFFF) + return address.is_global + + +class _PublicPeerOnly: + # Refuses a private address before connecting, so a refusal tells nothing about the private network's ports or + # addresses, and checks the address actually connected to, so DNS rebinding cannot get around it. + def _new_conn(self): + try: + resolved = [info[4][0] for info in socket.getaddrinfo(self.host, self.port, type=socket.SOCK_STREAM)] + except OSError: + resolved = [] # urllib3 reports the resolution failure + if not all(_is_public(address) for address in resolved): + raise PrivateAddressRefused(self, self.host) + sock = super()._new_conn() + if not _is_public(sock.getpeername()[0]): + sock.close() + raise PrivateAddressRefused(self, self.host) + return sock + + +class _PublicHTTPConnection(_PublicPeerOnly, HTTPConnection): + pass + + +class _PublicHTTPSConnection(_PublicPeerOnly, HTTPSConnection): + pass + + +class _PublicHTTPConnectionPool(HTTPConnectionPool): + ConnectionCls = _PublicHTTPConnection + + +class _PublicHTTPSConnectionPool(HTTPSConnectionPool): + ConnectionCls = _PublicHTTPSConnection + + +class PublicAddressAdapter(HTTPAdapter): + """Connects only to addresses routable on the public internet.""" + + def init_poolmanager(self, *args, **kwargs): + super().init_poolmanager(*args, **kwargs) + self.poolmanager.pool_classes_by_scheme = { + "http": _PublicHTTPConnectionPool, + "https": _PublicHTTPSConnectionPool, + } diff --git a/datacontract/imports/odata_importer.py b/datacontract/imports/odata_importer.py index db8a6ec27..3371daecf 100644 --- a/datacontract/imports/odata_importer.py +++ b/datacontract/imports/odata_importer.py @@ -12,6 +12,7 @@ from open_data_contract_standard.model import CustomProperty, OpenDataContractStandard, SchemaObject from datacontract.config import Config +from datacontract.engines.http_api import ApiSession, api_headers from datacontract.imports.importer import Importer from datacontract.imports.odcs_helper import ( create_odcs, @@ -160,21 +161,10 @@ def _read_file(path: str | Path, label: str) -> bytes: raise _schema_error(f"Failed to read OData {label} file {path}: {exc}") from exc -class _ODataSession(requests.Session): - """Keep Requests' redirect policy without introducing .netrc credentials.""" - - def rebuild_auth(self, prepared_request: requests.PreparedRequest, response: requests.Response) -> None: - if self.should_strip_auth(response.request.url, prepared_request.url): - prepared_request.headers.pop("Authorization", None) - - def _fetch_document(url: str, label: str, accept: str, config: Config) -> requests.Response: - headers = {"Accept": accept} - authorization = config.get_api_header_authorization() - if authorization is not None: - headers["Authorization"] = authorization + headers = {"Accept": accept, **api_headers(config)} try: - with _ODataSession() as session: + with ApiSession() as session: response = session.get( url, headers=headers, diff --git a/datacontract/imports/openapi_importer.py b/datacontract/imports/openapi_importer.py index 785d3b878..7a7cf096b 100644 --- a/datacontract/imports/openapi_importer.py +++ b/datacontract/imports/openapi_importer.py @@ -1,11 +1,11 @@ -"""Import the response of one GET operation of an OpenAPI 3.x document.""" +"""Import the response of one GET or POST operation of an OpenAPI 3.x document.""" import logging import re from urllib.parse import urlsplit import yaml -from open_data_contract_standard.model import OpenDataContractStandard +from open_data_contract_standard.model import CustomProperty, OpenDataContractStandard from datacontract.imports.importer import Importer from datacontract.imports.jsonschema_importer import SchemaResolver, to_schema_object @@ -30,7 +30,8 @@ def import_source(self, source: str, import_args: dict) -> OpenDataContractStand def import_openapi(source: str, operation: str | None = None) -> OpenDataContractStandard: - """The contract of the response of one GET operation, with an API server for each server of the document.""" + """The contract of the response of one GET or POST operation, with an API server for each server of the + document.""" try: with open(source, "r", encoding="utf-8") as file: spec = yaml.load(file, Loader=_SafeLoaderNoTimestamp) @@ -40,51 +41,81 @@ def import_openapi(source: str, operation: str | None = None) -> OpenDataContrac raise _schema_error(f"{source} is not an OpenAPI 3.x document.") resolver = SchemaResolver(spec) - path, path_item, get = _select_operation(spec, operation) - schema = _response_schema(resolver, path, get) + path, path_item, method, operation = _select_operation(spec, operation) + schema = _response_schema(resolver, f"{method} {path}", operation) resolver.warn() # an array response holds the records, which the schema describes record = schema["items"] if schema.get("type") == "array" and isinstance(schema.get("items"), dict) else schema if not record.get("properties"): - logger.warning(f"The response of GET {path} has no object properties, so the schema has no properties.") + logger.warning(f"The response of {method} {path} has no object properties, so the schema has no properties.") - name = get.get("operationId") or "_".join(re.findall(r"[A-Za-z0-9]+", path)) or "root" + name = operation.get("operationId") or "_".join(re.findall(r"[A-Za-z0-9]+", path)) or "root" odcs = create_odcs(name=spec.get("info", {}).get("title") or name) if spec.get("info", {}).get("version"): odcs.version = str(spec["info"]["version"]) odcs.schema_ = [ to_schema_object( - {**_to_json_schema(record), "type": "object"}, name, get.get("summary") or get.get("description") + {**_to_json_schema(record), "type": "object"}, + name, + operation.get("summary") or operation.get("description"), ) ] - odcs.servers = _servers(spec, _endpoint(resolver, path, path_item, get)) or None + endpoint, params = _endpoint(resolver, path, path_item, operation) + custom_properties = [CustomProperty(property="method", value="POST")] if method == "POST" else [] + if params: + custom_properties.append(CustomProperty(property="params", value=params)) + if method == "POST": + body = _request_body_example(resolver, operation) + if body is not None: + custom_properties.append(CustomProperty(property="body", value=body)) + elif resolver.resolve(operation.get("requestBody") or {}).get("required"): + logger.warning( + f"The request body of {method} {path} has no JSON example, so the contract has no body; " + f"add one to the servers' body custom property." + ) + odcs.servers = _servers(spec, endpoint, custom_properties) or None return odcs -def _select_operation(spec: dict, operation: str | None) -> tuple[str, dict, dict]: - """The path, path item, and GET operation that --operation names, or the only GET operation.""" - gets = [ - (path, item, item["get"]) +def _select_operation(spec: dict, operation: str | None) -> tuple[str, dict, str, dict]: + """The path, path item, method, and operation that --operation names, or the only GET operation. + + A POST operation is selected only by its operationId or as `POST /path`, as it may change data. + """ + operations = [ + (path, item, method.upper(), item[method]) for path, item in (spec.get("paths") or {}).items() - if isinstance(item, dict) and isinstance(item.get("get"), dict) + if isinstance(item, dict) + for method in ("get", "post") + if isinstance(item.get(method), dict) ] + gets = [o for o in operations if o[2] == "GET"] if operation is not None: - matches = [g for g in gets if operation in (g[2].get("operationId"), g[0], f"GET {g[0]}")] + method, _, path = operation.partition(" ") + matches = [ + o + for o in operations + if operation == o[3].get("operationId") + or (method.upper(), path) == (o[2], o[0]) + or (o[2] == "GET" and operation == o[0]) + ] else: matches = gets if len(gets) == 1 else [] if len(matches) == 1: return matches[0] - available = ", ".join(f"{g[2]['operationId']} (GET {g[0]})" if g[2].get("operationId") else g[0] for g in gets) - if not gets: - raise _schema_error("The OpenAPI document has no GET operation.") + available = ", ".join( + f"{o[3]['operationId']} ({o[2]} {o[0]})" if o[3].get("operationId") else f"{o[2]} {o[0]}" for o in operations + ) + if not operations: + raise _schema_error("The OpenAPI document has no GET or POST operation.") if operation is None: - raise _schema_error(f"Select one GET operation with --operation, by operationId or path: {available}") - raise _schema_error(f"No GET operation {operation!r}. Available: {available}") + raise _schema_error(f"Select one operation with --operation, by operationId or method and path: {available}") + raise _schema_error(f"No GET or POST operation {operation!r}. Available: {available}") -def _response_schema(resolver: SchemaResolver, path: str, get: dict) -> dict: +def _response_schema(resolver: SchemaResolver, label: str, operation: dict) -> dict: """The resolved schema of the first success response with a JSON or YAML body.""" - responses = get.get("responses") or {} + responses = operation.get("responses") or {} # 200 first, then the other success codes and the 2XX range for code in sorted((c for c in map(str, responses) if c.startswith("2")), key=lambda c: (c != "200", c)): response = resolver.resolve(responses.get(code) if code in responses else responses.get(int(code))) @@ -92,7 +123,7 @@ def _response_schema(resolver: SchemaResolver, path: str, get: dict) -> dict: if JSON_MEDIA_TYPE.match(media_type) or YAML_MEDIA_TYPE.match(media_type): if isinstance(body, dict) and isinstance(body.get("schema"), dict): return resolver.resolve(body["schema"]) - raise _schema_error(f"GET {path} has no success response with a JSON or YAML schema.") + raise _schema_error(f"{label} has no success response with a JSON or YAML schema.") def _to_json_schema(schema): @@ -129,10 +160,31 @@ def _admits_null(schema) -> bool: ) -def _parameters(resolver: SchemaResolver, path_item: dict, get: dict) -> dict: +def _request_body_example(resolver: SchemaResolver, operation: dict): + """The JSON example of the operation's request body: the media type's example, else its first named example, + else the schema's.""" + content = resolver.resolve(operation.get("requestBody") or {}).get("content") or {} + for media_type, body in content.items(): + if not JSON_MEDIA_TYPE.match(media_type) or not isinstance(body, dict): + continue + if "example" in body: + return body["example"] + for example in (body.get("examples") or {}).values(): + example = resolver.resolve(example) + if isinstance(example, dict) and "value" in example: + return example["value"] + schema = resolver.resolve(body.get("schema") or {}) + if "example" in schema: + return schema["example"] + if schema.get("examples"): + return schema["examples"][0] + return None + + +def _parameters(resolver: SchemaResolver, path_item: dict, operation: dict) -> dict: """The parameters of the operation by (in, name); the operation's own replace those of its path.""" parameters = {} - for parameter in (path_item.get("parameters") or []) + (get.get("parameters") or []): + for parameter in (path_item.get("parameters") or []) + (operation.get("parameters") or []): parameter = resolver.resolve(parameter) if isinstance(parameter, dict) and "name" in parameter: parameters[(parameter.get("in"), parameter["name"])] = parameter @@ -150,20 +202,20 @@ def _variable(parameter: dict) -> str: return f"${{{name}}}" if value is None else f"${{{name}:-{value}}}" -def _endpoint(resolver: SchemaResolver, path: str, path_item: dict, get: dict) -> str: - """The path with its parameters as variables, followed by the required query parameters.""" - parameters = _parameters(resolver, path_item, get) +def _endpoint(resolver: SchemaResolver, path: str, path_item: dict, operation: dict) -> tuple[str, dict]: + """The path with its parameters as variables, and the required query parameters as variables.""" + parameters = _parameters(resolver, path_item, operation) # every placeholder, even one without a declared parameter path = re.sub(r"\{([^}]+)\}", lambda m: _variable(parameters.get(("path", m.group(1)), {"name": m.group(1)})), path) - query = [ - f"{name}={_variable(parameter)}" + params = { + name: _variable(parameter) for (where, name), parameter in parameters.items() if where == "query" and parameter.get("required") - ] - return path + ("?" + "&".join(query) if query else "") + } + return path, params -def _servers(spec: dict, endpoint: str) -> list: +def _servers(spec: dict, endpoint: str, custom_properties: list) -> list: """An API server for each absolute server URL of the document, located at the endpoint.""" servers = [] for index, server in enumerate(spec.get("servers") or []): @@ -181,6 +233,7 @@ def _servers(spec: dict, endpoint: str) -> list: odcs_server = create_server(name=name, server_type="api", location=url.rstrip("/") + endpoint) if server.get("description"): odcs_server.description = server["description"] + odcs_server.customProperties = custom_properties or None servers.append(odcs_server) if not servers: logger.warning("The OpenAPI document has no absolute server URL, so the contract has no server to test.") diff --git a/datacontract/output/test_results_writer.py b/datacontract/output/test_results_writer.py index a9d5dc607..167efbb93 100644 --- a/datacontract/output/test_results_writer.py +++ b/datacontract/output/test_results_writer.py @@ -54,6 +54,9 @@ def write_test_result( details.append(f"project={server.project}") if server.account: details.append(f"account={server.account}") + method = next((p.value for p in server.customProperties or [] if p.property == "method"), None) + if server.type == "api" and method: + details.append(f"method={method}") if server.location: details.append(f"location={server.location}") if server.path: diff --git a/docs/docs/api.md b/docs/docs/api.md index db1311cbf..5e7678ed2 100644 --- a/docs/docs/api.md +++ b/docs/docs/api.md @@ -148,6 +148,7 @@ A data contract carries SQL and names the hosts to connect to, so a contract tha - a `publish_url` may only point at the **Entropy Data platform or the host set via `ENTROPY_DATA_HOST`** on the server — per-request `entropy-data-host` headers do not widen this, and other hosts are refused; - `authoritativeDefinitions` are **only resolved against the Entropy Data platform or the host set via `ENTROPY_DATA_HOST`** on the server — per-request `entropy-data-host` headers do not widen this; a URL on any other host, or a reference to a local file, is refused, and a failed lookup reports only the URL, never what the host answered. An API key goes only to the host it belongs to — the server's `ENTROPY_DATA_API_KEY` to exactly `ENTROPY_DATA_HOST`, an `entropy-data-api-key` sent to `/test` to that request's `entropy-data-host`, with all Entropy Data domains counting as one host; - `servers[].type: local` is **refused**, so a caller cannot read the files of the machine running the API; +- an `api` server's requests **may not reach a private network address**, so a caller cannot read or write services in the server's own network; proxies and CA bundles from the environment (`HTTPS_PROXY`, `REQUESTS_CA_BUNDLE`) are not used for them; - a `${VAR}` reference in the contract resolves **only from the variables the operator allow-listed**, so a posted contract cannot read the server's environment; - for a file-based server type (`s3`, `gcs`, `azure`), the DuckDB connection is **confined to the data locations the contract declares**. @@ -165,7 +166,13 @@ Likewise, a posted contract resolves no `${VAR}` references at all until you nam export DATACONTRACT_CLI_API_CONTRACT_VARIABLES='TABLE_*,CUTOFF_DATE' ``` -Both settings are also options on the command, which is the more visible place for them: `datacontract api --contract-variables 'TABLE_*' --allow-local-files`. The patterns are comma-separated fnmatch globs matched case-sensitively; a variable outside the list is indistinguishable from an unset one. `*` allows every variable, **including every credential in the server's environment**, so use it only where callers are trusted. +If the APIs to test live in the server's own network, allow private network addresses for `api` servers: + +```bash +export DATACONTRACT_CLI_API_ALLOW_PRIVATE_NETWORK=true +``` + +These settings are also options on the command: `datacontract api --contract-variables 'TABLE_*' --allow-local-files --allow-private-network`. The patterns are comma-separated fnmatch globs matched case-sensitively; a variable outside the list is indistinguishable from an unset one. `*` allows every variable, **including every credential in the server's environment**, so use it only where callers are trusted. ## Run as a Docker container diff --git a/docs/docs/configuration.md b/docs/docs/configuration.md index 52e688e59..6b15f1e3b 100644 --- a/docs/docs/configuration.md +++ b/docs/docs/configuration.md @@ -144,6 +144,7 @@ Every option, by its environment variable name and the matching `Config` field. | Environment variable | `Config` field | Type | Notes | |---|---|---|---| | `DATACONTRACT_API_HEADER_AUTHORIZATION` | `api_header_authorization` | string (secret) | | +| `DATACONTRACT_API_HEADER_X_API_KEY` | `api_header_x_api_key` | string (secret) | | | `DATACONTRACT_CUSTOM_QUALITY_CHECKS` | `custom_quality_checks` | string | | | `DATACONTRACT_MAX_ERRORS` | `max_errors` | integer | | | `DATACONTRACT_CONNECT_TIMEOUT` | `connect_timeout` | integer | | diff --git a/docs/docs/imports/index.md b/docs/docs/imports/index.md index 670438361..9e80c32ce 100644 --- a/docs/docs/imports/index.md +++ b/docs/docs/imports/index.md @@ -106,7 +106,7 @@ Each import page shows a runnable example: a small source file under [`examples/ - openapiA GET operation of an OpenAPI document. + openapiA GET or POST operation of an OpenAPI document. diff --git a/docs/docs/imports/openapi.md b/docs/docs/imports/openapi.md index dc58c642c..16fce8874 100644 --- a/docs/docs/imports/openapi.md +++ b/docs/docs/imports/openapi.md @@ -1,12 +1,12 @@ --- sidebar_position: 22 title: "Import: OpenAPI" -description: "Create a data contract from the response of a GET operation in an OpenAPI 3.x document." +description: "Create a data contract from the response of a GET or POST operation in an OpenAPI 3.x document." --- # Import: OpenAPI -Creates a data contract from the response of one GET operation in an [OpenAPI](https://www.openapis.org/) 3.0 or 3.1 document, in YAML or JSON. You can then [test the endpoint](../testing/api.md) against the contract. +Creates a data contract from the response of one GET or POST operation in an [OpenAPI](https://www.openapis.org/) 3.0 or 3.1 document, in YAML or JSON. You can then [test the endpoint](../testing/api.md) against the contract. ```bash datacontract import openapi --source openapi.yaml --operation listOrders @@ -14,12 +14,13 @@ datacontract import openapi --source openapi.yaml --operation listOrders ## What is imported -- **Operation**: select the GET operation with `--operation`, by its `operationId` (`listOrders`) or its path (`/orders`). If the document has only one GET operation, `--operation` can be omitted. +- **Operation**: select the operation with `--operation`, by its `operationId` (`listOrders`), its method and path (`GET /orders`, `POST /orders/search`), or the path of a GET operation (`/orders`). If the document has only one GET operation, `--operation` can be omitted. A POST operation is never selected by its path alone, because a POST may change data: import only POST operations that read, such as a search. - **Schema**: the contract gets one schema, named after the `operationId` (or the path), described by the operation's `summary` or `description`. Its properties come from the response schema of the first success response (`200` first, then other `2xx` codes) with a JSON (`application/json`, `application/*+json`) or YAML (`application/yaml`, `text/yaml`) body. - **Arrays**: if the response is an array, the schema describes its items, which are the records. - **Types**: the response schema is mapped like a [JSON Schema](jsonschema.md). Local `$ref`s (to `#/components/...`) are resolved and `allOf` is merged. OpenAPI 3.0's `example` is imported as `examples`. A property that admits `null` (`nullable: true`, or `type: [string, "null"]`) is not `required`, as ODCS `required` means not null. - **Servers**: every absolute `servers` URL becomes an `api` server whose `location` is the endpoint's URL. Server variables take their `default`. Relative server URLs (such as `/v1`) are skipped with a warning, because they cannot be tested. -- **Parameters**: path parameters and required query parameters become [variables](../configuration.md#variables-in-the-data-contract) in the `location`, defaulting to the parameter's `example` or `default`. For example, `/orders/{orderId}` with `example: ORD-1001` becomes `/orders/${orderId:-ORD-1001}`. Set `orderId` in the environment to test another order. +- **Parameters**: path parameters become [variables](../configuration.md#variables-in-the-data-contract) in the `location`, and required query parameters variables in the server's `params` custom property, defaulting to the parameter's `example` or `default`. For example, `/orders/{orderId}` with `example: ORD-1001` becomes `/orders/${orderId:-ORD-1001}`. Set `orderId` in the environment to test another order. +- **Request body**: for a POST operation, the server gets `method: POST` and a `body` custom property with the request body's JSON example: the media type's `example`, else its first entry in `examples`, else the schema's `example`. Without one, the contract has no `body` and a warning asks you to add it. Swagger 2.0 documents are not supported, and `$ref`s to other files are not followed. diff --git a/docs/docs/reference/api.md b/docs/docs/reference/api.md index ee3d5fc2f..862025c46 100644 --- a/docs/docs/reference/api.md +++ b/docs/docs/reference/api.md @@ -23,9 +23,22 @@ servers: | Variable | Example | Description | |---|---|---| -| `DATACONTRACT_API_HEADER_AUTHORIZATION` | `Bearer ` | Value for the `authorization` header (optional) | +| `DATACONTRACT_API_HEADER_AUTHORIZATION` | `Bearer ` | Value for the `Authorization` header (optional) | +| `DATACONTRACT_API_HEADER_X_API_KEY` | `` | Value for the `X-API-Key` header (optional) | -The `location` (URL) and `delimiter` (`new_line`, `array`, or `none`) come from the contract's `servers` block. The `location` may contain [variables](../configuration.md#variables-in-the-data-contract), such as `https://api.example.com/orders/${orderId:-ORD-1001}`. Only GET requests are supported. +The `location` (URL) and `delimiter` (`new_line`, `array`, or `none`) come from the contract's `servers` block. The `location` may contain [variables](../configuration.md#variables-in-the-data-contract), such as `https://api.example.com/orders/${orderId:-ORD-1001}`. + +## Request + +Custom properties of the server shape the request: + +| Custom property | Example | Description | +|---|---|---| +| `method` | `POST` | `GET` (default) or `POST`. | +| `params` | `{limit: 100, status: [placed, shipped]}` | The query parameters, each a string, number or boolean, or a list of them for a repeated parameter. | +| `body` | `{query: {status: shipped}}` | The request body. A mapping or list is sent as JSON, a string as the JSON text it holds. Requires `method: POST`. | + +A `${VAR}` [variable](../configuration.md#variables-in-the-data-contract) may appear in any string of `params` and `body`; it resolves to a string, so write a number the API requires as a literal, or put the reference into a string `body` without quotes: `'{"size": ${SIZE:-100}}'`. ## Data types diff --git a/docs/docs/testing/api.md b/docs/docs/testing/api.md index 0c38cf5cb..f74960b8b 100644 --- a/docs/docs/testing/api.md +++ b/docs/docs/testing/api.md @@ -1,12 +1,12 @@ --- sidebar_position: 15 title: "HTTP API" -description: "Test the JSON or YAML responses of a REST API's GET endpoint against a data contract, created from its OpenAPI document or a sample response." +description: "Test the JSON or YAML responses of a REST API's GET or POST endpoint against a data contract, created from its OpenAPI document or a sample response." --- # HTTP API -Test the response of a REST API's GET endpoint. `datacontract test` calls the endpoint and checks the response, JSON or YAML, against the contract. Only GET requests are supported. +Test the response of a REST API's GET or POST endpoint. `datacontract test` calls the endpoint and checks the response, JSON or YAML, against the contract. ## 1. Install @@ -20,11 +20,12 @@ See [Installation](../installation.md) for pip, pipx, and Docker. ## 2. Authenticate -If the API requires authentication, set the value for the `authorization` header: +If the API requires authentication, set the value for the `Authorization` header, or the API key for the `X-API-Key` header: ```bash # .env DATACONTRACT_API_HEADER_AUTHORIZATION="Bearer " +DATACONTRACT_API_HEADER_X_API_KEY="" ``` For public endpoints, skip this step. @@ -33,7 +34,7 @@ For public endpoints, skip this step. ### From the OpenAPI document -If the API has an OpenAPI 3.x document, import the GET operation you want to test, by its `operationId` or its path: +If the API has an OpenAPI 3.x document, import the operation you want to test, by its `operationId` or its method and path (`POST /orders/search`): ```bash datacontract import openapi --source openapi.yaml --operation listOrders --output datacontract.yaml @@ -46,17 +47,25 @@ servers: - server: production type: api description: Production - location: https://api.example.com/v1/orders?limit=${limit:-100} + customProperties: + - property: params + value: + limit: ${limit:-100} + location: https://api.example.com/v1/orders - server: staging type: api description: Staging - location: https://staging.api.example.com/v1/orders?limit=${limit:-100} + customProperties: + - property: params + value: + limit: ${limit:-100} + location: https://staging.api.example.com/v1/orders schema: - name: listOrders # ... ``` -Path parameters and required query parameters become [variables](../configuration.md#variables-in-the-data-contract) in the `location`, defaulting to the parameter's `example` or `default`. Set them in the environment to call the endpoint with other values, for example `limit=10 datacontract test datacontract.yaml`. See [Import: OpenAPI](../imports/openapi.md) for what is imported. +Path parameters become [variables](../configuration.md#variables-in-the-data-contract) in the `location`, and required query parameters variables in `params`, defaulting to the parameter's `example` or `default`. Set them in the environment to call the endpoint with other values, for example `limit=10 datacontract test datacontract.yaml`. See [Import: OpenAPI](../imports/openapi.md) for what is imported. ### From a response @@ -76,6 +85,31 @@ servers: location: "https://api.example.com/orders" ``` +### Shape the request + +Custom properties of the server set the method, the query parameters and the JSON body of the request. For a search API that only accepts POST: + +```yaml +servers: + - server: search + type: api + location: "https://api.example.com/orders/search" + customProperties: + - property: method + value: POST + - property: params + value: + limit: ${limit:-100} + status: [placed, shipped] + - property: body + value: + query: + customer_id: ${customer_id:-C-1} + size: 100 +``` + +This sends a POST request with the query parameters `limit=100`, `status=placed` and `status=shipped`, and the body `{"query": {"customer_id": "C-1"}, "size": 100}`. See the [reference](../reference/api.md#request) for the rules. + ## 4. Test the actual data ```bash @@ -86,7 +120,7 @@ The first server is tested unless you select one with `--server`, e.g. `--server ``` Testing datacontract.yaml -Server: production (type=api, location=https://api.example.com/v1/orders?limit=${limit:-100}) +Server: production (type=api, location=https://api.example.com/v1/orders) ╭────────┬───────────────────────────────────────────┬──────────┬─────────────────────────────╮ │ Result │ Check │ Field │ Details │ ├────────┼───────────────────────────────────────────┼──────────┼─────────────────────────────┤ @@ -112,6 +146,6 @@ Authentication options and data type handling: **[HTTP API Reference](../referen ## Troubleshooting -- **`401` / `403`**: set `DATACONTRACT_API_HEADER_AUTHORIZATION` including the scheme (e.g. `Bearer eyJ...`), not just the raw token. +- **`401` / `403`**: set `DATACONTRACT_API_HEADER_AUTHORIZATION` including the scheme (e.g. `Bearer eyJ...`), not just the raw token, or `DATACONTRACT_API_HEADER_X_API_KEY` if the API expects an `X-API-Key` header. - **`Variable orderId referenced in ... is not set`**: the endpoint has a path or query parameter without an example or default. Set it in the environment or a `.env` file, or write a default into the `location` (`${orderId:-ORD-1001}`). - **Schema checks fail on a wrapped response**: if the API returns `{"data": [...]}` instead of a plain array, model the wrapper object in the schema, or set `delimiter` accordingly (`array` for a JSON array of records, `none` for a single object). diff --git a/tests/conftest.py b/tests/conftest.py index 4fd9f0054..598667b27 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -5,6 +5,9 @@ import shutil import subprocess import sys +import threading +from dataclasses import dataclass, field +from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer from pathlib import Path import pytest @@ -108,3 +111,53 @@ def generated_docs(): if shutil.which("node") is None: pytest.skip("node is needed to generate docs/docs/release-notes.md") _generate(["node", "docs/scripts/generate-release-notes.mjs"]) + + +@dataclass +class Api: + """What the local API answers, and the requests it received. + + Attributes rather than one dict: what a request records must never end up in + the headers of a response. + """ + + url: str = "" + body: str = "" + content_type: str = "application/json" + status: int = 200 + paths: list = field(default_factory=list) + methods: list = field(default_factory=list) + headers: list = field(default_factory=list) + request_bodies: list = field(default_factory=list) + + +@pytest.fixture +def api(): + """A local API that answers every GET and POST with the body and content type set on it, and records the + requests.""" + api = Api() + + class Handler(BaseHTTPRequestHandler): + def do_GET(self): + api.paths.append(self.path) + api.methods.append(self.command) + api.headers.append(dict(self.headers)) + api.request_bodies.append(self.rfile.read(int(self.headers.get("Content-Length") or 0))) + body = api.body.encode() + self.send_response(api.status) + self.send_header("Content-Type", api.content_type) + self.send_header("Content-Length", str(len(body))) + self.end_headers() + self.wfile.write(body) + + do_POST = do_GET + + def log_message(self, *args): + pass + + server = ThreadingHTTPServer(("127.0.0.1", 0), Handler) + threading.Thread(target=server.serve_forever, daemon=True).start() + api.url = f"http://127.0.0.1:{server.server_port}" + yield api + server.shutdown() + server.server_close() diff --git a/tests/fixtures/import/openapi/orders.odcs.yaml b/tests/fixtures/import/openapi/orders.odcs.yaml index 5c3470009..1168da820 100644 --- a/tests/fixtures/import/openapi/orders.odcs.yaml +++ b/tests/fixtures/import/openapi/orders.odcs.yaml @@ -8,11 +8,19 @@ servers: - server: production type: api description: Production - location: https://eu.api.example.com/v1/orders?limit=${limit:-100} + customProperties: + - property: params + value: + limit: ${limit:-100} + location: https://eu.api.example.com/v1/orders - server: staging type: api description: Staging - location: https://staging.api.example.com/v1/orders?limit=${limit:-100} + customProperties: + - property: params + value: + limit: ${limit:-100} + location: https://staging.api.example.com/v1/orders schema: - name: listOrders physicalType: object diff --git a/tests/fixtures/import/openapi/orders.yaml b/tests/fixtures/import/openapi/orders.yaml index 08dff5176..cba2a322b 100644 --- a/tests/fixtures/import/openapi/orders.yaml +++ b/tests/fixtures/import/openapi/orders.yaml @@ -42,6 +42,36 @@ paths: responses: "201": description: Created. + /orders/search: + post: + operationId: searchOrders + summary: Orders matching a query. + parameters: + - name: limit + in: query + required: true + schema: + type: integer + default: 100 + requestBody: + required: true + content: + application/json: + schema: + type: object + examples: + shipped: + value: + status: shipped + responses: + "200": + description: The matching orders. + content: + application/json: + schema: + type: array + items: + $ref: "#/components/schemas/Order" /orders/{orderId}: parameters: - $ref: "#/components/parameters/OrderId" diff --git a/tests/test_api.py b/tests/test_api.py index 360a2fc59..d746ecb85 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -5,7 +5,7 @@ import responses from fastapi.testclient import TestClient -from datacontract.api import ALLOW_LOCAL_FILES_ENV, app +from datacontract.api import ALLOW_LOCAL_FILES_ENV, ALLOW_PRIVATE_NETWORK_ENV, app from datacontract.lint.resolve import clear_definition_cache from datacontract.model.exceptions import DataContractException from tests.dcs_deprecation import DCS_DEPRECATION @@ -838,6 +838,66 @@ def test_guard_allows_kafka_without_environment_secret(monkeypatch): _reject_environment_credentials_to_untrusted_host(_KAFKA_CONTRACT, "production", None) # no raise +def _api_contract(location: str) -> str: + return f"""\ +apiVersion: v3.2.0 +kind: DataContract +id: orders-api +version: 1.0.0 +status: active +servers: + - server: production + type: api + location: {location} +schema: + - name: orders + properties: + - name: order_id + logicalType: string +""" + + +@pytest.mark.parametrize("variable", ["DATACONTRACT_API_HEADER_AUTHORIZATION", "DATACONTRACT_API_HEADER_X_API_KEY"]) +def test_guard_blocks_env_api_header(monkeypatch, variable): + """Nothing pins an API server's location, so an environment-held API credential can never be paired with a + posted contract.""" + monkeypatch.setenv(variable, "prod-secret") + response = client.post( + "/test", content=_api_contract("https://attacker.example/orders"), headers={"Content-Type": "application/yaml"} + ) + assert response.status_code == 422 + assert f"credential for api servers in its environment ({variable})" in response.json()["detail"] + assert f"as request headers instead ({variable.lower().replace('_', '-')})" in response.json()["detail"] + + +@pytest.fixture +def no_api_headers(monkeypatch): + monkeypatch.delenv("DATACONTRACT_API_HEADER_AUTHORIZATION", raising=False) + monkeypatch.delenv("DATACONTRACT_API_HEADER_X_API_KEY", raising=False) + monkeypatch.delenv(ALLOW_PRIVATE_NETWORK_ENV, raising=False) + + +def test_api_server_requests_stay_off_private_networks(api, no_api_headers): + api.body = '[{"order_id": "ORD-1"}]' + response = client.post( + "/test", content=_api_contract(f"{api.url}/orders"), headers={"Content-Type": "application/yaml"} + ) + assert response.status_code == 200 + assert response.json()["result"] == "error" + assert "a posted data contract may not reach a private network address" in response.text + assert api.paths == [] + + +def test_api_server_requests_reach_private_networks_when_allowed(api, no_api_headers, monkeypatch): + monkeypatch.setenv(ALLOW_PRIVATE_NETWORK_ENV, "true") + api.body = '[{"order_id": "ORD-1"}]' + response = client.post( + "/test", content=_api_contract(f"{api.url}/orders"), headers={"Content-Type": "application/yaml"} + ) + assert response.json()["result"] == "passed", response.text + assert api.paths == ["/orders"] + + # --- publish_url is restricted to environment-pinned platform URLs --- from datacontract.api import ( # noqa: E402 diff --git a/tests/test_config.py b/tests/test_config.py index cd4f2fc14..a0f09b39f 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -182,6 +182,7 @@ def test_declared_snowflake_variables_are_not_reported(monkeypatch): "DATACONTRACT_CLI_DEBUG", "DATACONTRACT_CLI_API_KEY", "DATACONTRACT_CLI_API_ALLOW_LOCAL_FILES", + "DATACONTRACT_CLI_API_ALLOW_PRIVATE_NETWORK", "DATACONTRACT_CLI_API_CONTRACT_VARIABLES", "DATACONTRACT_SYSTEM_TRUSTSTORE", "DATACONTRACT_EDITOR_AI_PROVIDER", diff --git a/tests/test_import_odata.py b/tests/test_import_odata.py index 186548db4..732d0a2b0 100644 --- a/tests/test_import_odata.py +++ b/tests/test_import_odata.py @@ -58,7 +58,7 @@ def metadata_response(monkeypatch): response.content = (FIXTURES / "who-metadata.xml").read_bytes() response.headers = {"OData-Version": "4.0"} get = Mock(return_value=response) - monkeypatch.setattr("datacontract.imports.odata_importer._ODataSession.get", get) + monkeypatch.setattr("datacontract.engines.http_api.ApiSession.get", get) return response, get @@ -821,13 +821,16 @@ def test_redirect_authentication(http_transport, monkeypatch, authorization, tar routes[initial_url] = (302, {"Location": redirect_url}, b"") routes[redirect_url] = (302, {"Location": final_url}, b"") routes[final_url] = document - DataContract.import_from_source("odata", source=SERVICE_ROOT, config=Config(api_header_authorization=authorization)) + api_key = authorization and "redirect-key" + config = Config(api_header_authorization=authorization, api_header_x_api_key=api_key) + DataContract.import_from_source("odata", source=SERVICE_ROOT, config=config) requests_by_url = {request.url: request for request, _ in sent} - assert requests_by_url[initial_url].headers.get("Authorization") == authorization - expected = authorization if retained else None - assert requests_by_url[redirect_url].headers.get("Authorization") == expected + headers = ("Authorization", "X-API-Key") + assert [requests_by_url[initial_url].headers.get(h) for h in headers] == [authorization, api_key] + expected = [authorization, api_key] if retained else [None, None] + assert [requests_by_url[redirect_url].headers.get(h) for h in headers] == expected # Returning to the initial host must not restore credentials removed earlier. - assert requests_by_url[final_url].headers.get("Authorization") == expected + assert [requests_by_url[final_url].headers.get(h) for h in headers] == expected netrc.assert_not_called() diff --git a/tests/test_import_openapi.py b/tests/test_import_openapi.py index 082b55b65..0eea54e5e 100644 --- a/tests/test_import_openapi.py +++ b/tests/test_import_openapi.py @@ -60,6 +60,78 @@ def test_select_operation_by_id_or_path(operation): ] +@pytest.mark.parametrize("operation", ["searchOrders", "POST /orders/search", "post /orders/search"]) +def test_post_operation(operation): + result = import_openapi(ORDERS, operation) + + assert result.schema_[0].name == "searchOrders" + server = result.servers[0] + assert server.location == "https://eu.api.example.com/v1/orders/search" + assert [(p.property, p.value) for p in server.customProperties] == [ + ("method", "POST"), + ("params", {"limit": "${limit:-100}"}), + ("body", {"status": "shipped"}), + ] + + +def test_bare_path_selects_no_post_operation(): + """A POST operation may change data, so it is only selected by its operationId or method and path.""" + with pytest.raises(DataContractException) as e: + import_openapi(ORDERS, "/orders/search") + + assert "No GET or POST operation '/orders/search'" in e.value.reason + + +@pytest.mark.parametrize( + "media_type, body", + [ + pytest.param( + {"example": {"q": "a"}, "examples": {"b": {"value": {"q": "b"}}}, "schema": {"example": {"q": "c"}}}, + {"q": "a"}, + id="media-type-example", + ), + pytest.param( + {"examples": {"b": {"$ref": "#/components/examples/B"}}, "schema": {"example": {"q": "c"}}}, + {"q": "b"}, + id="first-named-example", + ), + pytest.param({"schema": {"$ref": "#/components/schemas/Query"}}, {"q": "c"}, id="schema-example"), + ], +) +def test_post_body_from_request_body_example(tmp_path: Path, media_type, body): + operation = get({"type": "object", "properties": {"id": {"type": "integer"}}})["get"] + operation["requestBody"] = {"content": {"application/json": media_type}} + source = write_spec( + tmp_path, + { + "servers": [{"url": "https://api.example.com"}], + "paths": {"/search": {"post": operation}}, + "components": { + "examples": {"B": {"value": {"q": "b"}}}, + "schemas": {"Query": {"type": "object", "example": {"q": "c"}}}, + }, + }, + ) + + result = import_openapi(source, "POST /search") + + assert {p.property: p.value for p in result.servers[0].customProperties}["body"] == body + + +def test_post_without_body_example_warns(tmp_path: Path, caplog): + operation = get({"type": "object", "properties": {"id": {"type": "integer"}}})["get"] + operation["requestBody"] = {"required": True, "content": {"application/json": {"schema": {"type": "object"}}}} + source = write_spec( + tmp_path, {"servers": [{"url": "https://api.example.com"}], "paths": {"/search": {"post": operation}}} + ) + + with caplog.at_level(logging.WARNING): + result = import_openapi(source, "POST /search") + + assert [p.property for p in result.servers[0].customProperties] == ["method"] + assert "The request body of POST /search has no JSON example" in caplog.text + + def test_skips_relative_server(caplog): with caplog.at_level(logging.WARNING): import_openapi(ORDERS, "listOrders") @@ -111,14 +183,17 @@ def test_several_get_operations_need_a_selection(): with pytest.raises(DataContractException) as e: import_openapi(ORDERS) - assert "listOrders (GET /orders), getOrder (GET /orders/{orderId}), /health" in e.value.reason + assert ( + "listOrders (GET /orders), createOrder (POST /orders), searchOrders (POST /orders/search), " + "getOrder (GET /orders/{orderId}), GET /health" in e.value.reason + ) def test_unknown_operation(): with pytest.raises(DataContractException) as e: - import_openapi(ORDERS, "createOrder") + import_openapi(ORDERS, "deleteOrder") - assert "No GET operation 'createOrder'" in e.value.reason + assert "No GET or POST operation 'deleteOrder'" in e.value.reason def test_operation_without_json_or_yaml_response(): diff --git a/tests/test_test_api.py b/tests/test_test_api.py index be020a24c..4aa17c05b 100644 --- a/tests/test_test_api.py +++ b/tests/test_test_api.py @@ -1,9 +1,16 @@ import json import logging +import socket from pathlib import Path from unittest.mock import MagicMock, patch +import pytest +import yaml +from urllib3.exceptions import NewConnectionError + +from datacontract.config import Config from datacontract.data_contract import DataContract +from datacontract.engines.http_api import _is_public, _PublicHTTPSConnection logging.basicConfig(level=logging.DEBUG, force=True) @@ -44,7 +51,7 @@ def mock_get(*args, **kwargs): return mock_response -@patch("datacontract.engines.data_contract_test.requests.get", side_effect=mock_get) +@patch("datacontract.engines.http_api.ApiSession.request", side_effect=mock_get) def test_test_api(_mock_get): with open(datacontract) as data_contract_file: data_contract_str = data_contract_file.read() @@ -70,7 +77,7 @@ def mock_get_array(*args, **kwargs): return mock_response -@patch("datacontract.engines.data_contract_test.requests.get", side_effect=mock_get_array) +@patch("datacontract.engines.http_api.ApiSession.request", side_effect=mock_get_array) def test_test_api_array_delimiter(_mock_get): with open(datacontract_array) as data_contract_file: data_contract_str = data_contract_file.read() @@ -81,3 +88,156 @@ def test_test_api_array_delimiter(_mock_get): print(run) assert run.result == "passed" assert all(check.result == "passed" for check in run.checks) + + +def search_contract(api, custom_properties: dict, location_query: str = "", **kwargs) -> DataContract: + """A contract for the local API's search endpoint, its request shaped by the custom properties.""" + contract = { + "apiVersion": "v3.2.0", + "kind": "DataContract", + "id": "orders", + "version": "1.0.0", + "status": "active", + "servers": [ + { + "server": "search", + "type": "api", + "location": f"{api.url}/orders/search{location_query}", + "customProperties": [{"property": k, "value": v} for k, v in custom_properties.items()], + } + ], + "schema": [{"name": "orders", "properties": [{"name": "order_id", "logicalType": "string", "required": True}]}], + } + return DataContract(data_contract_str=yaml.safe_dump(contract, sort_keys=False), **kwargs) + + +def test_post_request(api, monkeypatch): + api.body = json.dumps([{"order_id": "ORD-1"}]) + monkeypatch.setenv("SIZE", "5") + contract = search_contract( + api, + { + "method": "post", + "params": {"limit": "${LIMIT:-100}", "status": ["placed", "shipped"], "archived": False}, + "body": {"query": {"customer": "${CUSTOMER:-C-1}"}, "size": "${SIZE}"}, + }, + location_query="?tenant=eu", + ) + + run = contract.test() + + assert run.result == "passed", run.pretty() + assert api.methods == ["POST"] + assert api.paths == ["/orders/search?tenant=eu&limit=100&status=placed&status=shipped&archived=false"] + assert api.headers[0]["Content-Type"] == "application/json" + # a variable stays a string, such as a numeric customer or zip code + assert json.loads(api.request_bodies[0]) == {"query": {"customer": "C-1"}, "size": "5"} + + +def test_post_request_with_string_body(api, monkeypatch): + api.body = json.dumps([{"order_id": "ORD-1"}]) + monkeypatch.setenv("SIZE", "5") + body = '{"customer": "Müller €", "size": ${SIZE}}' + + run = search_contract(api, {"method": "POST", "body": body}).test() + + assert run.result == "passed", run.pretty() + assert api.headers[0]["Content-Type"] == "application/json" + assert json.loads(api.request_bodies[0]) == {"customer": "Müller €", "size": 5} + + +def test_credential_headers(api): + api.body = json.dumps([{"order_id": "ORD-1"}]) + config = Config(api_header_authorization="Bearer token", api_header_x_api_key="key") + + run = search_contract(api, {}, config=config).test() + + assert run.result == "passed", run.pretty() + assert api.methods == ["GET"] + assert (api.headers[0]["Authorization"], api.headers[0]["X-API-Key"]) == ("Bearer token", "key") + + +def test_error_response_is_explained(api): + api.status = 400 + api.body = '{"error": "unknown field size"}' + + run = search_contract(api, {}).test() + + assert run.result == "error" + assert any("400" in c.reason and "unknown field size" in c.reason for c in run.checks), run.pretty() + + +@pytest.mark.parametrize("dry_run", [False, True]) +@pytest.mark.parametrize( + "custom_properties, location_query, reason", + [ + ({"method": "PUT"}, "", "Method 'PUT' is not supported; use GET or POST."), + ({"body": {"q": "a"}}, "", "A body requires method POST."), + ({"method": "GET", "body": {"q": "a"}}, "", "A body requires method POST."), + ({"params": ["limit"]}, "", "The params custom property must map each query parameter name to its value."), + ({"params": {"filter": {"status": "a"}}}, "", "Query parameter 'filter' must be a string, number, or boolean"), + ( + {"params": {"limit": 10}}, + "?limit=100", + "Query parameter 'limit' is set both in the location and in the params custom property.", + ), + ], +) +def test_invalid_request(api, dry_run, custom_properties, location_query, reason): + run = search_contract(api, custom_properties, location_query, dry_run=dry_run).test() + + assert run.result == "failed" + assert any(c.name == "Build the API request" and c.reason.startswith(reason) for c in run.checks), run.pretty() + assert api.paths == [] + + +def test_untrusted_contract_stays_off_private_networks(api): + api.body = json.dumps([{"order_id": "ORD-1"}]) + run = search_contract(api, {"method": "POST", "body": {"q": "a"}}, untrusted_contract=True).test() + + assert run.result == "error" + assert any( + c.reason.startswith( + f"Failed to fetch API response from {api.url}/orders/search: Refused to connect to 127.0.0.1:" + ) + for c in run.checks + ), run.pretty() + assert api.paths == [] + + run = search_contract(api, {}, untrusted_contract=True, allow_private_network=True).test() + assert run.result == "passed", run.pretty() + + +def test_https_connection_stays_off_private_networks(api): + """The address is checked before the TLS handshake, so a plain HTTP server stands in for an HTTPS one.""" + port = int(api.url.rsplit(":", 1)[1]) + + with pytest.raises(NewConnectionError, match="Refused to connect to 127.0.0.1"): + _PublicHTTPSConnection("127.0.0.1", port)._new_conn() + + +def test_private_address_refused_before_connecting(): + """A closed port answers like an open one, and a hostname's private address is not disclosed.""" + with socket.socket() as probe: + probe.bind(("127.0.0.1", 0)) + closed_port = probe.getsockname()[1] + + with pytest.raises(NewConnectionError, match="Refused to connect to localhost:"): + _PublicHTTPSConnection("localhost", closed_port)._new_conn() + + +@pytest.mark.parametrize( + "address, public", + [ + ("93.184.215.14", True), + ("2606:2800:21f:cb07:6820:80da:af6b:8b2c", True), + ("::ffff:127.0.0.1", False), + ("64:ff9b::a9fe:a9fe", False), + ("64:ff9b::5db8:d70e", True), + ("::127.0.0.1", False), + ("::ffff:0:7f00:1", False), + ("::1", False), + ], +) +def test_embedded_ipv4_address_is_checked(address, public): + assert _is_public(address) is public diff --git a/tests/test_test_api_openapi_import.py b/tests/test_test_api_openapi_import.py index c3b5269e8..533ceddab 100644 --- a/tests/test_test_api_openapi_import.py +++ b/tests/test_test_api_openapi_import.py @@ -1,9 +1,6 @@ -"""`datacontract test` against a GET endpoint, with the contract imported from its OpenAPI document.""" +"""`datacontract test` against a GET or POST endpoint, with the contract imported from its OpenAPI document.""" import json -import threading -from dataclasses import dataclass, field -from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer import pytest @@ -41,46 +38,6 @@ ] -@dataclass -class Api: - """What the local API answers, and the paths it was asked for. - - Attributes rather than one dict: what a request records must never end up in - the headers of a response. - """ - - url: str = "" - body: str = "" - content_type: str = "application/json" - paths: list = field(default_factory=list) - - -@pytest.fixture -def api(): - """A local API that answers every GET with the body and content type set on it, and records the paths.""" - api = Api() - - class Handler(BaseHTTPRequestHandler): - def do_GET(self): - api.paths.append(self.path) - body = api.body.encode() - self.send_response(200) - self.send_header("Content-Type", api.content_type) - self.send_header("Content-Length", str(len(body))) - self.end_headers() - self.wfile.write(body) - - def log_message(self, *args): - pass - - server = ThreadingHTTPServer(("127.0.0.1", 0), Handler) - threading.Thread(target=server.serve_forever, daemon=True).start() - api.url = f"http://127.0.0.1:{server.server_port}" - yield api - server.shutdown() - server.server_close() - - def contract_for(api, operation): """The imported contract, its production server pointed at the local API.""" contract = DataContract.import_from_source("openapi", ORDERS, openapi_operation=operation)