From 9bcb60a316804850afeaea4f2553af1360c381d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Brunat?= Date: Tue, 18 Aug 2026 12:29:50 +0200 Subject: [PATCH 1/4] fix: address the security and correctness audit (#3) Rebuilds authentication, response handling and packaging around the audit findings, and adds the test suite and CI the repository was missing. Security: - Sign every OAuth request with HMAC-SHA512 over its method, URL, query and form body, with a timestamp, nonce and version. The previous header was static and replayable. PLAINTEXT stays as an explicit compatibility mode. - Redact secrets in the repr() of every credential dataclass. - Percent-encode path parameters, so an identifier cannot change the route. - Refuse a clear-text http:// base URL unless explicitly allowed. - Validate oauth_callback_confirmed and the callback token in the dance. - Truncate response bodies carried by exceptions. Correctness: - Handle body-less successful responses; reject unfollowed redirections and wrap undecodable JSON in InvalidResponseError. - Stop replacing missing dates with the current time; normalize to UTC. - Order runtime versions naturally, so 10 outranks 9. - Let HTTPX derive the Content-Type from the body actually sent. - Configure TLS/mTLS through an ssl.SSLContext instead of the arguments deprecated in HTTPX 0.28. - Classify 403 as AuthorizationError and wrap transport errors. - Retry idempotent requests on transient failures, honouring Retry-After. - Cache the instance catalogue per client. Packaging and tests: - Ship the py.typed marker the Typing :: Typed classifier promised. - Add 217 tests running entirely on httpx.MockTransport, plus CI running ruff, strict mypy and pytest on Python 3.11, 3.12 and 3.13. Breaking changes are listed in CHANGELOG.md. --- .github/workflows/ci.yml | 61 +++ .gitignore | 5 + CHANGELOG.md | 94 +++++ README.md | 101 ++++- pyproject.toml | 51 ++- src/clever_cloud/__init__.py | 39 +- src/clever_cloud/auth.py | 238 ++++++++++- src/clever_cloud/client.py | 539 +++++++++++++++++------- src/clever_cloud/exceptions.py | 59 ++- src/clever_cloud/models.py | 395 +++++++++++------- src/clever_cloud/oauth_dance.py | 364 ++++++++++------ src/clever_cloud/py.typed | 0 tests/conftest.py | 77 ++++ tests/test_auth.py | 330 +++++++++++++++ tests/test_client.py | 706 ++++++++++++++++++++++++++++++++ tests/test_models.py | 274 +++++++++++++ tests/test_networkgroups.py | 267 ++++++++++++ tests/test_oauth_dance.py | 403 ++++++++++++++++++ tests/test_tls.py | 186 +++++++++ 19 files changed, 3727 insertions(+), 462 deletions(-) create mode 100644 .github/workflows/ci.yml create mode 100644 CHANGELOG.md create mode 100644 src/clever_cloud/py.typed create mode 100644 tests/conftest.py create mode 100644 tests/test_auth.py create mode 100644 tests/test_client.py create mode 100644 tests/test_models.py create mode 100644 tests/test_networkgroups.py create mode 100644 tests/test_oauth_dance.py create mode 100644 tests/test_tls.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..fbf9851 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,61 @@ +name: CI + +on: + push: + branches: [main] + pull_request: + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +jobs: + test: + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + python-version: ["3.11", "3.12", "3.13"] + steps: + - uses: actions/checkout@v4 + + - name: Install uv + uses: astral-sh/setup-uv@v5 + with: + enable-cache: true + + - name: Set up Python ${{ matrix.python-version }} + run: uv python install ${{ matrix.python-version }} + + - name: Install the project + run: uv sync --python ${{ matrix.python-version }} --extra dev + + - name: Lint + run: uv run ruff check . + + - name: Type-check + run: uv run mypy + + - name: Test + run: uv run pytest --cov --cov-report=term-missing + + build: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - name: Install uv + uses: astral-sh/setup-uv@v5 + + - name: Build wheel and sdist + run: uv build + + - name: Check the wheel ships the py.typed marker + run: | + python -m zipfile -l dist/*.whl | grep -q 'clever_cloud/py.typed' \ + || { echo "py.typed missing from the wheel"; exit 1; } + + - uses: actions/upload-artifact@v4 + with: + name: dist + path: dist/ diff --git a/.gitignore b/.gitignore index 4c325a5..05fbbac 100644 --- a/.gitignore +++ b/.gitignore @@ -19,3 +19,8 @@ venv/ # uv uv.lock + +# Coverage +.coverage +coverage.xml +htmlcov/ diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..0682026 --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,94 @@ +# Changelog + +All notable changes to this project are documented in this file. + +## 0.2.0 + +Addresses the security, correctness and design audit tracked in +[issue #3](https://github.com/CleverCloud/clevercloud-sdk-python/issues/3). + +### Security + +- **OAuth requests are now fully signed.** Every request carries + `oauth_signature_method`, `oauth_timestamp`, `oauth_nonce` and `oauth_version`, + and is signed with HMAC-SHA512 over its method, URL, query and form body. The + previous header was static and could be replayed by anyone who observed it. + `SignatureMethod.PLAINTEXT` remains available as an explicitly selected + compatibility mode, and `SignatureMethod.HMAC_SHA256` is also supported. +- **Credentials no longer appear in representations.** `ApiTokenCredentials`, + `OAuthCredentials`, `OAuthConsumer` and `RequestToken` redact their secrets in + `repr()`, including when nested in a container that a logger reprs. +- **Path parameters are percent-encoded.** Identifiers such as `../self`, + `x?override=1` or `x/y` can no longer change which endpoint a request reaches. +- **Clear-text base URLs are refused.** An `http://` base URL raises unless + `allow_insecure_http=True` is passed explicitly. +- **The OAuth dance validates its callback.** `oauth_callback_confirmed` is + checked, and the verifier is only accepted when the callback carries the very + request token this dance obtained. +- Exception messages and attributes no longer copy an entire response body; + bodies are truncated to 2 KiB. + +### Fixed + +- Successful responses with an empty body (202, 205, and 200 on some endpoints) + no longer raise `JSONDecodeError`. Undecodable JSON now raises + `InvalidResponseError`. +- Unfollowed 3xx responses are no longer treated as successful responses. +- Missing dates are no longer replaced with the current time, and every parsed + date is normalized to a timezone-aware UTC datetime. +- Runtime versions are ordered naturally, so `resolve_instance_slug()` picks + `10` over `9`. +- The JSON `Content-Type` is no longer forced onto every request; HTTPX derives + it from the body actually sent. A GET carries no `Content-Type` at all, and a + form body is correctly labelled `application/x-www-form-urlencoded`. +- TLS and mTLS are configured through an `ssl.SSLContext` instead of the HTTPX + arguments deprecated in 0.28. Per-request cookies were removed from the OAuth + dance for the same reason. +- HTTP 403 is reported as `AuthorizationError` rather than an authentication + failure. +- Transport failures are wrapped in `TransportError`, inside the + `CleverCloudError` hierarchy. + +### Added + +- Idempotent requests (GET, HEAD, OPTIONS, PUT, DELETE) retry on 429, 502, 503, + 504 and network errors, with exponential backoff, jitter and `Retry-After` + support, capped by `max_retry_wait`. Each attempt is re-signed with a fresh + nonce. Configure with `max_retries` (2 by default; `0` disables retries). +- The instance catalogue is cached per client, so repeated `instance_slug` + resolutions no longer re-download it. `list_instances(refresh=True)` forces a + new fetch. +- `NotFoundError` and `RateLimitError` (which exposes `retry_after`). +- `OAuthDance.parse_callback_url()` for the browser-based flow, and a + configurable `mfa_kind` on `login()`. +- `OAuthCredentials.expiration_date` and `is_expired()`, populated from the + access-token exchange. +- A `py.typed` marker, so the declared `Typing :: Typed` classifier is honoured. +- A test suite (217 tests, no network access) plus CI running lint, strict type + checking and tests on Python 3.11, 3.12 and 3.13. + +### Breaking changes + +- `Auth.get_authorization_header()` now takes the request method and URL, since + a signature is bound to them. Custom `Auth` subclasses must be updated. +- Response models are parsed strictly: a payload missing a required field raises + `InvalidResponseError` instead of yielding a model filled with empty strings, + zeroes or a fabricated date. Genuinely optional fields are now typed + `| None` and default to `None` rather than `""`. +- `Profile.creation_date` and `Application.creation_date` are `datetime | None`. +- `NetworkGroup.members`, `.peers` and `.tags` are tuples, and + `PeerCreated.raw` is a read-only mapping, so `frozen=True` means what it says. +- An unknown `MemberKind` is rejected instead of being coerced to `EXTERNAL`. +- `list_domains()` and `get_primary_domain()` no longer swallow HTTP 404. They + raise `NotFoundError`, because the API reports "no such application" and "no + domain" with the same status; the caller decides how to treat it. +- HTTP 403 raises `AuthorizationError`, which is *not* a subclass of + `AuthenticationError`. Code catching `AuthenticationError` for 403 must be + updated. +- Redirections raise `InvalidResponseError` instead of returning the redirect + body. +- `httpx>=0.28` is now required. + +## 0.1.0 + +- Initial public release. diff --git a/README.md b/README.md index 62476e4..91c5ebb 100644 --- a/README.md +++ b/README.md @@ -37,9 +37,74 @@ async with CleverCloudClient(credentials) as client: ... ``` +Every OAuth request is signed with HMAC-SHA512 over its method, URL, query +string and form body, with a timestamp, a nonce and the OAuth version, so an +intercepted `Authorization` header cannot be replayed. To talk to a deployment +that still requires the legacy format, select the compatibility mode explicitly: + +```python +from clever_cloud import SignatureMethod + +credentials = OAuthCredentials(..., signature_method=SignatureMethod.PLAINTEXT) +``` + +### Obtaining OAuth credentials + +The browser flow is the supported way to obtain credentials: + +```python +import webbrowser +from clever_cloud import OAuthConsumer, OAuthDance + +with OAuthDance(OAuthConsumer(key="...", secret="..."), + callback_url="https://my-app.example/callback") as dance: + request_token = dance.get_request_token() + webbrowser.open(dance.get_authorization_url(request_token)) + + # ... your callback receives the redirect; pass its full URL back: + verifier = dance.parse_callback_url(callback_url, request_token) + credentials = dance.get_access_token(request_token, verifier) +``` + +`parse_callback_url()` checks that the callback carries the token this dance +requested before accepting the verifier. `OAuthDance.login()` remains available +for browser-less automation, but it drives the console's internal session +endpoints with the account password and is not a supported OAuth flow. + +### Errors + +All errors derive from `CleverCloudError`: + +| Exception | Raised when | +| --- | --- | +| `AuthenticationError` | HTTP 401: credentials missing or invalid | +| `AuthorizationError` | HTTP 403: credentials valid, access denied | +| `NotFoundError` | HTTP 404 | +| `RateLimitError` | HTTP 429, exposes `retry_after` | +| `HttpError` | Any other HTTP error status | +| `TransportError` | Network, timeout or TLS failure | +| `InvalidResponseError` | Undecodable body, unexpected redirect, or a payload that does not match the endpoint's contract | +| `OAuthError` | Failure during the OAuth dance, with its `step` | + +Response bodies attached to exceptions are truncated, so a large or sensitive +error payload does not end up whole in your logs. + +### Retries + +Idempotent requests (GET, HEAD, OPTIONS, PUT, DELETE) are retried on HTTP 429, +502, 503, 504 and on network errors, using exponential backoff with jitter and +honouring `Retry-After`. Each attempt is signed again with a fresh nonce. + +```python +async with CleverCloudClient(credentials, max_retries=0) as client: # opt out + ... +``` + ### Custom CA bundle and mTLS -The client accepts a custom CA bundle and a client certificate for mutual TLS, useful when targeting an API behind a private PKI or requiring client authentication: +The client accepts a custom CA bundle and a client certificate for mutual TLS, +useful when targeting an API behind a private PKI or requiring client +authentication: ```python async with CleverCloudClient( @@ -50,21 +115,36 @@ async with CleverCloudClient( ... ``` -`verify_ssl=False` disables server certificate verification entirely (not recommended outside of local testing). +Both are loaded into an `ssl.SSLContext`, so no deprecated HTTPX argument is +used. `verify_ssl=False` disables server certificate verification entirely (not +recommended outside of local testing). + +A clear-text `http://` base URL is refused by default, because credentials +would travel unencrypted; pass `allow_insecure_http=True` to override it against +a local development server. + +### Response models + +Models are parsed strictly: a response missing a field the endpoint is +documented to return raises `InvalidResponseError` rather than producing a model +filled with empty strings, zeroes or a fabricated timestamp. Optional fields are +typed `| None`, dates are timezone-aware UTC datetimes, and collections are +tuples, so `frozen=True` models are immutable all the way down. ## Available features -This SDK is still a work in progress, but it already provides the following features: +This SDK is still a work in progress, but it already provides the following +features: - Get user profile -- List instance types +- List instance types (cached per client) - Create application - Redeploy application - Create TCP redirection - List domains - Get primary domain - Custom CA bundle and mTLS client certificate support -- Tolerant response handling (non-JSON bodies, relaxed `Accept` header) +- Automatic retries with backoff on transient failures - NetworkGroups: create / get / delete / search, manage members, peers and external peers ### NetworkGroups example @@ -84,6 +164,17 @@ await client.create_networkgroup_member( ) ``` +## Development + +```bash +uv sync --extra dev +uv run pytest # test suite, no network access +uv run ruff check . # lint +uv run mypy # strict type checking +``` + +See [CHANGELOG.md](CHANGELOG.md) for release notes, including breaking changes. + ## License Apache 2.0 - See [LICENSE](LICENSE) for details. diff --git a/pyproject.toml b/pyproject.toml index ac5a826..fdc0e66 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "clevercloud-sdk" -version = "0.1.0" +version = "0.2.0" description = "Python SDK for Clever Cloud" readme = "README.md" license = "Apache-2.0" @@ -26,7 +26,16 @@ classifiers = [ "Typing :: Typed", ] dependencies = [ - "httpx>=0.25.0", + "httpx>=0.28.0", +] + +[project.optional-dependencies] +dev = [ + "pytest>=8.0", + "pytest-asyncio>=0.24", + "pytest-cov>=5.0", + "mypy>=1.11", + "ruff>=0.6", ] [project.urls] @@ -38,7 +47,45 @@ Issues = "https://github.com/CleverCloud/clevercloud-sdk-python/issues" [tool.hatch.build.targets.sdist] include = [ "/src", + "/tests", ] [tool.hatch.build.targets.wheel] packages = ["src/clever_cloud"] + +[tool.pytest.ini_options] +testpaths = ["tests"] +asyncio_mode = "auto" +addopts = "-q --strict-markers" + +[tool.coverage.run] +source = ["clever_cloud"] + +[tool.ruff] +line-length = 100 +target-version = "py311" +src = ["src", "tests"] + +[tool.ruff.lint] +select = [ + "E", "F", "W", # pycodestyle / pyflakes + "I", # isort + "B", # bugbear + "UP", # pyupgrade + "S", # bandit + "RUF", +] +ignore = [ + "S101", # assert is expected in tests + "UP042", # str+Enum is kept over StrEnum: str() of a member must not change +] + +[tool.ruff.lint.per-file-ignores] +"tests/*" = ["S105", "S106"] # hardcoded fake credentials in fixtures + +[tool.mypy] +python_version = "3.11" +strict = true +files = ["src", "tests"] +warn_unused_ignores = true +disallow_any_explicit = false diff --git a/src/clever_cloud/__init__.py b/src/clever_cloud/__init__.py index 7bfbeb2..c059b49 100644 --- a/src/clever_cloud/__init__.py +++ b/src/clever_cloud/__init__.py @@ -18,13 +18,23 @@ app = await client.create_application(owner_id="...", name="my-app", instance_slug="node") """ -from clever_cloud.auth import ApiTokenCredentials, OAuthCredentials +from clever_cloud.auth import ( + ApiTokenCredentials, + Auth, + OAuthCredentials, + SignatureMethod, +) from clever_cloud.client import CleverCloudClient from clever_cloud.exceptions import ( AuthenticationError, + AuthorizationError, CleverCloudError, HttpError, + InvalidResponseError, + NotFoundError, OAuthError, + RateLimitError, + TransportError, ) from clever_cloud.models import ( Application, @@ -42,29 +52,36 @@ ) from clever_cloud.oauth_dance import OAuthConsumer, OAuthDance, RequestToken -__version__ = "0.1.0" +__version__ = "0.2.0" __all__ = [ - "CleverCloudClient", - "OAuthCredentials", "ApiTokenCredentials", "Application", + "Auth", + "AuthenticationError", + "AuthorizationError", + "CleverCloudClient", + "CleverCloudError", "Domain", + "HttpError", + "InvalidResponseError", "MemberKind", "NetworkGroup", "NetworkGroupMember", "NetworkGroupPeer", + "NotFoundError", + "OAuthConsumer", + "OAuthCredentials", + "OAuthDance", + "OAuthError", "PeerCreated", "PeerKind", "PeerRole", "Profile", + "RateLimitError", + "RequestToken", + "SignatureMethod", "TcpRedirection", + "TransportError", "WireguardEndpoint", - "OAuthDance", - "OAuthConsumer", - "RequestToken", - "CleverCloudError", - "AuthenticationError", - "HttpError", - "OAuthError", ] diff --git a/src/clever_cloud/auth.py b/src/clever_cloud/auth.py index 74dc183..77b912c 100644 --- a/src/clever_cloud/auth.py +++ b/src/clever_cloud/auth.py @@ -1,58 +1,258 @@ -"""Authentication: OAuth v1 PLAINTEXT and API Token.""" +"""Authentication strategies: OAuth 1.0a signed requests and API Token.""" +from __future__ import annotations + +import base64 +import hashlib +import hmac +import secrets +import time from abc import ABC, abstractmethod -from dataclasses import dataclass -from urllib.parse import quote +from collections.abc import Iterable, Mapping, Sequence +from dataclasses import dataclass, field +from datetime import UTC, datetime +from enum import Enum +from urllib.parse import parse_qsl, quote, urlsplit import httpx +OAUTH_VERSION = "1.0" + +_DEFAULT_PORTS = {"http": 80, "https": 443} +_FORM_CONTENT_TYPE = "application/x-www-form-urlencoded" + + +class SignatureMethod(str, Enum): + """OAuth 1.0a signature methods supported by this SDK. + + ``HMAC-SHA512`` is the method recommended by Clever Cloud for production. + ``PLAINTEXT`` is a legacy compatibility mode: it sends the secrets in the + header and produces a static, replayable ``Authorization`` value, so it must + be selected explicitly. + """ + + HMAC_SHA512 = "HMAC-SHA512" + HMAC_SHA256 = "HMAC-SHA256" + PLAINTEXT = "PLAINTEXT" + + +HASH_ALGORITHMS = { + SignatureMethod.HMAC_SHA512: hashlib.sha512, + SignatureMethod.HMAC_SHA256: hashlib.sha256, +} + + +def percent_encode(value: str) -> str: + """Percent-encode a value per RFC 5849 §3.6 (only unreserved chars survive).""" + return quote(str(value), safe="~") + + +def normalize_url(url: str) -> str: + """Return the signature base URL: no query, no fragment, no default port.""" + parts = urlsplit(url) + scheme = parts.scheme.lower() + host = (parts.hostname or "").lower() + port = parts.port + netloc = host + if port is not None and port != _DEFAULT_PORTS.get(scheme): + netloc = f"{host}:{port}" + path = parts.path or "/" + return f"{scheme}://{netloc}{path}" + + +def normalize_parameters(params: Iterable[tuple[str, str]]) -> str: + """Encode, sort and join request parameters per RFC 5849 §3.4.1.3.2.""" + encoded = sorted( + (percent_encode(key), percent_encode(value)) for key, value in params + ) + return "&".join(f"{key}={value}" for key, value in encoded) + + +def build_signature_base_string( + method: str, + url: str, + params: Iterable[tuple[str, str]], +) -> str: + """Build the OAuth 1.0a signature base string (RFC 5849 §3.4.1.1).""" + return "&".join( + ( + method.upper(), + percent_encode(normalize_url(url)), + percent_encode(normalize_parameters(params)), + ) + ) + + +def _signing_key(consumer_secret: str, token_secret: str) -> str: + return f"{percent_encode(consumer_secret)}&{percent_encode(token_secret)}" + + +def redact(value: str | None) -> str: + """Represent a secret without disclosing it, keeping length as a hint.""" + if value is None: + return "None" + return f"'***redacted ({len(value)} chars)***'" if value else "''" + class Auth(ABC): """Base class for authentication strategies.""" @abstractmethod - def get_authorization_header(self) -> str: ... + def get_authorization_header( + self, + method: str, + url: str, + *, + body_params: Sequence[tuple[str, str]] = (), + ) -> str: + """Build the ``Authorization`` header value for one specific request. + + Args: + method: HTTP method, e.g. ``"GET"``. + url: Absolute request URL, including its query string. + body_params: Form-encoded body parameters, which take part in the + OAuth signature when the body is ``x-www-form-urlencoded``. + """ @abstractmethod - def get_base_url(self) -> str: ... + def get_base_url(self) -> str: + """Return the default API base URL for this credential kind.""" def apply_to_request(self, request: httpx.Request) -> httpx.Request: - request.headers["Authorization"] = self.get_authorization_header() + """Sign ``request`` in place and return it.""" + request.headers["Authorization"] = self.get_authorization_header( + request.method, + str(request.url), + body_params=_form_body_params(request), + ) return request -@dataclass(frozen=True, slots=True) +def _form_body_params(request: httpx.Request) -> list[tuple[str, str]]: + """Extract form-encoded body parameters, which must be signed.""" + content_type = request.headers.get("content-type", "") + if not content_type.startswith(_FORM_CONTENT_TYPE): + return [] + try: + body = request.content.decode("utf-8") + except UnicodeDecodeError: + return [] + return parse_qsl(body, keep_blank_values=True) + + +@dataclass(frozen=True, slots=True, repr=False) class OAuthCredentials(Auth): - """OAuth v1 PLAINTEXT credentials (4 tokens from OAuth dance or clever-tools).""" + """OAuth 1.0a credentials (4 tokens from the OAuth dance or clever-tools). + + Every request is signed with HMAC-SHA512 by default, including a timestamp, + a nonce and the OAuth version, so the ``Authorization`` header cannot be + replayed. Select ``SignatureMethod.PLAINTEXT`` only to talk to a server that + requires the legacy format. + """ consumer_key: str - consumer_secret: str + consumer_secret: str = field(repr=False) token: str - secret: str + secret: str = field(repr=False) base_url: str | None = None + signature_method: SignatureMethod = SignatureMethod.HMAC_SHA512 + expiration_date: datetime | None = None + """When the access token expires, as reported by the API, if it says so.""" + + def __repr__(self) -> str: + return ( + f"{type(self).__name__}(" + f"consumer_key={self.consumer_key!r}, " + f"consumer_secret={redact(self.consumer_secret)}, " + f"token={self.token!r}, " + f"secret={redact(self.secret)}, " + f"base_url={self.base_url!r}, " + f"signature_method={self.signature_method!r}, " + f"expiration_date={self.expiration_date!r})" + ) - def get_authorization_header(self) -> str: - """OAuth header with PLAINTEXT signature: consumerSecret%26tokenSecret.""" - signature = f"{quote(self.consumer_secret, safe='')}%26{quote(self.secret, safe='')}" + def get_authorization_header( + self, + method: str, + url: str, + *, + body_params: Sequence[tuple[str, str]] = (), + timestamp: int | None = None, + nonce: str | None = None, + ) -> str: + """Build a fully signed OAuth 1.0a ``Authorization`` header. + + ``timestamp`` and ``nonce`` are generated per call; they are only + accepted as arguments to make signatures reproducible in tests. + """ + oauth_params = { + "oauth_consumer_key": self.consumer_key, + "oauth_token": self.token, + "oauth_signature_method": self.signature_method.value, + "oauth_timestamp": str(timestamp if timestamp is not None else int(time.time())), + "oauth_nonce": nonce or secrets.token_hex(16), + "oauth_version": OAUTH_VERSION, + } + signature = self._sign(method, url, oauth_params, body_params) parts = [ - f'oauth_consumer_key="{self.consumer_key}"', - f'oauth_token="{self.token}"', - f'oauth_signature="{signature}"', + f'{percent_encode(key)}="{percent_encode(value)}"' + for key, value in sorted({**oauth_params, "oauth_signature": signature}.items()) ] return f"OAuth {', '.join(parts)}" + def _sign( + self, + method: str, + url: str, + oauth_params: Mapping[str, str], + body_params: Sequence[tuple[str, str]], + ) -> str: + key = _signing_key(self.consumer_secret, self.secret) + if self.signature_method is SignatureMethod.PLAINTEXT: + return key + params: list[tuple[str, str]] = [ + *parse_qsl(urlsplit(url).query, keep_blank_values=True), + *body_params, + *oauth_params.items(), + ] + base_string = build_signature_base_string(method, url, params) + digest = hmac.new( + key.encode("utf-8"), + base_string.encode("utf-8"), + HASH_ALGORITHMS[self.signature_method], + ).digest() + return base64.b64encode(digest).decode("ascii") + + def is_expired(self, *, now: datetime | None = None) -> bool: + """Whether the access token is past its expiration date, if known.""" + if self.expiration_date is None: + return False + return (now or datetime.now(tz=UTC)) >= self.expiration_date + def get_base_url(self) -> str: return self.base_url or "https://api.clever-cloud.com" -@dataclass(frozen=True, slots=True) +@dataclass(frozen=True, slots=True, repr=False) class ApiTokenCredentials(Auth): """API Token for the Clever Cloud API Bridge.""" - token: str + token: str = field(repr=False) base_url: str | None = None - def get_authorization_header(self) -> str: + def __repr__(self) -> str: + return ( + f"{type(self).__name__}(" + f"token={redact(self.token)}, base_url={self.base_url!r})" + ) + + def get_authorization_header( + self, + method: str = "", + url: str = "", + *, + body_params: Sequence[tuple[str, str]] = (), + ) -> str: return f"Bearer {self.token}" def get_base_url(self) -> str: diff --git a/src/clever_cloud/client.py b/src/clever_cloud/client.py index a056005..79fac35 100644 --- a/src/clever_cloud/client.py +++ b/src/clever_cloud/client.py @@ -1,11 +1,28 @@ """Clever Cloud async API client.""" +from __future__ import annotations + +import asyncio +import email.utils +import random +import re +import ssl +from datetime import UTC, datetime from typing import Any, Self +from urllib.parse import quote, urlsplit import httpx from clever_cloud.auth import Auth -from clever_cloud.exceptions import AuthenticationError, HttpError +from clever_cloud.exceptions import ( + AuthenticationError, + AuthorizationError, + HttpError, + InvalidResponseError, + NotFoundError, + RateLimitError, + TransportError, +) from clever_cloud.models import ( Application, Domain, @@ -19,6 +36,70 @@ TcpRedirection, ) +#: Methods that can safely be retried after a transient failure. +IDEMPOTENT_METHODS = frozenset({"GET", "HEAD", "OPTIONS", "PUT", "DELETE"}) + +#: Status codes worth retrying for an idempotent request. +RETRYABLE_STATUS_CODES = frozenset({429, 502, 503, 504}) + +_VERSION_PART = re.compile(r"(\d+)|([^\d]+)") + +# Sorts above any run, so a bare version outranks the same version with a suffix. +_VERSION_END: tuple[int, str] = (2, "") + + +def encode_path_segment(value: object, *, name: str) -> str: + """Percent-encode one path segment so it cannot alter the request route. + + Identifiers received from callers are interpolated into URLs; without + encoding, values such as ``../self``, ``x?override=1`` or ``x/y`` would + change which endpoint is reached on the authenticated host. + + Raises: + ValueError: If the value is empty or is a relative path element. + """ + if not isinstance(value, str) or not value: + msg = f"{name} must be a non-empty string, got {value!r}" + raise ValueError(msg) + if value in {".", ".."}: + msg = f"{name} must not be a relative path element, got {value!r}" + raise ValueError(msg) + return quote(value, safe="") + + +def _version_sort_key(version: object) -> tuple[tuple[int, int | str], ...]: + """Order versions naturally, so ``10`` sorts above ``9``. + + Splits a version into numeric and non-numeric runs; numeric runs compare as + integers, so ``10`` sorts above ``9`` where a string comparison did not. A + trailing end-marker sorts above any run, which keeps a pre-release suffix + such as ``1.0-beta`` below the bare ``1.0``. + """ + text = str(version) + key: list[tuple[int, int | str]] = [] + for digits, rest in _VERSION_PART.findall(text): + key.append((1, int(digits)) if digits else (0, rest)) + key.append(_VERSION_END) + return tuple(key) + + +def _retry_after_seconds(response: httpx.Response) -> float | None: + """Parse a ``Retry-After`` header, in delta-seconds or HTTP-date form.""" + raw = response.headers.get("retry-after") + if not raw: + return None + raw = raw.strip() + try: + return max(0.0, float(raw)) + except ValueError: + pass + parsed = email.utils.parsedate_to_datetime(raw) + if parsed is None: + return None + if parsed.tzinfo is None: + parsed = parsed.replace(tzinfo=UTC) + return max(0.0, (parsed - datetime.now(tz=UTC)).total_seconds()) + class CleverCloudClient: """Async client for the Clever Cloud API. @@ -43,39 +124,93 @@ def __init__( ca_bundle: str | None = None, client_cert: str | tuple[str, str] | tuple[str, str, str] | None = None, verify_ssl: bool = True, + allow_insecure_http: bool = False, + max_retries: int = 2, + max_retry_wait: float = 30.0, + transport: httpx.AsyncBaseTransport | None = None, ) -> None: + """Create a client. + + Args: + auth: Credentials used to sign every request. + base_url: Override the API base URL. Must be HTTPS unless + ``allow_insecure_http`` is set. + timeout: Per-request timeout, in seconds. + ca_bundle: Path to a CA bundle used to verify the server certificate. + client_cert: Client certificate for mTLS: a path, ``(cert, key)`` or + ``(cert, key, password)``. + verify_ssl: Set to ``False`` to disable certificate verification + entirely (local testing only). + allow_insecure_http: Allow a clear-text ``http://`` base URL. Off by + default, because credentials would travel unencrypted. + max_retries: Extra attempts for idempotent requests hitting a + transient failure (429/502/503/504 or a network error). + max_retry_wait: Upper bound, in seconds, for a single backoff wait. + transport: Custom transport, mainly useful for tests. + """ self._auth = auth self._base_url = base_url or auth.get_base_url() + self._validate_base_url(allow_insecure_http=allow_insecure_http) self._timeout = timeout self._ca_bundle = ca_bundle self._client_cert = client_cert self._verify_ssl = verify_ssl + self._max_retries = max(0, max_retries) + self._max_retry_wait = max_retry_wait + self._transport = transport self._client: httpx.AsyncClient | None = None + self._instances_cache: list[dict[str, Any]] | None = None + + def _validate_base_url(self, *, allow_insecure_http: bool) -> None: + scheme = urlsplit(self._base_url).scheme.lower() + if scheme == "https": + return + if scheme == "http" and allow_insecure_http: + return + msg = ( + f"Refusing to use base URL {self._base_url!r}: credentials must be sent " + "over HTTPS. Pass allow_insecure_http=True to override in development." + ) + raise ValueError(msg) - def _get_client(self) -> httpx.AsyncClient: - if self._client is None: - # Prepare SSL verification - # - ca_bundle: path to custom CA bundle file to verify server certificate - # - verify_ssl: True (default CA), False (disable verification) - verify: bool | str - if self._ca_bundle: - verify = self._ca_bundle - else: - verify = self._verify_ssl + def _build_ssl_context(self) -> ssl.SSLContext | bool: + """Build the TLS configuration as an :class:`ssl.SSLContext`. - # Prepare client certificate for mTLS - # - client_cert: path to cert file, or (cert, key), or (cert, key, password) - cert = self._client_cert + HTTPX deprecated passing file paths to ``verify=`` and the ``cert=`` + argument, so certificates are loaded into an explicit context instead. + """ + if not self._verify_ssl: + if self._client_cert is None: + return False + context = ssl.create_default_context() + context.check_hostname = False + context.verify_mode = ssl.CERT_NONE + else: + context = ssl.create_default_context(cafile=self._ca_bundle) + + cert = self._client_cert + if cert is not None: + if isinstance(cert, str): + context.load_cert_chain(certfile=cert) + elif len(cert) == 2: + context.load_cert_chain(certfile=cert[0], keyfile=cert[1]) + else: + context.load_cert_chain( + certfile=cert[0], keyfile=cert[1], password=cert[2] + ) + return context + def _get_client(self) -> httpx.AsyncClient: + if self._client is None: self._client = httpx.AsyncClient( base_url=self._base_url, timeout=self._timeout, - headers={ - "Accept": "*/*", - "Content-Type": "application/json", - }, - verify=verify, - cert=cert, + # Only Accept is set globally: the Content-Type must follow the + # body actually sent, which HTTPX derives from json=/data=. + headers={"Accept": "*/*"}, + verify=self._build_ssl_context(), + transport=self._transport, + follow_redirects=False, ) return self._client @@ -92,61 +227,135 @@ async def close(self) -> None: self._client = None def _handle_response(self, response: httpx.Response) -> Any: - if response.status_code == 401: + status = response.status_code + body = response.text + + if status == 401: raise AuthenticationError( - "Authentication failed", status_code=401, response_body=response.text + "Authentication failed", status_code=status, response_body=body ) - if response.status_code == 403: - raise AuthenticationError( - "Access forbidden", status_code=403, response_body=response.text + if status == 403: + raise AuthorizationError( + "Access forbidden", status_code=status, response_body=body + ) + if status == 404: + raise NotFoundError( + "Resource not found", status_code=status, response_body=body + ) + if status == 429: + raise RateLimitError( + "Rate limited", + status_code=status, + response_body=body, + retry_after=_retry_after_seconds(response), ) - if response.status_code >= 400: + if status >= 400: raise HttpError( - f"HTTP {response.status_code}: {response.text}", - status_code=response.status_code, - response_body=response.text, + f"HTTP {status}", status_code=status, response_body=body ) - if response.status_code == 204: - return {} + if 300 <= status < 400: + # Redirections are not followed: silently treating one as success + # would return the body of a page the caller never asked for. + location = response.headers.get("location", "") + msg = f"Unexpected redirection (HTTP {status}) to {location!r}" + raise InvalidResponseError(msg, response_body=body) + + # Any successful status may come without a body (204, but also 202 for + # accepted NetworkGroup operations, or 200 on some endpoints). + if not response.content: + return None + content_type = response.headers.get("content-type", "") if "json" in content_type: - return response.json() - return response.text + try: + return response.json() + except ValueError as exc: + msg = f"Server returned an undecodable JSON body: {exc}" + raise InvalidResponseError(msg, response_body=body) from exc + return body async def _request( self, method: str, path: str, *, - json: dict[str, Any] | None = None, + json: Any = None, data: dict[str, Any] | None = None, params: dict[str, Any] | None = None, headers: dict[str, str] | None = None, ) -> Any: client = self._get_client() - request = client.build_request( - method=method, - url=path, - json=json, - data=data, - params=params, - headers=headers, - ) - request = self._auth.apply_to_request(request) - response = await client.send(request) - return self._handle_response(response) + attempts = 1 + (self._max_retries if method.upper() in IDEMPOTENT_METHODS else 0) + + last_error: Exception | None = None + for attempt in range(attempts): + # Rebuilt and re-signed on every attempt: an OAuth signature carries + # a nonce and a timestamp that must not be reused. + request = client.build_request( + method=method, + url=path, + json=json, + data=data, + params=params, + headers=headers, + ) + request = self._auth.apply_to_request(request) + + wait: float | None = None + try: + response = await client.send(request) + except httpx.TransportError as exc: + last_error = TransportError(f"{type(exc).__name__}: {exc}") + last_error.__cause__ = exc + wait = self._backoff(attempt, None) + else: + if ( + response.status_code in RETRYABLE_STATUS_CODES + and attempt < attempts - 1 + ): + wait = self._backoff(attempt, _retry_after_seconds(response)) + else: + return self._handle_response(response) + + if attempt == attempts - 1: + break + await asyncio.sleep(wait or 0.0) + + if last_error is not None: + raise last_error + # Unreachable: the loop either returns, sleeps, or records an error. + msg = "Request failed without a response" + raise TransportError(msg) + + def _backoff(self, attempt: int, retry_after: float | None) -> float: + """Exponential backoff with jitter, capped, honouring ``Retry-After``.""" + if retry_after is not None: + return min(retry_after, self._max_retry_wait) + base = min(2.0**attempt, self._max_retry_wait) + return base * (0.5 + random.random() / 2) # noqa: S311 - jitter, not crypto async def get_profile(self) -> Profile: """Get the authenticated user's profile.""" data = await self._request("GET", "/v2/self") return Profile.from_api_response(data) - async def list_instances(self) -> list[dict[str, Any]]: - """List available instance types (runtimes).""" - return await self._request("GET", "/v2/products/instances") + async def list_instances(self, *, refresh: bool = False) -> list[dict[str, Any]]: + """List available instance types (runtimes). + + The catalogue is cached for the lifetime of the client, since it changes + rarely and every ``instance_slug`` resolution would otherwise re-download + it. Pass ``refresh=True`` to force a new fetch. + """ + if self._instances_cache is None or refresh: + data = await self._request("GET", "/v2/products/instances") + if not isinstance(data, list): + msg = f"Expected a list of instances, got {type(data).__name__}" + raise InvalidResponseError(msg) + self._instances_cache = data + return self._instances_cache async def resolve_instance_slug(self, slug: str) -> tuple[str, str, str]: - """Resolve an instance slug to (type, version, variant_id). + """Resolve an instance slug to ``(type, version, variant_id)``. Args: slug: Instance slug like "static", "node", "python", etc. @@ -161,22 +370,32 @@ async def resolve_instance_slug(self, slug: str) -> tuple[str, str, str]: matching = [ i for i in instances - if i.get("enabled") and i.get("variant", {}).get("slug") == slug + if isinstance(i, dict) + and i.get("enabled") + and (i.get("variant") or {}).get("slug") == slug ] if not matching: available = sorted( { - i.get("variant", {}).get("slug") + candidate for i in instances - if i.get("enabled") + if isinstance(i, dict) and i.get("enabled") + for candidate in [(i.get("variant") or {}).get("slug")] + if isinstance(candidate, str) } ) msg = f"Unknown instance slug: {slug}. Available: {available}" raise ValueError(msg) - # Sort by version descending to get latest - matching.sort(key=lambda x: x.get("version", ""), reverse=True) + # Natural version ordering, so "10" is newer than "9". + matching.sort( + key=lambda x: _version_sort_key(x.get("version", "")), reverse=True + ) best = matching[0] - return best["type"], best["version"], best["variant"]["id"] + try: + return best["type"], best["version"], best["variant"]["id"] + except (KeyError, TypeError) as exc: + msg = f"Incomplete instance entry for slug {slug!r}: {exc}" + raise InvalidResponseError(msg) from exc async def create_application( self, @@ -222,6 +441,7 @@ async def create_application( Either instance_slug OR (instance_type, instance_version, instance_variant) must be provided. """ + owner = encode_path_segment(owner_id, name="owner_id") if instance_slug: ( instance_type, @@ -229,7 +449,10 @@ async def create_application( instance_variant, ) = await self.resolve_instance_slug(instance_slug) elif not (instance_type and instance_version and instance_variant): - msg = "Either instance_slug or (instance_type, instance_version, instance_variant) required" + msg = ( + "Either instance_slug or " + "(instance_type, instance_version, instance_variant) is required" + ) raise ValueError(msg) body: dict[str, Any] = { @@ -262,7 +485,7 @@ async def create_application( body["publicGitRepositoryUrl"] = public_git_repository_url data = await self._request( - "POST", f"/v2/organisations/{owner_id}/applications", json=body + "POST", f"/v2/organisations/{owner}/applications", json=body ) return Application.from_api_response(data) @@ -282,6 +505,8 @@ async def redeploy_application( commit: Specific commit to deploy (optional) use_cache: Whether to use build cache (optional) """ + owner = encode_path_segment(owner_id, name="owner_id") + app = encode_path_segment(app_id, name="app_id") params: dict[str, Any] = {} if commit is not None: params["commit"] = commit @@ -290,8 +515,8 @@ async def redeploy_application( await self._request( "POST", - f"/v2/organisations/{owner_id}/applications/{app_id}/instances", - params=params if params else None, + f"/v2/organisations/{owner}/applications/{app}/instances", + params=params or None, ) async def create_tcp_redirection( @@ -311,18 +536,16 @@ async def create_tcp_redirection( Returns: TcpRedirection with namespace and assigned port """ + owner = encode_path_segment(owner_id, name="owner_id") + app = encode_path_segment(app_id, name="app_id") data = await self._request( "POST", - f"/v2/organisations/{owner_id}/applications/{app_id}/tcpRedirs", + f"/v2/organisations/{owner}/applications/{app}/tcpRedirs", json={"namespace": namespace}, ) return TcpRedirection.from_api_response(data) - async def list_domains( - self, - owner_id: str, - app_id: str, - ) -> list[Domain]: + async def list_domains(self, owner_id: str, app_id: str) -> list[Domain]: """List all domains (vhosts) for an application. Args: @@ -331,17 +554,50 @@ async def list_domains( Returns: List of domains for the application + + Raises: + NotFoundError: If the organisation or the application does not exist. + An existing application with no domain returns an empty list. """ - try: - data = await self._request( - "GET", - f"/v2/organisations/{owner_id}/applications/{app_id}/vhosts", - ) - return [Domain.from_api_response(d) for d in data] - except HttpError as e: - if e.status_code == 404: - return [] - raise + owner = encode_path_segment(owner_id, name="owner_id") + app = encode_path_segment(app_id, name="app_id") + data = await self._request( + "GET", f"/v2/organisations/{owner}/applications/{app}/vhosts" + ) + if not isinstance(data, list): + msg = f"Expected a list of domains, got {type(data).__name__}" + raise InvalidResponseError(msg) + return [Domain.from_api_response(d) for d in data] + + async def get_primary_domain(self, owner_id: str, app_id: str) -> Domain | None: + """Get the primary domain (vhost) for an application. + + Args: + owner_id: Organisation or user ID that owns the application + app_id: Application ID + + Returns: + Domain with the primary vhost, or ``None`` if the API answered + successfully with no content. + + Raises: + NotFoundError: If the application has no primary domain, or does not + exist. The API reports both with HTTP 404, so the caller decides + how to treat it. + """ + owner = encode_path_segment(owner_id, name="owner_id") + app = encode_path_segment(app_id, name="app_id") + data = await self._request( + "GET", f"/v2/organisations/{owner}/applications/{app}/vhosts/favourite" + ) + if data is None: + return None + # Some deployments answer with a single-element list. + if isinstance(data, list): + if not data: + return None + data = data[0] + return Domain.from_api_response(data, is_primary=True) async def create_networkgroup( self, @@ -367,6 +623,7 @@ async def create_networkgroup( members: Optional initial members (list of WannabeNetworkgroupMember dicts: {id, domainName, kind, label?}). """ + owner = encode_path_segment(owner_id, name="owner_id") body: dict[str, Any] = {"label": label} if description is not None: body["description"] = description @@ -378,37 +635,31 @@ async def create_networkgroup( body["members"] = members await self._request( "POST", - f"/v4/networkgroups/organisations/{owner_id}/networkgroups", + f"/v4/networkgroups/organisations/{owner}/networkgroups", json=body, ) - async def get_networkgroup( - self, - owner_id: str, - ng_id: str, - ) -> NetworkGroup: + async def get_networkgroup(self, owner_id: str, ng_id: str) -> NetworkGroup: """Get a NetworkGroup. GET /v4/networkgroups/organisations/{ownerId}/networkgroups/{networkGroupId} """ + owner = encode_path_segment(owner_id, name="owner_id") + ng = encode_path_segment(ng_id, name="ng_id") data = await self._request( - "GET", - f"/v4/networkgroups/organisations/{owner_id}/networkgroups/{ng_id}", + "GET", f"/v4/networkgroups/organisations/{owner}/networkgroups/{ng}" ) return NetworkGroup.from_api_response(data) - async def delete_networkgroup( - self, - owner_id: str, - ng_id: str, - ) -> None: + async def delete_networkgroup(self, owner_id: str, ng_id: str) -> None: """Delete a NetworkGroup. DELETE /v4/networkgroups/organisations/{ownerId}/networkgroups/{networkGroupId} """ + owner = encode_path_segment(owner_id, name="owner_id") + ng = encode_path_segment(ng_id, name="ng_id") await self._request( - "DELETE", - f"/v4/networkgroups/organisations/{owner_id}/networkgroups/{ng_id}", + "DELETE", f"/v4/networkgroups/organisations/{owner}/networkgroups/{ng}" ) async def search_networkgroup_components( @@ -425,12 +676,19 @@ async def search_networkgroup_components( (CleverPeer | ExternalPeer | Member | NetworkGroup) that callers typically discriminate by inspecting fields. """ + owner = encode_path_segment(owner_id, name="owner_id") params = {"query": query} if query is not None else None - return await self._request( + data = await self._request( "GET", - f"/v4/networkgroups/organisations/{owner_id}/networkgroups/search", + f"/v4/networkgroups/organisations/{owner}/networkgroups/search", params=params, ) + if data is None: + return [] + if not isinstance(data, list): + msg = f"Expected a list of components, got {type(data).__name__}" + raise InvalidResponseError(msg) + return data async def create_networkgroup_member( self, @@ -455,6 +713,8 @@ async def create_networkgroup_member( kind: ADDON | APPLICATION | EXTERNAL | LOADBALANCER. label: Optional human-readable label. """ + owner = encode_path_segment(owner_id, name="owner_id") + ng = encode_path_segment(ng_id, name="ng_id") body: dict[str, Any] = { "id": member_id, "domainName": domain_name, @@ -464,69 +724,73 @@ async def create_networkgroup_member( body["label"] = label await self._request( "POST", - f"/v4/networkgroups/organisations/{owner_id}/networkgroups/{ng_id}/members", + f"/v4/networkgroups/organisations/{owner}/networkgroups/{ng}/members", json=body, ) async def get_networkgroup_member( - self, - owner_id: str, - ng_id: str, - member_id: str, + self, owner_id: str, ng_id: str, member_id: str ) -> NetworkGroupMember: """Get a member of a NetworkGroup. GET .../networkgroups/{networkGroupId}/members/{memberId} """ + owner = encode_path_segment(owner_id, name="owner_id") + ng = encode_path_segment(ng_id, name="ng_id") + member = encode_path_segment(member_id, name="member_id") data = await self._request( "GET", - f"/v4/networkgroups/organisations/{owner_id}/networkgroups/{ng_id}/members/{member_id}", + f"/v4/networkgroups/organisations/{owner}/networkgroups/{ng}/members/{member}", ) return NetworkGroupMember.from_api_response(data) async def delete_networkgroup_member( - self, - owner_id: str, - ng_id: str, - member_id: str, + self, owner_id: str, ng_id: str, member_id: str ) -> None: """Remove a member from a NetworkGroup. DELETE .../networkgroups/{networkGroupId}/members/{memberId} """ + owner = encode_path_segment(owner_id, name="owner_id") + ng = encode_path_segment(ng_id, name="ng_id") + member = encode_path_segment(member_id, name="member_id") await self._request( "DELETE", - f"/v4/networkgroups/organisations/{owner_id}/networkgroups/{ng_id}/members/{member_id}", + f"/v4/networkgroups/organisations/{owner}/networkgroups/{ng}/members/{member}", ) async def list_networkgroup_peers( - self, - owner_id: str, - ng_id: str, + self, owner_id: str, ng_id: str ) -> list[NetworkGroupPeer]: """List peers of a NetworkGroup. GET .../networkgroups/{networkGroupId}/peers """ + owner = encode_path_segment(owner_id, name="owner_id") + ng = encode_path_segment(ng_id, name="ng_id") data = await self._request( - "GET", - f"/v4/networkgroups/organisations/{owner_id}/networkgroups/{ng_id}/peers", + "GET", f"/v4/networkgroups/organisations/{owner}/networkgroups/{ng}/peers" ) - return [NetworkGroupPeer.from_api_response(p) for p in data or []] + if data is None: + return [] + if not isinstance(data, list): + msg = f"Expected a list of peers, got {type(data).__name__}" + raise InvalidResponseError(msg) + return [NetworkGroupPeer.from_api_response(p) for p in data] async def get_networkgroup_peer( - self, - owner_id: str, - ng_id: str, - peer_id: str, + self, owner_id: str, ng_id: str, peer_id: str ) -> NetworkGroupPeer: """Get a peer of a NetworkGroup. GET .../networkgroups/{networkGroupId}/peers/{peerId} """ + owner = encode_path_segment(owner_id, name="owner_id") + ng = encode_path_segment(ng_id, name="ng_id") + peer = encode_path_segment(peer_id, name="peer_id") data = await self._request( "GET", - f"/v4/networkgroups/organisations/{owner_id}/networkgroups/{ng_id}/peers/{peer_id}", + f"/v4/networkgroups/organisations/{owner}/networkgroups/{ng}/peers/{peer}", ) return NetworkGroupPeer.from_api_response(data) @@ -551,6 +815,8 @@ async def create_networkgroup_peer( POST .../networkgroups/{networkGroupId}/peers """ + owner = encode_path_segment(owner_id, name="owner_id") + ng = encode_path_segment(ng_id, name="ng_id") body: dict[str, Any] = { "id": peer_id, "parentMember": parent_member, @@ -570,24 +836,24 @@ async def create_networkgroup_peer( body[key] = value data = await self._request( "POST", - f"/v4/networkgroups/organisations/{owner_id}/networkgroups/{ng_id}/peers", + f"/v4/networkgroups/organisations/{owner}/networkgroups/{ng}/peers", json=body, ) - return PeerCreated.from_api_response(data or {}) + return PeerCreated.from_api_response(data) async def delete_networkgroup_peer( - self, - owner_id: str, - ng_id: str, - peer_id: str, + self, owner_id: str, ng_id: str, peer_id: str ) -> None: """Delete a peer of a NetworkGroup. DELETE .../networkgroups/{networkGroupId}/peers/{peerId} """ + owner = encode_path_segment(owner_id, name="owner_id") + ng = encode_path_segment(ng_id, name="ng_id") + peer = encode_path_segment(peer_id, name="peer_id") await self._request( "DELETE", - f"/v4/networkgroups/organisations/{owner_id}/networkgroups/{ng_id}/peers/{peer_id}", + f"/v4/networkgroups/organisations/{owner}/networkgroups/{ng}/peers/{peer}", ) async def create_networkgroup_external_peer( @@ -608,6 +874,8 @@ async def create_networkgroup_external_peer( POST .../networkgroups/{networkGroupId}/external-peers """ + owner = encode_path_segment(owner_id, name="owner_id") + ng = encode_path_segment(ng_id, name="ng_id") body: dict[str, Any] = { "parentMember": parent_member, "peerRole": peer_role.value if isinstance(peer_role, PeerRole) else peer_role, @@ -624,32 +892,7 @@ async def create_networkgroup_external_peer( body[key] = value data = await self._request( "POST", - f"/v4/networkgroups/organisations/{owner_id}/networkgroups/{ng_id}/external-peers", + f"/v4/networkgroups/organisations/{owner}/networkgroups/{ng}/external-peers", json=body, ) - return PeerCreated.from_api_response(data or {}) - - async def get_primary_domain( - self, - owner_id: str, - app_id: str, - ) -> Domain | None: - """Get the primary domain (vhost) for an application. - - Args: - owner_id: Organisation or user ID that owns the application - app_id: Application ID - - Returns: - Domain with the primary vhost, or None if not set - """ - try: - data = await self._request( - "GET", - f"/v2/organisations/{owner_id}/applications/{app_id}/vhosts/favourite", - ) - return Domain.from_api_response(data, is_primary=True) - except HttpError as e: - if e.status_code == 404: - return None - raise + return PeerCreated.from_api_response(data) diff --git a/src/clever_cloud/exceptions.py b/src/clever_cloud/exceptions.py index 304ee07..f835267 100644 --- a/src/clever_cloud/exceptions.py +++ b/src/clever_cloud/exceptions.py @@ -1,5 +1,15 @@ """Exception hierarchy for Clever Cloud SDK.""" +MAX_BODY_LENGTH = 2048 +"""Maximum number of characters of a response body kept in an exception.""" + + +def truncate_body(body: str, *, limit: int = MAX_BODY_LENGTH) -> str: + """Truncate a response body so exceptions never carry unbounded payloads.""" + if len(body) <= limit: + return body + return f"{body[:limit]}... [truncated, {len(body)} characters total]" + class CleverCloudError(Exception): """Base exception for all SDK errors.""" @@ -9,8 +19,24 @@ def __init__(self, message: str) -> None: super().__init__(message) +class TransportError(CleverCloudError): + """Network-level failure (connection, timeout, TLS) raised by the transport.""" + + +class InvalidResponseError(CleverCloudError): + """The server returned a response the SDK cannot interpret. + + Raised for undecodable JSON bodies, unexpected redirections, or payloads + whose shape does not match what the endpoint is documented to return. + """ + + def __init__(self, message: str, *, response_body: str = "") -> None: + super().__init__(message) + self.response_body = truncate_body(response_body) + + class HttpError(CleverCloudError): - """HTTP error with status code and response body.""" + """HTTP error with status code and (truncated) response body.""" def __init__( self, @@ -21,11 +47,36 @@ def __init__( ) -> None: super().__init__(message) self.status_code = status_code - self.response_body = response_body + self.response_body = truncate_body(response_body) class AuthenticationError(HttpError): - """Authentication failure (HTTP 401/403).""" + """Authentication failure (HTTP 401): credentials missing or invalid.""" + + +class AuthorizationError(HttpError): + """Authorization failure (HTTP 403): credentials valid but access denied.""" + + +class NotFoundError(HttpError): + """The requested resource does not exist (HTTP 404).""" + + +class RateLimitError(HttpError): + """The API rejected the request for rate limiting (HTTP 429).""" + + def __init__( + self, + message: str, + *, + status_code: int, + response_body: str, + retry_after: float | None = None, + ) -> None: + super().__init__( + message, status_code=status_code, response_body=response_body + ) + self.retry_after = retry_after class OAuthError(CleverCloudError): @@ -40,4 +91,4 @@ def __init__( ) -> None: super().__init__(message) self.step = step - self.details = details + self.details = truncate_body(details) if details is not None else None diff --git a/src/clever_cloud/models.py b/src/clever_cloud/models.py index 7daad5d..b20aa20 100644 --- a/src/clever_cloud/models.py +++ b/src/clever_cloud/models.py @@ -1,71 +1,148 @@ -"""Data models for Clever Cloud API responses.""" +"""Data models for Clever Cloud API responses. +Parsing is strict on purpose: a payload that does not carry the fields an +endpoint is documented to return raises :class:`InvalidResponseError` instead of +producing a model filled with empty strings, zeroes or a fabricated timestamp. +""" + +from __future__ import annotations + +from collections.abc import Mapping from dataclasses import dataclass, field from datetime import UTC, datetime from enum import Enum -from typing import Any, Self +from types import MappingProxyType +from typing import Any, Self, TypeVar + +from clever_cloud.exceptions import InvalidResponseError + +def _parse_date(raw: Any, *, model: str, key: str) -> datetime | None: + """Parse an API date into a timezone-aware UTC datetime. -def _parse_date(raw: Any) -> datetime: - """Parse API date (timestamp in ms or ISO string).""" + Accepts a millisecond epoch integer or an ISO-8601 string. An absent date + yields ``None`` — it is never replaced with the current time. A present but + unparsable date is an error, not a silent fallback. + """ + if raw is None or raw == "": + return None + if isinstance(raw, bool): + msg = f"{model}.{key}: expected a date, got a boolean" + raise InvalidResponseError(msg) if isinstance(raw, int): - return datetime.fromtimestamp(raw / 1000, tz=UTC) - if isinstance(raw, str) and raw: - return datetime.fromisoformat(raw.replace("Z", "+00:00")) - return datetime.now(tz=UTC) + try: + return datetime.fromtimestamp(raw / 1000, tz=UTC) + except (OverflowError, OSError, ValueError) as exc: + msg = f"{model}.{key}: invalid epoch timestamp {raw!r}" + raise InvalidResponseError(msg) from exc + if isinstance(raw, str): + try: + parsed = datetime.fromisoformat(raw.replace("Z", "+00:00")) + except ValueError as exc: + msg = f"{model}.{key}: invalid ISO-8601 date {raw!r}" + raise InvalidResponseError(msg) from exc + # Normalize: a date without offset is interpreted as UTC, and any other + # offset is converted, so every model exposes UTC-aware datetimes. + if parsed.tzinfo is None: + return parsed.replace(tzinfo=UTC) + return parsed.astimezone(UTC) + msg = f"{model}.{key}: unsupported date type {type(raw).__name__}" + raise InvalidResponseError(msg) + + +def _mapping(data: Any, *, model: str) -> Mapping[str, Any]: + if not isinstance(data, Mapping): + msg = f"{model}: expected a JSON object, got {type(data).__name__}" + raise InvalidResponseError(msg) + return data + + +def _require_str(data: Mapping[str, Any], key: str, *, model: str) -> str: + value = data.get(key) + if not isinstance(value, str) or not value: + msg = f"{model}: missing or invalid required field {key!r}" + raise InvalidResponseError(msg) + return value + + +def _require_int(data: Mapping[str, Any], key: str, *, model: str) -> int: + value = data.get(key) + if isinstance(value, bool) or not isinstance(value, int): + msg = f"{model}: missing or invalid required field {key!r}" + raise InvalidResponseError(msg) + return value + + +def _optional_str(data: Mapping[str, Any], key: str) -> str | None: + value = data.get(key) + return value if isinstance(value, str) and value else None + + +def _optional_int(data: Mapping[str, Any], key: str) -> int | None: + value = data.get(key) + if isinstance(value, bool) or not isinstance(value, int): + return None + return value + + +def _flag(data: Mapping[str, Any], key: str) -> bool: + return bool(data.get(key, False)) @dataclass(frozen=True, slots=True) class Profile: - """User profile from GET /v2/self.""" + """User profile from ``GET /v2/self``.""" id: str email: str - name: str - phone: str - address: str - city: str - zipcode: str - country: str - avatar: str - creation_date: datetime - lang: str - email_validated: bool - is_linked_to_github: bool - admin: bool - can_pay: bool - preferred_mfa: str | None - has_password: bool - partner_id: str | None - partner_name: str | None - partner_console_url: str | None + name: str | None = None + phone: str | None = None + address: str | None = None + city: str | None = None + zipcode: str | None = None + country: str | None = None + avatar: str | None = None + creation_date: datetime | None = None + lang: str | None = None + email_validated: bool = False + is_linked_to_github: bool = False + admin: bool = False + can_pay: bool = False + preferred_mfa: str | None = None + has_password: bool = False + partner_id: str | None = None + partner_name: str | None = None + partner_console_url: str | None = None @classmethod - def from_api_response(cls, data: dict[str, Any]) -> Self: + def from_api_response(cls, data: Any) -> Self: + data = _mapping(data, model="Profile") oauth_apps = data.get("oauthApps", []) is_linked_to_github = isinstance(oauth_apps, list) and "github" in oauth_apps return cls( - id=data.get("id", ""), - email=data.get("email", ""), - name=data.get("name", ""), - phone=data.get("phone", ""), - address=data.get("address", ""), - city=data.get("city", ""), - zipcode=data.get("zipcode", ""), - country=data.get("country", ""), - avatar=data.get("avatar", ""), - creation_date=_parse_date(data.get("creationDate", "")), - lang=data.get("lang", ""), - email_validated=data.get("emailValidated", False), + id=_require_str(data, "id", model="Profile"), + email=_require_str(data, "email", model="Profile"), + name=_optional_str(data, "name"), + phone=_optional_str(data, "phone"), + address=_optional_str(data, "address"), + city=_optional_str(data, "city"), + zipcode=_optional_str(data, "zipcode"), + country=_optional_str(data, "country"), + avatar=_optional_str(data, "avatar"), + creation_date=_parse_date( + data.get("creationDate"), model="Profile", key="creationDate" + ), + lang=_optional_str(data, "lang"), + email_validated=_flag(data, "emailValidated"), is_linked_to_github=is_linked_to_github, - admin=data.get("admin", False), - can_pay=data.get("canPay", False), - preferred_mfa=data.get("preferredMFA"), - has_password=data.get("hasPassword", False), - partner_id=data.get("partnerId"), - partner_name=data.get("partnerName"), - partner_console_url=data.get("partnerConsoleUrl"), + admin=_flag(data, "admin"), + can_pay=_flag(data, "canPay"), + preferred_mfa=_optional_str(data, "preferredMFA"), + has_password=_flag(data, "hasPassword"), + partner_id=_optional_str(data, "partnerId"), + partner_name=_optional_str(data, "partnerName"), + partner_console_url=_optional_str(data, "partnerConsoleUrl"), ) @@ -77,12 +154,10 @@ class Domain: is_primary: bool @classmethod - def from_api_response( - cls, data: dict[str, Any], *, is_primary: bool = False - ) -> Self: - fqdn = data.get("fqdn", "").rstrip("/") + def from_api_response(cls, data: Any, *, is_primary: bool = False) -> Self: + data = _mapping(data, model="Domain") return cls( - domain=fqdn, + domain=_require_str(data, "fqdn", model="Domain").rstrip("/"), is_primary=is_primary, ) @@ -95,10 +170,11 @@ class TcpRedirection: port: int @classmethod - def from_api_response(cls, data: dict[str, Any]) -> Self: + def from_api_response(cls, data: Any) -> Self: + data = _mapping(data, model="TcpRedirection") return cls( - namespace=data.get("namespace", "default"), - port=data.get("port", 0), + namespace=_require_str(data, "namespace", model="TcpRedirection"), + port=_require_int(data, "port", model="TcpRedirection"), ) @@ -125,22 +201,39 @@ class PeerKind(str, Enum): EXTERNAL = "EXTERNAL" +_E = TypeVar("_E", bound=Enum) + + +def _parse_enum(enum_cls: type[_E], raw: Any, *, model: str, key: str) -> _E: + try: + return enum_cls(raw) + except ValueError as exc: + msg = f"{model}.{key}: unknown value {raw!r}" + raise InvalidResponseError(msg) from exc + + @dataclass(frozen=True, slots=True) class NetworkGroupMember: - """Member of a NetworkGroup (GET .../members/{memberId}).""" + """Member of a NetworkGroup (``GET .../members/{memberId}``).""" id: str domain_name: str kind: MemberKind - label: str + label: str | None = None @classmethod - def from_api_response(cls, data: dict[str, Any]) -> Self: + def from_api_response(cls, data: Any) -> Self: + data = _mapping(data, model="NetworkGroupMember") return cls( - id=data.get("id", ""), - domain_name=data.get("domainName", ""), - kind=MemberKind(data.get("kind", "EXTERNAL")), - label=data.get("label", ""), + id=_require_str(data, "id", model="NetworkGroupMember"), + domain_name=_require_str(data, "domainName", model="NetworkGroupMember"), + kind=_parse_enum( + MemberKind, + _require_str(data, "kind", model="NetworkGroupMember"), + model="NetworkGroupMember", + key="kind", + ), + label=_optional_str(data, "label"), ) @@ -148,14 +241,15 @@ def from_api_response(cls, data: dict[str, Any]) -> Self: class WireguardEndpoint: """Wireguard endpoint (private/public address pair).""" - private_address: str - public_address: str + private_address: str | None = None + public_address: str | None = None @classmethod - def from_api_response(cls, data: dict[str, Any]) -> Self: + def from_api_response(cls, data: Any) -> Self: + data = _mapping(data, model="WireguardEndpoint") return cls( - private_address=str(data.get("privateAddress", "")), - public_address=str(data.get("publicAddress", "")), + private_address=_optional_str(data, "privateAddress"), + public_address=_optional_str(data, "publicAddress"), ) @@ -164,88 +258,95 @@ class NetworkGroupPeer: """Peer of a NetworkGroup (CleverPeer or ExternalPeer flattened).""" id: str - public_key: str parent_member: str - endpoint: WireguardEndpoint | None - hostname: str - label: str kind: PeerKind - hv: str | None + public_key: str | None = None + endpoint: WireguardEndpoint | None = None + hostname: str | None = None + label: str | None = None + hv: str | None = None @classmethod - def from_api_response(cls, data: dict[str, Any]) -> Self: + def from_api_response(cls, data: Any) -> Self: + data = _mapping(data, model="NetworkGroupPeer") endpoint_data = data.get("endpoint") endpoint = ( WireguardEndpoint.from_api_response(endpoint_data) - if isinstance(endpoint_data, dict) + if isinstance(endpoint_data, Mapping) else None ) - # CleverPeer has "hv" field; ExternalPeer does not. - hv = data.get("hv") - kind = PeerKind.CLEVER if hv is not None else PeerKind.EXTERNAL + # CleverPeer carries an "hv" field; ExternalPeer does not. + hv = _optional_str(data, "hv") return cls( - id=data.get("id", ""), - public_key=data.get("publicKey", ""), - parent_member=data.get("parentMember", ""), + id=_require_str(data, "id", model="NetworkGroupPeer"), + parent_member=_require_str(data, "parentMember", model="NetworkGroupPeer"), + kind=PeerKind.CLEVER if hv is not None else PeerKind.EXTERNAL, + public_key=_optional_str(data, "publicKey"), endpoint=endpoint, - hostname=data.get("hostname", ""), - label=data.get("label", ""), - kind=kind, + hostname=_optional_str(data, "hostname"), + label=_optional_str(data, "label"), hv=hv, ) @dataclass(frozen=True, slots=True) class NetworkGroup: - """NetworkGroup from GET .../networkgroups/{networkGroupId}.""" + """NetworkGroup from ``GET .../networkgroups/{networkGroupId}``. + + Collections are exposed as tuples so the model is deeply immutable, as + ``frozen=True`` advertises. + """ id: str owner_id: str label: str - description: str - dns_sanitized_label: str - network_ip: str - last_allocated_ip: str version: int - members: list[NetworkGroupMember] = field(default_factory=list) - peers: list[NetworkGroupPeer] = field(default_factory=list) - tags: list[str] = field(default_factory=list) + description: str | None = None + dns_sanitized_label: str | None = None + network_ip: str | None = None + last_allocated_ip: str | None = None + members: tuple[NetworkGroupMember, ...] = () + peers: tuple[NetworkGroupPeer, ...] = () + tags: tuple[str, ...] = () @classmethod - def from_api_response(cls, data: dict[str, Any]) -> Self: + def from_api_response(cls, data: Any) -> Self: + data = _mapping(data, model="NetworkGroup") return cls( - id=data.get("id", ""), - owner_id=data.get("ownerId", ""), - label=data.get("label", ""), - description=data.get("description", ""), - dns_sanitized_label=data.get("dnsSanitizedLabel", ""), - network_ip=data.get("networkIp", ""), - last_allocated_ip=data.get("lastAllocatedIp", ""), - version=data.get("version", 0), - members=[ + id=_require_str(data, "id", model="NetworkGroup"), + owner_id=_require_str(data, "ownerId", model="NetworkGroup"), + label=_require_str(data, "label", model="NetworkGroup"), + version=_require_int(data, "version", model="NetworkGroup"), + description=_optional_str(data, "description"), + dns_sanitized_label=_optional_str(data, "dnsSanitizedLabel"), + network_ip=_optional_str(data, "networkIp"), + last_allocated_ip=_optional_str(data, "lastAllocatedIp"), + members=tuple( NetworkGroupMember.from_api_response(m) - for m in data.get("members") or [] - ], - peers=[ - NetworkGroupPeer.from_api_response(p) for p in data.get("peers") or [] - ], - tags=list(data.get("tags") or []), + for m in data.get("members") or () + ), + peers=tuple( + NetworkGroupPeer.from_api_response(p) for p in data.get("peers") or () + ), + tags=tuple(str(tag) for tag in data.get("tags") or ()), ) @dataclass(frozen=True, slots=True) class PeerCreated: - """Response of POST .../peers and .../external-peers.""" + """Response of ``POST .../peers`` and ``.../external-peers``.""" peer_id: str - raw: dict[str, Any] + raw: Mapping[str, Any] = field(default_factory=lambda: MappingProxyType({})) @classmethod - def from_api_response(cls, data: dict[str, Any]) -> Self: - return cls( - peer_id=data.get("id", data.get("peerId", "")), - raw=data, - ) + def from_api_response(cls, data: Any) -> Self: + data = _mapping(data, model="PeerCreated") + peer_id = data.get("id") or data.get("peerId") + if not isinstance(peer_id, str) or not peer_id: + msg = "PeerCreated: missing or invalid required field 'id'" + raise InvalidResponseError(msg) + return cls(peer_id=peer_id, raw=MappingProxyType(dict(data))) @dataclass(frozen=True, slots=True) @@ -254,37 +355,45 @@ class Application: id: str name: str - description: str - zone: str - instance_type: str - instance_version: str - instance_variant: str - min_instances: int - max_instances: int - min_flavor: str - max_flavor: str - deploy_url: str - creation_date: datetime - state: str + zone: str | None = None + description: str | None = None + instance_type: str | None = None + instance_version: str | None = None + instance_variant: str | None = None + min_instances: int | None = None + max_instances: int | None = None + min_flavor: str | None = None + max_flavor: str | None = None + deploy_url: str | None = None + creation_date: datetime | None = None + state: str | None = None @classmethod - def from_api_response(cls, data: dict[str, Any]) -> Self: - instance = data.get("instance", {}) - variant = instance.get("variant", {}) + def from_api_response(cls, data: Any) -> Self: + data = _mapping(data, model="Application") + instance = data.get("instance") + instance = instance if isinstance(instance, Mapping) else {} + variant = instance.get("variant") + variant = variant if isinstance(variant, Mapping) else {} return cls( - id=data.get("id", ""), - name=data.get("name", ""), - description=data.get("description", ""), - zone=data.get("zone", ""), - instance_type=instance.get("type", data.get("instanceType", "")), - instance_version=instance.get("version", data.get("instanceVersion", "")), - instance_variant=variant.get("id", data.get("instanceVariant", "")), - min_instances=data.get("minInstances", 1), - max_instances=data.get("maxInstances", 1), - min_flavor=data.get("minFlavor", ""), - max_flavor=data.get("maxFlavor", ""), - deploy_url=data.get("deployUrl", ""), - creation_date=_parse_date(data.get("creationDate", "")), - state=data.get("state", ""), + id=_require_str(data, "id", model="Application"), + name=_require_str(data, "name", model="Application"), + zone=_optional_str(data, "zone"), + description=_optional_str(data, "description"), + instance_type=_optional_str(instance, "type") + or _optional_str(data, "instanceType"), + instance_version=_optional_str(instance, "version") + or _optional_str(data, "instanceVersion"), + instance_variant=_optional_str(variant, "id") + or _optional_str(data, "instanceVariant"), + min_instances=_optional_int(data, "minInstances"), + max_instances=_optional_int(data, "maxInstances"), + min_flavor=_optional_str(data, "minFlavor"), + max_flavor=_optional_str(data, "maxFlavor"), + deploy_url=_optional_str(data, "deployUrl"), + creation_date=_parse_date( + data.get("creationDate"), model="Application", key="creationDate" + ), + state=_optional_str(data, "state"), ) diff --git a/src/clever_cloud/oauth_dance.py b/src/clever_cloud/oauth_dance.py index 2cb01cb..51c892f 100644 --- a/src/clever_cloud/oauth_dance.py +++ b/src/clever_cloud/oauth_dance.py @@ -1,41 +1,74 @@ -"""OAuth 1.0 dance to obtain access credentials. +"""OAuth 1.0a dance to obtain access credentials. -Flow: get_request_token() -> login() or browser auth -> get_access_token() +Flow: ``get_request_token()`` -> browser authorization (or ``login()``) +-> ``get_access_token()``. """ -from dataclasses import dataclass -from typing import Self -from urllib.parse import parse_qs, urlencode, urlparse +from __future__ import annotations + +import base64 +import hmac +import secrets +import time +from dataclasses import dataclass, field +from datetime import UTC, datetime +from typing import Any, Self +from urllib.parse import parse_qs, parse_qsl, urlencode, urlsplit import httpx -from clever_cloud.auth import OAuthCredentials +from clever_cloud.auth import ( + HASH_ALGORITHMS, + OAUTH_VERSION, + OAuthCredentials, + SignatureMethod, + build_signature_base_string, + percent_encode, + redact, +) from clever_cloud.exceptions import OAuthError +_FORM_HEADERS = { + "Content-Type": "application/x-www-form-urlencoded", + "Accept": "application/x-www-form-urlencoded", +} +_REDIRECT_STATUSES = frozenset({301, 302, 303, 307, 308}) + -@dataclass(frozen=True, slots=True) +@dataclass(frozen=True, slots=True, repr=False) class OAuthConsumer: - """OAuth consumer credentials from Clever Cloud console.""" + """OAuth consumer credentials from the Clever Cloud console.""" key: str - secret: str + secret: str = field(repr=False) + + def __repr__(self) -> str: + return f"OAuthConsumer(key={self.key!r}, secret={redact(self.secret)})" -@dataclass(frozen=True, slots=True) +@dataclass(frozen=True, slots=True, repr=False) class RequestToken: - """Temporary token used during OAuth dance.""" + """Temporary token used during the OAuth dance.""" token: str - secret: str + secret: str = field(repr=False) + callback_confirmed: bool = False + + def __repr__(self) -> str: + return ( + f"RequestToken(token={self.token!r}, secret={redact(self.secret)}, " + f"callback_confirmed={self.callback_confirmed!r})" + ) class OAuthDance: - """Performs OAuth 1.0 dance to obtain OAuthCredentials. + """Performs the OAuth 1.0a dance to obtain :class:`OAuthCredentials`. Example: with OAuthDance(OAuthConsumer(key="...", secret="...")) as dance: request_token = dance.get_request_token() - verifier = dance.login(request_token, email="...", password="...") + webbrowser.open(dance.get_authorization_url(request_token)) + verifier = dance.parse_callback_url(callback_url, request_token) credentials = dance.get_access_token(request_token, verifier) """ @@ -47,10 +80,29 @@ def __init__( *, callback_url: str = "oob", timeout: float = 30.0, + signature_method: SignatureMethod = SignatureMethod.HMAC_SHA512, + api_url: str | None = None, + transport: httpx.BaseTransport | None = None, ) -> None: + """Create a dance. + + Args: + consumer: Consumer key/secret issued by the Clever Cloud console. + callback_url: Callback the API redirects to, or ``"oob"`` for + out-of-band (the verifier is then shown to the user). + timeout: Per-request timeout, in seconds. + signature_method: Signature method; HMAC-SHA512 by default. + ``PLAINTEXT`` is a legacy compatibility mode. + api_url: Override the API root (testing, private deployments). + transport: Custom transport, mainly useful for tests. + """ self._consumer = consumer self._callback_url = callback_url - self._client = httpx.Client(base_url=self.API_URL, timeout=timeout) + self._signature_method = signature_method + self._api_url = (api_url or self.API_URL).rstrip("/") + self._client = httpx.Client( + base_url=self._api_url, timeout=timeout, transport=transport + ) def close(self) -> None: self._client.close() @@ -61,52 +113,120 @@ def __enter__(self) -> Self: def __exit__(self, *args: object) -> None: self.close() - def _get_oauth_signature(self, token_secret: str = "") -> str: - return f"{self._consumer.secret}&{token_secret}" - - def get_request_token(self) -> RequestToken: - """Step 1: Get temporary request token.""" - body = { + def _signed_params( + self, + url: str, + extra: dict[str, str], + *, + token_secret: str = "", + timestamp: int | None = None, + nonce: str | None = None, + ) -> dict[str, str]: + """Build the fully signed OAuth parameters for a dance request. + + Every request carries a signature method, timestamp, nonce and version, + so it cannot be replayed — the legacy format omitted all four. + """ + params = { "oauth_consumer_key": self._consumer.key, - "oauth_signature_method": "PLAINTEXT", - "oauth_signature": self._get_oauth_signature(), - "oauth_callback": self._callback_url, + "oauth_signature_method": self._signature_method.value, + "oauth_timestamp": str(timestamp if timestamp is not None else int(time.time())), + "oauth_nonce": nonce or secrets.token_hex(16), + "oauth_version": OAUTH_VERSION, + **extra, } - - response = self._client.post( - "/v2/oauth/request_token", - data=body, - headers={ - "Content-Type": "application/x-www-form-urlencoded", - "Accept": "application/x-www-form-urlencoded", - }, - ) + key = f"{percent_encode(self._consumer.secret)}&{percent_encode(token_secret)}" + if self._signature_method is SignatureMethod.PLAINTEXT: + signature = key + else: + base_string = build_signature_base_string("POST", url, params.items()) + digest = hmac.new( + key.encode("utf-8"), + base_string.encode("utf-8"), + HASH_ALGORITHMS[self._signature_method], + ).digest() + signature = base64.b64encode(digest).decode("ascii") + return {**params, "oauth_signature": signature} + + def _post_form(self, path: str, body: dict[str, str], *, step: str) -> dict[str, str]: + try: + response = self._client.post(path, data=body, headers=_FORM_HEADERS) + except httpx.TransportError as exc: + msg = f"Network failure during {step}: {type(exc).__name__}: {exc}" + raise OAuthError(msg, step=step) from exc if response.status_code != 200: raise OAuthError( - f"Failed to get request token: {response.text}", - step="request_token", + f"Step {step} failed with HTTP {response.status_code}", + step=step, details=response.text, ) + return dict(parse_qsl(response.text, keep_blank_values=True)) - # Parse the response (format: oauth_token=...&oauth_token_secret=...) - params = parse_qs(response.text) - token = params.get("oauth_token", [""])[0] - secret = params.get("oauth_token_secret", [""])[0] + def get_request_token(self) -> RequestToken: + """Step 1: get a temporary request token.""" + url = f"{self._api_url}/v2/oauth/request_token" + body = self._signed_params(url, {"oauth_callback": self._callback_url}) + params = self._post_form("/v2/oauth/request_token", body, step="request_token") + token = params.get("oauth_token", "") + secret = params.get("oauth_token_secret", "") if not token or not secret: raise OAuthError( "Invalid request token response", step="request_token", - details=response.text, + details=str(params), ) - return RequestToken(token=token, secret=secret) + # RFC 5849 §2.1: the server must confirm it recorded the callback. + confirmed = params.get("oauth_callback_confirmed", "").lower() == "true" + if not confirmed and self._callback_url != "oob": + raise OAuthError( + "Server did not confirm the OAuth callback " + "(oauth_callback_confirmed is not 'true'); the authorization " + "could be redirected to an address you did not request", + step="request_token", + details=str(params), + ) + + return RequestToken(token=token, secret=secret, callback_confirmed=confirmed) def get_authorization_url(self, request_token: RequestToken) -> str: - """Get URL for browser-based authorization.""" + """Get the URL for browser-based authorization.""" params = urlencode({"oauth_token": request_token.token}) - return f"{self.API_URL}/v2/oauth/authorize?{params}" + return f"{self._api_url}/v2/oauth/authorize?{params}" + + def parse_callback_url(self, callback_url: str, request_token: RequestToken) -> str: + """Extract and validate the verifier from the URL the callback received. + + Verifies that the callback carries the very token this dance requested, + so a verifier obtained for another authorization cannot be injected. + + Raises: + OAuthError: If the token does not match or the verifier is missing. + """ + params = parse_qs(urlsplit(callback_url).query) + returned_token = params.get("oauth_token", [""])[0] + verifier = params.get("oauth_verifier", [""])[0] + + if not hmac.compare_digest(returned_token, request_token.token): + raise OAuthError( + "Callback oauth_token does not match the request token", + step="callback", + ) + if not verifier: + raise OAuthError("Callback is missing oauth_verifier", step="callback") + return verifier + + def _verifier_from_redirect( + self, response: httpx.Response, request_token: RequestToken + ) -> str | None: + if response.status_code not in _REDIRECT_STATUSES: + return None + location = response.headers.get("Location", "") + if "oauth_verifier=" not in location: + return None + return self.parse_callback_url(location, request_token) def login( self, @@ -115,116 +235,96 @@ def login( email: str, password: str, mfa_code: str | None = None, + mfa_kind: str = "TOTP", ) -> str: - """Step 2: Login with email/password and return OAuth verifier.""" - # First, login to create a session - login_body: dict[str, str] = { - "email": email, - "pass": password, - "from_authorize": "true", - } - + """Step 2 (non-interactive): log in and return the OAuth verifier. + + Warning: + This drives the console's internal session endpoints with the + account password, which is not a supported OAuth flow: prefer + :meth:`get_authorization_url` plus :meth:`parse_callback_url` and let + the user authorize in a browser. It is kept for automation contexts + where no browser is available. + + Args: + request_token: Token returned by :meth:`get_request_token`. + email: Account email. + password: Account password. + mfa_code: One-time code, when the account has MFA enabled. + mfa_kind: MFA kind expected by the API (``"TOTP"`` by default). + """ + # Session cookies are kept by the HTTPX client itself: passing them + # per-request is deprecated and makes persistence ambiguous. login_response = self._client.post( "/v2/sessions/login", - data=login_body, - headers={ - "Content-Type": "application/x-www-form-urlencoded", - }, + data={"email": email, "pass": password, "from_authorize": "true"}, + headers={"Content-Type": "application/x-www-form-urlencoded"}, follow_redirects=False, ) - # Check for MFA requirement (status 200 means MFA form returned) + # A 200 means the MFA form was returned instead of a session redirect. if login_response.status_code == 200: if mfa_code is None: raise OAuthError( "MFA code required", step="login", - details="Please provide mfa_code parameter", + details="Please provide the mfa_code parameter", ) - # Submit MFA code mfa_response = self._client.post( "/v2/sessions/mfa_login", data={ "mfa_attempt": mfa_code, - "mfa_kind": "TOTP", + "mfa_kind": mfa_kind, "email": email, }, - headers={ - "Content-Type": "application/x-www-form-urlencoded", - }, - cookies=login_response.cookies, + headers={"Content-Type": "application/x-www-form-urlencoded"}, follow_redirects=False, ) if mfa_response.status_code == 401: raise OAuthError( - "Invalid MFA code", - step="mfa_login", - details=mfa_response.text, + "Invalid MFA code", step="mfa_login", details=mfa_response.text ) if mfa_response.status_code != 303: raise OAuthError( - f"MFA login failed: {mfa_response.text}", + f"MFA login failed with HTTP {mfa_response.status_code}", step="mfa_login", details=mfa_response.text, ) - session_cookies = mfa_response.cookies elif login_response.status_code == 401: raise OAuthError( - "Invalid credentials", - step="login", - details=login_response.text, + "Invalid credentials", step="login", details=login_response.text ) elif login_response.status_code == 303: - # Login successful, got session cookie - session_cookies = login_response.cookies + pass # Session cookie recorded on the client by HTTPX. else: raise OAuthError( - f"Login failed: {login_response.text}", + f"Login failed with HTTP {login_response.status_code}", step="login", details=login_response.text, ) - # Now authorize the OAuth application auth_response = self._client.get( "/v2/oauth/authorize", params={"oauth_token": request_token.token}, - cookies=session_cookies, follow_redirects=False, ) + verifier = self._verifier_from_redirect(auth_response, request_token) + if verifier: + return verifier - # The response should redirect with oauth_verifier - if auth_response.status_code in (301, 302, 303, 307, 308): - location = auth_response.headers.get("Location", "") - if "oauth_verifier=" in location: - # Extract verifier from redirect URL - parsed = urlparse(location) - params = parse_qs(parsed.query) - verifier = params.get("oauth_verifier", [""])[0] - if verifier: - return verifier - - # If we get 200, user needs to approve OAuth rights + # HTTP 200 means the consent screen was returned: approve it explicitly. if auth_response.status_code == 200: - # Auto-approve by posting to authorize approve_response = self._client.post( "/v2/oauth/authorize", data={"oauth_token": request_token.token}, - cookies=session_cookies, follow_redirects=False, ) - if approve_response.status_code in (301, 302, 303, 307, 308): - location = approve_response.headers.get("Location", "") - if "oauth_verifier=" in location: - parsed = urlparse(location) - params = parse_qs(parsed.query) - verifier = params.get("oauth_verifier", [""])[0] - if verifier: - return verifier + verifier = self._verifier_from_redirect(approve_response, request_token) + if verifier: + return verifier raise OAuthError( - "Failed to get OAuth verifier", - step="authorize", - details=auth_response.text, + "Failed to get OAuth verifier", step="authorize", details=auth_response.text ) def get_access_token( @@ -232,41 +332,22 @@ def get_access_token( request_token: RequestToken, verifier: str, ) -> OAuthCredentials: - """Step 3: Exchange request token + verifier for final credentials.""" - body = { - "oauth_consumer_key": self._consumer.key, - "oauth_signature_method": "PLAINTEXT", - "oauth_signature": self._get_oauth_signature(request_token.secret), - "oauth_token": request_token.token, - "oauth_verifier": verifier, - } - - response = self._client.post( - "/v2/oauth/access_token", - data=body, - headers={ - "Content-Type": "application/x-www-form-urlencoded", - "Accept": "application/x-www-form-urlencoded", - }, + """Step 3: exchange the request token and verifier for final credentials.""" + url = f"{self._api_url}/v2/oauth/access_token" + body = self._signed_params( + url, + {"oauth_token": request_token.token, "oauth_verifier": verifier}, + token_secret=request_token.secret, ) + params = self._post_form("/v2/oauth/access_token", body, step="access_token") - if response.status_code != 200: - raise OAuthError( - f"Failed to get access token: {response.text}", - step="access_token", - details=response.text, - ) - - # Parse the response - params = parse_qs(response.text) - token = params.get("oauth_token", [""])[0] - secret = params.get("oauth_token_secret", [""])[0] - + token = params.get("oauth_token", "") + secret = params.get("oauth_token_secret", "") if not token or not secret: raise OAuthError( "Invalid access token response", step="access_token", - details=response.text, + details=str(params), ) return OAuthCredentials( @@ -274,4 +355,27 @@ def get_access_token( consumer_secret=self._consumer.secret, token=token, secret=secret, + signature_method=self._signature_method, + expiration_date=_parse_expiration(params.get("expiration_date")), ) + + +def _parse_expiration(raw: Any) -> datetime | None: + """Parse the ``expiration_date`` the API returns with access tokens.""" + if raw is None or raw == "": + return None + text = str(raw) + if text.isdigit(): + value = int(text) + # Values are seconds or milliseconds depending on the endpoint. + if value > 10_000_000_000: + value //= 1000 + try: + return datetime.fromtimestamp(value, tz=UTC) + except (OverflowError, OSError, ValueError): + return None + try: + parsed = datetime.fromisoformat(text.replace("Z", "+00:00")) + except ValueError: + return None + return parsed.replace(tzinfo=UTC) if parsed.tzinfo is None else parsed.astimezone(UTC) diff --git a/src/clever_cloud/py.typed b/src/clever_cloud/py.typed new file mode 100644 index 0000000..e69de29 diff --git a/tests/conftest.py b/tests/conftest.py new file mode 100644 index 0000000..959bf2f --- /dev/null +++ b/tests/conftest.py @@ -0,0 +1,77 @@ +"""Shared fixtures: fake credentials and an HTTP layer backed by MockTransport. + +No test performs a real network call — every request is served by a handler +declared in the test itself. +""" + +from __future__ import annotations + +from collections.abc import Callable + +import httpx +import pytest + +from clever_cloud import ApiTokenCredentials, CleverCloudClient, OAuthCredentials + +Handler = Callable[[httpx.Request], httpx.Response] + +BASE_URL = "https://api.example.test" + + +@pytest.fixture +def token_auth() -> ApiTokenCredentials: + return ApiTokenCredentials(token="test-bearer-token", base_url=BASE_URL) + + +@pytest.fixture +def oauth_auth() -> OAuthCredentials: + return OAuthCredentials( + consumer_key="consumer-key", + consumer_secret="consumer-secret", + token="access-token", + secret="access-secret", + base_url=BASE_URL, + ) + + +@pytest.fixture +def make_client( + token_auth: ApiTokenCredentials, +) -> Callable[..., CleverCloudClient]: + """Build a client whose transport is a caller-supplied request handler.""" + + def factory(handler: Handler, **kwargs: object) -> CleverCloudClient: + kwargs.setdefault("auth", token_auth) + kwargs.setdefault("base_url", BASE_URL) + kwargs.setdefault("max_retries", 0) + auth = kwargs.pop("auth") + return CleverCloudClient( + auth, # type: ignore[arg-type] + transport=httpx.MockTransport(handler), + **kwargs, # type: ignore[arg-type] + ) + + return factory + + +@pytest.fixture +def record_requests() -> tuple[list[httpx.Request], Callable[..., Handler]]: + """Capture requests while returning a canned response.""" + seen: list[httpx.Request] = [] + + def handler_factory( + status_code: int = 200, + json: object = None, + *, + content: bytes | None = None, + headers: dict[str, str] | None = None, + ) -> Handler: + def handler(request: httpx.Request) -> httpx.Response: + seen.append(request) + if content is not None: + return httpx.Response(status_code, content=content, headers=headers) + return httpx.Response(status_code, json=json, headers=headers) + + return handler + + return seen, handler_factory diff --git a/tests/test_auth.py b/tests/test_auth.py new file mode 100644 index 0000000..2ee556a --- /dev/null +++ b/tests/test_auth.py @@ -0,0 +1,330 @@ +"""Tests for the authentication strategies (issue #3, findings 1 and 2).""" + +from __future__ import annotations + +import base64 +import hashlib +import hmac +import re +from datetime import UTC, datetime, timedelta + +import httpx +import pytest + +from clever_cloud import ApiTokenCredentials, OAuthCredentials, SignatureMethod +from clever_cloud.auth import ( + build_signature_base_string, + normalize_parameters, + normalize_url, + percent_encode, +) + +HEADER_PARAM = re.compile(r'(\w+)="([^"]*)"') + + +def parse_header(header: str) -> dict[str, str]: + assert header.startswith("OAuth ") + return dict(HEADER_PARAM.findall(header)) + + +class TestPercentEncoding: + @pytest.mark.parametrize( + ("raw", "expected"), + [ + ("abcABC123", "abcABC123"), + ("-._~", "-._~"), + ("+", "%2B"), + (" ", "%20"), + ("/", "%2F"), + ("=", "%3D"), + ("&", "%26"), + ("é", "%C3%A9"), + ], + ) + def test_only_unreserved_characters_survive(self, raw: str, expected: str) -> None: + assert percent_encode(raw) == expected + + +class TestNormalizeUrl: + def test_drops_query_and_fragment(self) -> None: + url = "https://api.example.com/v2/self?a=1#frag" + assert normalize_url(url) == "https://api.example.com/v2/self" + + def test_drops_default_port_and_lowercases_authority(self) -> None: + assert normalize_url("HTTPS://API.Example.COM:443/v2/x") == ( + "https://api.example.com/v2/x" + ) + + def test_keeps_non_default_port(self) -> None: + assert normalize_url("https://api.example.com:8443/v2/x") == ( + "https://api.example.com:8443/v2/x" + ) + + +class TestSignatureBaseString: + def test_matches_the_rfc_5849_example(self) -> None: + """The worked example from RFC 5849 section 3.4.1.1.""" + url = "http://example.com/request?b5=%3D%253D&a3=a&c%40=&a2=r%20b" + params = [ + ("b5", "=%3D"), + ("a3", "a"), + ("c@", ""), + ("a2", "r b"), + ("c2", ""), + ("a3", "2 q"), + ("oauth_consumer_key", "9djdj82h48djs9d2"), + ("oauth_token", "kkk9d7dh3k39sjv7"), + ("oauth_signature_method", "HMAC-SHA1"), + ("oauth_timestamp", "137131201"), + ("oauth_nonce", "7d8f3e4a"), + ] + expected = ( + "POST&http%3A%2F%2Fexample.com%2Frequest&a2%3Dr%2520b%26a3%3D2%2520q" + "%26a3%3Da%26b5%3D%253D%25253D%26c%2540%3D%26c2%3D%26oauth_consumer_" + "key%3D9djdj82h48djs9d2%26oauth_nonce%3D7d8f3e4a%26oauth_signature_m" + "ethod%3DHMAC-SHA1%26oauth_timestamp%3D137131201%26oauth_token%3Dkkk" + "9d7dh3k39sjv7" + ) + assert build_signature_base_string("POST", url, params) == expected + + def test_duplicate_keys_are_ordered_by_value(self) -> None: + assert normalize_parameters([("a", "2"), ("a", "1")]) == "a=1&a=2" + + +class TestOAuthHeader: + def test_carries_every_required_oauth_parameter( + self, oauth_auth: OAuthCredentials + ) -> None: + """Finding 1: the legacy header omitted method, timestamp, nonce, version.""" + header = oauth_auth.get_authorization_header("GET", "https://api.example.test/v2/self") + params = parse_header(header) + assert params["oauth_consumer_key"] == "consumer-key" + assert params["oauth_token"] == "access-token" + assert params["oauth_signature_method"] == "HMAC-SHA512" + assert params["oauth_version"] == "1.0" + assert params["oauth_nonce"] + assert params["oauth_timestamp"].isdigit() + assert params["oauth_signature"] + + def test_signature_is_not_replayable(self, oauth_auth: OAuthCredentials) -> None: + """Two calls for the same request must not produce the same header.""" + url = "https://api.example.test/v2/self" + first = oauth_auth.get_authorization_header("GET", url) + second = oauth_auth.get_authorization_header("GET", url) + assert first != second + assert parse_header(first)["oauth_nonce"] != parse_header(second)["oauth_nonce"] + + def test_signature_does_not_disclose_the_secrets( + self, oauth_auth: OAuthCredentials + ) -> None: + header = oauth_auth.get_authorization_header("GET", "https://api.example.test/v2/self") + assert "consumer-secret" not in header + assert "access-secret" not in header + + def test_signature_binds_the_http_method(self, oauth_auth: OAuthCredentials) -> None: + url = "https://api.example.test/v2/self" + get = oauth_auth.get_authorization_header("GET", url, timestamp=1, nonce="n") + post = oauth_auth.get_authorization_header("POST", url, timestamp=1, nonce="n") + assert parse_header(get)["oauth_signature"] != parse_header(post)["oauth_signature"] + + def test_signature_binds_the_url(self, oauth_auth: OAuthCredentials) -> None: + one = oauth_auth.get_authorization_header( + "GET", "https://api.example.test/v2/self", timestamp=1, nonce="n" + ) + two = oauth_auth.get_authorization_header( + "GET", "https://api.example.test/v2/other", timestamp=1, nonce="n" + ) + assert parse_header(one)["oauth_signature"] != parse_header(two)["oauth_signature"] + + def test_signature_binds_the_query_parameters( + self, oauth_auth: OAuthCredentials + ) -> None: + one = oauth_auth.get_authorization_header( + "GET", "https://api.example.test/v2/x?a=1", timestamp=1, nonce="n" + ) + two = oauth_auth.get_authorization_header( + "GET", "https://api.example.test/v2/x?a=2", timestamp=1, nonce="n" + ) + assert parse_header(one)["oauth_signature"] != parse_header(two)["oauth_signature"] + + def test_signature_binds_a_form_encoded_body( + self, oauth_auth: OAuthCredentials + ) -> None: + url = "https://api.example.test/v2/x" + plain = oauth_auth.get_authorization_header("POST", url, timestamp=1, nonce="n") + with_body = oauth_auth.get_authorization_header( + "POST", url, body_params=[("a", "1")], timestamp=1, nonce="n" + ) + assert parse_header(plain)["oauth_signature"] != parse_header(with_body)["oauth_signature"] + + def test_hmac_sha512_value_is_reproducible(self, oauth_auth: OAuthCredentials) -> None: + """Recompute the signature independently and compare.""" + url = "https://api.example.test/v2/self" + header = oauth_auth.get_authorization_header( + method="GET", url=url, timestamp=1700000000, nonce="fixed-nonce" + ) + params = parse_header(header) + + signed_params = [ + ("oauth_consumer_key", "consumer-key"), + ("oauth_token", "access-token"), + ("oauth_signature_method", "HMAC-SHA512"), + ("oauth_timestamp", "1700000000"), + ("oauth_nonce", "fixed-nonce"), + ("oauth_version", "1.0"), + ] + base_string = build_signature_base_string("GET", url, signed_params) + key = "consumer-secret&access-secret" + expected = base64.b64encode( + hmac.new(key.encode(), base_string.encode(), hashlib.sha512).digest() + ).decode() + + # The header value is percent-encoded; compare against the encoded form. + assert params["oauth_signature"] == percent_encode(expected) + + def test_hmac_sha256_is_selectable(self, oauth_auth: OAuthCredentials) -> None: + creds = OAuthCredentials( + consumer_key="k", + consumer_secret="cs", + token="t", + secret="ts", + signature_method=SignatureMethod.HMAC_SHA256, + ) + params = parse_header(creds.get_authorization_header("GET", "https://x.test/a")) + assert params["oauth_signature_method"] == "HMAC-SHA256" + + def test_plaintext_stays_available_as_an_explicit_compatibility_mode(self) -> None: + creds = OAuthCredentials( + consumer_key="k", + consumer_secret="cs", + token="t", + secret="ts", + signature_method=SignatureMethod.PLAINTEXT, + ) + params = parse_header(creds.get_authorization_header("GET", "https://x.test/a")) + assert params["oauth_signature_method"] == "PLAINTEXT" + assert params["oauth_signature"] == percent_encode("cs&ts") + # Even in legacy mode, the anti-replay parameters are present. + assert params["oauth_nonce"] + assert params["oauth_timestamp"] + + def test_default_method_is_hmac_sha512(self) -> None: + creds = OAuthCredentials(consumer_key="k", consumer_secret="cs", token="t", secret="ts") + assert creds.signature_method is SignatureMethod.HMAC_SHA512 + + def test_secrets_needing_encoding_are_handled(self) -> None: + creds = OAuthCredentials( + consumer_key="k", + consumer_secret="a b&c", + token="t", + secret="d/e", + signature_method=SignatureMethod.PLAINTEXT, + ) + params = parse_header(creds.get_authorization_header("GET", "https://x.test/a")) + assert params["oauth_signature"] == percent_encode("a%20b%26c&d%2Fe") + + +class TestApplyToRequest: + def test_signs_the_actual_request_url_and_method( + self, oauth_auth: OAuthCredentials + ) -> None: + request = httpx.Request("GET", "https://api.example.test/v2/self?a=1") + oauth_auth.apply_to_request(request) + assert request.headers["Authorization"].startswith("OAuth ") + + def test_form_body_takes_part_in_the_signature( + self, oauth_auth: OAuthCredentials + ) -> None: + url = "https://api.example.test/v2/oauth/x" + with_body = httpx.Request("POST", url, data={"a": "1"}) + without_body = httpx.Request("POST", url) + oauth_auth.apply_to_request(with_body) + oauth_auth.apply_to_request(without_body) + # Different signed material, so the two headers cannot be identical. + assert with_body.headers["Authorization"] != without_body.headers["Authorization"] + + def test_json_body_is_not_treated_as_form_parameters( + self, oauth_auth: OAuthCredentials + ) -> None: + request = httpx.Request("POST", "https://api.example.test/v2/x", json={"a": "1"}) + oauth_auth.apply_to_request(request) + assert "OAuth" in request.headers["Authorization"] + + +class TestApiToken: + def test_bearer_header(self, token_auth: ApiTokenCredentials) -> None: + assert token_auth.get_authorization_header("GET", "https://x.test") == ( + "Bearer test-bearer-token" + ) + + def test_applies_to_request(self, token_auth: ApiTokenCredentials) -> None: + request = httpx.Request("GET", "https://x.test/v2/self") + token_auth.apply_to_request(request) + assert request.headers["Authorization"] == "Bearer test-bearer-token" + + def test_default_base_url_is_the_api_bridge(self) -> None: + assert ApiTokenCredentials(token="t").get_base_url() == ( + "https://api-bridge.clever-cloud.com" + ) + + +class TestCredentialRedaction: + """Finding 2: secrets must not appear in a representation.""" + + def test_api_token_repr_hides_the_token(self) -> None: + creds = ApiTokenCredentials(token="BEARER_SECRET") + assert "BEARER_SECRET" not in repr(creds) + assert "redacted" in repr(creds) + + def test_oauth_repr_hides_both_secrets(self) -> None: + creds = OAuthCredentials( + consumer_key="KEY", + consumer_secret="CONSUMER_SECRET", + token="TOKEN", + secret="TOKEN_SECRET", + ) + text = repr(creds) + assert "CONSUMER_SECRET" not in text + assert "TOKEN_SECRET" not in text + # Non-secret identifiers stay visible, so the repr is still useful. + assert "KEY" in text + assert "TOKEN" in text + + def test_str_and_format_also_hide_secrets(self) -> None: + creds = OAuthCredentials( + consumer_key="KEY", + consumer_secret="CONSUMER_SECRET", + token="TOKEN", + secret="TOKEN_SECRET", + ) + assert "CONSUMER_SECRET" not in str(creds) + assert "CONSUMER_SECRET" not in f"{creds}" + assert "CONSUMER_SECRET" not in f"{creds!r}" + + def test_secrets_do_not_leak_through_a_container_repr(self) -> None: + """Structured logging often reprs a whole dict or list.""" + creds = ApiTokenCredentials(token="BEARER_SECRET") + assert "BEARER_SECRET" not in repr({"auth": creds}) + assert "BEARER_SECRET" not in repr([creds]) + + +class TestExpiration: + def test_credentials_without_expiration_never_expire(self) -> None: + creds = OAuthCredentials(consumer_key="k", consumer_secret="cs", token="t", secret="ts") + assert creds.is_expired() is False + + def test_expired_credentials_are_reported(self) -> None: + past = datetime.now(tz=UTC) - timedelta(hours=1) + creds = OAuthCredentials( + consumer_key="k", consumer_secret="cs", token="t", secret="ts", + expiration_date=past, + ) + assert creds.is_expired() is True + + def test_future_expiration_is_not_expired(self) -> None: + future = datetime.now(tz=UTC) + timedelta(hours=1) + creds = OAuthCredentials( + consumer_key="k", consumer_secret="cs", token="t", secret="ts", + expiration_date=future, + ) + assert creds.is_expired() is False diff --git a/tests/test_client.py b/tests/test_client.py new file mode 100644 index 0000000..d6ba2fb --- /dev/null +++ b/tests/test_client.py @@ -0,0 +1,706 @@ +"""Tests for the HTTP client (issue #3, findings 3, 4, 6, 7, 9).""" + +from __future__ import annotations + +from collections.abc import Callable + +import httpx +import pytest + +from clever_cloud import ( + ApiTokenCredentials, + AuthenticationError, + AuthorizationError, + CleverCloudClient, + HttpError, + InvalidResponseError, + NotFoundError, + OAuthCredentials, + RateLimitError, + TransportError, +) +from clever_cloud.client import _version_sort_key, encode_path_segment +from conftest import BASE_URL + +PROFILE = {"id": "user_1", "email": "user@example.test"} + + +class TestPathSegmentEncoding: + """Finding 3: identifiers were interpolated raw into URLs.""" + + @pytest.mark.parametrize( + ("raw", "expected"), + [ + ("orga_1234", "orga_1234"), + ("../self", "..%2Fself"), + ("x?override=1", "x%3Foverride%3D1"), + ("x/y", "x%2Fy"), + ("x#frag", "x%23frag"), + ("a b", "a%20b"), + ("100%", "100%25"), + ], + ) + def test_encoding(self, raw: str, expected: str) -> None: + assert encode_path_segment(raw, name="owner_id") == expected + + @pytest.mark.parametrize("bad", ["", ".", ".."]) + def test_rejected_values(self, bad: str) -> None: + with pytest.raises(ValueError, match="owner_id"): + encode_path_segment(bad, name="owner_id") + + def test_non_string_is_rejected(self) -> None: + with pytest.raises(ValueError, match="non-empty string"): + encode_path_segment(None, name="owner_id") + + async def test_traversal_cannot_change_the_route( + self, + make_client: Callable[..., CleverCloudClient], + record_requests: tuple[list[httpx.Request], Callable[..., object]], + ) -> None: + seen, factory = record_requests + client = make_client(factory(200, [])) + async with client: + await client.list_domains("../self", "app_1") + # raw_path is what actually goes on the wire; .path is the decoded view. + assert seen[0].url.raw_path == ( + b"/v2/organisations/..%2Fself/applications/app_1/vhosts" + ) + + async def test_query_injection_cannot_add_parameters( + self, + make_client: Callable[..., CleverCloudClient], + record_requests: tuple[list[httpx.Request], Callable[..., object]], + ) -> None: + seen, factory = record_requests + client = make_client(factory(200, [])) + async with client: + await client.list_domains("x?override=1", "app_1") + assert seen[0].url.params.get("override") is None + assert "override" not in str(seen[0].url.query) + + async def test_extra_path_segment_cannot_be_injected( + self, + make_client: Callable[..., CleverCloudClient], + record_requests: tuple[list[httpx.Request], Callable[..., object]], + ) -> None: + seen, factory = record_requests + client = make_client(factory(200, [])) + async with client: + await client.list_domains("orga_1", "x/y") + assert seen[0].url.raw_path == ( + b"/v2/organisations/orga_1/applications/x%2Fy/vhosts" + ) + + +class TestResponseHandling: + """Finding 4: empty bodies, redirections and undecodable JSON.""" + + @pytest.mark.parametrize("status", [200, 201, 202, 204, 205]) + async def test_empty_body_on_any_success_status( + self, make_client: Callable[..., CleverCloudClient], status: int + ) -> None: + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response( + status, content=b"", headers={"content-type": "application/json"} + ) + + client = make_client(handler) + async with client: + # create_networkgroup is documented to answer 202 with no body. + await client.create_networkgroup("orga_1", label="ng") + + async def test_json_content_type_with_empty_body_does_not_raise( + self, make_client: Callable[..., CleverCloudClient] + ) -> None: + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response(200, content=b"", headers={"content-type": "application/json"}) + + client = make_client(handler) + async with client: + assert await client.search_networkgroup_components("orga_1") == [] + + async def test_undecodable_json_is_wrapped_in_a_domain_exception( + self, make_client: Callable[..., CleverCloudClient] + ) -> None: + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response( + 200, content=b"{not json", headers={"content-type": "application/json"} + ) + + client = make_client(handler) + async with client: + with pytest.raises(InvalidResponseError, match="undecodable JSON"): + await client.get_profile() + + async def test_plain_text_body_is_returned_as_text( + self, make_client: Callable[..., CleverCloudClient] + ) -> None: + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response(200, text="hello", headers={"content-type": "text/plain"}) + + client = make_client(handler) + async with client: + with pytest.raises(InvalidResponseError, match="expected a JSON object"): + await client.get_profile() + + @pytest.mark.parametrize("status", [301, 302, 303, 307, 308]) + async def test_unfollowed_redirection_is_not_a_success( + self, make_client: Callable[..., CleverCloudClient], status: int + ) -> None: + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response(status, headers={"location": "https://evil.test/"}, text="body") + + client = make_client(handler) + async with client: + with pytest.raises(InvalidResponseError, match="Unexpected redirection"): + await client.get_profile() + + async def test_successful_profile( + self, make_client: Callable[..., CleverCloudClient] + ) -> None: + client = make_client(lambda r: httpx.Response(200, json=PROFILE)) + async with client: + profile = await client.get_profile() + assert profile.id == "user_1" + + +class TestErrorClassification: + @pytest.mark.parametrize( + ("status", "expected"), + [ + (401, AuthenticationError), + (403, AuthorizationError), + (404, NotFoundError), + (429, RateLimitError), + (400, HttpError), + (500, HttpError), + ], + ) + async def test_status_maps_to_exception( + self, + make_client: Callable[..., CleverCloudClient], + status: int, + expected: type[Exception], + ) -> None: + client = make_client(lambda r: httpx.Response(status, text="boom")) + async with client: + with pytest.raises(expected) as excinfo: + await client.get_profile() + assert excinfo.value.status_code == status # type: ignore[attr-defined] + + async def test_403_is_authorization_not_authentication( + self, make_client: Callable[..., CleverCloudClient] + ) -> None: + """403 used to be reported as an authentication failure.""" + client = make_client(lambda r: httpx.Response(403, text="nope")) + async with client: + with pytest.raises(AuthorizationError): + await client.get_profile() + assert not issubclass(AuthorizationError, AuthenticationError) + + async def test_large_error_body_is_truncated( + self, make_client: Callable[..., CleverCloudClient] + ) -> None: + big = "x" * 100_000 + client = make_client(lambda r: httpx.Response(500, text=big)) + async with client: + with pytest.raises(HttpError) as excinfo: + await client.get_profile() + assert len(excinfo.value.response_body) < 3000 + assert "truncated" in excinfo.value.response_body + + async def test_error_message_does_not_embed_the_whole_body( + self, make_client: Callable[..., CleverCloudClient] + ) -> None: + client = make_client(lambda r: httpx.Response(500, text="secret-token-in-body")) + async with client: + with pytest.raises(HttpError) as excinfo: + await client.get_profile() + assert "secret-token-in-body" not in str(excinfo.value) + + async def test_rate_limit_exposes_retry_after( + self, make_client: Callable[..., CleverCloudClient] + ) -> None: + client = make_client( + lambda r: httpx.Response(429, headers={"retry-after": "12"}, text="slow down") + ) + async with client: + with pytest.raises(RateLimitError) as excinfo: + await client.get_profile() + assert excinfo.value.retry_after == 12.0 + + async def test_transport_error_is_wrapped_in_the_sdk_hierarchy( + self, make_client: Callable[..., CleverCloudClient] + ) -> None: + """A network failure used to escape CleverCloudError entirely.""" + def handler(request: httpx.Request) -> httpx.Response: + raise httpx.ConnectError("connection refused") + + client = make_client(handler) + async with client: + with pytest.raises(TransportError, match="ConnectError"): + await client.get_profile() + + +class TestRequestShape: + """Finding 7: a JSON Content-Type was forced onto every request.""" + + async def test_get_has_no_content_type( + self, + make_client: Callable[..., CleverCloudClient], + record_requests: tuple[list[httpx.Request], Callable[..., object]], + ) -> None: + seen, factory = record_requests + client = make_client(factory(200, PROFILE)) + async with client: + await client.get_profile() + assert "content-type" not in seen[0].headers + + async def test_json_body_gets_a_json_content_type( + self, + make_client: Callable[..., CleverCloudClient], + record_requests: tuple[list[httpx.Request], Callable[..., object]], + ) -> None: + seen, factory = record_requests + client = make_client(factory(202)) + async with client: + await client.create_networkgroup("orga_1", label="ng") + assert seen[0].headers["content-type"] == "application/json" + + async def test_form_body_gets_a_form_content_type( + self, + make_client: Callable[..., CleverCloudClient], + record_requests: tuple[list[httpx.Request], Callable[..., object]], + ) -> None: + """data={"x": "y"} used to be sent form-encoded under a JSON Content-Type.""" + seen, factory = record_requests + client = make_client(factory(200, PROFILE)) + async with client: + await client._request("POST", "/v2/x", data={"x": "y"}) + assert seen[0].headers["content-type"] == "application/x-www-form-urlencoded" + assert seen[0].content == b"x=y" + + async def test_authorization_header_is_applied( + self, + make_client: Callable[..., CleverCloudClient], + record_requests: tuple[list[httpx.Request], Callable[..., object]], + oauth_auth: OAuthCredentials, + ) -> None: + seen, factory = record_requests + client = make_client(factory(200, PROFILE), auth=oauth_auth) + async with client: + await client.get_profile() + assert seen[0].headers["authorization"].startswith("OAuth ") + + +class TestRetries: + @pytest.mark.parametrize("status", [429, 502, 503, 504]) + async def test_idempotent_request_is_retried( + self, token_auth: ApiTokenCredentials, status: int + ) -> None: + calls: list[int] = [] + + def handler(request: httpx.Request) -> httpx.Response: + calls.append(1) + if len(calls) < 3: + return httpx.Response(status, headers={"retry-after": "0"}) + return httpx.Response(200, json=PROFILE) + + client = CleverCloudClient( + token_auth, + base_url=BASE_URL, + max_retries=2, + transport=httpx.MockTransport(handler), + ) + async with client: + profile = await client.get_profile() + assert profile.id == "user_1" + assert len(calls) == 3 + + async def test_non_idempotent_request_is_not_retried( + self, token_auth: ApiTokenCredentials + ) -> None: + calls: list[int] = [] + + def handler(request: httpx.Request) -> httpx.Response: + calls.append(1) + return httpx.Response(503, headers={"retry-after": "0"}) + + client = CleverCloudClient( + token_auth, + base_url=BASE_URL, + max_retries=3, + transport=httpx.MockTransport(handler), + ) + async with client: + with pytest.raises(HttpError): + await client.create_networkgroup("orga_1", label="ng") + assert len(calls) == 1 + + async def test_retries_are_bounded(self, token_auth: ApiTokenCredentials) -> None: + calls: list[int] = [] + + def handler(request: httpx.Request) -> httpx.Response: + calls.append(1) + return httpx.Response(503, headers={"retry-after": "0"}) + + client = CleverCloudClient( + token_auth, + base_url=BASE_URL, + max_retries=2, + transport=httpx.MockTransport(handler), + ) + async with client: + with pytest.raises(HttpError): + await client.get_profile() + assert len(calls) == 3 + + async def test_transport_error_is_retried_then_raised( + self, token_auth: ApiTokenCredentials + ) -> None: + calls: list[int] = [] + + def handler(request: httpx.Request) -> httpx.Response: + calls.append(1) + raise httpx.ConnectTimeout("timeout") + + client = CleverCloudClient( + token_auth, + base_url=BASE_URL, + max_retries=1, + max_retry_wait=0, + transport=httpx.MockTransport(handler), + ) + async with client: + with pytest.raises(TransportError): + await client.get_profile() + assert len(calls) == 2 + + async def test_each_retry_is_signed_again(self, oauth_auth: OAuthCredentials) -> None: + """An OAuth nonce must never be reused across attempts.""" + headers: list[str] = [] + + def handler(request: httpx.Request) -> httpx.Response: + headers.append(request.headers["authorization"]) + if len(headers) < 2: + return httpx.Response(503, headers={"retry-after": "0"}) + return httpx.Response(200, json=PROFILE) + + client = CleverCloudClient( + oauth_auth, + base_url=BASE_URL, + max_retries=1, + transport=httpx.MockTransport(handler), + ) + async with client: + await client.get_profile() + assert headers[0] != headers[1] + + +class TestVersionOrdering: + """Finding 6: versions were compared as strings, so "9" beat "10".""" + + @pytest.mark.parametrize( + ("lower", "higher"), + [("9", "10"), ("1.9", "1.10"), ("8", "11"), ("3.9", "3.12"), ("1.0-beta", "1.0")], + ) + def test_natural_ordering(self, lower: str, higher: str) -> None: + assert _version_sort_key(lower) < _version_sort_key(higher) + + async def test_resolve_instance_slug_picks_the_highest_version( + self, make_client: Callable[..., CleverCloudClient] + ) -> None: + instances = [ + {"enabled": True, "type": "node", "version": "9", + "variant": {"slug": "node", "id": "var_9"}}, + {"enabled": True, "type": "node", "version": "10", + "variant": {"slug": "node", "id": "var_10"}}, + ] + client = make_client(lambda r: httpx.Response(200, json=instances)) + async with client: + resolved = await client.resolve_instance_slug("node") + assert resolved == ("node", "10", "var_10") + + async def test_disabled_instances_are_ignored( + self, make_client: Callable[..., CleverCloudClient] + ) -> None: + instances = [ + {"enabled": False, "type": "node", "version": "20", + "variant": {"slug": "node", "id": "var_20"}}, + {"enabled": True, "type": "node", "version": "18", + "variant": {"slug": "node", "id": "var_18"}}, + ] + client = make_client(lambda r: httpx.Response(200, json=instances)) + async with client: + assert (await client.resolve_instance_slug("node"))[1] == "18" + + async def test_unknown_slug_lists_the_available_ones( + self, make_client: Callable[..., CleverCloudClient] + ) -> None: + instances = [ + {"enabled": True, "type": "node", "version": "20", + "variant": {"slug": "node", "id": "v"}} + ] + client = make_client(lambda r: httpx.Response(200, json=instances)) + async with client: + with pytest.raises(ValueError, match="Unknown instance slug: ruby"): + await client.resolve_instance_slug("ruby") + + +class TestInstanceCatalogueCaching: + async def test_catalogue_is_fetched_once_per_client( + self, make_client: Callable[..., CleverCloudClient] + ) -> None: + calls: list[int] = [] + instances = [ + {"enabled": True, "type": "node", "version": "20", + "variant": {"slug": "node", "id": "var_20"}} + ] + + def handler(request: httpx.Request) -> httpx.Response: + if request.url.path == "/v2/products/instances": + calls.append(1) + return httpx.Response(200, json=instances) + return httpx.Response(200, json={"id": "app_1", "name": "n"}) + + client = make_client(handler) + async with client: + await client.create_application("orga_1", "a", instance_slug="node") + await client.create_application("orga_1", "b", instance_slug="node") + assert len(calls) == 1 + + async def test_refresh_forces_a_new_fetch( + self, make_client: Callable[..., CleverCloudClient] + ) -> None: + calls: list[int] = [] + + def handler(request: httpx.Request) -> httpx.Response: + calls.append(1) + return httpx.Response(200, json=[]) + + client = make_client(handler) + async with client: + await client.list_instances() + await client.list_instances() + await client.list_instances(refresh=True) + assert len(calls) == 2 + + +class TestTlsConfiguration: + """Finding 9: deprecated HTTPX TLS APIs and clear-text base URLs.""" + + def test_http_base_url_is_refused_by_default( + self, token_auth: ApiTokenCredentials + ) -> None: + with pytest.raises(ValueError, match="must be sent over HTTPS"): + CleverCloudClient(token_auth, base_url="http://api.example.test") + + def test_http_base_url_requires_an_explicit_override( + self, token_auth: ApiTokenCredentials + ) -> None: + client = CleverCloudClient( + token_auth, base_url="http://localhost:8080", allow_insecure_http=True + ) + assert client._base_url == "http://localhost:8080" + + def test_credentials_default_base_url_is_https( + self, token_auth: ApiTokenCredentials + ) -> None: + CleverCloudClient(ApiTokenCredentials(token="t")) + + def test_ca_bundle_builds_an_ssl_context_without_deprecation( + self, token_auth: ApiTokenCredentials, tmp_path: object + ) -> None: + import ssl + + client = CleverCloudClient(token_auth, base_url=BASE_URL) + context = client._build_ssl_context() + assert isinstance(context, ssl.SSLContext) + assert context.verify_mode is ssl.CERT_REQUIRED + + def test_verify_ssl_false_disables_verification( + self, token_auth: ApiTokenCredentials + ) -> None: + client = CleverCloudClient(token_auth, base_url=BASE_URL, verify_ssl=False) + assert client._build_ssl_context() is False + + def test_no_deprecation_warning_when_creating_the_transport( + self, token_auth: ApiTokenCredentials, recwarn: pytest.WarningsRecorder + ) -> None: + client = CleverCloudClient(token_auth, base_url=BASE_URL) + client._get_client() + assert [w for w in recwarn if issubclass(w.category, DeprecationWarning)] == [] + + +class TestEndpoints: + async def test_list_domains( + self, make_client: Callable[..., CleverCloudClient] + ) -> None: + client = make_client( + lambda r: httpx.Response(200, json=[{"fqdn": "a.test"}, {"fqdn": "b.test/"}]) + ) + async with client: + domains = await client.list_domains("orga_1", "app_1") + assert [d.domain for d in domains] == ["a.test", "b.test"] + + async def test_list_domains_no_longer_hides_a_missing_application( + self, make_client: Callable[..., CleverCloudClient] + ) -> None: + """A 404 used to be reported as "this application has no domain".""" + client = make_client(lambda r: httpx.Response(404, text="app not found")) + async with client: + with pytest.raises(NotFoundError): + await client.list_domains("orga_1", "unknown_app") + + async def test_get_primary_domain( + self, make_client: Callable[..., CleverCloudClient] + ) -> None: + client = make_client(lambda r: httpx.Response(200, json={"fqdn": "a.test"})) + async with client: + domain = await client.get_primary_domain("orga_1", "app_1") + assert domain is not None + assert domain.is_primary is True + + async def test_get_primary_domain_surfaces_404( + self, make_client: Callable[..., CleverCloudClient] + ) -> None: + client = make_client(lambda r: httpx.Response(404)) + async with client: + with pytest.raises(NotFoundError): + await client.get_primary_domain("orga_1", "app_1") + + async def test_create_tcp_redirection( + self, + make_client: Callable[..., CleverCloudClient], + record_requests: tuple[list[httpx.Request], Callable[..., object]], + ) -> None: + seen, factory = record_requests + client = make_client(factory(200, {"namespace": "cleverapps", "port": 4242})) + async with client: + redir = await client.create_tcp_redirection("orga_1", "app_1") + assert redir.port == 4242 + assert seen[0].url.path.endswith("/tcpRedirs") + + async def test_redeploy_sends_optional_parameters( + self, + make_client: Callable[..., CleverCloudClient], + record_requests: tuple[list[httpx.Request], Callable[..., object]], + ) -> None: + seen, factory = record_requests + client = make_client(factory(200)) + async with client: + await client.redeploy_application("orga_1", "app_1", commit="abc", use_cache=False) + assert seen[0].url.params["commit"] == "abc" + assert seen[0].url.params["useCache"] == "false" + + async def test_create_application_body( + self, + make_client: Callable[..., CleverCloudClient], + record_requests: tuple[list[httpx.Request], Callable[..., object]], + ) -> None: + import json as jsonlib + + seen, factory = record_requests + client = make_client(factory(200, {"id": "app_1", "name": "my-app"})) + async with client: + app = await client.create_application( + "orga_1", + "my-app", + instance_type="node", + instance_version="20", + instance_variant="var_1", + environment=[{"name": "K", "value": "V"}], + ) + body = jsonlib.loads(seen[0].content) + assert body["name"] == "my-app" + assert body["env"] == {"K": "V"} + assert app.id == "app_1" + + async def test_create_application_requires_instance_information( + self, make_client: Callable[..., CleverCloudClient] + ) -> None: + client = make_client(lambda r: httpx.Response(200, json={})) + async with client: + with pytest.raises(ValueError, match="instance_slug"): + await client.create_application("orga_1", "my-app") + + async def test_networkgroup_roundtrip( + self, + make_client: Callable[..., CleverCloudClient], + record_requests: tuple[list[httpx.Request], Callable[..., object]], + ) -> None: + seen, factory = record_requests + ng = {"id": "ng_1", "ownerId": "orga_1", "label": "l", "version": 1} + client = make_client(factory(200, ng)) + async with client: + result = await client.get_networkgroup("orga_1", "ng_1") + assert result.id == "ng_1" + assert seen[0].url.path == ( + "/v4/networkgroups/organisations/orga_1/networkgroups/ng_1" + ) + + async def test_create_networkgroup_member_body( + self, + make_client: Callable[..., CleverCloudClient], + record_requests: tuple[list[httpx.Request], Callable[..., object]], + ) -> None: + import json as jsonlib + + from clever_cloud import MemberKind + + seen, factory = record_requests + client = make_client(factory(202)) + async with client: + await client.create_networkgroup_member( + "orga_1", + "ng_1", + member_id="app_1", + domain_name="d.members", + kind=MemberKind.APPLICATION, + label="app", + ) + body = jsonlib.loads(seen[0].content) + assert body == { + "id": "app_1", + "domainName": "d.members", + "kind": "APPLICATION", + "label": "app", + } + + async def test_list_networkgroup_peers_handles_a_null_body( + self, make_client: Callable[..., CleverCloudClient] + ) -> None: + client = make_client(lambda r: httpx.Response(200, json=None)) + async with client: + assert await client.list_networkgroup_peers("orga_1", "ng_1") == [] + + async def test_delete_networkgroup_peer_uses_the_delete_verb( + self, + make_client: Callable[..., CleverCloudClient], + record_requests: tuple[list[httpx.Request], Callable[..., object]], + ) -> None: + seen, factory = record_requests + client = make_client(factory(204)) + async with client: + await client.delete_networkgroup_peer("orga_1", "ng_1", "peer_1") + assert seen[0].method == "DELETE" + assert seen[0].url.path.endswith("/peers/peer_1") + + +class TestClientLifecycle: + async def test_close_is_idempotent( + self, make_client: Callable[..., CleverCloudClient] + ) -> None: + client = make_client(lambda r: httpx.Response(200, json=PROFILE)) + async with client: + await client.get_profile() + await client.close() + + async def test_client_is_reusable_after_close( + self, make_client: Callable[..., CleverCloudClient] + ) -> None: + client = make_client(lambda r: httpx.Response(200, json=PROFILE)) + async with client: + await client.get_profile() + async with client: + assert (await client.get_profile()).id == "user_1" diff --git a/tests/test_models.py b/tests/test_models.py new file mode 100644 index 0000000..8f9f781 --- /dev/null +++ b/tests/test_models.py @@ -0,0 +1,274 @@ +"""Tests for response model parsing (issue #3, finding 5).""" + +from __future__ import annotations + +from datetime import UTC, datetime + +import pytest + +from clever_cloud import ( + Application, + Domain, + InvalidResponseError, + MemberKind, + NetworkGroup, + NetworkGroupMember, + NetworkGroupPeer, + PeerCreated, + PeerKind, + Profile, + TcpRedirection, +) + +MINIMAL_PROFILE = {"id": "user_1", "email": "user@example.test"} + + +class TestDateParsing: + """A missing date must never be replaced with the current time.""" + + def test_missing_date_is_none_not_now(self) -> None: + profile = Profile.from_api_response(MINIMAL_PROFILE) + assert profile.creation_date is None + + def test_empty_string_date_is_none(self) -> None: + profile = Profile.from_api_response({**MINIMAL_PROFILE, "creationDate": ""}) + assert profile.creation_date is None + + def test_epoch_milliseconds(self) -> None: + profile = Profile.from_api_response( + {**MINIMAL_PROFILE, "creationDate": 1700000000000} + ) + assert profile.creation_date == datetime(2023, 11, 14, 22, 13, 20, tzinfo=UTC) + + def test_iso_string_with_offset_is_converted_to_utc(self) -> None: + profile = Profile.from_api_response( + {**MINIMAL_PROFILE, "creationDate": "2023-11-14T23:13:20+01:00"} + ) + assert profile.creation_date == datetime(2023, 11, 14, 22, 13, 20, tzinfo=UTC) + + def test_iso_string_with_z_suffix(self) -> None: + profile = Profile.from_api_response( + {**MINIMAL_PROFILE, "creationDate": "2023-11-14T22:13:20Z"} + ) + assert profile.creation_date == datetime(2023, 11, 14, 22, 13, 20, tzinfo=UTC) + + def test_naive_iso_string_is_assumed_utc_not_left_naive(self) -> None: + """Integer and string dates used to produce inconsistent awareness.""" + profile = Profile.from_api_response( + {**MINIMAL_PROFILE, "creationDate": "2023-11-14T22:13:20"} + ) + assert profile.creation_date is not None + assert profile.creation_date.tzinfo is not None + assert profile.creation_date == datetime(2023, 11, 14, 22, 13, 20, tzinfo=UTC) + + def test_unparsable_date_is_rejected(self) -> None: + with pytest.raises(InvalidResponseError, match="invalid ISO-8601"): + Profile.from_api_response({**MINIMAL_PROFILE, "creationDate": "not-a-date"}) + + def test_wrong_type_date_is_rejected(self) -> None: + with pytest.raises(InvalidResponseError, match="unsupported date type"): + Profile.from_api_response({**MINIMAL_PROFILE, "creationDate": ["x"]}) + + +class TestProfile: + def test_required_fields_are_enforced(self) -> None: + with pytest.raises(InvalidResponseError, match="'id'"): + Profile.from_api_response({"email": "a@b.test"}) + with pytest.raises(InvalidResponseError, match="'email'"): + Profile.from_api_response({"id": "user_1"}) + + def test_missing_optional_fields_are_none_not_empty_strings(self) -> None: + profile = Profile.from_api_response(MINIMAL_PROFILE) + assert profile.name is None + assert profile.country is None + assert profile.preferred_mfa is None + + def test_full_payload(self) -> None: + profile = Profile.from_api_response( + { + "id": "user_1", + "email": "user@example.test", + "name": "Ada", + "phone": "+33100000000", + "country": "FR", + "lang": "fr", + "emailValidated": True, + "admin": False, + "canPay": True, + "hasPassword": True, + "preferredMFA": "TOTP", + "oauthApps": ["github"], + } + ) + assert profile.name == "Ada" + assert profile.email_validated is True + assert profile.is_linked_to_github is True + assert profile.preferred_mfa == "TOTP" + + def test_github_link_absent(self) -> None: + profile = Profile.from_api_response({**MINIMAL_PROFILE, "oauthApps": []}) + assert profile.is_linked_to_github is False + + def test_non_object_payload_is_rejected(self) -> None: + with pytest.raises(InvalidResponseError, match="expected a JSON object"): + Profile.from_api_response(["not", "an", "object"]) + + +class TestDomain: + def test_parses_and_strips_trailing_slash(self) -> None: + domain = Domain.from_api_response({"fqdn": "app.example.test/"}) + assert domain.domain == "app.example.test" + assert domain.is_primary is False + + def test_primary_flag(self) -> None: + domain = Domain.from_api_response({"fqdn": "a.test"}, is_primary=True) + assert domain.is_primary is True + + def test_missing_fqdn_is_rejected(self) -> None: + with pytest.raises(InvalidResponseError, match="'fqdn'"): + Domain.from_api_response({}) + + +class TestTcpRedirection: + def test_parses_namespace_and_port(self) -> None: + redir = TcpRedirection.from_api_response({"namespace": "cleverapps", "port": 4242}) + assert redir.namespace == "cleverapps" + assert redir.port == 4242 + + def test_missing_port_is_rejected_rather_than_defaulting_to_zero(self) -> None: + with pytest.raises(InvalidResponseError, match="'port'"): + TcpRedirection.from_api_response({"namespace": "cleverapps"}) + + def test_boolean_is_not_accepted_as_a_port(self) -> None: + with pytest.raises(InvalidResponseError, match="'port'"): + TcpRedirection.from_api_response({"namespace": "n", "port": True}) + + +class TestApplication: + def test_reads_instance_from_the_nested_object(self) -> None: + app = Application.from_api_response( + { + "id": "app_1", + "name": "my-app", + "zone": "par", + "instance": {"type": "node", "version": "20", "variant": {"id": "var_1"}}, + "creationDate": 1700000000000, + } + ) + assert app.instance_type == "node" + assert app.instance_version == "20" + assert app.instance_variant == "var_1" + + def test_falls_back_to_the_flat_fields(self) -> None: + app = Application.from_api_response( + { + "id": "app_1", + "name": "my-app", + "instanceType": "python", + "instanceVersion": "3.12", + "instanceVariant": "var_2", + } + ) + assert app.instance_type == "python" + assert app.instance_variant == "var_2" + + def test_required_fields_are_enforced(self) -> None: + with pytest.raises(InvalidResponseError, match="'id'"): + Application.from_api_response({"name": "x"}) + with pytest.raises(InvalidResponseError, match="'name'"): + Application.from_api_response({"id": "app_1"}) + + def test_unknown_instance_shape_does_not_crash(self) -> None: + app = Application.from_api_response( + {"id": "app_1", "name": "x", "instance": "unexpected"} + ) + assert app.instance_type is None + + +class TestNetworkGroupModels: + def test_member_parsing(self) -> None: + member = NetworkGroupMember.from_api_response( + {"id": "app_1", "domainName": "a.members", "kind": "APPLICATION", "label": "app"} + ) + assert member.kind is MemberKind.APPLICATION + assert member.label == "app" + + def test_unknown_member_kind_is_rejected(self) -> None: + """The old code silently coerced an unknown kind into EXTERNAL.""" + with pytest.raises(InvalidResponseError, match="unknown value"): + NetworkGroupMember.from_api_response( + {"id": "x", "domainName": "d", "kind": "SOMETHING_NEW"} + ) + + def test_missing_member_kind_is_rejected(self) -> None: + with pytest.raises(InvalidResponseError, match="'kind'"): + NetworkGroupMember.from_api_response({"id": "x", "domainName": "d"}) + + def test_clever_peer_is_detected_by_its_hv_field(self) -> None: + peer = NetworkGroupPeer.from_api_response( + { + "id": "peer_1", + "parentMember": "app_1", + "hv": "hv-1", + "endpoint": {"privateAddress": "10.0.0.1", "publicAddress": "1.2.3.4"}, + } + ) + assert peer.kind is PeerKind.CLEVER + assert peer.endpoint is not None + assert peer.endpoint.private_address == "10.0.0.1" + + def test_external_peer_has_no_hv(self) -> None: + peer = NetworkGroupPeer.from_api_response( + {"id": "peer_2", "parentMember": "m_1", "publicKey": "pk"} + ) + assert peer.kind is PeerKind.EXTERNAL + assert peer.endpoint is None + + def test_networkgroup_parsing(self) -> None: + ng = NetworkGroup.from_api_response( + { + "id": "ng_1", + "ownerId": "orga_1", + "label": "my-ng", + "version": 3, + "members": [{"id": "app_1", "domainName": "d", "kind": "APPLICATION"}], + "peers": [{"id": "peer_1", "parentMember": "app_1"}], + "tags": ["a", "b"], + } + ) + assert ng.version == 3 + assert len(ng.members) == 1 + assert len(ng.peers) == 1 + assert ng.tags == ("a", "b") + + def test_collections_are_immutable(self) -> None: + """frozen=True advertised an immutability the lists did not provide.""" + ng = NetworkGroup.from_api_response( + {"id": "ng_1", "ownerId": "orga_1", "label": "l", "version": 1} + ) + assert isinstance(ng.members, tuple) + assert isinstance(ng.peers, tuple) + assert isinstance(ng.tags, tuple) + with pytest.raises(AttributeError): + ng.tags.append("x") # type: ignore[attr-defined] + + def test_null_collections_are_empty(self) -> None: + ng = NetworkGroup.from_api_response( + {"id": "ng_1", "ownerId": "orga_1", "label": "l", "version": 1, + "members": None, "peers": None, "tags": None} + ) + assert ng.members == () + + def test_peer_created_requires_an_id(self) -> None: + with pytest.raises(InvalidResponseError, match="'id'"): + PeerCreated.from_api_response({}) + + def test_peer_created_accepts_the_peer_id_alias(self) -> None: + created = PeerCreated.from_api_response({"peerId": "peer_9"}) + assert created.peer_id == "peer_9" + + def test_peer_created_raw_mapping_is_read_only(self) -> None: + created = PeerCreated.from_api_response({"id": "peer_1", "extra": 1}) + assert created.raw["extra"] == 1 + with pytest.raises(TypeError): + created.raw["extra"] = 2 # type: ignore[index] diff --git a/tests/test_networkgroups.py b/tests/test_networkgroups.py new file mode 100644 index 0000000..f4e80f5 --- /dev/null +++ b/tests/test_networkgroups.py @@ -0,0 +1,267 @@ +"""Endpoint-level tests for the NetworkGroups API surface.""" + +from __future__ import annotations + +import json as jsonlib +from collections.abc import Callable + +import httpx +import pytest + +from clever_cloud import ( + CleverCloudClient, + InvalidResponseError, + MemberKind, + PeerKind, + PeerRole, +) + +NG_ROOT = "/v4/networkgroups/organisations/orga_1/networkgroups" + + +Handler = Callable[[httpx.Request], httpx.Response] + + +def capture( + status: int = 200, payload: object = None +) -> tuple[list[httpx.Request], Handler]: + seen: list[httpx.Request] = [] + + def handler(request: httpx.Request) -> httpx.Response: + seen.append(request) + if payload is None: + return httpx.Response(status, content=b"") + return httpx.Response(status, json=payload) + + return seen, handler + + +class TestCreateNetworkGroup: + async def test_minimal_body(self, make_client: Callable[..., CleverCloudClient]) -> None: + seen, handler = capture(202) + client = make_client(handler) + async with client: + await client.create_networkgroup("orga_1", label="my-ng") + assert seen[0].method == "POST" + assert seen[0].url.path == NG_ROOT + assert jsonlib.loads(seen[0].content) == {"label": "my-ng"} + + async def test_full_body(self, make_client: Callable[..., CleverCloudClient]) -> None: + seen, handler = capture(202) + client = make_client(handler) + async with client: + await client.create_networkgroup( + "orga_1", + label="my-ng", + description="desc", + ng_id="ng_custom", + tags=["a"], + members=[{"id": "app_1", "domainName": "d", "kind": "APPLICATION"}], + ) + body = jsonlib.loads(seen[0].content) + assert body["id"] == "ng_custom" + assert body["description"] == "desc" + assert body["tags"] == ["a"] + assert body["members"][0]["id"] == "app_1" + + async def test_owner_id_is_encoded( + self, make_client: Callable[..., CleverCloudClient] + ) -> None: + seen, handler = capture(202) + client = make_client(handler) + async with client: + await client.create_networkgroup("../orga_2", label="x") + assert b"..%2Forga_2" in seen[0].url.raw_path + + +class TestDeleteNetworkGroup: + async def test_delete(self, make_client: Callable[..., CleverCloudClient]) -> None: + seen, handler = capture(204) + client = make_client(handler) + async with client: + await client.delete_networkgroup("orga_1", "ng_1") + assert seen[0].method == "DELETE" + assert seen[0].url.path == f"{NG_ROOT}/ng_1" + + +class TestSearchComponents: + async def test_query_is_sent(self, make_client: Callable[..., CleverCloudClient]) -> None: + seen, handler = capture(200, [{"id": "ng_1"}]) + client = make_client(handler) + async with client: + result = await client.search_networkgroup_components("orga_1", query="ng") + assert seen[0].url.params["query"] == "ng" + assert result == [{"id": "ng_1"}] + + async def test_no_query(self, make_client: Callable[..., CleverCloudClient]) -> None: + seen, handler = capture(200, []) + client = make_client(handler) + async with client: + await client.search_networkgroup_components("orga_1") + assert not seen[0].url.params + + async def test_unexpected_shape_is_rejected( + self, make_client: Callable[..., CleverCloudClient] + ) -> None: + client = make_client(lambda r: httpx.Response(200, json={"not": "a list"})) + async with client: + with pytest.raises(InvalidResponseError, match="Expected a list"): + await client.search_networkgroup_components("orga_1") + + +class TestMembers: + async def test_get_member(self, make_client: Callable[..., CleverCloudClient]) -> None: + seen, handler = capture( + 200, {"id": "app_1", "domainName": "d.members", "kind": "APPLICATION"} + ) + client = make_client(handler) + async with client: + member = await client.get_networkgroup_member("orga_1", "ng_1", "app_1") + assert member.kind is MemberKind.APPLICATION + assert seen[0].url.path == f"{NG_ROOT}/ng_1/members/app_1" + + async def test_delete_member(self, make_client: Callable[..., CleverCloudClient]) -> None: + seen, handler = capture(204) + client = make_client(handler) + async with client: + await client.delete_networkgroup_member("orga_1", "ng_1", "app_1") + assert seen[0].method == "DELETE" + assert seen[0].url.path == f"{NG_ROOT}/ng_1/members/app_1" + + async def test_kind_accepts_a_plain_string( + self, make_client: Callable[..., CleverCloudClient] + ) -> None: + seen, handler = capture(202) + client = make_client(handler) + async with client: + await client.create_networkgroup_member( + "orga_1", "ng_1", member_id="app_1", domain_name="d", kind="ADDON" + ) + assert jsonlib.loads(seen[0].content)["kind"] == "ADDON" + + async def test_label_is_omitted_when_absent( + self, make_client: Callable[..., CleverCloudClient] + ) -> None: + seen, handler = capture(202) + client = make_client(handler) + async with client: + await client.create_networkgroup_member( + "orga_1", "ng_1", member_id="app_1", domain_name="d", kind=MemberKind.ADDON + ) + assert "label" not in jsonlib.loads(seen[0].content) + + +class TestPeers: + async def test_list_peers(self, make_client: Callable[..., CleverCloudClient]) -> None: + seen, handler = capture( + 200, [{"id": "peer_1", "parentMember": "app_1", "hv": "hv1"}] + ) + client = make_client(handler) + async with client: + peers = await client.list_networkgroup_peers("orga_1", "ng_1") + assert peers[0].kind is PeerKind.CLEVER + assert seen[0].url.path == f"{NG_ROOT}/ng_1/peers" + + async def test_list_peers_rejects_an_unexpected_shape( + self, make_client: Callable[..., CleverCloudClient] + ) -> None: + client = make_client(lambda r: httpx.Response(200, json={"x": 1})) + async with client: + with pytest.raises(InvalidResponseError, match="Expected a list of peers"): + await client.list_networkgroup_peers("orga_1", "ng_1") + + async def test_get_peer(self, make_client: Callable[..., CleverCloudClient]) -> None: + seen, handler = capture(200, {"id": "peer_1", "parentMember": "app_1"}) + client = make_client(handler) + async with client: + peer = await client.get_networkgroup_peer("orga_1", "ng_1", "peer_1") + assert peer.id == "peer_1" + assert seen[0].url.path == f"{NG_ROOT}/ng_1/peers/peer_1" + + async def test_create_peer_body( + self, make_client: Callable[..., CleverCloudClient] + ) -> None: + seen, handler = capture(200, {"id": "peer_1"}) + client = make_client(handler) + async with client: + created = await client.create_networkgroup_peer( + "orga_1", + "ng_1", + peer_id="peer_1", + parent_member="app_1", + peer_role=PeerRole.SERVER, + public_key="pk", + ip="10.0.0.1", + port=51820, + hostname="h", + label="l", + hv="hv1", + parent_event="evt", + ) + body = jsonlib.loads(seen[0].content) + assert body["peerRole"] == "SERVER" + assert body["peerKind"] == "CLEVER" + assert body["publicKey"] == "pk" + assert body["port"] == 51820 + assert body["parentEvent"] == "evt" + assert created.peer_id == "peer_1" + + async def test_create_peer_omits_absent_optional_fields( + self, make_client: Callable[..., CleverCloudClient] + ) -> None: + seen, handler = capture(200, {"id": "peer_1"}) + client = make_client(handler) + async with client: + await client.create_networkgroup_peer( + "orga_1", "ng_1", peer_id="p", parent_member="m", peer_role="CLIENT" + ) + body = jsonlib.loads(seen[0].content) + assert set(body) == {"id", "parentMember", "peerRole", "peerKind"} + + async def test_create_external_peer( + self, make_client: Callable[..., CleverCloudClient] + ) -> None: + seen, handler = capture(200, {"id": "peer_ext"}) + client = make_client(handler) + async with client: + created = await client.create_networkgroup_external_peer( + "orga_1", + "ng_1", + parent_member="m_1", + peer_role=PeerRole.CLIENT, + public_key="pk", + label="laptop", + ip="1.2.3.4", + port=51820, + hostname="host", + parent_event="evt", + ) + assert seen[0].url.path == f"{NG_ROOT}/ng_1/external-peers" + body = jsonlib.loads(seen[0].content) + assert body["peerRole"] == "CLIENT" + assert body["label"] == "laptop" + assert created.peer_id == "peer_ext" + + async def test_external_peer_omits_absent_optional_fields( + self, make_client: Callable[..., CleverCloudClient] + ) -> None: + seen, handler = capture(200, {"id": "p"}) + client = make_client(handler) + async with client: + await client.create_networkgroup_external_peer( + "orga_1", "ng_1", parent_member="m", peer_role="CLIENT", + public_key="pk", label="l", + ) + body = jsonlib.loads(seen[0].content) + assert set(body) == {"parentMember", "peerRole", "publicKey", "label"} + + async def test_peer_response_without_id_is_rejected( + self, make_client: Callable[..., CleverCloudClient] + ) -> None: + """The old code accepted an empty body and returned an empty peer id.""" + client = make_client(lambda r: httpx.Response(200, content=b"")) + async with client: + with pytest.raises(InvalidResponseError): + await client.create_networkgroup_peer( + "orga_1", "ng_1", peer_id="p", parent_member="m", peer_role="CLIENT" + ) diff --git a/tests/test_oauth_dance.py b/tests/test_oauth_dance.py new file mode 100644 index 0000000..70a0c5a --- /dev/null +++ b/tests/test_oauth_dance.py @@ -0,0 +1,403 @@ +"""Tests for the OAuth dance (issue #3, findings 1, 2 and 8).""" + +from __future__ import annotations + +from collections.abc import Callable +from datetime import UTC, datetime +from urllib.parse import parse_qsl + +import httpx +import pytest + +from clever_cloud import OAuthConsumer, OAuthDance, OAuthError, RequestToken, SignatureMethod + +API_URL = "https://api.example.test" +CONSUMER = OAuthConsumer(key="consumer-key", secret="consumer-secret") + +Handler = Callable[[httpx.Request], httpx.Response] + + +def make_dance(handler: Handler, **kwargs: object) -> OAuthDance: + return OAuthDance( + CONSUMER, + api_url=API_URL, + transport=httpx.MockTransport(handler), + **kwargs, # type: ignore[arg-type] + ) + + +def form(response_text: str) -> dict[str, str]: + return dict(parse_qsl(response_text)) + + +def request_token_response(**extra: str) -> str: + params = { + "oauth_token": "req-token", + "oauth_token_secret": "req-secret", + "oauth_callback_confirmed": "true", + **extra, + } + return "&".join(f"{k}={v}" for k, v in params.items()) + + +class TestRequestTokenSigning: + """Finding 1: the exchange omitted timestamp, nonce and version.""" + + def test_body_carries_every_oauth_parameter(self) -> None: + seen: list[dict[str, str]] = [] + + def handler(request: httpx.Request) -> httpx.Response: + seen.append(dict(parse_qsl(request.content.decode()))) + return httpx.Response(200, text=request_token_response()) + + with make_dance(handler) as dance: + dance.get_request_token() + + body = seen[0] + assert body["oauth_consumer_key"] == "consumer-key" + assert body["oauth_signature_method"] == "HMAC-SHA512" + assert body["oauth_version"] == "1.0" + assert body["oauth_nonce"] + assert body["oauth_timestamp"].isdigit() + assert body["oauth_signature"] + assert body["oauth_callback"] == "oob" + + def test_signature_does_not_expose_the_consumer_secret(self) -> None: + seen: list[str] = [] + + def handler(request: httpx.Request) -> httpx.Response: + seen.append(request.content.decode()) + return httpx.Response(200, text=request_token_response()) + + with make_dance(handler) as dance: + dance.get_request_token() + assert "consumer-secret" not in seen[0] + + def test_two_dances_produce_different_nonces(self) -> None: + nonces: list[str] = [] + + def handler(request: httpx.Request) -> httpx.Response: + nonces.append(dict(parse_qsl(request.content.decode()))["oauth_nonce"]) + return httpx.Response(200, text=request_token_response()) + + with make_dance(handler) as dance: + dance.get_request_token() + dance.get_request_token() + assert nonces[0] != nonces[1] + + def test_plaintext_mode_still_carries_the_secret_by_design(self) -> None: + seen: list[dict[str, str]] = [] + + def handler(request: httpx.Request) -> httpx.Response: + seen.append(dict(parse_qsl(request.content.decode()))) + return httpx.Response(200, text=request_token_response()) + + with make_dance(handler, signature_method=SignatureMethod.PLAINTEXT) as dance: + dance.get_request_token() + assert seen[0]["oauth_signature_method"] == "PLAINTEXT" + assert seen[0]["oauth_signature"] == "consumer-secret&" + + def test_returns_the_token(self) -> None: + with make_dance(lambda r: httpx.Response(200, text=request_token_response())) as dance: + token = dance.get_request_token() + assert token.token == "req-token" + assert token.secret == "req-secret" + assert token.callback_confirmed is True + + def test_http_error_is_reported_with_its_step(self) -> None: + with make_dance(lambda r: httpx.Response(500, text="boom")) as dance: + with pytest.raises(OAuthError) as excinfo: + dance.get_request_token() + assert excinfo.value.step == "request_token" + + def test_incomplete_response_is_rejected(self) -> None: + with make_dance(lambda r: httpx.Response(200, text="oauth_token=only")) as dance: + with pytest.raises(OAuthError, match="Invalid request token"): + dance.get_request_token() + + def test_network_failure_becomes_an_oauth_error(self) -> None: + def handler(request: httpx.Request) -> httpx.Response: + raise httpx.ConnectError("refused") + + with make_dance(handler) as dance: + with pytest.raises(OAuthError, match="Network failure"): + dance.get_request_token() + + +class TestCallbackConfirmation: + """Finding 8: oauth_callback_confirmed was ignored.""" + + def test_unconfirmed_callback_is_refused(self) -> None: + text = request_token_response(oauth_callback_confirmed="false") + with make_dance( + lambda r: httpx.Response(200, text=text), + callback_url="https://app.example.test/cb", + ) as dance: + with pytest.raises(OAuthError, match="did not confirm"): + dance.get_request_token() + + def test_missing_confirmation_is_refused_for_a_real_callback(self) -> None: + text = "oauth_token=t&oauth_token_secret=s" + with make_dance( + lambda r: httpx.Response(200, text=text), + callback_url="https://app.example.test/cb", + ) as dance: + with pytest.raises(OAuthError, match="did not confirm"): + dance.get_request_token() + + def test_out_of_band_flow_does_not_require_confirmation(self) -> None: + text = "oauth_token=t&oauth_token_secret=s" + with make_dance(lambda r: httpx.Response(200, text=text)) as dance: + token = dance.get_request_token() + assert token.callback_confirmed is False + + +class TestCallbackValidation: + """Finding 8: the callback token was never compared to the request token.""" + + @pytest.fixture + def token(self) -> RequestToken: + return RequestToken(token="req-token", secret="req-secret") + + def test_valid_callback_returns_the_verifier(self, token: RequestToken) -> None: + with make_dance(lambda r: httpx.Response(200)) as dance: + verifier = dance.parse_callback_url( + "https://app.example.test/cb?oauth_token=req-token&oauth_verifier=v-123", + token, + ) + assert verifier == "v-123" + + def test_mismatched_token_is_refused(self, token: RequestToken) -> None: + with make_dance(lambda r: httpx.Response(200)) as dance: + with pytest.raises(OAuthError, match="does not match"): + dance.parse_callback_url( + "https://app.example.test/cb?oauth_token=other&oauth_verifier=v", + token, + ) + + def test_absent_token_is_refused(self, token: RequestToken) -> None: + with make_dance(lambda r: httpx.Response(200)) as dance: + with pytest.raises(OAuthError, match="does not match"): + dance.parse_callback_url( + "https://app.example.test/cb?oauth_verifier=v", token + ) + + def test_missing_verifier_is_refused(self, token: RequestToken) -> None: + with make_dance(lambda r: httpx.Response(200)) as dance: + with pytest.raises(OAuthError, match="missing oauth_verifier"): + dance.parse_callback_url( + "https://app.example.test/cb?oauth_token=req-token", token + ) + + def test_authorization_url(self, token: RequestToken) -> None: + with make_dance(lambda r: httpx.Response(200)) as dance: + url = dance.get_authorization_url(token) + assert url == f"{API_URL}/v2/oauth/authorize?oauth_token=req-token" + + +class TestAccessToken: + @pytest.fixture + def token(self) -> RequestToken: + return RequestToken(token="req-token", secret="req-secret") + + def test_exchange_is_fully_signed(self, token: RequestToken) -> None: + seen: list[dict[str, str]] = [] + + def handler(request: httpx.Request) -> httpx.Response: + seen.append(dict(parse_qsl(request.content.decode()))) + return httpx.Response( + 200, text="oauth_token=access&oauth_token_secret=access-secret" + ) + + with make_dance(handler) as dance: + dance.get_access_token(token, "verifier-1") + + body = seen[0] + assert body["oauth_token"] == "req-token" + assert body["oauth_verifier"] == "verifier-1" + assert body["oauth_signature_method"] == "HMAC-SHA512" + assert body["oauth_nonce"] + assert body["oauth_timestamp"].isdigit() + assert body["oauth_version"] == "1.0" + assert "consumer-secret" not in request_body_text(seen[0]) + + def test_returns_usable_credentials(self, token: RequestToken) -> None: + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response( + 200, text="oauth_token=access&oauth_token_secret=access-secret" + ) + + with make_dance(handler) as dance: + credentials = dance.get_access_token(token, "v") + + assert credentials.token == "access" + assert credentials.secret == "access-secret" + assert credentials.consumer_key == "consumer-key" + assert credentials.signature_method is SignatureMethod.HMAC_SHA512 + # The credentials can immediately sign a request. + assert credentials.get_authorization_header("GET", "https://x.test/v2/self") + + def test_expiration_date_is_kept(self, token: RequestToken) -> None: + """Finding 8: the returned expiration_date was discarded.""" + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response( + 200, + text=( + "oauth_token=access&oauth_token_secret=s" + "&expiration_date=2030-01-01T00%3A00%3A00Z" + ), + ) + + with make_dance(handler) as dance: + credentials = dance.get_access_token(token, "v") + assert credentials.expiration_date == datetime(2030, 1, 1, tzinfo=UTC) + assert credentials.is_expired() is False + + def test_epoch_expiration_is_parsed(self, token: RequestToken) -> None: + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response( + 200, text="oauth_token=a&oauth_token_secret=s&expiration_date=1700000000" + ) + + with make_dance(handler) as dance: + credentials = dance.get_access_token(token, "v") + assert credentials.expiration_date == datetime(2023, 11, 14, 22, 13, 20, tzinfo=UTC) + + def test_absent_expiration_is_none(self, token: RequestToken) -> None: + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response(200, text="oauth_token=a&oauth_token_secret=s") + + with make_dance(handler) as dance: + credentials = dance.get_access_token(token, "v") + assert credentials.expiration_date is None + + def test_incomplete_response_is_rejected(self, token: RequestToken) -> None: + with make_dance(lambda r: httpx.Response(200, text="oauth_token=a")) as dance: + with pytest.raises(OAuthError, match="Invalid access token"): + dance.get_access_token(token, "v") + + +def request_body_text(body: dict[str, str]) -> str: + return "&".join(f"{k}={v}" for k, v in body.items()) + + +class TestLogin: + """The password-driven shortcut, kept for browser-less automation.""" + + @pytest.fixture + def token(self) -> RequestToken: + return RequestToken(token="req-token", secret="req-secret") + + def test_successful_login_returns_the_verifier(self, token: RequestToken) -> None: + def handler(request: httpx.Request) -> httpx.Response: + if request.url.path == "/v2/sessions/login": + return httpx.Response(303, headers={"location": "/"}) + return httpx.Response( + 302, + headers={ + "location": "https://app.test/cb?oauth_token=req-token&oauth_verifier=v-9" + }, + ) + + with make_dance(handler) as dance: + assert dance.login(token, email="a@b.test", password="pw") == "v-9" + + def test_verifier_from_a_foreign_token_is_refused(self, token: RequestToken) -> None: + """A redirect for another authorization must not be accepted.""" + def handler(request: httpx.Request) -> httpx.Response: + if request.url.path == "/v2/sessions/login": + return httpx.Response(303, headers={"location": "/"}) + return httpx.Response( + 302, + headers={ + "location": "https://app.test/cb?oauth_token=someone-else&oauth_verifier=v" + }, + ) + + with make_dance(handler) as dance: + with pytest.raises(OAuthError, match="does not match"): + dance.login(token, email="a@b.test", password="pw") + + def test_invalid_credentials(self, token: RequestToken) -> None: + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response(401, text="bad") + + with make_dance(handler) as dance: + with pytest.raises(OAuthError, match="Invalid credentials"): + dance.login(token, email="a@b.test", password="pw") + + def test_mfa_required_without_code(self, token: RequestToken) -> None: + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response(200, text="") + + with make_dance(handler) as dance: + with pytest.raises(OAuthError, match="MFA code required"): + dance.login(token, email="a@b.test", password="pw") + + def test_mfa_kind_is_configurable(self, token: RequestToken) -> None: + """The MFA kind used to be hard-coded to TOTP.""" + seen: list[dict[str, str]] = [] + + def handler(request: httpx.Request) -> httpx.Response: + if request.url.path == "/v2/sessions/login": + return httpx.Response(200, text="") + if request.url.path == "/v2/sessions/mfa_login": + seen.append(dict(parse_qsl(request.content.decode()))) + return httpx.Response(303, headers={"location": "/"}) + return httpx.Response( + 302, + headers={ + "location": "https://app.test/cb?oauth_token=req-token&oauth_verifier=v" + }, + ) + + with make_dance(handler) as dance: + dance.login( + token, email="a@b.test", password="pw", mfa_code="123456", mfa_kind="WEBAUTHN" + ) + assert seen[0]["mfa_kind"] == "WEBAUTHN" + + def test_invalid_mfa_code(self, token: RequestToken) -> None: + def handler(request: httpx.Request) -> httpx.Response: + if request.url.path == "/v2/sessions/login": + return httpx.Response(200, text="") + return httpx.Response(401, text="bad code") + + with make_dance(handler) as dance: + with pytest.raises(OAuthError, match="Invalid MFA code"): + dance.login(token, email="a@b.test", password="pw", mfa_code="000000") + + def test_consent_screen_is_approved(self, token: RequestToken) -> None: + def handler(request: httpx.Request) -> httpx.Response: + if request.url.path == "/v2/sessions/login": + return httpx.Response(303, headers={"location": "/"}) + if request.method == "GET": + return httpx.Response(200, text="") + return httpx.Response( + 303, + headers={ + "location": "https://app.test/cb?oauth_token=req-token&oauth_verifier=v-c" + }, + ) + + with make_dance(handler) as dance: + assert dance.login(token, email="a@b.test", password="pw") == "v-c" + + +class TestDanceRedaction: + """Finding 2: dance credentials leaked through repr().""" + + def test_consumer_secret_is_hidden(self) -> None: + text = repr(OAuthConsumer(key="KEY", secret="CONSUMER_SECRET")) + assert "CONSUMER_SECRET" not in text + assert "KEY" in text + + def test_request_token_secret_is_hidden(self) -> None: + text = repr(RequestToken(token="TOKEN", secret="TOKEN_SECRET")) + assert "TOKEN_SECRET" not in text + assert "TOKEN" in text + + def test_oauth_error_details_are_truncated(self) -> None: + error = OAuthError("failed", step="login", details="x" * 100_000) + assert error.details is not None + assert len(error.details) < 3000 diff --git a/tests/test_tls.py b/tests/test_tls.py new file mode 100644 index 0000000..254c2df --- /dev/null +++ b/tests/test_tls.py @@ -0,0 +1,186 @@ +"""TLS, mTLS and Retry-After handling (issue #3, finding 9 and retry gap).""" + +from __future__ import annotations + +import ssl +from collections.abc import Callable +from datetime import UTC, datetime, timedelta + +import httpx +import pytest + +from clever_cloud import ApiTokenCredentials, CleverCloudClient, RateLimitError +from clever_cloud.client import _retry_after_seconds +from conftest import BASE_URL + + +class RecordingContext: + """Stand-in capturing what would be loaded into the SSL context.""" + + def __init__(self) -> None: + self.calls: list[dict[str, object]] = [] + + def __call__(self, **kwargs: object) -> None: + self.calls.append(kwargs) + + +@pytest.fixture +def recorded_cert_chain(monkeypatch: pytest.MonkeyPatch) -> RecordingContext: + recorder = RecordingContext() + + def fake_load(self: ssl.SSLContext, **kwargs: object) -> None: + recorder(**kwargs) + + monkeypatch.setattr(ssl.SSLContext, "load_cert_chain", fake_load) + return recorder + + +class TestClientCertificate: + def test_single_file_certificate( + self, token_auth: ApiTokenCredentials, recorded_cert_chain: RecordingContext + ) -> None: + client = CleverCloudClient( + token_auth, base_url=BASE_URL, client_cert="/etc/pki/client.pem" + ) + client._build_ssl_context() + assert recorded_cert_chain.calls == [{"certfile": "/etc/pki/client.pem"}] + + def test_certificate_and_key_pair( + self, token_auth: ApiTokenCredentials, recorded_cert_chain: RecordingContext + ) -> None: + client = CleverCloudClient( + token_auth, base_url=BASE_URL, client_cert=("/c.crt", "/c.key") + ) + client._build_ssl_context() + assert recorded_cert_chain.calls == [ + {"certfile": "/c.crt", "keyfile": "/c.key"} + ] + + def test_certificate_key_and_password( + self, token_auth: ApiTokenCredentials, recorded_cert_chain: RecordingContext + ) -> None: + client = CleverCloudClient( + token_auth, base_url=BASE_URL, client_cert=("/c.crt", "/c.key", "pw") + ) + client._build_ssl_context() + assert recorded_cert_chain.calls == [ + {"certfile": "/c.crt", "keyfile": "/c.key", "password": "pw"} + ] + + def test_client_cert_without_verification_still_builds_a_context( + self, token_auth: ApiTokenCredentials, recorded_cert_chain: RecordingContext + ) -> None: + """verify_ssl=False must not silently drop the client certificate.""" + client = CleverCloudClient( + token_auth, + base_url=BASE_URL, + client_cert=("/c.crt", "/c.key"), + verify_ssl=False, + ) + context = client._build_ssl_context() + assert isinstance(context, ssl.SSLContext) + assert context.verify_mode is ssl.CERT_NONE + assert context.check_hostname is False + assert recorded_cert_chain.calls + + +class TestCaBundle: + def test_missing_ca_bundle_is_reported( + self, token_auth: ApiTokenCredentials + ) -> None: + client = CleverCloudClient( + token_auth, base_url=BASE_URL, ca_bundle="/does/not/exist.pem" + ) + with pytest.raises(FileNotFoundError): + client._build_ssl_context() + + def test_ca_bundle_is_loaded( + self, token_auth: ApiTokenCredentials, tmp_path: object + ) -> None: + """A real PEM file is loaded without any deprecation warning.""" + import warnings + from pathlib import Path + + # Reuse the system trust store contents as a valid PEM bundle. + default_paths = ssl.get_default_verify_paths() + source = default_paths.cafile + if not source or not Path(source).exists(): + pytest.skip("no system CA bundle available to reuse") + + client = CleverCloudClient(token_auth, base_url=BASE_URL, ca_bundle=source) + with warnings.catch_warnings(): + warnings.simplefilter("error", DeprecationWarning) + context = client._build_ssl_context() + assert isinstance(context, ssl.SSLContext) + assert context.get_ca_certs() + + +class TestRetryAfterParsing: + def test_delta_seconds(self) -> None: + response = httpx.Response(429, headers={"retry-after": "30"}) + assert _retry_after_seconds(response) == 30.0 + + def test_http_date(self) -> None: + future = datetime.now(tz=UTC) + timedelta(seconds=60) + stamp = future.strftime("%a, %d %b %Y %H:%M:%S GMT") + response = httpx.Response(429, headers={"retry-after": stamp}) + value = _retry_after_seconds(response) + assert value is not None + assert 30 <= value <= 61 + + def test_past_http_date_is_clamped_to_zero(self) -> None: + past = datetime.now(tz=UTC) - timedelta(hours=1) + stamp = past.strftime("%a, %d %b %Y %H:%M:%S GMT") + response = httpx.Response(429, headers={"retry-after": stamp}) + assert _retry_after_seconds(response) == 0.0 + + def test_absent_header(self) -> None: + assert _retry_after_seconds(httpx.Response(429)) is None + + def test_negative_value_is_clamped(self) -> None: + response = httpx.Response(429, headers={"retry-after": "-5"}) + assert _retry_after_seconds(response) == 0.0 + + async def test_retry_after_is_capped_by_max_retry_wait( + self, token_auth: ApiTokenCredentials, monkeypatch: pytest.MonkeyPatch + ) -> None: + """A hostile Retry-After must not park the client for hours.""" + import asyncio + + waits: list[float] = [] + + async def fake_sleep(delay: float) -> None: + waits.append(delay) + + monkeypatch.setattr(asyncio, "sleep", fake_sleep) + calls: list[int] = [] + + def handler(request: httpx.Request) -> httpx.Response: + calls.append(1) + if len(calls) == 1: + return httpx.Response(503, headers={"retry-after": "86400"}) + return httpx.Response(200, json={"id": "u", "email": "e@x.test"}) + + client = CleverCloudClient( + token_auth, + base_url=BASE_URL, + max_retries=1, + max_retry_wait=5, + transport=httpx.MockTransport(handler), + ) + async with client: + await client.get_profile() + assert waits == [5.0] + + async def test_rate_limit_error_carries_a_date_based_retry_after( + self, make_client: Callable[..., CleverCloudClient] + ) -> None: + future = datetime.now(tz=UTC) + timedelta(seconds=45) + stamp = future.strftime("%a, %d %b %Y %H:%M:%S GMT") + client = make_client( + lambda r: httpx.Response(429, headers={"retry-after": stamp}) + ) + async with client: + with pytest.raises(RateLimitError) as excinfo: + await client.get_profile() + assert excinfo.value.retry_after is not None From 28c59e4862b82c9b34255e6df8b6a16413b61bce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Brunat?= Date: Tue, 18 Aug 2026 13:21:14 +0200 Subject: [PATCH 2/4] docs: fix the welcome example for the now-optional profile name Profile.name is str | None since parsing became strict, so the example would have printed "Hello, None!" for an account without a name. --- README.md | 3 ++- src/clever_cloud/__init__.py | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 91c5ebb..ad0b4c4 100644 --- a/README.md +++ b/README.md @@ -18,7 +18,8 @@ from clever_cloud import CleverCloudClient, ApiTokenCredentials async with CleverCloudClient(ApiTokenCredentials(token="...")) as client: profile = await client.get_profile() - print(f"Hello, {profile.name}!") + # name is optional on the API side, hence the fallback + print(f"Hello, {profile.name or profile.email}!") ``` You can also use OAuth credentials: diff --git a/src/clever_cloud/__init__.py b/src/clever_cloud/__init__.py index c059b49..5ea159c 100644 --- a/src/clever_cloud/__init__.py +++ b/src/clever_cloud/__init__.py @@ -5,7 +5,8 @@ async with CleverCloudClient(ApiTokenCredentials(token="...")) as client: profile = await client.get_profile() - print(f"Hello, {profile.name}!") + # name is optional on the API side, hence the fallback + print(f"Hello, {profile.name or profile.email}!") Example with OAuth (full access): from clever_cloud import CleverCloudClient, OAuthCredentials From 0a6fe440a63dfd9b26e5852a5ab3c8af20e27ff9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Brunat?= Date: Tue, 18 Aug 2026 13:34:45 +0200 Subject: [PATCH 3/4] docs: document the public API for library users The docstrings inherited from the original code were uneven: 30 of 34 public methods documented no exception, several NetworkGroups methods carried only their HTTP route, and the models did not say which fields can be None. - Document the error hierarchy once on CleverCloudClient, so each method only states what it adds on top of it. - Add Args/Returns/Raises to the NetworkGroups and creation methods. - Note that create_networkgroup/create_networkgroup_member answer 202: success means accepted, not ready. - Add Attributes sections to the models, calling out the nullable fields and the guaranteed ones. - Document the OAuth dance step by step, including which values to store and which failures map to which step. Raises is now documented on 23 of 35 methods; the remaining 12 are pure methods that raise nothing specific. --- src/clever_cloud/auth.py | 57 ++++++- src/clever_cloud/client.py | 267 ++++++++++++++++++++++++++++---- src/clever_cloud/models.py | 92 ++++++++++- src/clever_cloud/oauth_dance.py | 58 ++++++- 4 files changed, 434 insertions(+), 40 deletions(-) diff --git a/src/clever_cloud/auth.py b/src/clever_cloud/auth.py index 77b912c..c4c71ad 100644 --- a/src/clever_cloud/auth.py +++ b/src/clever_cloud/auth.py @@ -112,14 +112,31 @@ def get_authorization_header( url: Absolute request URL, including its query string. body_params: Form-encoded body parameters, which take part in the OAuth signature when the body is ``x-www-form-urlencoded``. + + Returns: + The value to set as the ``Authorization`` header. """ @abstractmethod def get_base_url(self) -> str: - """Return the default API base URL for this credential kind.""" + """Return the default API base URL for this credential kind. + + Returns: + The base URL used when the client is created without an explicit + ``base_url``. + """ def apply_to_request(self, request: httpx.Request) -> httpx.Request: - """Sign ``request`` in place and return it.""" + """Sign ``request`` in place and return it. + + Args: + request: The request to authenticate. Its method, URL and + form-encoded body all take part in an OAuth signature, so it + must be fully built before this is called. + + Returns: + The same request, with its ``Authorization`` header set. + """ request.headers["Authorization"] = self.get_authorization_header( request.method, str(request.url), @@ -182,8 +199,22 @@ def get_authorization_header( ) -> str: """Build a fully signed OAuth 1.0a ``Authorization`` header. + Args: + method: HTTP method of the request being signed. + url: Absolute request URL, including its query string. + body_params: Form-encoded body parameters, when the request carries + an ``x-www-form-urlencoded`` body. + timestamp: Override the OAuth timestamp. For tests only. + nonce: Override the OAuth nonce. For tests only. + ``timestamp`` and ``nonce`` are generated per call; they are only accepted as arguments to make signatures reproducible in tests. + + Returns: + The header value, e.g. ``OAuth oauth_consumer_key="...", ...``. + Every value is percent-encoded, and the signature covers the method, + URL, query string and form body, so it is valid for this request + only. """ oauth_params = { "oauth_consumer_key": self.consumer_key, @@ -224,7 +255,15 @@ def _sign( return base64.b64encode(digest).decode("ascii") def is_expired(self, *, now: datetime | None = None) -> bool: - """Whether the access token is past its expiration date, if known.""" + """Whether the access token is past its expiration date. + + Args: + now: Reference time; defaults to the current UTC time. + + Returns: + ``False`` when no expiration date is known — the API does not + always report one, so this is not proof the token still works. + """ if self.expiration_date is None: return False return (now or datetime.now(tz=UTC)) >= self.expiration_date @@ -253,6 +292,18 @@ def get_authorization_header( *, body_params: Sequence[tuple[str, str]] = (), ) -> str: + """Build the ``Authorization`` header. + + Args: + method: Unused; a bearer token is not bound to the request. + url: Unused, for the same reason. + body_params: Unused, for the same reason. + + Returns: + ``"Bearer "``. Unlike an OAuth signature, this value is the + same for every request, so it must be protected in transit and + kept out of logs. + """ return f"Bearer {self.token}" def get_base_url(self) -> str: diff --git a/src/clever_cloud/client.py b/src/clever_cloud/client.py index 79fac35..1f985d0 100644 --- a/src/clever_cloud/client.py +++ b/src/clever_cloud/client.py @@ -104,6 +104,9 @@ def _retry_after_seconds(response: httpx.Response) -> float | None: class CleverCloudClient: """Async client for the Clever Cloud API. + Use it as an async context manager so the underlying connection pool is + closed on exit. The client is reusable after ``close()``. + Example: credentials = OAuthCredentials( consumer_key="...", consumer_secret="...", @@ -112,7 +115,31 @@ class CleverCloudClient: async with CleverCloudClient(credentials) as client: profile = await client.get_profile() - app = await client.create_application(...) + app = await client.create_application( + owner_id="orga_...", name="my-app", instance_slug="node" + ) + + Errors: + Every method can raise the following, all deriving from + :class:`CleverCloudError`. Individual methods only document what they + add on top of this. + + - :class:`AuthenticationError` — HTTP 401, credentials missing or + invalid. + - :class:`AuthorizationError` — HTTP 403, credentials valid but the + account may not perform this operation. + - :class:`NotFoundError` — HTTP 404, the organisation, application or + resource does not exist. + - :class:`RateLimitError` — HTTP 429; carries ``retry_after``. Only + raised once the automatic retries are exhausted. + - :class:`HttpError` — any other error status. + - :class:`TransportError` — network, timeout or TLS failure. + - :class:`InvalidResponseError` — the response could not be decoded, was + an unexpected redirection, or did not match the endpoint's contract. + + Note: + Idempotent requests are retried automatically on transient failures; + see the ``max_retries`` argument. """ def __init__( @@ -335,7 +362,14 @@ def _backoff(self, attempt: int, retry_after: float | None) -> float: return base * (0.5 + random.random() / 2) # noqa: S311 - jitter, not crypto async def get_profile(self) -> Profile: - """Get the authenticated user's profile.""" + """Get the authenticated user's profile. + + ``GET /v2/self`` + + Returns: + The profile. Only ``id`` and ``email`` are guaranteed; every other + field is ``None`` when the account does not carry it. + """ data = await self._request("GET", "/v2/self") return Profile.from_api_response(data) @@ -344,7 +378,16 @@ async def list_instances(self, *, refresh: bool = False) -> list[dict[str, Any]] The catalogue is cached for the lifetime of the client, since it changes rarely and every ``instance_slug`` resolution would otherwise re-download - it. Pass ``refresh=True`` to force a new fetch. + it. + + Args: + refresh: Fetch the catalogue again instead of reusing the cache. + + Returns: + The raw instance entries, as returned by the API. + + Raises: + InvalidResponseError: If the API does not return a list. """ if self._instances_cache is None or refresh: data = await self._request("GET", "/v2/products/instances") @@ -360,11 +403,17 @@ async def resolve_instance_slug(self, slug: str) -> tuple[str, str, str]: Args: slug: Instance slug like "static", "node", "python", etc. + Versions are compared naturally, so ``10`` is newer than ``9``. + Returns: - Tuple of (instance_type, version, variant_id) + Tuple of ``(instance_type, version, variant_id)`` for the newest + enabled version of that runtime. Raises: - ValueError: If the slug cannot be resolved + ValueError: If no enabled instance matches the slug. The message + lists the slugs that are available. + InvalidResponseError: If the matching catalogue entry is missing + the fields needed to create an application. """ instances = await self.list_instances() matching = [ @@ -440,6 +489,16 @@ async def create_application( Either instance_slug OR (instance_type, instance_version, instance_variant) must be provided. + + Returns: + The created application, including its ``id`` and ``deploy_url``. + + Raises: + ValueError: If neither the slug nor the full instance triplet is + given, or if an identifier is empty. + NotFoundError: If the organisation does not exist. + HttpError: If the API rejects the application, for instance on a + duplicate name or an unavailable zone. """ owner = encode_path_segment(owner_id, name="owner_id") if instance_slug: @@ -504,6 +563,10 @@ async def redeploy_application( app_id: Application ID to redeploy commit: Specific commit to deploy (optional) use_cache: Whether to use build cache (optional) + + Raises: + NotFoundError: If the organisation or the application does not + exist. """ owner = encode_path_segment(owner_id, name="owner_id") app = encode_path_segment(app_id, name="app_id") @@ -534,7 +597,13 @@ async def create_tcp_redirection( namespace: TCP redirection namespace (default: "cleverapps") Returns: - TcpRedirection with namespace and assigned port + The redirection, with the port the platform assigned. + + Raises: + NotFoundError: If the organisation or the application does not + exist. + HttpError: If no port is available in that namespace, or the + application already has a redirection there. """ owner = encode_path_segment(owner_id, name="owner_id") app = encode_path_segment(app_id, name="app_id") @@ -622,6 +691,14 @@ async def create_networkgroup( tags: Optional tags. members: Optional initial members (list of WannabeNetworkgroupMember dicts: {id, domainName, kind, label?}). + + Note: + The API answers 202 with no body: creation is asynchronous, so a + successful call means the request was accepted, not that the + NetworkGroup is ready. Poll :meth:`get_networkgroup` to observe it. + + Raises: + NotFoundError: If the organisation does not exist. """ owner = encode_path_segment(owner_id, name="owner_id") body: dict[str, Any] = {"label": label} @@ -640,9 +717,21 @@ async def create_networkgroup( ) async def get_networkgroup(self, owner_id: str, ng_id: str) -> NetworkGroup: - """Get a NetworkGroup. + """Get a NetworkGroup, with its members and peers. + + ``GET /v4/networkgroups/organisations/{ownerId}/networkgroups/{networkGroupId}`` - GET /v4/networkgroups/organisations/{ownerId}/networkgroups/{networkGroupId} + Args: + owner_id: Organisation ID (``orga_*``). + ng_id: NetworkGroup ID (``ng_*``). + + Returns: + The NetworkGroup, whose ``members``, ``peers`` and ``tags`` are + tuples. + + Raises: + NotFoundError: If the organisation or the NetworkGroup does not + exist. """ owner = encode_path_segment(owner_id, name="owner_id") ng = encode_path_segment(ng_id, name="ng_id") @@ -652,9 +741,17 @@ async def get_networkgroup(self, owner_id: str, ng_id: str) -> NetworkGroup: return NetworkGroup.from_api_response(data) async def delete_networkgroup(self, owner_id: str, ng_id: str) -> None: - """Delete a NetworkGroup. + """Delete a NetworkGroup and everything it contains. - DELETE /v4/networkgroups/organisations/{ownerId}/networkgroups/{networkGroupId} + ``DELETE /v4/networkgroups/organisations/{ownerId}/networkgroups/{networkGroupId}`` + + Args: + owner_id: Organisation ID (``orga_*``). + ng_id: NetworkGroup ID (``ng_*``). + + Raises: + NotFoundError: If the organisation or the NetworkGroup does not + exist. """ owner = encode_path_segment(owner_id, name="owner_id") ng = encode_path_segment(ng_id, name="ng_id") @@ -670,11 +767,21 @@ async def search_networkgroup_components( ) -> list[dict[str, Any]]: """Search NetworkGroup components (NGs, members, peers). - GET /v4/networkgroups/organisations/{ownerId}/networkgroups/search + ``GET /v4/networkgroups/organisations/{ownerId}/networkgroups/search`` + + Args: + owner_id: Organisation ID (``orga_*``). + query: Free-text filter. Omit it to list every component. + + Returns: + The raw component list, as returned by the API. The response is a + ``oneOf`` union (CleverPeer | ExternalPeer | Member | NetworkGroup) + which this SDK deliberately does not discriminate: inspect the + dictionaries yourself, typically on the ``kind`` or ``peerKind`` + key. An empty result is an empty list. - Returns the raw component list — the response is a oneOf union - (CleverPeer | ExternalPeer | Member | NetworkGroup) that callers - typically discriminate by inspecting fields. + Raises: + InvalidResponseError: If the API does not return a list. """ owner = encode_path_segment(owner_id, name="owner_id") params = {"query": query} if query is not None else None @@ -710,8 +817,17 @@ async def create_networkgroup_member( ng_id: NetworkGroup ID (ng_*). member_id: ID of the entity to add (app_*, addon_*, ...). domain_name: Internal domain name to assign to the member. - kind: ADDON | APPLICATION | EXTERNAL | LOADBALANCER. + kind: ADDON | APPLICATION | EXTERNAL | LOADBALANCER, as a + :class:`MemberKind` or its string value. label: Optional human-readable label. + + Note: + The API answers 202 with no body, so a successful call means the + request was accepted, not that the member is attached. + + Raises: + NotFoundError: If the organisation or the NetworkGroup does not + exist. """ owner = encode_path_segment(owner_id, name="owner_id") ng = encode_path_segment(ng_id, name="ng_id") @@ -731,9 +847,22 @@ async def create_networkgroup_member( async def get_networkgroup_member( self, owner_id: str, ng_id: str, member_id: str ) -> NetworkGroupMember: - """Get a member of a NetworkGroup. + """Get one member of a NetworkGroup. + + ``GET .../networkgroups/{networkGroupId}/members/{memberId}`` - GET .../networkgroups/{networkGroupId}/members/{memberId} + Args: + owner_id: Organisation ID (``orga_*``). + ng_id: NetworkGroup ID (``ng_*``). + member_id: ID of the member (``app_*``, ``addon_*``, ...). + + Returns: + The member and the internal domain name assigned to it. + + Raises: + NotFoundError: If the NetworkGroup or the member does not exist. + InvalidResponseError: If the API reports a member kind this SDK + version does not know. """ owner = encode_path_segment(owner_id, name="owner_id") ng = encode_path_segment(ng_id, name="ng_id") @@ -749,7 +878,15 @@ async def delete_networkgroup_member( ) -> None: """Remove a member from a NetworkGroup. - DELETE .../networkgroups/{networkGroupId}/members/{memberId} + ``DELETE .../networkgroups/{networkGroupId}/members/{memberId}`` + + Args: + owner_id: Organisation ID (``orga_*``). + ng_id: NetworkGroup ID (``ng_*``). + member_id: ID of the member to remove. + + Raises: + NotFoundError: If the NetworkGroup or the member does not exist. """ owner = encode_path_segment(owner_id, name="owner_id") ng = encode_path_segment(ng_id, name="ng_id") @@ -762,9 +899,20 @@ async def delete_networkgroup_member( async def list_networkgroup_peers( self, owner_id: str, ng_id: str ) -> list[NetworkGroupPeer]: - """List peers of a NetworkGroup. + """List the peers of a NetworkGroup. + + ``GET .../networkgroups/{networkGroupId}/peers`` + + Args: + owner_id: Organisation ID (``orga_*``). + ng_id: NetworkGroup ID (``ng_*``). + + Returns: + The peers, Clever and external alike; check ``peer.kind`` to tell + them apart. A NetworkGroup with no peer yields an empty list. - GET .../networkgroups/{networkGroupId}/peers + Raises: + NotFoundError: If the NetworkGroup does not exist. """ owner = encode_path_segment(owner_id, name="owner_id") ng = encode_path_segment(ng_id, name="ng_id") @@ -781,9 +929,21 @@ async def list_networkgroup_peers( async def get_networkgroup_peer( self, owner_id: str, ng_id: str, peer_id: str ) -> NetworkGroupPeer: - """Get a peer of a NetworkGroup. + """Get one peer of a NetworkGroup. + + ``GET .../networkgroups/{networkGroupId}/peers/{peerId}`` + + Args: + owner_id: Organisation ID (``orga_*``). + ng_id: NetworkGroup ID (``ng_*``). + peer_id: ID of the peer. - GET .../networkgroups/{networkGroupId}/peers/{peerId} + Returns: + The peer. ``kind`` is ``CLEVER`` when the API reports an ``hv`` + field, ``EXTERNAL`` otherwise. + + Raises: + NotFoundError: If the NetworkGroup or the peer does not exist. """ owner = encode_path_segment(owner_id, name="owner_id") ng = encode_path_segment(ng_id, name="ng_id") @@ -811,9 +971,33 @@ async def create_networkgroup_peer( hv: str | None = None, parent_event: str | None = None, ) -> PeerCreated: - """Add a peer to a member of a NetworkGroup. + """Add a Clever peer to a member of a NetworkGroup. + + ``POST .../networkgroups/{networkGroupId}/peers`` + + Args: + owner_id: Organisation ID (``orga_*``). + ng_id: NetworkGroup ID (``ng_*``). + peer_id: ID to assign to the peer. + parent_member: ID of the member this peer belongs to. + peer_role: CLIENT or SERVER, as a :class:`PeerRole` or its value. + peer_kind: Peer kind; ``"CLEVER"`` by default. + public_key: Wireguard public key. + ip: Peer IP address. + port: Wireguard port. + hostname: Peer hostname. + label: Human-readable label. + hv: Hypervisor identifier, for a Clever peer. + parent_event: Event this peer creation belongs to. + + Returns: + The created peer id, plus the raw payload in ``raw`` for fields + this SDK does not model. - POST .../networkgroups/{networkGroupId}/peers + Raises: + NotFoundError: If the NetworkGroup or the parent member does not + exist. + InvalidResponseError: If the API does not return a peer id. """ owner = encode_path_segment(owner_id, name="owner_id") ng = encode_path_segment(ng_id, name="ng_id") @@ -846,7 +1030,15 @@ async def delete_networkgroup_peer( ) -> None: """Delete a peer of a NetworkGroup. - DELETE .../networkgroups/{networkGroupId}/peers/{peerId} + ``DELETE .../networkgroups/{networkGroupId}/peers/{peerId}`` + + Args: + owner_id: Organisation ID (``orga_*``). + ng_id: NetworkGroup ID (``ng_*``). + peer_id: ID of the peer to delete. + + Raises: + NotFoundError: If the NetworkGroup or the peer does not exist. """ owner = encode_path_segment(owner_id, name="owner_id") ng = encode_path_segment(ng_id, name="ng_id") @@ -872,7 +1064,30 @@ async def create_networkgroup_external_peer( ) -> PeerCreated: """Add an external peer to a member of a NetworkGroup. - POST .../networkgroups/{networkGroupId}/external-peers + Use this for a machine outside Clever Cloud — a laptop or an on-premise + server — joining the NetworkGroup with its own Wireguard key. + + ``POST .../networkgroups/{networkGroupId}/external-peers`` + + Args: + owner_id: Organisation ID (``orga_*``). + ng_id: NetworkGroup ID (``ng_*``). + parent_member: ID of the member this peer belongs to. + peer_role: CLIENT or SERVER, as a :class:`PeerRole` or its value. + public_key: Wireguard public key of the external machine. + label: Human-readable label. + ip: Peer IP address. + port: Wireguard port. + hostname: Peer hostname. + parent_event: Event this peer creation belongs to. + + Returns: + The created peer id, plus the raw payload in ``raw``. + + Raises: + NotFoundError: If the NetworkGroup or the parent member does not + exist. + InvalidResponseError: If the API does not return a peer id. """ owner = encode_path_segment(owner_id, name="owner_id") ng = encode_path_segment(ng_id, name="ng_id") diff --git a/src/clever_cloud/models.py b/src/clever_cloud/models.py index b20aa20..5b0163a 100644 --- a/src/clever_cloud/models.py +++ b/src/clever_cloud/models.py @@ -91,7 +91,21 @@ def _flag(data: Mapping[str, Any], key: str) -> bool: @dataclass(frozen=True, slots=True) class Profile: - """User profile from ``GET /v2/self``.""" + """User profile from ``GET /v2/self``. + + Only ``id`` and ``email`` are guaranteed by the API. Every other field is + ``None`` when the account does not carry it, so it is never confused with a + value the user actually set. + + Attributes: + id: Account identifier (``user_*``). + email: Account email address. + creation_date: When the account was created, as a timezone-aware UTC + datetime, or ``None`` if the API did not report it. + email_validated: Whether the email address has been confirmed. + is_linked_to_github: Whether a GitHub account is linked. + preferred_mfa: Preferred MFA kind, e.g. ``"TOTP"``, or ``None``. + """ id: str email: str @@ -148,7 +162,13 @@ def from_api_response(cls, data: Any) -> Self: @dataclass(frozen=True, slots=True) class Domain: - """Domain (vhost) for an application.""" + """Domain (vhost) for an application. + + Attributes: + domain: Fully-qualified domain name, without a trailing slash. + is_primary: Whether this is the application's primary domain. Set by + the method that produced it, not by the payload. + """ domain: str is_primary: bool @@ -164,7 +184,13 @@ def from_api_response(cls, data: Any, *, is_primary: bool = False) -> Self: @dataclass(frozen=True, slots=True) class TcpRedirection: - """TCP redirection for an application.""" + """TCP redirection for an application. + + Attributes: + namespace: Redirection namespace, e.g. ``"cleverapps"``. + port: Port the platform assigned. Never a placeholder: a payload + without a port is rejected. + """ namespace: str port: int @@ -214,7 +240,15 @@ def _parse_enum(enum_cls: type[_E], raw: Any, *, model: str, key: str) -> _E: @dataclass(frozen=True, slots=True) class NetworkGroupMember: - """Member of a NetworkGroup (``GET .../members/{memberId}``).""" + """Member of a NetworkGroup (``GET .../members/{memberId}``). + + Attributes: + id: Identifier of the underlying entity (``app_*``, ``addon_*``, ...). + domain_name: Internal domain name assigned inside the NetworkGroup. + kind: What the member is. An unknown kind is rejected rather than + silently coerced. + label: Human-readable label, or ``None``. + """ id: str domain_name: str @@ -239,7 +273,12 @@ def from_api_response(cls, data: Any) -> Self: @dataclass(frozen=True, slots=True) class WireguardEndpoint: - """Wireguard endpoint (private/public address pair).""" + """Wireguard endpoint (private/public address pair). + + Attributes: + private_address: Address inside the NetworkGroup, or ``None``. + public_address: Publicly reachable address, or ``None``. + """ private_address: str | None = None public_address: str | None = None @@ -255,7 +294,18 @@ def from_api_response(cls, data: Any) -> Self: @dataclass(frozen=True, slots=True) class NetworkGroupPeer: - """Peer of a NetworkGroup (CleverPeer or ExternalPeer flattened).""" + """Peer of a NetworkGroup (CleverPeer and ExternalPeer, flattened). + + Attributes: + id: Peer identifier. + parent_member: Member this peer belongs to. + kind: ``CLEVER`` when the API reports an ``hv`` field, ``EXTERNAL`` + otherwise. + public_key: Wireguard public key, or ``None``. + endpoint: Private/public address pair, or ``None`` if the peer is not + connected yet. + hv: Hypervisor identifier; set only on a Clever peer. + """ id: str parent_member: str @@ -295,6 +345,14 @@ class NetworkGroup: Collections are exposed as tuples so the model is deeply immutable, as ``frozen=True`` advertises. + + Attributes: + id: NetworkGroup identifier (``ng_*``). + owner_id: Owning organisation (``orga_*``). + version: Configuration version, incremented by the platform on change. + members: Members attached to the group. + peers: Peers, Clever and external alike. + network_ip: CIDR the group allocates addresses from, or ``None``. """ id: str @@ -334,7 +392,13 @@ def from_api_response(cls, data: Any) -> Self: @dataclass(frozen=True, slots=True) class PeerCreated: - """Response of ``POST .../peers`` and ``.../external-peers``.""" + """Response of ``POST .../peers`` and ``.../external-peers``. + + Attributes: + peer_id: Identifier of the created peer. + raw: The full payload, as a read-only mapping, for fields this SDK does + not model yet. + """ peer_id: str raw: Mapping[str, Any] = field(default_factory=lambda: MappingProxyType({})) @@ -351,7 +415,19 @@ def from_api_response(cls, data: Any) -> Self: @dataclass(frozen=True, slots=True) class Application: - """Application from the Clever Cloud API.""" + """Application from the Clever Cloud API. + + Only ``id`` and ``name`` are guaranteed; the rest is ``None`` when the + payload does not carry it, which happens on the trimmed-down objects some + endpoints return. + + Attributes: + id: Application identifier (``app_*``). + name: Application name. + deploy_url: Git remote to push to, or ``None``. + creation_date: Timezone-aware UTC datetime, or ``None``. + state: Current state reported by the API, or ``None``. + """ id: str name: str diff --git a/src/clever_cloud/oauth_dance.py b/src/clever_cloud/oauth_dance.py index 51c892f..b71784c 100644 --- a/src/clever_cloud/oauth_dance.py +++ b/src/clever_cloud/oauth_dance.py @@ -105,6 +105,7 @@ def __init__( ) def close(self) -> None: + """Close the underlying HTTP client. Called on context-manager exit.""" self._client.close() def __enter__(self) -> Self: @@ -164,7 +165,18 @@ def _post_form(self, path: str, body: dict[str, str], *, step: str) -> dict[str, return dict(parse_qsl(response.text, keep_blank_values=True)) def get_request_token(self) -> RequestToken: - """Step 1: get a temporary request token.""" + """Step 1: get a temporary request token. + + Returns: + The request token to pass to :meth:`get_authorization_url`, then to + :meth:`get_access_token`. Keep it for the whole dance: its secret + signs the final exchange, and its token validates the callback. + + Raises: + OAuthError: If the API rejects the request, answers an incomplete + body, does not confirm the callback, or is unreachable. Check + ``.step`` to see which stage failed. + """ url = f"{self._api_url}/v2/oauth/request_token" body = self._signed_params(url, {"oauth_callback": self._callback_url}) params = self._post_form("/v2/oauth/request_token", body, step="request_token") @@ -192,7 +204,16 @@ def get_request_token(self) -> RequestToken: return RequestToken(token=token, secret=secret, callback_confirmed=confirmed) def get_authorization_url(self, request_token: RequestToken) -> str: - """Get the URL for browser-based authorization.""" + """Get the URL to send the user to for authorization. + + Args: + request_token: Token from :meth:`get_request_token`. + + Returns: + The URL to open in a browser. Once the user approves, the API + redirects to the callback given to the constructor; pass that + callback URL to :meth:`parse_callback_url`. + """ params = urlencode({"oauth_token": request_token.token}) return f"{self._api_url}/v2/oauth/authorize?{params}" @@ -202,6 +223,13 @@ def parse_callback_url(self, callback_url: str, request_token: RequestToken) -> Verifies that the callback carries the very token this dance requested, so a verifier obtained for another authorization cannot be injected. + Args: + callback_url: The full URL your callback received, query included. + request_token: The token returned by :meth:`get_request_token`. + + Returns: + The verifier to pass to :meth:`get_access_token`. + Raises: OAuthError: If the token does not match or the verifier is missing. """ @@ -252,6 +280,14 @@ def login( password: Account password. mfa_code: One-time code, when the account has MFA enabled. mfa_kind: MFA kind expected by the API (``"TOTP"`` by default). + + Returns: + The OAuth verifier, to pass to :meth:`get_access_token`. + + Raises: + OAuthError: If the credentials or the MFA code are rejected, if MFA + is required but no code was given, or if no verifier could be + obtained. ``.step`` names the stage that failed. """ # Session cookies are kept by the HTTPX client itself: passing them # per-request is deprecated and makes persistence ambiguous. @@ -332,7 +368,23 @@ def get_access_token( request_token: RequestToken, verifier: str, ) -> OAuthCredentials: - """Step 3: exchange the request token and verifier for final credentials.""" + """Step 3: exchange the request token and verifier for credentials. + + Args: + request_token: Token from :meth:`get_request_token`. + verifier: Verifier from :meth:`parse_callback_url` or + :meth:`login`. + + Returns: + Long-lived credentials, ready to be passed to + :class:`CleverCloudClient`. Store all four values: the consumer + pair is needed to sign requests, not only the access token. + + Raises: + OAuthError: If the exchange is rejected — commonly an expired + request token or a verifier already used — or if the response + is incomplete. + """ url = f"{self._api_url}/v2/oauth/access_token" body = self._signed_params( url, From 927b868e2cad5547f372651e820dff2aed8aeca1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Brunat?= Date: Tue, 18 Aug 2026 14:33:21 +0200 Subject: [PATCH 4/4] fix: address the four P2 review findings - Retry-After: parsedate_to_datetime() raises on an arbitrary header, so a server-controlled value escaped the error hierarchy and cancelled the retry it was meant to schedule. Malformed values are now treated as absent, including NaN and infinity, which parse as floats but are not usable delays. - Undecodable JSON: a JSONDecodeError holds the whole payload in .doc, and `raise ... from exc` kept it reachable. The error is now raised outside the except block, keeping only the reason as a string: `from None` alone would have cleared __cause__ while leaving __context__ pointing at the same object. - OAuth login(): all four HTTP calls translate a transport failure into an OAuthError carrying the login, mfa_login or authorize step, like the token exchanges already did. - Access token: the credentials now pin the API root that issued them, so handing them to CleverCloudClient no longer sends the token to the public API when the dance targeted a private deployment. Adds 19 regression tests, one per failure mode, including the full dance-to-client handoff against a private root. --- src/clever_cloud/client.py | 25 +++++-- src/clever_cloud/oauth_dance.py | 81 ++++++++++++++-------- tests/test_client.py | 53 +++++++++++++++ tests/test_oauth_dance.py | 116 ++++++++++++++++++++++++++++++++ tests/test_tls.py | 46 +++++++++++++ 5 files changed, 288 insertions(+), 33 deletions(-) diff --git a/src/clever_cloud/client.py b/src/clever_cloud/client.py index 1f985d0..db19a0b 100644 --- a/src/clever_cloud/client.py +++ b/src/clever_cloud/client.py @@ -4,6 +4,7 @@ import asyncio import email.utils +import math import random import re import ssl @@ -90,10 +91,18 @@ def _retry_after_seconds(response: httpx.Response) -> float | None: return None raw = raw.strip() try: - return max(0.0, float(raw)) + seconds = float(raw) except ValueError: pass - parsed = email.utils.parsedate_to_datetime(raw) + else: + # NaN and infinity parse as floats but are not usable delays. + return max(0.0, seconds) if math.isfinite(seconds) else None + try: + parsed = email.utils.parsedate_to_datetime(raw) + except (ValueError, TypeError, OverflowError): + # The header is server-controlled: a malformed value is treated as + # absent rather than allowed to escape and cancel the retry. + return None if parsed is None: return None if parsed.tzinfo is None: @@ -294,11 +303,19 @@ def _handle_response(self, response: httpx.Response) -> Any: content_type = response.headers.get("content-type", "") if "json" in content_type: + decode_error: str | None = None try: return response.json() except ValueError as exc: - msg = f"Server returned an undecodable JSON body: {exc}" - raise InvalidResponseError(msg, response_body=body) from exc + # Only keep the reason as a string. A JSONDecodeError holds the + # entire payload in its .doc attribute, and raising from inside + # this block would keep that object reachable through __cause__ + # *and* __context__, defeating the truncation below. + decode_error = str(exc) + # Raised outside the except block, so no exception context is + # attached and the undecoded payload is not retained. + msg = f"Server returned an undecodable JSON body: {decode_error}" + raise InvalidResponseError(msg, response_body=body) return body async def _request( diff --git a/src/clever_cloud/oauth_dance.py b/src/clever_cloud/oauth_dance.py index b71784c..e15fad1 100644 --- a/src/clever_cloud/oauth_dance.py +++ b/src/clever_cloud/oauth_dance.py @@ -10,6 +10,8 @@ import hmac import secrets import time +from collections.abc import Iterator +from contextlib import contextmanager from dataclasses import dataclass, field from datetime import UTC, datetime from typing import Any, Self @@ -149,13 +151,24 @@ def _signed_params( signature = base64.b64encode(digest).decode("ascii") return {**params, "oauth_signature": signature} - def _post_form(self, path: str, body: dict[str, str], *, step: str) -> dict[str, str]: + @contextmanager + def _transport_errors_as_oauth(self, step: str) -> Iterator[None]: + """Translate a network failure into an :class:`OAuthError` for ``step``. + + Every HTTP call of the dance goes through this, so a connection or TLS + failure never escapes the documented SDK error hierarchy as a raw + ``httpx`` exception. + """ try: - response = self._client.post(path, data=body, headers=_FORM_HEADERS) + yield except httpx.TransportError as exc: msg = f"Network failure during {step}: {type(exc).__name__}: {exc}" raise OAuthError(msg, step=step) from exc + def _post_form(self, path: str, body: dict[str, str], *, step: str) -> dict[str, str]: + with self._transport_errors_as_oauth(step): + response = self._client.post(path, data=body, headers=_FORM_HEADERS) + if response.status_code != 200: raise OAuthError( f"Step {step} failed with HTTP {response.status_code}", @@ -291,12 +304,13 @@ def login( """ # Session cookies are kept by the HTTPX client itself: passing them # per-request is deprecated and makes persistence ambiguous. - login_response = self._client.post( - "/v2/sessions/login", - data={"email": email, "pass": password, "from_authorize": "true"}, - headers={"Content-Type": "application/x-www-form-urlencoded"}, - follow_redirects=False, - ) + with self._transport_errors_as_oauth("login"): + login_response = self._client.post( + "/v2/sessions/login", + data={"email": email, "pass": password, "from_authorize": "true"}, + headers={"Content-Type": "application/x-www-form-urlencoded"}, + follow_redirects=False, + ) # A 200 means the MFA form was returned instead of a session redirect. if login_response.status_code == 200: @@ -306,16 +320,17 @@ def login( step="login", details="Please provide the mfa_code parameter", ) - mfa_response = self._client.post( - "/v2/sessions/mfa_login", - data={ - "mfa_attempt": mfa_code, - "mfa_kind": mfa_kind, - "email": email, - }, - headers={"Content-Type": "application/x-www-form-urlencoded"}, - follow_redirects=False, - ) + with self._transport_errors_as_oauth("mfa_login"): + mfa_response = self._client.post( + "/v2/sessions/mfa_login", + data={ + "mfa_attempt": mfa_code, + "mfa_kind": mfa_kind, + "email": email, + }, + headers={"Content-Type": "application/x-www-form-urlencoded"}, + follow_redirects=False, + ) if mfa_response.status_code == 401: raise OAuthError( "Invalid MFA code", step="mfa_login", details=mfa_response.text @@ -339,22 +354,24 @@ def login( details=login_response.text, ) - auth_response = self._client.get( - "/v2/oauth/authorize", - params={"oauth_token": request_token.token}, - follow_redirects=False, - ) + with self._transport_errors_as_oauth("authorize"): + auth_response = self._client.get( + "/v2/oauth/authorize", + params={"oauth_token": request_token.token}, + follow_redirects=False, + ) verifier = self._verifier_from_redirect(auth_response, request_token) if verifier: return verifier # HTTP 200 means the consent screen was returned: approve it explicitly. if auth_response.status_code == 200: - approve_response = self._client.post( - "/v2/oauth/authorize", - data={"oauth_token": request_token.token}, - follow_redirects=False, - ) + with self._transport_errors_as_oauth("authorize"): + approve_response = self._client.post( + "/v2/oauth/authorize", + data={"oauth_token": request_token.token}, + follow_redirects=False, + ) verifier = self._verifier_from_redirect(approve_response, request_token) if verifier: return verifier @@ -378,7 +395,9 @@ def get_access_token( Returns: Long-lived credentials, ready to be passed to :class:`CleverCloudClient`. Store all four values: the consumer - pair is needed to sign requests, not only the access token. + pair is needed to sign requests, not only the access token. Their + ``base_url`` points at the API root that issued them, so a private + deployment keeps being addressed. Raises: OAuthError: If the exchange is rejected — commonly an expired @@ -407,6 +426,10 @@ def get_access_token( consumer_secret=self._consumer.secret, token=token, secret=secret, + # Pin the API root that issued the token: handing these credentials + # to CleverCloudClient must not send them to the public API when + # they came from a private deployment. + base_url=self._api_url, signature_method=self._signature_method, expiration_date=_parse_expiration(params.get("expiration_date")), ) diff --git a/tests/test_client.py b/tests/test_client.py index d6ba2fb..a6619f3 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -704,3 +704,56 @@ async def test_client_is_reusable_after_close( await client.get_profile() async with client: assert (await client.get_profile()).id == "user_1" + + +class TestNoBodyRetentionThroughCause: + """A truncated body must not stay reachable through the exception chain.""" + + async def test_invalid_json_body_is_not_retained_by_the_cause( + self, make_client: Callable[..., CleverCloudClient] + ) -> None: + # A JSONDecodeError keeps the whole payload in its .doc attribute. + big = "{" + "x" * 200_000 + client = make_client( + lambda r: httpx.Response( + 200, content=big.encode(), headers={"content-type": "application/json"} + ) + ) + async with client: + with pytest.raises(InvalidResponseError) as excinfo: + await client.get_profile() + + error = excinfo.value + assert len(error.response_body) < 3000 + assert error.__cause__ is None + assert error.__context__ is None or not hasattr(error.__context__, "doc") + + async def test_invalid_json_message_stays_informative( + self, make_client: Callable[..., CleverCloudClient] + ) -> None: + """Suppressing the cause must not cost the failure position.""" + client = make_client( + lambda r: httpx.Response( + 200, content=b"{not json", headers={"content-type": "application/json"} + ) + ) + async with client: + with pytest.raises(InvalidResponseError, match="line 1 column"): + await client.get_profile() + + async def test_secret_in_an_invalid_body_is_not_reachable_from_the_chain( + self, make_client: Callable[..., CleverCloudClient] + ) -> None: + payload = '{"token": "SUPER_SECRET_VALUE", ' + "x" * 5000 + client = make_client( + lambda r: httpx.Response( + 200, + content=payload.encode(), + headers={"content-type": "application/json"}, + ) + ) + async with client: + with pytest.raises(InvalidResponseError) as excinfo: + await client.get_profile() + assert "SUPER_SECRET_VALUE" not in str(excinfo.value) + assert excinfo.value.__cause__ is None diff --git a/tests/test_oauth_dance.py b/tests/test_oauth_dance.py index 70a0c5a..8b7f392 100644 --- a/tests/test_oauth_dance.py +++ b/tests/test_oauth_dance.py @@ -401,3 +401,119 @@ def test_oauth_error_details_are_truncated(self) -> None: error = OAuthError("failed", step="login", details="x" * 100_000) assert error.details is not None assert len(error.details) < 3000 + + +class TestLoginTransportErrors: + """Every dance call must translate a network failure into an OAuthError.""" + + @pytest.fixture + def token(self) -> RequestToken: + return RequestToken(token="req-token", secret="req-secret") + + def _failing_at(self, failing_path: str) -> Handler: + def handler(request: httpx.Request) -> httpx.Response: + if request.url.path == failing_path: + raise httpx.ConnectError("connection refused") + if request.url.path == "/v2/sessions/login": + return httpx.Response(200, text="") + if request.url.path == "/v2/sessions/mfa_login": + return httpx.Response(303, headers={"location": "/"}) + return httpx.Response(200, text="") + + return handler + + def test_login_network_failure(self, token: RequestToken) -> None: + with make_dance(self._failing_at("/v2/sessions/login")) as dance: + with pytest.raises(OAuthError) as excinfo: + dance.login(token, email="a@b.test", password="pw") + assert excinfo.value.step == "login" + assert "ConnectError" in excinfo.value.message + + def test_mfa_network_failure(self, token: RequestToken) -> None: + with make_dance(self._failing_at("/v2/sessions/mfa_login")) as dance: + with pytest.raises(OAuthError) as excinfo: + dance.login(token, email="a@b.test", password="pw", mfa_code="123456") + assert excinfo.value.step == "mfa_login" + + def test_authorize_network_failure(self, token: RequestToken) -> None: + with make_dance(self._failing_at("/v2/oauth/authorize")) as dance: + with pytest.raises(OAuthError) as excinfo: + dance.login(token, email="a@b.test", password="pw", mfa_code="123456") + assert excinfo.value.step == "authorize" + + def test_approve_network_failure(self, token: RequestToken) -> None: + """The consent POST is the fourth call and was unguarded too.""" + calls: list[str] = [] + + def handler(request: httpx.Request) -> httpx.Response: + calls.append(f"{request.method} {request.url.path}") + if request.url.path == "/v2/sessions/login": + return httpx.Response(303, headers={"location": "/"}) + if request.method == "GET": + return httpx.Response(200, text="") + raise httpx.ReadTimeout("timeout") + + with make_dance(handler) as dance: + with pytest.raises(OAuthError) as excinfo: + dance.login(token, email="a@b.test", password="pw") + assert excinfo.value.step == "authorize" + + def test_no_raw_httpx_error_escapes(self, token: RequestToken) -> None: + """Nothing outside the SDK hierarchy reaches the caller.""" + from clever_cloud import CleverCloudError + + with make_dance(self._failing_at("/v2/sessions/login")) as dance: + with pytest.raises(CleverCloudError): + dance.login(token, email="a@b.test", password="pw") + + +class TestCredentialsTargetTheIssuingApi: + """Credentials must keep addressing the deployment that issued them.""" + + @pytest.fixture + def token(self) -> RequestToken: + return RequestToken(token="req-token", secret="req-secret") + + def _access_token_handler(self) -> Handler: + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response(200, text="oauth_token=access&oauth_token_secret=s") + + return handler + + def test_custom_api_root_is_preserved(self, token: RequestToken) -> None: + with make_dance(self._access_token_handler()) as dance: + credentials = dance.get_access_token(token, "v") + assert credentials.base_url == API_URL + assert credentials.get_base_url() == API_URL + + def test_default_api_root_is_unchanged(self, token: RequestToken) -> None: + dance = OAuthDance( + CONSUMER, transport=httpx.MockTransport(self._access_token_handler()) + ) + with dance: + credentials = dance.get_access_token(token, "v") + assert credentials.get_base_url() == "https://api.clever-cloud.com" + + async def test_client_handoff_targets_the_private_deployment( + self, token: RequestToken + ) -> None: + """The full flow: dance on a private root, then use the credentials.""" + from clever_cloud import CleverCloudClient + + with make_dance(self._access_token_handler()) as dance: + credentials = dance.get_access_token(token, "v") + + seen: list[httpx.Request] = [] + + def api_handler(request: httpx.Request) -> httpx.Response: + seen.append(request) + return httpx.Response(200, json={"id": "u_1", "email": "u@example.test"}) + + # No base_url passed: the client must follow the credentials. + async with CleverCloudClient( + credentials, transport=httpx.MockTransport(api_handler) + ) as client: + await client.get_profile() + + assert str(seen[0].url).startswith(API_URL) + assert "api.clever-cloud.com" not in str(seen[0].url) diff --git a/tests/test_tls.py b/tests/test_tls.py index 254c2df..2168e1b 100644 --- a/tests/test_tls.py +++ b/tests/test_tls.py @@ -184,3 +184,49 @@ async def test_rate_limit_error_carries_a_date_based_retry_after( with pytest.raises(RateLimitError) as excinfo: await client.get_profile() assert excinfo.value.retry_after is not None + + +class TestMalformedRetryAfter: + """A server-controlled header must never escape the error hierarchy.""" + + @pytest.mark.parametrize( + "value", ["bogus", "Mon, 32 Foo 2026 99:99:99 GMT", "", " ", "NaN", "1e999"] + ) + def test_malformed_value_is_treated_as_absent(self, value: str) -> None: + response = httpx.Response(429, headers={"retry-after": value}) + assert _retry_after_seconds(response) is None + + async def test_malformed_value_does_not_cancel_the_retry( + self, token_auth: ApiTokenCredentials + ) -> None: + """A 503 with `Retry-After: bogus` used to raise ValueError instead.""" + calls: list[int] = [] + + def handler(request: httpx.Request) -> httpx.Response: + calls.append(1) + if len(calls) == 1: + return httpx.Response(503, headers={"retry-after": "bogus"}) + return httpx.Response(200, json={"id": "u", "email": "e@x.test"}) + + client = CleverCloudClient( + token_auth, + base_url=BASE_URL, + max_retries=1, + max_retry_wait=0, + transport=httpx.MockTransport(handler), + ) + async with client: + profile = await client.get_profile() + assert profile.id == "u" + assert len(calls) == 2 + + async def test_malformed_value_on_429_still_raises_the_sdk_error( + self, make_client: Callable[..., CleverCloudClient] + ) -> None: + client = make_client( + lambda r: httpx.Response(429, headers={"retry-after": "bogus"}) + ) + async with client: + with pytest.raises(RateLimitError) as excinfo: + await client.get_profile() + assert excinfo.value.retry_after is None