diff --git a/README.md b/README.md index 01dcd2a..f838181 100644 --- a/README.md +++ b/README.md @@ -367,6 +367,13 @@ cd examples MCP_TRANSPORT_TYPE=http MCP_STATELESS=true NITROSTACK_APP_MODE=universal python widgets_example.py ``` +### OAuth client + +Use `OAuth2Client` for authorization-code/PKCE and client-credentials flows, +managed token storage, refresh, discovery, registration and revocation. Tokens +stay in memory unless a file store is explicitly configured. See the +[OAuth client guide](docs/oauth-client.md) for examples and security responsibilities. + ### Running Tests To run the automated test suite, execute: ```bash diff --git a/docs/oauth-client-implementation.md b/docs/oauth-client-implementation.md new file mode 100644 index 0000000..dd12fa7 --- /dev/null +++ b/docs/oauth-client-implementation.md @@ -0,0 +1,153 @@ +# Issue #68 implementation walkthrough + +This change adds client-side OAuth support alongside the existing server-side +authentication code. The starting point is the two unmerged files from PR #4; +their responsibilities and encrypted file format are retained, while their +concurrency, persistence, validation and transport behavior are revised. + +## Token records and time + +`auth/token_store.py` defines `StoredToken` as a frozen dataclass. A frozen record +lets stores return the same object without allowing a caller to mutate a stored +credential accidentally. Construction validates the required token strings, +integer millisecond deadline and optional string fields. Credential-bearing +fields have `repr=False`; explicit `to_dict()` is deliberately different because +persistence needs their real values. + +`calculate_expiration()` validates a finite nonnegative numeric duration, converts +seconds to milliseconds, adds the current Unix time, and returns an integer. +`is_token_expired()` compares the clock with that deadline minus a configurable +margin, also converting its seconds to milliseconds. Equality counts as expired. + +`token_response_to_stored()` is the conversion boundary for provider responses. +Only an absent or null expiry receives the one-hour fallback. Zero remains zero. +During refresh, a missing refresh token or scope inherits the previous value; +the original resource association remains attached to the replacement record. + +## The storage contract + +The asynchronous `TokenStore` protocol describes the five operations needed by +the client without requiring inheritance. Applications can provide another store +by implementing that contract. `MemoryTokenStore` holds immutable records under +a small process lock. It deliberately does not purge expired access tokens, +because doing so would also discard refresh credentials. + +`FileTokenStore` delegates blocking work to `asyncio.to_thread()`. Its lock is +selected by canonical path, so different instances targeting the same file share +the same process-local lock. The complete load/modify/save operation holds that +lock, preventing a stale reader from overwriting another caller's newly saved key. + +The read path rejects symlinks, opens without following the final symlink where +supported, requires a regular file, and tightens POSIX permissions. Missing files +mean an empty store. Invalid JSON, invalid records, decryption failures and +permission failures remain errors rather than becoming an empty document. + +The write path performs these steps: + +1. Serialize and optionally encrypt all records before touching the destination. +2. Create the parent directory and an exclusive temporary file beside the target. +3. Set restrictive permissions before writing credentials. +4. Write, flush and synchronize the temporary file. +5. Recheck the destination and replace it atomically. +6. Remove a remaining temporary file when a write or replacement fails. + +The old destination remains intact until replacement. Tests inject failures at +serialization, synchronization and replacement to verify this property. + +Encryption is imported only when selected. AES-GCM, key derivation and the hex +`iv:tag:ciphertext` representation preserve the older implementation's format. +`token-encryption` advertises the optional dependency, and the development extra +includes it so encryption tests always execute. Neither store coordinates +multiple writer processes; Windows directory privacy depends on ACLs. + +## Client configuration and transport + +`auth/client.py` defines typed, redacted config, authorization-flow and +registration results. Constructor mappings use the same snake_case names as the +config model. The client creates a memory store unless the caller supplies one. +Request timeout and response limits are validated once at construction. + +The transport has two layers. `_request()` owns validation, DNS safety, status +handling and sanitized errors. `_send()` is an asynchronous seam that delegates +urllib work to a worker thread; tests can replace this seam without contacting an +identity provider. `_send_sync()` builds the request, rejects redirects, bounds +the response before JSON parsing, and captures status and headers. Revocation +does not require JSON, allowing an empty successful response. + +Constructor endpoint URLs represent explicit operator configuration. Other +destinations call the existing CIMD safety checker. The returned address is used +for the actual socket connection; the original hostname is retained for TLS +certificate checks and SNI, and the URL's port is preserved. Proxy routing is +disabled for pinned requests so a proxy cannot defeat the selected destination. + +`auth/cimd.py` now separates generic fetch URL validation from the CIMD-specific +non-root document-path rule. Existing CIMD entry points retain their original +default. OAuth can validate a root resource URL explicitly. The existing pinned +connection classes and redirect denial are shared; the CIMD document size limit +remains independent from the OAuth response limit. + +## Discovery and authorization + +Protected-resource discovery considers an explicit metadata URL first, otherwise +a Bearer challenge and well-known candidates. It requires the returned resource +identifier to match and validates the listed authorization-server URLs. + +Authorization-server discovery probes the supported well-known locations and +checks the exact expected issuer plus relevant endpoint fields. It replaces +cached metadata only after the entire document validates. Endpoint lookup always +checks explicit configuration before discovery results. + +Authorization flow creation reuses `generate_pkce_params("S256")`, creates random +state unless the caller supplies it, and constructs the authorization query with +the client ID, redirect URI, challenge and optional scope/resource. Existing +reserved query parameters are replaced rather than duplicated. The returned +verifier and state belong to the application's pending flow; callback receipt, +state validation and replay prevention remain application responsibilities. + +PKCE metadata checks apply when starting authorization-code flows. Generic +discovery and client-credentials grants do not require PKCE advertising. + +## Acquisition, refresh and revocation + +Code exchange and client-credentials acquisition share `_obtain_token()`. It +constructs a form request, applies Basic authentication for confidential clients, +converts the response to `StoredToken`, and awaits persistence before returning. +A failed save is a reported failure, not an apparently successful acquisition. + +`refresh_token()` maintains a dictionary from token key to the in-progress task. +The first caller creates the task; simultaneous callers await that same task. +`asyncio.shield()` separates a waiter's cancellation from the shared operation. +A completion callback removes the task and retrieves its exception, including +when all waiters cancelled. A failed operation can therefore be retried later. + +Different keys have different locks and refresh tasks. Same-key acquisition, +refresh and revocation hold a shared lock within the client instance. In +particular, revocation waits for an already-running refresh and then revokes the +rotated credential, preventing that refresh from restoring a record after local +logout. The lock registry uses weak references so unused keys do not accumulate +permanent lock entries. + +Refresh reads the existing record even when expired, uses its refresh credential, +preserves omitted fields, and saves the replacement. Failure leaves the local +record unchanged. A provider-side rotation can still have happened before a local +save failure, which an application must handle by reauthorization if necessary. + +Revocation defaults to the stored refresh token when available and otherwise the +access token. An explicit hint selects either credential. It removes the local +record only after upstream success. Client registration sends JSON and returns a +redacted typed result; it does not implicitly change client configuration. + +## Error boundaries and verification + +`OAuthClientError` exposes operation, HTTP status and whitelisted standard OAuth +codes. Arbitrary provider descriptions and underlying exception messages are +suppressed with sanitized exceptions. Token-store errors follow the same rule. +Tests use recognizable credential sentinels to inspect representations, +tracebacks, logs, stdout and stderr. Explicit serialization remains secret-bearing. + +The three new test modules cover storage/expiry, OAuth lifecycle/concurrency, and +urllib/pinned transport behavior respectively. They use fake upstream responses +and controlled coordination events instead of live providers. Existing OAuth, +PKCE, CIMD, auth and full-suite tests exercise integration with the SDK. See the +separate verification report for exact platform results and the Windows socket +limitation; local Linux runs are not a claim of hosted CI success. diff --git a/docs/oauth-client-verification.md b/docs/oauth-client-verification.md new file mode 100644 index 0000000..8eb90c3 --- /dev/null +++ b/docs/oauth-client-verification.md @@ -0,0 +1,89 @@ +# Issue #68 verification + +Validation date: 2026-10-28. Feature branch: +`feature/issue-68-oauth-client`, based directly on upstream `develop` at +`b3a2e13ae9f6af024f568b10d7c689927ae8ec2a`. PR #36 and spillover commits are not +part of this branch. + +## Scope and checks + +The three new test modules add 78 cases to the unchanged 889-test baseline. +Complete collection succeeds with 967 tests. The new feature selection passes +all 78 tests on Windows Python 3.12.14. It covers token records and expiry, +plaintext/encrypted stores, atomic failure recovery, concurrent writers, +authorization and client credentials, refresh rotation and cancellation, +revocation, registration, discovery, bounded urllib responses, DNS pins, TLS +identity, ports, redirect rejection and secret-safe output. + +`git diff --check`, compilation/AST checks, public-export inspection and +`uv lock --check` pass. Runtime requirements are unchanged; cryptography is an +optional encryption extra and is also included in development dependencies. +No HTTP client dependency was added. No test exclusions, skips or xfail markers +were added. + +## Complete suites + +All runs use MCP 2.2.0 and install `.[dev,token-encryption]`. Linux runs use Ubuntu +under WSL. These are local compatibility runs, not hosted GitHub Actions results. + +| Platform | Python | Result | Total coverage | +|---|---|---|---| +| Linux | 3.10.21 | 967 passed | 86.38% | +| Linux | 3.11.16 | 967 passed | 86.36% | +| Linux | 3.12.14 | 967 passed | 86.36% | +| Windows | 3.10.21 | 967 passed | 86.37% | +| Windows | 3.11.16 | 967 passed | 86.35% | +| Windows | 3.12.14 | 966 passed, 1 failed | 86.35% | + +Commands, with an environment-specific interpreter and temporary directory: + +```shell +python -m pytest tests --collect-only -o addopts= -q +python -m pytest tests/test_token_store.py tests/test_oauth_client.py tests/test_oauth_client_transport.py -o addopts= -q --timeout=60 +python -m pytest tests --cov=nitrostack --cov-report=term-missing --timeout=60 --tb=short +git diff --check +uv lock --check +``` + +Full-run logs are retained locally in `.pytest_cache/linux-3.10.log`, +`linux-3.11.log`, `linux-3.12.log`, `windows-3.10.log`, `windows-3.11.log`, and +`windows-final.log`. The cache is ignored by Git. + +## Windows socket limitation + +The final Windows 3.12 run fails the unchanged +`tests/test_oauth.py::test_discovery_server_idempotent_start_and_dcr_http` while +reading a live localhost POST response: `ConnectionAbortedError: [WinError 10053]`. +Two earlier complete feature runs each reported 964 passed and one failure in +`test_discovery_dcr_404_when_disabled_or_missing_client_id` with the same socket +error. Those runs preceded the final two additional storage cases. + +Comparison used an isolated archive of the exact unchanged develop commit and +the same Windows 3.12.14 environment/MCP version: + +- Two complete baseline runs passed all 889 tests (85.78% coverage). +- The failing disabled-registration test passed five isolated repetitions on + both baseline and feature code. +- Running the existing OAuth/PKCE/CIMD/authentication modules together on the + unchanged baseline reproduced WinError 10053 in the disabled-registration test: + 83 passed, 1 failed. +- The corresponding feature selection reported 160 passed, 2 failed, both live + discovery socket tests. The new feature-only selection remained 78 passed. + +Baseline reproduction command: + +```shell +python -m pytest tests/test_oauth.py tests/test_pkce.py tests/test_mcp20_oauth_cimd.py tests/test_auth_modules.py tests/test_oauth_discovery.py -o addopts= -q --timeout=60 --tb=short +``` + +This demonstrates an intermittent baseline socket failure under matching local +conditions. It does not establish the operating-system-level cause, and the +complete Windows 3.12 suite is not reported as passing. No socket test, server +implementation or assertion was changed to conceal the result. + +## Publication status + +Implementation and local validation were completed before publication. Branch +publication was subsequently authorized by the user. No PR creation or reviewer +message is included. Hosted CI remains to be verified on the published head; +the local matrix results above are not hosted CI results. diff --git a/docs/oauth-client.md b/docs/oauth-client.md new file mode 100644 index 0000000..6416c41 --- /dev/null +++ b/docs/oauth-client.md @@ -0,0 +1,198 @@ +# OAuth client and token stores + +`OAuth2Client` manages OAuth credentials for an application calling a protected +service. It is separate from the SDK's server-side OAuth guards. It saves every +successfully acquired or refreshed token to its configured store before returning +it. It does not launch browsers, receive callbacks, or intercept API requests. + +## Client credentials + +```python +import asyncio +import os + +from nitrostack import OAuth2Client, OAuth2ClientConfig, is_token_expired + + +async def main(): + client = OAuth2Client(OAuth2ClientConfig( + client_id="reporting-app", + client_secret=os.environ["OAUTH_CLIENT_SECRET"], + token_endpoint="https://identity.example.com/oauth/token", + )) + key = "identity.example.com|reporting-app|service-account|reports-api" + token = await client.store.get_token(key) + if token is None or is_token_expired(token): + token = await client.get_client_credentials_token( + key, scope="reports:read", resource="https://reports.example.com", + ) + # Pass token.access_token to your HTTP client's Authorization header. + # Do not print the credential or its serialized representation. + + +asyncio.run(main()) +``` + +An equivalent mapping with snake_case keys can replace `OAuth2ClientConfig`. +Confidential clients authenticate using HTTP Basic; public clients send their +client ID without a secret. Token requests use form encoding. HTTP uses the +standard library, with a default 10-second socket timeout and a 1-MiB response +limit. Override these using `timeout_seconds` and `max_response_bytes`. + +## Authorization code and PKCE + +```python +from nitrostack import OAuth2Client + +client = OAuth2Client({ + "client_id": "desktop-app", + "issuer": "https://identity.example.com", + "redirect_uri": "http://127.0.0.1:8765/callback", +}) + +# Run these awaits inside your application's async lifecycle. +metadata = await client.discover_authorization_server_metadata() +flow = await client.start_authorization_flow(scope="profile", resource="https://api.example.com") +# Your application presents flow.authorization_url and securely retains flow. +``` + +The application must associate the pending flow with the correct user/session, +validate the callback's state against `flow.state`, reject unsolicited/replayed +callbacks, and consume the pending flow once. Only after that validation should it +exchange the code: + +```python +token = await client.exchange_code_for_token( + "identity.example.com|desktop-app|user-123|api.example.com", + code=callback_code, + code_verifier=flow.code_verifier, + resource="https://api.example.com", +) +``` + +The SDK reuses its S256 PKCE implementation and generates random state when none +is supplied. `AuthorizationFlow` hides the URL, state and verifier in its repr. +The application still needs to protect those explicit attributes. No callback +listener is supplied. PKCE capability validation applies to authorization-code +flows after discovery; a client-credentials-only server does not need PKCE. + +## Discovery and configured endpoints + +`discover_protected_resource_metadata(resource_url, +www_authenticate_header=...)` supports a Bearer challenge's `resource_metadata` +URL and path-specific/root well-known locations. Without a supplied challenge, +it first probes the resource for its challenge. Its returned `authorization_servers` +can be used with `discover_authorization_server_metadata(issuer=...)`. + +Authorization-server discovery validates the issuer and endpoint fields before +replacing cached metadata. Explicit constructor endpoints take precedence over +discovered endpoints. Discovery does not register the application automatically; +`register_client(metadata)` is a separate JSON request and returns a redacted +`ClientRegistration` containing the returned credentials. + +HTTPS is required. HTTP is allowed only for explicitly configured loopback +development endpoints. Constructor endpoint URLs are trusted operator settings; +untrusted destinations are DNS-checked and connected using the validated IP, +preserving the original hostname for TLS verification/SNI and the original port. +Redirects are rejected. Root resource paths are permitted without weakening CIMD's +existing non-root client-identifier rule. CIMD retains its separate document limit. + +## Storage and key selection + +The default `MemoryTokenStore` writes no files. Both it and `FileTokenStore` +implement the asynchronous `TokenStore` protocol: `save_token`, `get_token`, +`delete_token`, `list_keys`, and `clear`. + +Applications sharing a store must construct keys that distinguish provider, +client, principal, and resource. The store does not infer identity or add access +control between code sharing the same store. Avoid secrets in key names. + +```python +from pathlib import Path +from nitrostack import FileTokenStore, OAuth2Client + +store = FileTokenStore(Path.home() / ".my-app" / "oauth-tokens.json") +client = OAuth2Client(config, store=store) +``` + +File persistence is explicit. `create_default_token_store()` selects +`~/.nitrostack/tokens.json`; `get_default_store_path()` returns that path. Neither +changes the OAuth client's default memory storage. + +File updates serialize under a canonical-path, process-local lock and atomically +replace the destination using an exclusive temporary file in the same directory. +Failed serialization or writes preserve the previous valid file. Symlink +destinations are refused. A missing file is empty; corruption, decryption and +permission failures raise `TokenStoreError` rather than silently discarding data. +Filesystem operations run in worker threads. + +On POSIX, destination and temporary credential files use mode `0600` before +credentials are written. Windows uses ACLs rather than Unix permission semantics; +use an application-private directory with suitable ACLs. Cross-process concurrent +writers are unsupported. Keep token files out of source control and backups that +are not approved for credentials. + +### Optional encryption + +```shell +pip install 'nitrostack[token-encryption]' +``` + +```python +store = FileTokenStore( + Path.home() / ".my-app" / "oauth-tokens.enc", + encryption_key=os.environ["TOKEN_STORE_ENCRYPTION_KEY"], +) +``` + +Encryption imports are lazy. The file format remains compatible with PR #4: +AES-GCM with a random 16-byte IV, hex `iv:tag:ciphertext`, and its existing +PBKDF2-HMAC-SHA256 key derivation (100,000 iterations, `nitrostack-token-store` +salt). Keep the encryption key separate from the file. Switching an existing +plaintext file to encryption requires an explicit migration by the application. + +## Expiry and refresh + +`StoredToken` is immutable. `expires_at` is an integer Unix timestamp in +**milliseconds**. `calculate_expiration(expires_in)` and `is_token_expired(token, +skew=30)` take **seconds**. Missing/`None` `expires_in` uses one hour; zero expires +immediately. Negative, nonnumeric, boolean and nonfinite durations are rejected. + +Stores retain expired records so the refresh credential remains available: + +```python +token = await client.store.get_token(key) +if token is not None and is_token_expired(token): + if token.refresh_token: + token = await client.refresh_token(key) + else: + # Start the appropriate acquisition flow again. + ... +``` + +Share one client instance to coalesce concurrent refresh calls for a key into one +upstream request. Different keys run independently. Cancelling a waiter does not +cancel the shared refresh. Completed/failed tasks are removed so a later call can +retry. Refresh preserves an omitted refresh token and scope, and the existing +resource association. Persistence failures are reported; failed refreshes leave +the local record unchanged. A provider may already have rotated a credential when +a local persistence failure occurs, so applications must handle reauthorization. + +Acquisition, refresh and revocation serialize for the same key within the client. +Revocation selects the refresh credential when available (or the access token), +and deletes the local record only after upstream success. Pass +`token_type_hint="access_token"` to select that credential explicitly. An empty +successful revocation response is accepted. Coordination is not shared between +client instances or processes. + +## Errors and secret handling + +Catch `OAuthClientError` and `TokenStoreError` for sanitized failures. OAuth errors +expose the operation, HTTP status when available, and recognized standard error +codes. Upstream error descriptions, URLs with query parameters, request bodies and +underlying exception details are suppressed. + +Config, token, pending-flow and registration representations hide credentials. +Explicit `token.to_dict()` and `registration.to_dict()` contain credentials for +interoperability/persistence: **never log these mappings**. Redacted repr does not +make arbitrary attribute access, application logs or memory dumps safe. diff --git a/nitrostack/__init__.py b/nitrostack/__init__.py index 9f1c3d5..8d97a62 100644 --- a/nitrostack/__init__.py +++ b/nitrostack/__init__.py @@ -103,6 +103,27 @@ validate_pkce_support, verify_pkce, ) +from nitrostack.auth.client import ( + AuthorizationFlow, + ClientRegistration, + OAuth2Client, + OAuth2ClientConfig, + OAuthClientError, + get_well_known_metadata_uris, + parse_www_authenticate_header, +) +from nitrostack.auth.token_store import ( + FileTokenStore, + MemoryTokenStore, + StoredToken, + TokenStore, + TokenStoreError, + calculate_expiration, + create_default_token_store, + get_default_store_path, + is_token_expired, + token_response_to_stored, +) from nitrostack.auth.scopes import ( has_all_scopes, has_any_scope, @@ -139,6 +160,23 @@ __all__ = [ + "AuthorizationFlow", + "ClientRegistration", + "OAuth2Client", + "OAuth2ClientConfig", + "OAuthClientError", + "FileTokenStore", + "MemoryTokenStore", + "StoredToken", + "TokenStore", + "TokenStoreError", + "calculate_expiration", + "create_default_token_store", + "get_default_store_path", + "get_well_known_metadata_uris", + "is_token_expired", + "parse_www_authenticate_header", + "token_response_to_stored", "tool", "resource", "prompt", diff --git a/nitrostack/auth/cimd.py b/nitrostack/auth/cimd.py index 5ff4366..a0e7659 100644 --- a/nitrostack/auth/cimd.py +++ b/nitrostack/auth/cimd.py @@ -59,8 +59,15 @@ class CimdFetchError(ValueError): def validate_client_identifier_url(client_id_url: str, *, allow_loopback: bool = False) -> str: + """CIMD identifiers additionally require a non-root document path.""" + return validate_fetch_url(client_id_url, allow_loopback=allow_loopback, require_non_root_path=True) + + +def validate_fetch_url( + client_id_url: str, *, allow_loopback: bool = False, require_non_root_path: bool = False, +) -> str: """ - Validate a CIMD ``client_id`` URL before any network fetch. + Validate a fetch URL, optionally requiring CIMD's non-root document path. Returns the normalized URL string on success. """ @@ -85,10 +92,10 @@ def validate_client_identifier_url(client_id_url: str, *, allow_loopback: bool = raise CimdValidationError("client_id URL must not contain a fragment") path = parsed.path or "" - if not path or path == "/": + if require_non_root_path and (not path or path == "/"): raise CimdValidationError("client_id URL must include a non-root path") - segments = [segment for segment in path.split("/") if segment] + segments = [segment for segment in urllib.parse.unquote(path).split("/") if segment] if any(segment in {".", ".."} for segment in segments): raise CimdValidationError("client_id URL path must not contain '.' or '..' segments") @@ -154,9 +161,11 @@ def cimd_host_matches_request( return cimd_url_matches_request_host(cimd_url, headers, peer, trusted=trusted) -async def assert_safe_fetch_target(url_str: str, *, allow_loopback: bool = False) -> Optional[str]: +async def assert_safe_fetch_target( + url_str: str, *, allow_loopback: bool = False, require_non_root_path: bool = True, +) -> Optional[str]: """DNS pre-resolution and IP range filtering. Returns the pinned destination IP.""" - validate_client_identifier_url(url_str, allow_loopback=allow_loopback) + validate_fetch_url(url_str, allow_loopback=allow_loopback, require_non_root_path=require_non_root_path) parsed = urllib.parse.urlparse(url_str) hostname = parsed.hostname if not hostname: @@ -216,27 +225,29 @@ def connect(self) -> None: class _PinnedHTTPHandler(urllib.request.HTTPHandler): - def __init__(self, hostname: str, pinned_ip: str) -> None: + def __init__(self, hostname: str, pinned_ip: str, port: Optional[int] = None) -> None: super().__init__() self._hostname = hostname self._pinned_ip = pinned_ip + self._port = port def http_open(self, req: urllib.request.Request): return self.do_open( - lambda host, **kwargs: _PinnedHTTPConnection(self._hostname, self._pinned_ip, **kwargs), + lambda host, **kwargs: _PinnedHTTPConnection(self._hostname, self._pinned_ip, port=self._port, **kwargs), req, ) class _PinnedHTTPSHandler(urllib.request.HTTPSHandler): - def __init__(self, hostname: str, pinned_ip: str) -> None: + def __init__(self, hostname: str, pinned_ip: str, port: Optional[int] = None) -> None: super().__init__() self._hostname = hostname self._pinned_ip = pinned_ip + self._port = port def https_open(self, req: urllib.request.Request): return self.do_open( - lambda host, **kwargs: _PinnedHTTPSConnection(self._hostname, self._pinned_ip, **kwargs), + lambda host, **kwargs: _PinnedHTTPSConnection(self._hostname, self._pinned_ip, port=self._port, **kwargs), req, ) @@ -252,22 +263,24 @@ def redirect_request(self, req, fp, code, msg, headers, newurl): # noqa: ANN001 ) +def _build_fetch_opener(url_str: str, pinned_ip: Optional[str] = None): + """Share redirect denial and DNS-pinned, hostname-verified connections.""" + parsed = urllib.parse.urlparse(url_str) + handlers: list[urllib.request.BaseHandler] = [_NoRedirectHandler()] + if pinned_ip and parsed.hostname: + handlers.append(urllib.request.ProxyHandler({})) + handler = _PinnedHTTPHandler if parsed.scheme == "http" else _PinnedHTTPSHandler + handlers.append(handler(parsed.hostname, pinned_ip, parsed.port)) + return urllib.request.build_opener(*handlers) + + def _fetch_cimd_bytes( url_str: str, *, timeout_sec: float, pinned_ip: Optional[str] = None, ) -> bytes: - parsed = urllib.parse.urlparse(url_str) - hostname = parsed.hostname - handlers: list[urllib.request.BaseHandler] = [_NoRedirectHandler()] - if pinned_ip and hostname: - if (parsed.scheme or "").lower() == "http": - handlers.append(_PinnedHTTPHandler(hostname, pinned_ip)) - else: - handlers.append(_PinnedHTTPSHandler(hostname, pinned_ip)) - - opener = urllib.request.build_opener(*handlers) + opener = _build_fetch_opener(url_str, pinned_ip) request = urllib.request.Request(url_str, headers={"Accept": "application/json"}, method="GET") try: diff --git a/nitrostack/auth/client.py b/nitrostack/auth/client.py new file mode 100644 index 0000000..92d2663 --- /dev/null +++ b/nitrostack/auth/client.py @@ -0,0 +1,462 @@ +"""Reusable OAuth client, adapted from PR #4 with managed, secret-safe tokens.""" + +from __future__ import annotations + +import asyncio +import base64 +import json +import math +import re +import secrets +import urllib.error +import urllib.parse +import urllib.request +import weakref +from dataclasses import dataclass, field, fields +from types import MappingProxyType +from typing import Any, Mapping + +from nitrostack.auth import cimd +from nitrostack.auth.pkce import generate_pkce_params, is_valid_code_verifier, validate_pkce_support +from nitrostack.auth.token_store import MemoryTokenStore, StoredToken, TokenStore, token_response_to_stored + + +_OAUTH_ERRORS = frozenset({ + "invalid_request", "invalid_client", "invalid_grant", "unauthorized_client", + "unsupported_grant_type", "invalid_scope", "access_denied", "server_error", + "temporarily_unavailable", "invalid_token", "unsupported_token_type", + "invalid_redirect_uri", "invalid_client_metadata", +}) + + +class OAuthClientError(ValueError): + """Only operation, status, and recognized OAuth error codes are exposed.""" + + def __init__(self, operation: str, *, status: int | None = None, error_code=None): + self.operation = operation + self.status = status + self.error_code = error_code if isinstance(error_code, str) and error_code in _OAUTH_ERRORS else None + suffix = f" (HTTP {status})" if status is not None else "" + if self.error_code: + suffix += f": {self.error_code}" + super().__init__(f"OAuth {operation} failed{suffix}") + + +@dataclass(frozen=True) +class OAuth2ClientConfig: + client_id: str | None = field(default=None, repr=False) + client_secret: str | None = field(default=None, repr=False) + issuer: str | None = field(default=None, repr=False) + redirect_uri: str | None = field(default=None, repr=False) + authorization_endpoint: str | None = field(default=None, repr=False) + token_endpoint: str | None = field(default=None, repr=False) + revocation_endpoint: str | None = field(default=None, repr=False) + registration_endpoint: str | None = field(default=None, repr=False) + protected_resource_metadata_url: str | None = field(default=None, repr=False) + authorization_server_metadata_url: str | None = field(default=None, repr=False) + + def __post_init__(self): + if any(value is not None and (not isinstance(value, str) or not value) + for value in (getattr(self, item.name) for item in fields(self))): + raise OAuthClientError("configuration") + + +@dataclass(frozen=True) +class AuthorizationFlow: + authorization_url: str = field(repr=False) + state: str = field(repr=False) + code_verifier: str = field(repr=False) + code_challenge: str = field(repr=False) + code_challenge_method: str = "S256" + + +@dataclass(frozen=True) +class ClientRegistration: + client_id: str = field(repr=False) + client_secret: str | None = field(default=None, repr=False) + metadata: Mapping[str, Any] = field(default_factory=dict, repr=False) + + def __post_init__(self): + if (not isinstance(self.client_id, str) or not self.client_id + or (self.client_secret is not None and not isinstance(self.client_secret, str))): + raise OAuthClientError("registration response") + object.__setattr__(self, "metadata", MappingProxyType(dict(self.metadata))) + + def to_dict(self) -> dict[str, Any]: + """Explicit credential-bearing output; never log this mapping.""" + result = {**self.metadata, "client_id": self.client_id} + if self.client_secret is not None: + result["client_secret"] = self.client_secret + return result + + +@dataclass(frozen=True) +class _Request: + method: str + url: str = field(repr=False) + headers: Mapping[str, str] = field(repr=False) + body: bytes | None = field(repr=False) + timeout: float + pinned_ip: str | None = field(default=None, repr=False) + + +@dataclass(frozen=True) +class _Response: + status: int + headers: Mapping[str, str] = field(repr=False) + body: bytes = field(repr=False) + + +def get_well_known_metadata_uris(resource_url: str) -> list[str]: + parsed = urllib.parse.urlsplit(resource_url) + origin = f"{parsed.scheme}://{parsed.netloc}" + urls = [] + if parsed.path and parsed.path != "/": + urls.append(f"{origin}/.well-known/oauth-protected-resource{parsed.path}") + return list(dict.fromkeys([*urls, f"{origin}/.well-known/oauth-protected-resource"])) + + +def parse_www_authenticate_header(value: str) -> dict[str, str] | None: + match = re.search(r"(?:^|,)\s*Bearer\s+", value, re.IGNORECASE) + if not match: + return None + result = {"scheme": "Bearer"} + for item in re.finditer(r'([\w-]+)\s*=\s*(?:"((?:\\.|[^"\\])*)"|([^,\s]+))', value[match.end():]): + text = item.group(2) if item.group(2) is not None else item.group(3) + result[item.group(1).lower()] = re.sub(r"\\(.)", r"\1", text) + return result + + +class OAuth2Client: + """One configured OAuth client; share it to coalesce refreshes per token key. + + Store keys must distinguish provider, client, principal, and resource. This + client never launches a browser or listens for authorization callbacks. + """ + + def __init__( + self, config: OAuth2ClientConfig | Mapping[str, Any], *, store: TokenStore | None = None, + timeout_seconds: float = 10, max_response_bytes: int = 1024 * 1024, + ): + try: + self.config = config if isinstance(config, OAuth2ClientConfig) else OAuth2ClientConfig(**dict(config)) + except (TypeError, ValueError): + raise OAuthClientError("configuration") from None + try: + valid_timeout = (not isinstance(timeout_seconds, bool) and isinstance(timeout_seconds, (int, float)) + and math.isfinite(timeout_seconds) and timeout_seconds > 0) + except OverflowError: + valid_timeout = False + if not valid_timeout: + raise OAuthClientError("configuration") + if type(max_response_bytes) is not int or max_response_bytes <= 0: + raise OAuthClientError("configuration") + self.store = MemoryTokenStore() if store is None else store + self.timeout_seconds = timeout_seconds + self.max_response_bytes = max_response_bytes + self._metadata: dict[str, Any] = {} + self._refresh_tasks: dict[str, asyncio.Task[StoredToken]] = {} + self._key_locks: weakref.WeakValueDictionary[str, asyncio.Lock] = weakref.WeakValueDictionary() + self._trusted_urls = { + value for item in fields(self.config) + if item.name.endswith(("endpoint", "metadata_url")) or item.name == "issuer" + if (value := getattr(self.config, item.name)) is not None + } + for url in self._trusted_urls: + self._validate_url(url) + + def _validate_url(self, url: str) -> None: + try: + if not isinstance(url, str) or url != url.strip(): + raise ValueError() + cimd.validate_fetch_url(url, allow_loopback=url in self._trusted_urls) + # Trigger port validation before urllib or socket sees malformed input. + urllib.parse.urlsplit(url).port + except (TypeError, ValueError): + raise OAuthClientError("endpoint validation") from None + + def _endpoint(self, name: str) -> str: + value = getattr(self.config, name) or self._metadata.get(name) + if not value: + raise OAuthClientError("endpoint configuration") + self._validate_url(value) + return value + + async def _send(self, request: _Request) -> _Response: + """Internal async transport seam; tests replace it without real network IO.""" + return await asyncio.to_thread(self._send_sync, request) + + def _send_sync(self, request: _Request) -> _Response: + opener = cimd._build_fetch_opener(request.url, request.pinned_ip) + outgoing = urllib.request.Request(request.url, data=request.body, + headers=dict(request.headers), method=request.method) + try: + response = opener.open(outgoing, timeout=request.timeout) + except urllib.error.HTTPError as exc: + response = exc + with response: + length = response.headers.get("Content-Length") + if length is not None and not 0 <= int(length) <= self.max_response_bytes: + raise OAuthClientError("response size") + body = response.read(self.max_response_bytes + 1) + if len(body) > self.max_response_bytes: + raise OAuthClientError("response size") + return _Response(response.code, dict(response.headers), body) + + async def _request( + self, operation: str, method: str, url: str, *, body: bytes | None = None, + headers: Mapping[str, str] | None = None, allowed_statuses: tuple[int, ...] = (), + ) -> _Response: + try: + self._validate_url(url) + pinned_ip = None + if url not in self._trusted_urls: + pinned_ip = await cimd.assert_safe_fetch_target(url, require_non_root_path=False) + if not pinned_ip: + raise OAuthClientError("endpoint validation") + response = await self._send(_Request( + method, url, {"Accept": "application/json", **(headers or {})}, body, + self.timeout_seconds, pinned_ip, + )) + if len(response.body) > self.max_response_bytes: + raise OAuthClientError("response size") + except OAuthClientError: + raise + except Exception: + raise OAuthClientError(operation) from None + if not (200 <= response.status < 300 or response.status in allowed_statuses): + error_code = None + try: + error_code = json.loads(response.body).get("error") + except (ValueError, AttributeError, TypeError): + pass + raise OAuthClientError(operation, status=response.status, error_code=error_code) + return response + + @staticmethod + def _json(response: _Response, operation: str) -> dict[str, Any]: + try: + result = json.loads(response.body) + if not isinstance(result, dict): + raise ValueError() + return result + except (ValueError, TypeError, UnicodeError): + raise OAuthClientError(operation) from None + + async def discover_protected_resource_metadata( + self, resource_url: str, *, www_authenticate_header: str | None = None, + ) -> dict[str, Any]: + self._validate_url(resource_url) + configured = self.config.protected_resource_metadata_url + if configured: + candidates = [configured] + else: + if www_authenticate_header is None: + try: + response = await self._request("resource discovery", "GET", resource_url, + allowed_statuses=(401,)) + www_authenticate_header = next((value for key, value in response.headers.items() + if key.lower() == "www-authenticate"), "") + except OAuthClientError: + www_authenticate_header = "" + challenge = parse_www_authenticate_header(www_authenticate_header or "") or {} + candidates = list(dict.fromkeys([ + *([challenge["resource_metadata"]] if challenge.get("resource_metadata") else []), + *get_well_known_metadata_uris(resource_url), + ])) + for url in candidates: + try: + metadata = self._json(await self._request("resource discovery", "GET", url), "resource metadata") + if metadata.get("resource") != resource_url: + raise OAuthClientError("resource metadata") + servers = metadata.get("authorization_servers") + if not isinstance(servers, list) or not servers: + raise OAuthClientError("resource metadata") + for issuer in servers: + self._validate_url(issuer) + return metadata + except OAuthClientError: + continue + raise OAuthClientError("resource discovery") + + async def discover_authorization_server_metadata(self, issuer: str | None = None) -> dict[str, Any]: + issuer = issuer or self.config.issuer + self._validate_url(issuer) + parsed = urllib.parse.urlsplit(issuer) + if parsed.query: + raise OAuthClientError("issuer validation") + origin = f"{parsed.scheme}://{parsed.netloc}" + path = parsed.path.rstrip("/") + configured = self.config.authorization_server_metadata_url + candidates = [configured] if configured else list(dict.fromkeys([ + f"{origin}/.well-known/oauth-authorization-server{path}", + f"{origin}/.well-known/openid-configuration{path}", + f"{issuer.rstrip('/')}/.well-known/openid-configuration", + ])) + for url in candidates: + try: + metadata = self._json(await self._request("authorization discovery", "GET", url), "authorization metadata") + if metadata.get("issuer") != issuer or not metadata.get("token_endpoint"): + raise OAuthClientError("authorization metadata") + for name in ("authorization_endpoint", "token_endpoint", "registration_endpoint", "revocation_endpoint"): + if name in metadata: + self._validate_url(metadata[name]) + self._metadata = dict(metadata) + return dict(metadata) + except OAuthClientError: + continue + raise OAuthClientError("authorization discovery") + + def _client_id(self) -> str: + if not self.config.client_id: + raise OAuthClientError("client configuration") + return self.config.client_id + + def _key_lock(self, key: str) -> asyncio.Lock: + if not isinstance(key, str) or not key: + raise OAuthClientError("token key") + lock = self._key_locks.get(key) + if lock is None: + lock = asyncio.Lock() + self._key_locks[key] = lock + return lock + + async def start_authorization_flow( + self, *, scope: str | None = None, state: str | None = None, + resource: str | None = None, redirect_uri: str | None = None, + ) -> AuthorizationFlow: + redirect_uri = redirect_uri or self.config.redirect_uri + if not redirect_uri or (state is not None and (not isinstance(state, str) or not state)): + raise OAuthClientError("authorization configuration") + if self._metadata and not validate_pkce_support(self._metadata.get("code_challenge_methods_supported")): + raise OAuthClientError("PKCE support") + pkce = generate_pkce_params("S256") + state = secrets.token_urlsafe(32) if state is None else state + params = {"response_type": "code", "client_id": self._client_id(), "redirect_uri": redirect_uri, + "state": state, "code_challenge": pkce["code_challenge"], "code_challenge_method": "S256"} + if scope is not None: + params["scope"] = scope + if resource is not None: + params["resource"] = resource + endpoint = urllib.parse.urlsplit(self._endpoint("authorization_endpoint")) + reserved = {"response_type", "client_id", "redirect_uri", "state", "code_challenge", + "code_challenge_method", "scope", "resource"} + query = [(key, value) for key, value in urllib.parse.parse_qsl(endpoint.query, keep_blank_values=True) + if key not in reserved] + authorization_url = urllib.parse.urlunsplit(endpoint._replace(query=urllib.parse.urlencode([*query, *params.items()]))) + return AuthorizationFlow(authorization_url, state, pkce["code_verifier"], pkce["code_challenge"]) + + def _authenticate(self, params: dict[str, Any]) -> dict[str, str]: + params["client_id"] = self._client_id() + headers = {"Content-Type": "application/x-www-form-urlencoded"} + if self.config.client_secret is not None: + credentials = ":".join(urllib.parse.quote_plus(value, safe="") for value in + (self._client_id(), self.config.client_secret)) + headers["Authorization"] = "Basic " + base64.b64encode(credentials.encode("ascii")).decode("ascii") + return headers + + async def _obtain_token(self, key, params, *, resource=None, previous=None): + if not isinstance(key, str) or not key: + raise OAuthClientError("token key") + headers = self._authenticate(params) + if resource is not None: + params["resource"] = resource + response = await self._request("token request", "POST", self._endpoint("token_endpoint"), + body=urllib.parse.urlencode(params).encode("utf-8"), headers=headers) + try: + token = token_response_to_stored(self._json(response, "token response"), resource, previous=previous) + await self.store.save_token(key, token) + except Exception: + raise OAuthClientError("token persistence or response") from None + return token + + async def exchange_code_for_token( + self, key: str, *, code: str, code_verifier: str, + redirect_uri: str | None = None, resource: str | None = None, + ) -> StoredToken: + redirect_uri = redirect_uri or self.config.redirect_uri + if (not isinstance(code, str) or not code or not isinstance(code_verifier, str) + or not is_valid_code_verifier(code_verifier) or not redirect_uri): + raise OAuthClientError("code exchange configuration") + async with self._key_lock(key): + return await self._obtain_token(key, {"grant_type": "authorization_code", "code": code, + "code_verifier": code_verifier, "redirect_uri": redirect_uri}, resource=resource) + + async def get_client_credentials_token( + self, key: str, *, scope: str | None = None, resource: str | None = None, + ) -> StoredToken: + if not self.config.client_secret: + raise OAuthClientError("client credentials configuration") + params = {"grant_type": "client_credentials"} + if scope is not None: + params["scope"] = scope + async with self._key_lock(key): + return await self._obtain_token(key, params, resource=resource) + + async def _refresh(self, key: str) -> StoredToken: + async with self._key_lock(key): + return await self._refresh_locked(key) + + async def _refresh_locked(self, key: str) -> StoredToken: + try: + previous = await self.store.get_token(key) + except Exception: + raise OAuthClientError("token retrieval") from None + if previous is None or not previous.refresh_token: + raise OAuthClientError("refresh credential") + params = {"grant_type": "refresh_token", "refresh_token": previous.refresh_token} + if previous.scope is not None: + params["scope"] = previous.scope + return await self._obtain_token(key, params, resource=previous.resource, previous=previous) + + async def refresh_token(self, key: str) -> StoredToken: + if not isinstance(key, str) or not key: + raise OAuthClientError("token key") + task = self._refresh_tasks.get(key) + if task is None: + task = asyncio.create_task(self._refresh(key)) + self._refresh_tasks[key] = task + + def completed(done): + if self._refresh_tasks.get(key) is done: + self._refresh_tasks.pop(key, None) + if not done.cancelled(): + done.exception() # Consume failures even if every waiter cancelled. + + task.add_done_callback(completed) + return await asyncio.shield(task) + + async def revoke_token(self, key: str, *, token_type_hint: str | None = None) -> None: + async with self._key_lock(key): + await self._revoke_locked(key, token_type_hint) + + async def _revoke_locked(self, key: str, token_type_hint: str | None) -> None: + try: + stored = await self.store.get_token(key) + if stored is None: + return + hint = token_type_hint or ("refresh_token" if stored.refresh_token else "access_token") + if hint not in ("access_token", "refresh_token") or not getattr(stored, hint): + raise OAuthClientError("revocation configuration") + params = {"token": getattr(stored, hint), "token_type_hint": hint} + headers = self._authenticate(params) + await self._request("revocation", "POST", self._endpoint("revocation_endpoint"), + body=urllib.parse.urlencode(params).encode("utf-8"), headers=headers) + await self.store.delete_token(key) + except OAuthClientError: + raise + except Exception: + raise OAuthClientError("revocation persistence") from None + + async def register_client(self, metadata: Mapping[str, Any]) -> ClientRegistration: + try: + body = json.dumps(dict(metadata)).encode("utf-8") + except Exception: + raise OAuthClientError("registration configuration") from None + response = await self._request("registration", "POST", self._endpoint("registration_endpoint"), + body=body, headers={"Content-Type": "application/json"}) + result = self._json(response, "registration response") + try: + return ClientRegistration(result["client_id"], result.get("client_secret"), result) + except (KeyError, TypeError, ValueError): + raise OAuthClientError("registration response") from None diff --git a/nitrostack/auth/token_store.py b/nitrostack/auth/token_store.py new file mode 100644 index 0000000..9534466 --- /dev/null +++ b/nitrostack/auth/token_store.py @@ -0,0 +1,293 @@ +"""Async token persistence adapted from the unmerged full-parity implementation. + +Expiry is milliseconds since the Unix epoch; durations and skew are seconds. +Expired records remain readable so their refresh credentials are not discarded. +""" + +from __future__ import annotations + +import asyncio +import hashlib +import json +import math +import os +import stat +import tempfile +import threading +import time +from dataclasses import dataclass, field +from pathlib import Path +from typing import Any, Mapping, Protocol + + +class TokenStoreError(ValueError): + """Persistence or record validation failed; messages never contain credentials.""" + + +@dataclass(frozen=True) +class StoredToken: + access_token: str = field(repr=False) + token_type: str = field(repr=False) + expires_at: int + refresh_token: str | None = field(default=None, repr=False) + scope: str | None = field(default=None, repr=False) + resource: str | None = field(default=None, repr=False) + + def __post_init__(self) -> None: + if not isinstance(self.access_token, str) or not self.access_token: + raise TokenStoreError("Invalid access token") + if not isinstance(self.token_type, str) or not self.token_type: + raise TokenStoreError("Invalid token type") + if type(self.expires_at) is not int or self.expires_at < 0: + raise TokenStoreError("Invalid token expiry") + if any(value is not None and not isinstance(value, str) + for value in (self.refresh_token, self.scope, self.resource)): + raise TokenStoreError("Invalid optional token fields") + + def to_dict(self) -> dict[str, Any]: + """Explicit credential-bearing serialization. Never log this mapping.""" + return {"access_token": self.access_token, "token_type": self.token_type, + "expires_at": self.expires_at, "refresh_token": self.refresh_token, + "scope": self.scope, "resource": self.resource} + + @classmethod + def from_dict(cls, value: Mapping[str, Any]) -> StoredToken: + try: + return cls(**dict(value)) + except (TypeError, ValueError): + raise TokenStoreError("Invalid stored token record") from None + + +def _seconds(value: float, label: str) -> float: + try: + valid = not isinstance(value, bool) and isinstance(value, (int, float)) and math.isfinite(value) and value >= 0 + except OverflowError: + valid = False + if not valid: + raise ValueError(f"{label} must be finite, nonnegative seconds") + return value + + +def calculate_expiration(expires_in: float) -> int: + deadline = time.time() * 1000 + _seconds(expires_in, "expires_in") * 1000 + if not math.isfinite(deadline): + raise ValueError("expires_in is too large") + return int(deadline) + + +def is_token_expired(token: StoredToken, skew: float = 30) -> bool: + return time.time() * 1000 >= token.expires_at - _seconds(skew, "skew") * 1000 + + +def token_response_to_stored( + response: Mapping[str, Any], resource: str | None = None, + *, previous: StoredToken | None = None, +) -> StoredToken: + """Use a one-hour fallback only for absent expiry, preserving refresh rotation.""" + try: + expires_in = response.get("expires_in") + return StoredToken( + access_token=response["access_token"], + token_type=response.get("token_type", "Bearer"), + expires_at=calculate_expiration(3600 if expires_in is None else expires_in), + refresh_token=response.get("refresh_token", previous.refresh_token if previous else None), + scope=response.get("scope", previous.scope if previous else None), + resource=previous.resource if previous else resource, + ) + except (KeyError, TypeError, ValueError, OverflowError): + raise TokenStoreError("Invalid OAuth token response") from None + + +class TokenStore(Protocol): + async def save_token(self, key: str, token: StoredToken) -> None: ... + async def get_token(self, key: str) -> StoredToken | None: ... + async def delete_token(self, key: str) -> None: ... + async def list_keys(self) -> list[str]: ... + async def clear(self) -> None: ... + + +def _validate_key(key: str) -> None: + if not isinstance(key, str) or not key: + raise TokenStoreError("A nonempty token key is required") + + +class MemoryTokenStore: + def __init__(self) -> None: + self._tokens: dict[str, StoredToken] = {} + self._lock = threading.RLock() + + async def save_token(self, key: str, token: StoredToken) -> None: + _validate_key(key) + if not isinstance(token, StoredToken): + raise TokenStoreError("Expected a StoredToken") + with self._lock: + self._tokens[key] = token + + async def get_token(self, key: str) -> StoredToken | None: + _validate_key(key) + with self._lock: + return self._tokens.get(key) + + async def delete_token(self, key: str) -> None: + _validate_key(key) + with self._lock: + self._tokens.pop(key, None) + + async def list_keys(self) -> list[str]: + with self._lock: + return list(self._tokens) + + async def clear(self) -> None: + with self._lock: + self._tokens.clear() + + +_path_locks: dict[str, Any] = {} +_path_locks_guard = threading.Lock() + + +class FileTokenStore: + """Atomic single-process storage, serialized across instances of the same path. + + POSIX files are mode 0600. Windows deployments must use a private parent ACL. + Encryption retains the prior AES-GCM ``iv:tag:ciphertext`` hex file format. + """ + + def __init__(self, store_path: str | Path, encryption_key: str | None = None): + self._path = Path(store_path).expanduser().absolute() + if encryption_key is not None and (not isinstance(encryption_key, str) or not encryption_key): + raise TokenStoreError("Encryption key must be a nonempty string") + self._encryption_key = encryption_key + canonical = os.path.normcase(str(self._path.parent.resolve() / self._path.name)) + with _path_locks_guard: + self._lock = _path_locks.setdefault(canonical, threading.RLock()) + + def _cipher_parts(self): + try: + from cryptography.hazmat.primitives.ciphers import Cipher, algorithms, modes + except ImportError: + raise TokenStoreError("Install nitrostack[token-encryption] for encrypted storage") from None + key = hashlib.pbkdf2_hmac( + "sha256", self._encryption_key.encode("utf-8"), b"nitrostack-token-store", 100000, 32, + ) + return Cipher, algorithms, modes, key + + def _encrypt(self, text: str) -> str: + if self._encryption_key is None: + return text + Cipher, algorithms, modes, key = self._cipher_parts() + iv = os.urandom(16) + encryptor = Cipher(algorithms.AES(key), modes.GCM(iv)).encryptor() + ciphertext = encryptor.update(text.encode("utf-8")) + encryptor.finalize() + return f"{iv.hex()}:{encryptor.tag.hex()}:{ciphertext.hex()}" + + def _decrypt(self, text: str) -> str: + if self._encryption_key is None: + return text + Cipher, algorithms, modes, key = self._cipher_parts() + iv, tag, ciphertext = (bytes.fromhex(part) for part in text.split(":")) + decryptor = Cipher(algorithms.AES(key), modes.GCM(iv, tag)).decryptor() + return (decryptor.update(ciphertext) + decryptor.finalize()).decode("utf-8") + + def _check_path(self) -> None: + if self._path.is_symlink(): + raise TokenStoreError("Token store cannot be a symlink") + + def _load(self) -> dict[str, StoredToken]: + self._check_path() + try: + descriptor = os.open(self._path, os.O_RDONLY | getattr(os, "O_NOFOLLOW", 0)) + except FileNotFoundError: + return {} + with os.fdopen(descriptor, "r", encoding="utf-8") as stream: + if not stat.S_ISREG(os.fstat(stream.fileno()).st_mode): + raise TokenStoreError("Token store must be a regular file") + if os.name == "posix": + os.fchmod(stream.fileno(), 0o600) + text = stream.read() + try: + plaintext = self._decrypt(text) + except TokenStoreError: + raise + except Exception: + raise TokenStoreError("Token store decryption failed") from None + try: + raw = json.loads(plaintext) + except (ValueError, TypeError): + raise TokenStoreError("Token store contains invalid JSON") from None + if not isinstance(raw, dict): + raise TokenStoreError("Invalid token store document") + for key in raw: + _validate_key(key) + return {key: StoredToken.from_dict(value) for key, value in raw.items()} + + def _save(self, tokens: dict[str, StoredToken]) -> None: + self._check_path() + data = self._encrypt(json.dumps({key: value.to_dict() for key, value in tokens.items()})) + self._path.parent.mkdir(mode=0o700, parents=True, exist_ok=True) + descriptor, temporary = tempfile.mkstemp(prefix=".oauth-token-", dir=self._path.parent) + try: + with os.fdopen(descriptor, "w", encoding="utf-8") as stream: + if os.name == "posix": + os.fchmod(stream.fileno(), 0o600) + else: + os.chmod(temporary, 0o600) + stream.write(data) + stream.flush() + os.fsync(stream.fileno()) + self._check_path() + os.replace(temporary, self._path) + finally: + Path(temporary).unlink(missing_ok=True) + + def _operate(self, operation: str, key=None, token=None): + try: + with self._lock: + tokens = self._load() + if operation == "get": + return tokens.get(key) + if operation == "list": + return list(tokens) + if operation == "save": + tokens[key] = token + elif operation == "delete": + tokens.pop(key, None) + elif operation == "clear": + tokens.clear() + self._save(tokens) + except TokenStoreError: + raise + except PermissionError: + raise TokenStoreError("Token store permission denied") from None + except Exception: + raise TokenStoreError("Token store operation failed") from None + + async def save_token(self, key: str, token: StoredToken) -> None: + _validate_key(key) + if not isinstance(token, StoredToken): + raise TokenStoreError("Expected a StoredToken") + await asyncio.to_thread(self._operate, "save", key, token) + + async def get_token(self, key: str) -> StoredToken | None: + _validate_key(key) + return await asyncio.to_thread(self._operate, "get", key) + + async def delete_token(self, key: str) -> None: + _validate_key(key) + await asyncio.to_thread(self._operate, "delete", key) + + async def list_keys(self) -> list[str]: + return await asyncio.to_thread(self._operate, "list") + + async def clear(self) -> None: + await asyncio.to_thread(self._operate, "clear") + + +def get_default_store_path() -> str: + return str(Path.home() / ".nitrostack" / "tokens.json") + + +def create_default_token_store( + store_path: str | Path | None = None, encryption_key: str | None = None, +) -> FileTokenStore: + return FileTokenStore(store_path or get_default_store_path(), encryption_key) diff --git a/pyproject.toml b/pyproject.toml index 6f480c5..4c39763 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -21,7 +21,9 @@ dependencies = [ ] [project.optional-dependencies] +token-encryption = ["cryptography>=41"] dev = [ + "cryptography>=41", "pytest-cov>=4.1.0", "pytest>=8", "pytest-asyncio>=0.23", @@ -87,4 +89,4 @@ exclude_lines = [ "pragma: no cover", "if TYPE_CHECKING:", "if __name__ == .__main__.:", -] \ No newline at end of file +] diff --git a/tests/test_oauth_client.py b/tests/test_oauth_client.py new file mode 100644 index 0000000..4622146 --- /dev/null +++ b/tests/test_oauth_client.py @@ -0,0 +1,447 @@ +"""Issue #68 client lifecycle, discovery trust, and single-flight refresh tests.""" + +import asyncio +import base64 +import json +import logging +import traceback +import urllib.error +import urllib.parse +import urllib.request +from unittest.mock import AsyncMock + +import pytest + +from nitrostack import ( + AuthorizationFlow, ClientRegistration, FileTokenStore, MemoryTokenStore, OAuth2Client, + OAuth2ClientConfig, OAuthClientError, StoredToken, get_well_known_metadata_uris, + parse_www_authenticate_header, +) +from nitrostack.auth import cimd +from nitrostack.auth.client import _Response +from nitrostack.auth.pkce import generate_code_challenge, is_valid_code_verifier + +ACCESS = "ACCESS_SENTINEL_NEVER_LOG" +REFRESH = "REFRESH_SENTINEL_NEVER_LOG" +SECRET = "CLIENT_SENTINEL_NEVER_LOG: with spaces" +ENDPOINT = "https://issuer.example/token" + + +def response(value=None, *, status=200, headers=None, raw=None): + body = raw if raw is not None else json.dumps(value if value is not None else {"access_token": ACCESS}).encode() + return _Response(status, headers or {}, body) + + +def client(**overrides): + return OAuth2Client({"client_id": "client:id", "client_secret": SECRET, + "issuer": "https://issuer.example", "redirect_uri": "http://127.0.0.1/callback", + "authorization_endpoint": "https://issuer.example/authorize?audience=api&state=old&scope=old", + "token_endpoint": ENDPOINT, "revocation_endpoint": "https://issuer.example/revoke", + "registration_endpoint": "https://issuer.example/register", **overrides}) + + +def install_transport(monkeypatch, oauth, *responses): + send = AsyncMock(side_effect=list(responses) if responses else None) + if not responses: + send.return_value = response() + monkeypatch.setattr(oauth, "_send", send) + return send + + +async def saved(oauth, key="alice", refresh=REFRESH): + token = StoredToken(ACCESS, "Bearer", 1, refresh, "read", "https://resource.example") + await oauth.store.save_token(key, token) + return token + + +@pytest.mark.asyncio +async def test_client_credentials_are_encoded_saved_and_redacted(monkeypatch): + oauth = client() + send = install_transport(monkeypatch, oauth, response({"access_token": ACCESS, "refresh_token": REFRESH})) + result = await oauth.get_client_credentials_token("alice", scope="read", resource="https://resource.example") + assert isinstance(result, StoredToken) and await oauth.store.get_token("alice") == result + request = send.call_args.args[0] + params = urllib.parse.parse_qs(request.body.decode()) + assert params == {"grant_type": ["client_credentials"], "client_id": ["client:id"], + "scope": ["read"], "resource": ["https://resource.example"]} + assert base64.b64decode(request.headers["Authorization"][6:]).decode() == ( + urllib.parse.quote_plus("client:id", safe="") + ":" + urllib.parse.quote_plus(SECRET, safe="") + ) + assert request.timeout == 10 and request.pinned_ip is None + assert all(secret not in repr(request) + repr(result) + repr(oauth.config) for secret in (ACCESS, REFRESH, SECRET)) + + +@pytest.mark.asyncio +@pytest.mark.parametrize("encryption_key", [None, "encryption-key"]) +async def test_client_persists_and_refreshes_after_file_store_reopening(tmp_path, monkeypatch, encryption_key): + path = tmp_path / "tokens.json" + config = OAuth2ClientConfig(client_id="client", client_secret=SECRET, token_endpoint=ENDPOINT) + oauth = OAuth2Client(config, store=FileTokenStore(path, encryption_key)) + install_transport(monkeypatch, oauth, response({"access_token": ACCESS, "refresh_token": REFRESH, "expires_in": 0})) + await oauth.get_client_credentials_token("alice", scope="read", resource="https://resource.example") + reopened = OAuth2Client(config, store=FileTokenStore(path, encryption_key)) + install_transport(monkeypatch, reopened, response({"access_token": "replacement"})) + token = await reopened.refresh_token("alice") + assert token.access_token == "replacement" and token.refresh_token == REFRESH + assert token.resource == "https://resource.example" + assert await reopened.store.get_token("alice") == token + + +@pytest.mark.asyncio +async def test_authorization_url_pkce_state_and_public_code_exchange(monkeypatch): + oauth = client(client_secret=None) + flow = await oauth.start_authorization_flow(scope="read", resource="https://resource.example") + second = await oauth.start_authorization_flow() + assert isinstance(flow, AuthorizationFlow) and flow.state != second.state + assert is_valid_code_verifier(flow.code_verifier) + assert generate_code_challenge(flow.code_verifier) == flow.code_challenge + query = urllib.parse.parse_qs(urllib.parse.urlsplit(flow.authorization_url).query) + assert query["state"] == [flow.state] and query["audience"] == ["api"] + assert query["code_challenge_method"] == ["S256"] + assert flow.code_verifier not in flow.authorization_url + assert "scope" not in urllib.parse.parse_qs(urllib.parse.urlsplit(second.authorization_url).query) + assert flow.code_verifier not in repr(flow) and flow.state not in repr(flow) + send = install_transport(monkeypatch, oauth) + token = await oauth.exchange_code_for_token("alice", code="one-time-code", code_verifier=flow.code_verifier) + request = send.call_args.args[0] + assert "Authorization" not in request.headers + assert urllib.parse.parse_qs(request.body.decode())["code_verifier"] == [flow.code_verifier] + assert await oauth.store.get_token("alice") == token + + +@pytest.mark.asyncio +async def test_refresh_is_single_flight_and_preserves_rotation_fields(monkeypatch): + oauth = client() + previous = await saved(oauth) + entered, release = asyncio.Event(), asyncio.Event() + calls = [] + + async def send(request): + calls.append(request) + entered.set() + await release.wait() + return response({"access_token": "replacement"}) + + monkeypatch.setattr(oauth, "_send", send) + tasks = [asyncio.create_task(oauth.refresh_token("alice")) for _ in range(50)] + await asyncio.wait_for(entered.wait(), 2) + await asyncio.sleep(0) + assert len(calls) == 1 + release.set() + results = await asyncio.gather(*tasks) + assert all(result is results[0] for result in results) + assert results[0].access_token == "replacement" + assert results[0].refresh_token == previous.refresh_token + assert results[0].scope == previous.scope and results[0].resource == previous.resource + assert await oauth.store.get_token("alice") == results[0] + assert oauth._refresh_tasks == {} + + +@pytest.mark.asyncio +async def test_different_keys_refresh_independently_and_one_waiter_can_cancel(monkeypatch): + oauth = client() + await saved(oauth, "alice", "alice-refresh") + await saved(oauth, "bob", "bob-refresh") + both_started, release = asyncio.Event(), asyncio.Event() + calls = [] + + async def send(request): + calls.append(urllib.parse.parse_qs(request.body.decode())["refresh_token"][0]) + if len(calls) == 2: + both_started.set() + await release.wait() + return response() + + monkeypatch.setattr(oauth, "_send", send) + cancelled = asyncio.create_task(oauth.refresh_token("alice")) + survivor = asyncio.create_task(oauth.refresh_token("alice")) + bob = asyncio.create_task(oauth.refresh_token("bob")) + await asyncio.wait_for(both_started.wait(), 2) + cancelled.cancel() + with pytest.raises(asyncio.CancelledError): + await cancelled + assert not survivor.done() + release.set() + await asyncio.gather(survivor, bob) + assert sorted(calls) == ["alice-refresh", "bob-refresh"] + + +@pytest.mark.asyncio +async def test_refresh_shared_failure_can_retry_without_losing_credentials(monkeypatch): + oauth = client() + previous = await saved(oauth) + release = asyncio.Event() + calls = [] + + async def send(request): + calls.append(request) + await release.wait() + return response({"error": "invalid_grant", "error_description": ACCESS + REFRESH}, status=400) + + monkeypatch.setattr(oauth, "_send", send) + tasks = [asyncio.create_task(oauth.refresh_token("alice")) for _ in range(20)] + await asyncio.sleep(0) + await asyncio.sleep(0) + release.set() + errors = await asyncio.gather(*tasks, return_exceptions=True) + assert len(calls) == 1 and all(isinstance(error, OAuthClientError) for error in errors) + assert all(error is errors[0] for error in errors) + assert "invalid_grant" in str(errors[0]) and ACCESS not in repr(errors[0]) + assert await oauth.store.get_token("alice") == previous + send = install_transport(monkeypatch, oauth, response({"access_token": "new", "refresh_token": "rotated"})) + result = await oauth.refresh_token("alice") + assert send.await_count == 1 and result.refresh_token == "rotated" + + +@pytest.mark.asyncio +async def test_persistence_failure_does_not_claim_refresh_success(monkeypatch): + oauth = client() + previous = await saved(oauth) + install_transport(monkeypatch, oauth) + monkeypatch.setattr(oauth.store, "save_token", AsyncMock(side_effect=OSError(ACCESS))) + with pytest.raises(OAuthClientError) as exc: + await oauth.refresh_token("alice") + assert ACCESS not in str(exc.value) + assert await oauth.store.get_token("alice") == previous + + +@pytest.mark.asyncio +async def test_revocation_waits_for_refresh_and_removes_rotated_credential(monkeypatch): + oauth = client() + await saved(oauth) + entered, release = asyncio.Event(), asyncio.Event() + revoked = [] + + async def send(request): + if request.url == ENDPOINT: + entered.set() + await release.wait() + return response({"access_token": "new-access", "refresh_token": "new-refresh"}) + revoked.append(urllib.parse.parse_qs(request.body.decode())["token"][0]) + return response(raw=b"") + + monkeypatch.setattr(oauth, "_send", send) + refresh = asyncio.create_task(oauth.refresh_token("alice")) + await asyncio.wait_for(entered.wait(), 2) + revoke = asyncio.create_task(oauth.revoke_token("alice")) + await asyncio.sleep(0) + assert revoked == [] + release.set() + await asyncio.gather(refresh, revoke) + assert revoked == ["new-refresh"] + assert await oauth.store.get_token("alice") is None + + +@pytest.mark.asyncio +async def test_all_waiters_can_cancel_without_leaking_unhandled_failures(monkeypatch): + oauth = client() + await saved(oauth) + entered, release = asyncio.Event(), asyncio.Event() + errors = [] + loop = asyncio.get_running_loop() + original_handler = loop.get_exception_handler() + loop.set_exception_handler(lambda loop, context: errors.append(context)) + + async def send(request): + entered.set() + await release.wait() + raise OSError(ACCESS) + + monkeypatch.setattr(oauth, "_send", send) + try: + waiter = asyncio.create_task(oauth.refresh_token("alice")) + await asyncio.wait_for(entered.wait(), 2) + shared = oauth._refresh_tasks["alice"] + waiter.cancel() + with pytest.raises(asyncio.CancelledError): + await waiter + assert not shared.cancelled() + release.set() + with pytest.raises(OAuthClientError): + await shared + await asyncio.sleep(0) + assert oauth._refresh_tasks == {} and errors == [] + finally: + loop.set_exception_handler(original_handler) + + +@pytest.mark.asyncio +async def test_revocation_success_failure_and_registration(monkeypatch): + oauth = client() + await saved(oauth) + send = install_transport(monkeypatch, oauth, response(raw=b"", status=200)) + await oauth.revoke_token("alice") + assert await oauth.store.get_token("alice") is None + assert urllib.parse.parse_qs(send.call_args.args[0].body.decode())["token"] == [REFRESH] + await saved(oauth) + install_transport(monkeypatch, oauth, response({"error": "invalid_client"}, status=401)) + with pytest.raises(OAuthClientError): + await oauth.revoke_token("alice", token_type_hint="access_token") + assert await oauth.store.get_token("alice") is not None + send = install_transport(monkeypatch, oauth, response({"client_id": "registered", "client_secret": SECRET})) + result = await oauth.register_client({"redirect_uris": ["http://127.0.0.1/callback"]}) + assert isinstance(result, ClientRegistration) and result.client_secret == SECRET + assert SECRET not in repr(result) + assert result.to_dict()["client_secret"] == SECRET + assert json.loads(send.call_args.args[0].body)["redirect_uris"] == ["http://127.0.0.1/callback"] + + +@pytest.mark.asyncio +async def test_errors_logs_and_transport_representations_do_not_leak(monkeypatch, caplog, capsys): + oauth = client() + caplog.set_level(logging.DEBUG) + send = install_transport(monkeypatch, oauth, response({"error": ACCESS, "error_description": SECRET}, status=401)) + with pytest.raises(OAuthClientError) as exc: + await oauth.get_client_credentials_token("alice") + rendered = "".join(traceback.format_exception(exc.type, exc.value, exc.tb)) + logging.getLogger(__name__).debug("client=%r config=%r request=%r error=%r", oauth, oauth.config, + send.call_args.args[0], exc.value) + output = rendered + caplog.text + str(capsys.readouterr()) + assert all(secret not in output for secret in (ACCESS, REFRESH, SECRET)) + install_transport(monkeypatch, oauth, OSError(ACCESS + SECRET)) + with pytest.raises(OAuthClientError) as exc: + await oauth.get_client_credentials_token("alice") + assert all(secret not in "".join(traceback.format_exception(exc.type, exc.value, exc.tb)) for secret in (ACCESS, SECRET)) + + +@pytest.mark.asyncio +@pytest.mark.parametrize("raw", [b"not-json", b"[]", b"null", b'{}', b'{"access_token":null}', + b'{"access_token":"token","expires_in":-1}']) +async def test_invalid_token_responses_are_not_saved(monkeypatch, raw): + oauth = client() + install_transport(monkeypatch, oauth, response(raw=raw)) + with pytest.raises(OAuthClientError): + await oauth.get_client_credentials_token("alice") + assert await oauth.store.list_keys() == [] + + +@pytest.mark.asyncio +async def test_discovery_pins_unknown_urls_and_does_not_require_pkce_for_client_credentials(monkeypatch): + oauth = OAuth2Client({"client_id": "client", "client_secret": SECRET, "issuer": "https://issuer.example"}) + safe = AsyncMock(return_value="8.8.8.8") + monkeypatch.setattr(cimd, "assert_safe_fetch_target", safe) + metadata = {"issuer": "https://issuer.example", "token_endpoint": ENDPOINT, + "authorization_endpoint": "https://issuer.example/authorize"} + send = install_transport(monkeypatch, oauth, response(metadata), response()) + assert await oauth.discover_authorization_server_metadata() == metadata + result = await oauth.get_client_credentials_token("alice") + assert result.access_token == ACCESS + assert safe.await_count == 2 + assert all(call.args[0].pinned_ip == "8.8.8.8" for call in send.call_args_list) + with pytest.raises(OAuthClientError, match="PKCE"): + await oauth.start_authorization_flow(redirect_uri="http://127.0.0.1/callback") + + +@pytest.mark.asyncio +async def test_discovery_failure_preserves_previous_metadata_and_explicit_endpoint_wins(monkeypatch): + oauth = client() + monkeypatch.setattr(cimd, "assert_safe_fetch_target", AsyncMock(return_value="8.8.8.8")) + valid = {"issuer": "https://issuer.example", "token_endpoint": "https://discovered.example/token"} + send = install_transport(monkeypatch, oauth, response(valid)) + await oauth.discover_authorization_server_metadata() + assert oauth._endpoint("token_endpoint") == ENDPOINT + install_transport(monkeypatch, oauth, response({"issuer": "https://wrong.example"}), + response(raw=b"[]")) + with pytest.raises(OAuthClientError): + await oauth.discover_authorization_server_metadata() + assert oauth._metadata == valid + + +@pytest.mark.asyncio +async def test_protected_resource_challenge_and_well_known_fallback(monkeypatch): + oauth = client() + safe = AsyncMock(return_value="8.8.8.8") + monkeypatch.setattr(cimd, "assert_safe_fetch_target", safe) + resource = "https://resource.example/reports" + metadata = {"resource": resource, "authorization_servers": ["https://issuer.example"]} + send = install_transport(monkeypatch, oauth, + response({}, status=401, headers={"WWW-Authenticate": 'Bearer resource_metadata="https://metadata.example/info"'}), + response(metadata)) + assert await oauth.discover_protected_resource_metadata(resource) == metadata + assert send.call_args_list[1].args[0].url == "https://metadata.example/info" + send = install_transport(monkeypatch, oauth, response({}, status=404), response(metadata)) + assert await oauth.discover_protected_resource_metadata(resource) == metadata + assert send.call_args_list[1].args[0].url.endswith("/.well-known/oauth-protected-resource/reports") + + +@pytest.mark.asyncio +async def test_operator_configured_loopback_metadata_avoids_ssrf_exception_flags(monkeypatch): + issuer = "http://127.0.0.1:9876" + oauth = OAuth2Client({"issuer": issuer, "token_endpoint": issuer + "/token", + "authorization_server_metadata_url": issuer + "/.well-known/oauth-authorization-server"}) + guard = AsyncMock(side_effect=AssertionError("must not fetch untrusted URLs")) + monkeypatch.setattr(cimd, "assert_safe_fetch_target", guard) + send = install_transport(monkeypatch, oauth, response({"issuer": issuer, "token_endpoint": issuer + "/token"})) + await oauth.discover_authorization_server_metadata() + guard.assert_not_called() + assert send.call_args.args[0].pinned_ip is None + + +@pytest.mark.asyncio +async def test_untrusted_private_dns_is_rejected_before_credentials_are_sent(monkeypatch): + oauth = client(token_endpoint=None) + oauth._metadata = {"token_endpoint": "https://internal.example/token"} + monkeypatch.setattr(cimd.socket, "getaddrinfo", lambda *args, **kwargs: [(2, 1, 6, "", ("10.0.0.1", 0))]) + send = install_transport(monkeypatch, oauth) + with pytest.raises(OAuthClientError): + await oauth.get_client_credentials_token("alice") + send.assert_not_called() + + +def test_metadata_helpers_and_snake_case_only(): + assert get_well_known_metadata_uris("https://resource.example/reports") == [ + "https://resource.example/.well-known/oauth-protected-resource/reports", + "https://resource.example/.well-known/oauth-protected-resource", + ] + parsed = parse_www_authenticate_header('Basic realm="other", bEaReR realm="a,b", resource_metadata="https://safe.example/info"') + assert parsed["realm"] == "a,b" and parsed["resource_metadata"] == "https://safe.example/info" + assert parse_www_authenticate_header('Basic realm="other"') is None + assert not hasattr(client(), "refreshToken") + assert not hasattr(MemoryTokenStore(), "getToken") + + +@pytest.mark.asyncio +async def test_supplied_state_and_discovered_pkce_support(monkeypatch): + oauth = client() + oauth._metadata = {"code_challenge_methods_supported": ["S256"]} + flow = await oauth.start_authorization_flow(state="caller-state") + assert flow.state == "caller-state" and flow.code_challenge_method == "S256" + with pytest.raises(OAuthClientError): + await oauth.start_authorization_flow(state="") + with pytest.raises(OAuthClientError): + await oauth.exchange_code_for_token("alice", code="code", code_verifier="bad") + with pytest.raises(OAuthClientError): + await oauth.refresh_token("missing") + with pytest.raises(OAuthClientError): + await oauth.refresh_token("") + assert isinstance(OAuth2Client({}).store, MemoryTokenStore) + + +@pytest.mark.asyncio +async def test_configured_protected_metadata_and_failed_document_do_not_leak(monkeypatch): + metadata_url = "https://operator.example/resource-metadata" + oauth = client(protected_resource_metadata_url=metadata_url) + guard = AsyncMock(side_effect=AssertionError("operator endpoint must be trusted")) + monkeypatch.setattr(cimd, "assert_safe_fetch_target", guard) + metadata = {"resource": "https://resource.example/", "authorization_servers": ["https://issuer.example"]} + send = install_transport(monkeypatch, oauth, response(metadata)) + assert await oauth.discover_protected_resource_metadata("https://resource.example/") == metadata + assert send.call_args.args[0].url == metadata_url + guard.assert_not_called() + install_transport(monkeypatch, oauth, response({"resource": "https://wrong.example/"})) + with pytest.raises(OAuthClientError): + await oauth.discover_protected_resource_metadata("https://resource.example/") + + +@pytest.mark.parametrize("options", [{"timeout_seconds": 0}, {"timeout_seconds": float("inf")}, + {"max_response_bytes": 0}, {"max_response_bytes": True}]) +def test_invalid_client_limits(options): + with pytest.raises(OAuthClientError): + OAuth2Client({}, **options) + + +@pytest.mark.parametrize("url", ["http://public.example/token", "https://user:secret@public.example/token", + "https://public.example/token#fragment", "https://public.example:bad/token"]) +def test_invalid_operator_url_is_rejected(url): + with pytest.raises(OAuthClientError): + client(token_endpoint=url) diff --git a/tests/test_oauth_client_transport.py b/tests/test_oauth_client_transport.py new file mode 100644 index 0000000..8006bc9 --- /dev/null +++ b/tests/test_oauth_client_transport.py @@ -0,0 +1,177 @@ +"""Exercise the urllib seam, DNS pins, TLS identity, and bounded IO.""" + +import asyncio +import io +import ssl +import threading +import traceback +import urllib.error +import urllib.request +from unittest.mock import AsyncMock, MagicMock + +import pytest + +from nitrostack import OAuth2Client, OAuthClientError +from nitrostack.auth import cimd +from nitrostack.auth.client import _Response + +ENDPOINT = "https://issuer.example:8443/token" +SECRET = "TRANSPORT_SENTINEL_DO_NOT_LOG" + + +class WireResponse(io.BytesIO): + def __init__(self, body, *, status=200, headers=None): + super().__init__(body) + self.code = status + self.headers = headers or {} + + +def make_client(**limits): + return OAuth2Client({"client_id": "client", "client_secret": SECRET, "token_endpoint": ENDPOINT}, **limits) + + +@pytest.mark.asyncio +async def test_real_urllib_seam_carries_form_headers_and_timeout(monkeypatch): + oauth = make_client(timeout_seconds=4) + opener = MagicMock() + opener.open.return_value = WireResponse(b'{"access_token":"ok"}') + builder = MagicMock(return_value=opener) + monkeypatch.setattr(cimd, "_build_fetch_opener", builder) + result = await oauth.get_client_credentials_token("alice") + assert result.access_token == "ok" + outgoing = opener.open.call_args.args[0] + assert outgoing.full_url == ENDPOINT and outgoing.get_method() == "POST" + assert outgoing.get_header("Content-type") == "application/x-www-form-urlencoded" + assert outgoing.get_header("Authorization").startswith("Basic ") + assert opener.open.call_args.kwargs["timeout"] == 4 + builder.assert_called_once_with(ENDPOINT, None) + + +@pytest.mark.asyncio +@pytest.mark.parametrize("body,headers", [ + (b"x" * 33, {}), (b"{}", {"Content-Length": "33"}), + (b"{}", {"Content-Length": "-1"}), (b"{}", {"Content-Length": "not-a-number"}), +]) +async def test_urllib_limits_response_before_parsing(monkeypatch, body, headers): + oauth = make_client(max_response_bytes=32) + opener = MagicMock() + opener.open.return_value = WireResponse(body, headers=headers) + monkeypatch.setattr(cimd, "_build_fetch_opener", lambda *args: opener) + with pytest.raises(OAuthClientError): + await oauth.get_client_credentials_token("alice") + assert await oauth.store.list_keys() == [] + + +@pytest.mark.asyncio +async def test_async_fake_transport_cannot_bypass_response_limit(monkeypatch): + oauth = make_client(max_response_bytes=32) + monkeypatch.setattr(oauth, "_send", AsyncMock(return_value=_Response(200, {}, b"x" * 33))) + with pytest.raises(OAuthClientError, match="response size"): + await oauth.get_client_credentials_token("alice") + + +@pytest.mark.asyncio +async def test_blocking_transport_runs_on_worker_thread(monkeypatch): + oauth = make_client() + loop = asyncio.get_running_loop() + entered, release = asyncio.Event(), threading.Event() + thread_ids = [] + + def blocking(request): + thread_ids.append(threading.get_ident()) + loop.call_soon_threadsafe(entered.set) + assert release.wait(3) + return _Response(200, {}, b'{"access_token":"ok"}') + + monkeypatch.setattr(oauth, "_send_sync", blocking) + task = asyncio.create_task(oauth.get_client_credentials_token("alice")) + try: + await asyncio.wait_for(entered.wait(), 2) + assert thread_ids == [thread_ids[0]] and thread_ids[0] != threading.get_ident() + finally: + release.set() + await task + + +@pytest.mark.asyncio +async def test_timeout_and_http_error_bodies_are_sanitized(monkeypatch): + oauth = make_client() + opener = MagicMock() + monkeypatch.setattr(cimd, "_build_fetch_opener", lambda *args: opener) + for failure in [TimeoutError(SECRET), urllib.error.HTTPError( + ENDPOINT + "?token=" + SECRET, 401, SECRET, {}, + io.BytesIO(b'{"error":"invalid_client","error_description":"' + SECRET.encode() + b'"}'), + )]: + opener.open.side_effect = failure + with pytest.raises(OAuthClientError) as exc: + await oauth.get_client_credentials_token("alice") + rendered = "".join(traceback.format_exception(exc.type, exc.value, exc.tb)) + assert SECRET not in rendered + + +@pytest.mark.asyncio +@pytest.mark.parametrize("status", [301, 302, 303, 307, 308]) +async def test_redirect_responses_are_terminal_not_followed(monkeypatch, status): + oauth = make_client() + send = AsyncMock(return_value=_Response(status, {"Location": "http://127.0.0.1/private"}, b"")) + monkeypatch.setattr(oauth, "_send", send) + with pytest.raises(OAuthClientError) as exc: + await oauth.get_client_credentials_token("alice") + assert exc.value.status == status and send.await_count == 1 + request = urllib.request.Request(ENDPOINT) + with pytest.raises(urllib.error.HTTPError): + cimd._NoRedirectHandler().redirect_request(request, None, status, "redirect", {}, "http://127.0.0.1/private") + + +@pytest.mark.asyncio +async def test_untrusted_target_uses_checked_ip_and_disables_proxy_redirection(monkeypatch): + oauth = OAuth2Client({"client_id": "client", "client_secret": SECRET}) + oauth._metadata = {"token_endpoint": ENDPOINT} + guard = AsyncMock(return_value="8.8.8.8") + monkeypatch.setattr(cimd, "assert_safe_fetch_target", guard) + send = AsyncMock(return_value=_Response(200, {}, b'{"access_token":"ok"}')) + monkeypatch.setattr(oauth, "_send", send) + await oauth.get_client_credentials_token("alice") + guard.assert_awaited_once_with(ENDPOINT, require_non_root_path=False) + assert send.call_args.args[0].pinned_ip == "8.8.8.8" + builder = MagicMock() + monkeypatch.setattr(urllib.request, "build_opener", builder) + cimd._build_fetch_opener(ENDPOINT, "8.8.8.8") + handlers = builder.call_args.args + assert any(isinstance(handler, cimd._NoRedirectHandler) for handler in handlers) + assert any(isinstance(handler, urllib.request.ProxyHandler) and handler.proxies == {} for handler in handlers) + secure = next(handler for handler in handlers if isinstance(handler, cimd._PinnedHTTPSHandler)) + assert secure._hostname == "issuer.example" and secure._port == 8443 + + +def test_pinned_tls_connection_keeps_port_and_hostname(monkeypatch): + sock = MagicMock() + sock.getpeername.return_value = ("8.8.8.8", 8443) + connect = MagicMock(return_value=sock) + monkeypatch.setattr(cimd.socket, "create_connection", connect) + context = ssl.create_default_context() + assert context.check_hostname and context.verify_mode == ssl.CERT_REQUIRED + wrap = MagicMock(return_value=sock) + monkeypatch.setattr(context, "wrap_socket", wrap) + connection = cimd._PinnedHTTPSConnection("issuer.example", "8.8.8.8", port=8443, context=context) + connection.connect() + assert connect.call_args.args[0] == ("8.8.8.8", 8443) + wrap.assert_called_once_with(sock, server_hostname="issuer.example") + sock.getpeername.return_value = ("127.0.0.1", 8443) + with pytest.raises(cimd.CimdFetchError): + connection.connect() + assert sock.close.called + + +@pytest.mark.asyncio +async def test_generic_root_url_validation_does_not_weaken_cimd(monkeypatch): + url = "https://public.example/" + assert cimd.validate_fetch_url(url) == url + with pytest.raises(cimd.CimdValidationError, match="non-root"): + cimd.validate_client_identifier_url(url) + with pytest.raises(cimd.CimdValidationError, match="non-root"): + await cimd.assert_safe_fetch_target(url) + monkeypatch.setattr(cimd.socket, "getaddrinfo", lambda *args, **kwargs: [(2, 1, 6, "", ("8.8.8.8", 0))]) + assert await cimd.assert_safe_fetch_target(url, require_non_root_path=False) == "8.8.8.8" + with pytest.raises(cimd.CimdValidationError): + cimd.validate_fetch_url("https://public.example/%2e%2e/private") diff --git a/tests/test_token_store.py b/tests/test_token_store.py new file mode 100644 index 0000000..e4a29bd --- /dev/null +++ b/tests/test_token_store.py @@ -0,0 +1,211 @@ +"""Issue #68 storage, expiry, encryption compatibility, and secret safety.""" + +import asyncio +import builtins +import hashlib +import json +import os +import stat +from dataclasses import FrozenInstanceError +from pathlib import Path + +import pytest + +from nitrostack import ( + FileTokenStore, MemoryTokenStore, StoredToken, TokenStoreError, + calculate_expiration, create_default_token_store, get_default_store_path, + is_token_expired, token_response_to_stored, +) + +ACCESS = "ACCESS_SENTINEL_NOT_FOR_LOGS" +REFRESH = "REFRESH_SENTINEL_NOT_FOR_LOGS" +KEY = "ENCRYPTION_SENTINEL_NOT_FOR_LOGS" + + +def token(**overrides): + return StoredToken(**{"access_token": ACCESS, "token_type": "Bearer", "expires_at": 123, + "refresh_token": REFRESH, "scope": "read", "resource": "https://resource.example", + **overrides}) + + +@pytest.fixture(params=["memory", "file", "encrypted"]) +def store(request, tmp_path): + if request.param == "memory": + return MemoryTokenStore() + return FileTokenStore(tmp_path / "tokens.json", KEY if request.param == "encrypted" else None) + + +@pytest.mark.asyncio +async def test_roundtrip_expired_record_and_management(store): + original = token() + assert is_token_expired(original) + await store.save_token("issuer:client:alice:resource", original) + await store.save_token("bob", token(access_token="other")) + result = await store.get_token("issuer:client:alice:resource") + assert result == original + assert result.to_dict() == original.to_dict() + assert result.refresh_token == REFRESH + with pytest.raises(FrozenInstanceError): + result.access_token = "changed" + assert set(await store.list_keys()) == {"issuer:client:alice:resource", "bob"} + await store.delete_token("bob") + await store.delete_token("missing") + assert await store.get_token("bob") is None + await store.clear() + assert await store.list_keys() == [] + + +@pytest.mark.asyncio +async def test_store_and_record_representations_are_safe(store, caplog, capsys): + original = token() + await store.save_token("key", original) + caplog.set_level("DEBUG") + import logging + logging.getLogger(__name__).debug("store=%r token=%r", store, original) + output = repr(original) + str(original) + repr(store) + caplog.text + str(capsys.readouterr()) + assert all(secret not in output for secret in (ACCESS, REFRESH, KEY)) + + +@pytest.mark.asyncio +async def test_file_reopening_and_permissions(tmp_path, monkeypatch): + path = tmp_path / "private" / "tokens.json" + store = FileTokenStore(path) + replace = os.replace + modes = [] + + def observe(source, destination): + modes.append(stat.S_IMODE(Path(source).stat().st_mode)) + replace(source, destination) + + monkeypatch.setattr(os, "replace", observe) + await store.save_token("alice", token()) + assert await FileTokenStore(path).get_token("alice") == token() + assert list(path.parent.glob(".oauth-token-*")) == [] + if os.name == "posix": + assert modes == [0o600] + assert stat.S_IMODE(path.stat().st_mode) == 0o600 + path.chmod(0o644) + await store.get_token("alice") + assert stat.S_IMODE(path.stat().st_mode) == 0o600 + + +@pytest.mark.asyncio +@pytest.mark.parametrize("failure_point", ["replace", "fsync", "serialization"]) +async def test_atomic_failure_preserves_old_file(tmp_path, monkeypatch, failure_point): + path = tmp_path / "tokens.json" + store = FileTokenStore(path) + await store.save_token("alice", token()) + original = path.read_bytes() + + def fail(*args): + raise PermissionError(ACCESS + REFRESH) + + if failure_point == "serialization": + monkeypatch.setattr(store, "_encrypt", fail) + else: + monkeypatch.setattr(os, failure_point, fail) + with pytest.raises(TokenStoreError) as exc: + await store.save_token("bob", token(access_token="replacement")) + assert ACCESS not in str(exc.value) and REFRESH not in repr(exc.value) + assert path.read_bytes() == original + assert list(tmp_path.glob(".oauth-token-*")) == [] + assert await store.get_token("alice") == token() + + +@pytest.mark.asyncio +async def test_concurrent_instances_do_not_lose_keys(tmp_path): + stores = [FileTokenStore(tmp_path / "tokens.json") for _ in range(2)] + await asyncio.gather(*(stores[i % 2].save_token(str(i), token(access_token=str(i))) for i in range(30))) + assert set(await stores[0].list_keys()) == {str(i) for i in range(30)} + assert [(await stores[1].get_token(str(i))).access_token for i in range(30)] == [str(i) for i in range(30)] + + +@pytest.mark.asyncio +async def test_legacy_encryption_format_and_wrong_key(tmp_path): + from cryptography.hazmat.primitives.ciphers import Cipher, algorithms, modes + + path = tmp_path / "tokens.json" + derived = hashlib.pbkdf2_hmac("sha256", KEY.encode(), b"nitrostack-token-store", 100000, 32) + iv = os.urandom(16) + encryptor = Cipher(algorithms.AES(derived), modes.GCM(iv)).encryptor() + ciphertext = encryptor.update(json.dumps({"alice": token().to_dict()}).encode()) + encryptor.finalize() + path.write_text(f"{iv.hex()}:{encryptor.tag.hex()}:{ciphertext.hex()}") + assert await FileTokenStore(path, KEY).get_token("alice") == token() + original = path.read_bytes() + with pytest.raises(TokenStoreError) as exc: + await FileTokenStore(path, "wrong").get_token("alice") + assert ACCESS not in str(exc.value) + assert path.read_bytes() == original + await FileTokenStore(path, KEY).save_token("bob", token()) + assert ACCESS not in path.read_text() and REFRESH not in path.read_text() + assert await FileTokenStore(path, KEY).get_token("bob") == token() + + +@pytest.mark.asyncio +@pytest.mark.parametrize("content", ["broken", "[]", '{"alice":null}', '{"alice":{"access_token":"secret"}}']) +async def test_corruption_is_an_error_not_an_empty_store(tmp_path, content): + path = tmp_path / "tokens.json" + path.write_text(content) + with pytest.raises(TokenStoreError): + await FileTokenStore(path).get_token("alice") + assert path.read_text() == content + + +@pytest.mark.asyncio +async def test_missing_file_is_empty_and_symlink_is_refused(tmp_path, monkeypatch): + path = tmp_path / "tokens.json" + store = FileTokenStore(path) + assert await store.list_keys() == [] + assert not path.exists() + if os.name == "posix": + target = tmp_path / "target.json" + target.write_text("{}") + path.symlink_to(target) + else: + monkeypatch.setattr(Path, "is_symlink", lambda value: value == path) + with pytest.raises(TokenStoreError, match="symlink"): + await store.save_token("alice", token()) + + +@pytest.mark.asyncio +async def test_optional_encryption_import_is_lazy(tmp_path, monkeypatch): + original = builtins.__import__ + + def deny(name, *args, **kwargs): + if name.startswith("cryptography"): + raise ImportError(ACCESS) + return original(name, *args, **kwargs) + + monkeypatch.setattr(builtins, "__import__", deny) + await FileTokenStore(tmp_path / "plain.json").save_token("a", token()) + with pytest.raises(TokenStoreError, match="token-encryption"): + await FileTokenStore(tmp_path / "encrypted.json", KEY).save_token("a", token()) + assert not (tmp_path / "encrypted.json").exists() + + +def test_expiry_units_boundaries_rotation_and_one_hour_fallback(monkeypatch): + monkeypatch.setattr("nitrostack.auth.token_store.time.time", lambda: 1000) + assert calculate_expiration(60) == 1060000 + assert not is_token_expired(token(expires_at=1030001)) + assert is_token_expired(token(expires_at=1030000)) + assert is_token_expired(token(expires_at=1000000), skew=0) + for response in [{"access_token": ACCESS}, {"access_token": ACCESS, "expires_in": None}]: + assert token_response_to_stored(response).expires_at == 4600000 + assert token_response_to_stored({"access_token": ACCESS, "expires_in": 0}).expires_at == 1000000 + refreshed = token_response_to_stored({"access_token": "new"}, previous=token()) + assert refreshed.refresh_token == REFRESH and refreshed.scope == "read" + assert refreshed.resource == token().resource + + +@pytest.mark.parametrize("duration", [-1, float("inf"), float("nan"), "60", None, True, 10 ** 400, 1e308], + ids=["negative", "infinite", "nan", "text", "none", "bool", "huge-int", "huge-float"]) +def test_invalid_durations_are_rejected(duration): + with pytest.raises(ValueError): + calculate_expiration(duration) + + +def test_default_store_helpers_are_explicit(tmp_path, monkeypatch): + monkeypatch.setattr(Path, "home", lambda: tmp_path) + assert Path(get_default_store_path()) == tmp_path / ".nitrostack" / "tokens.json" + assert isinstance(create_default_token_store(), FileTokenStore) + assert not (tmp_path / ".nitrostack").exists() diff --git a/uv.lock b/uv.lock index 4c444be..9cff2e0 100644 --- a/uv.lock +++ b/uv.lock @@ -593,6 +593,7 @@ dependencies = [ [package.optional-dependencies] dev = [ + { name = "cryptography" }, { name = "httpx" }, { name = "pytest" }, { name = "pytest-asyncio" }, @@ -600,11 +601,16 @@ dev = [ { name = "pytest-timeout" }, { name = "tox" }, ] +token-encryption = [ + { name = "cryptography" }, +] [package.metadata] requires-dist = [ { name = "anyio", specifier = ">=4.0.0" }, { name = "certifi", specifier = ">=2026.10.19" }, + { name = "cryptography", marker = "extra == 'dev'", specifier = ">=41" }, + { name = "cryptography", marker = "extra == 'token-encryption'", specifier = ">=41" }, { name = "httpx", marker = "extra == 'dev'", specifier = ">=0.27.0" }, { name = "mcp", specifier = ">=2,<3" }, { name = "packaging", specifier = ">=23.0.0" }, @@ -619,7 +625,7 @@ requires-dist = [ { name = "uvicorn", specifier = ">=0.20.0" }, { name = "watchfiles", specifier = ">=0.18.0" }, ] -provides-extras = ["dev"] +provides-extras = ["token-encryption", "dev"] [[package]] name = "opentelemetry-api"