diff --git a/packages/auth/src/pipefy_auth/__init__.py b/packages/auth/src/pipefy_auth/__init__.py index 26a07eb2..addf035f 100644 --- a/packages/auth/src/pipefy_auth/__init__.py +++ b/packages/auth/src/pipefy_auth/__init__.py @@ -38,6 +38,7 @@ resolve_pipefy_auth, tier_for, ) +from pipefy_auth.responses import OAuthErrorResponse, TokenResponse from pipefy_auth.revoke import ( RevocationError, RevocationUnsupportedError, @@ -62,6 +63,7 @@ "DiscoveryPolicy", "LoginError", "LoginResult", + "OAuthErrorResponse", "OidcClient", "ProviderMetadata", "RefreshError", @@ -75,6 +77,7 @@ "SessionDeleteError", "StaticBearerAuth", "StoredSession", + "TokenResponse", "__version__", "configure_keychain_backend", "delete_session", diff --git a/packages/auth/src/pipefy_auth/discovery.py b/packages/auth/src/pipefy_auth/discovery.py index dc8128cc..73c5e80a 100644 --- a/packages/auth/src/pipefy_auth/discovery.py +++ b/packages/auth/src/pipefy_auth/discovery.py @@ -81,7 +81,7 @@ def fetch_provider_metadata( if response.status_code != 200: # Status-only; never echo the raw body. Discovery isn't OAuth so there's # no RFC 6749 ``error`` field to surface, and a `[:N]` window of the - # body is the same echo-channel class scrubbed in ``_format_token_error``. + # body is the same echo-channel class scrubbed in ``OAuthErrorResponse``. raise ValueError(f"OIDC discovery failed ({response.status_code}) at {url}") try: diff --git a/packages/auth/src/pipefy_auth/flow.py b/packages/auth/src/pipefy_auth/flow.py index 214a7e91..1cfdeea0 100644 --- a/packages/auth/src/pipefy_auth/flow.py +++ b/packages/auth/src/pipefy_auth/flow.py @@ -9,6 +9,7 @@ from urllib.parse import urlencode import httpx +from pydantic import ValidationError from pipefy_auth import _http from pipefy_auth.discovery import ( @@ -18,6 +19,11 @@ ) from pipefy_auth.loopback import CallbackResult, LoopbackCapture from pipefy_auth.pkce import challenge_from_verifier, generate_verifier +from pipefy_auth.responses import ( + OAuthErrorResponse, + TokenResponse, + _format_validation_error, +) _DEFAULT_SCOPES = ("openid", "profile", "email", "offline_access") _TOKEN_EXCHANGE_TIMEOUT_S = 30.0 @@ -29,10 +35,10 @@ class LoginError(RuntimeError): @dataclass(frozen=True) class LoginResult: - """The token response, plus the resolved issuer (post-discovery).""" + """The OAuth token, plus the resolved issuer (post-discovery).""" issuer: str - token_response: dict[str, object] + token: TokenResponse def build_authorization_url( @@ -65,7 +71,7 @@ def exchange_code( redirect_uri: str, code_verifier: str, client: httpx.Client, -) -> dict[str, object]: +) -> TokenResponse: """Exchange an authorization code for tokens at the issuer's token endpoint.""" try: response = client.post( @@ -82,42 +88,28 @@ def exchange_code( raise LoginError(f"Token exchange request failed: {exc}") from exc if response.status_code != 200: - raise LoginError(_format_token_error(response)) + # ``error_description`` is free-form per RFC 6749 §5.2 — its content + # reflects the IdP's framing, and surfacing it at all is part of trusting + # the IdP. A length cap wouldn't change that (an attacker can truncate + # to fit any cap, and a tight cap kills legitimate error context). + raise LoginError( + OAuthErrorResponse.from_response(response).render( + fallback=f"Token endpoint returned HTTP {response.status_code}", + prefix="Token exchange failed", + ) + ) try: payload = response.json() except ValueError as exc: raise LoginError(f"Token endpoint returned non-JSON response: {exc}") from exc if not isinstance(payload, dict): raise LoginError("Token endpoint returned a non-object JSON payload.") - return payload - - -def _format_token_error(response: httpx.Response) -> str: - """Render a token-exchange non-200 as a user-safe message. - - Only OAuth-standard ``error`` / ``error_description`` fields are surfaced; - raw bodies are never echoed (RFC 6749 doesn't forbid IdPs from including - submitted params like ``code_verifier`` in error responses, and a `[:N]` - snippet would be a guaranteed echo channel under a hostile IdP). - """ - generic = f"Token endpoint returned HTTP {response.status_code}" try: - payload = response.json() - except ValueError: - return generic - if not isinstance(payload, dict): - return generic - error = payload.get("error") - if not error: - return generic - # ``error_description`` is free-form per RFC 6749 §5.2 — its content reflects - # the IdP's framing, and surfacing it at all is part of trusting the IdP. A - # length cap wouldn't change that (an attacker can truncate to fit any cap, - # and a tight cap kills legitimate error context). - description = payload.get("error_description") - if description: - return f"Token exchange failed: {error}: {description}" - return f"Token exchange failed: {error}" + return TokenResponse.from_payload(payload) + except ValidationError as exc: + raise LoginError(_format_validation_error(exc)) from exc + except ValueError as exc: + raise LoginError(str(exc)) from exc def run_login( @@ -188,7 +180,7 @@ def run_login( _ensure_callback_ok(callback, expected_state=state) code = cast(str, callback.code) - token_response = exchange_code( + token = exchange_code( metadata=metadata, client_id=client_id, code=code, @@ -196,7 +188,7 @@ def run_login( code_verifier=verifier, client=http, ) - return LoginResult(issuer=metadata.issuer, token_response=token_response) + return LoginResult(issuer=metadata.issuer, token=token) def _ensure_callback_ok(callback: CallbackResult, *, expected_state: str) -> None: diff --git a/packages/auth/src/pipefy_auth/refresh.py b/packages/auth/src/pipefy_auth/refresh.py index 65a51059..715e521c 100644 --- a/packages/auth/src/pipefy_auth/refresh.py +++ b/packages/auth/src/pipefy_auth/refresh.py @@ -5,10 +5,16 @@ import time import httpx +from pydantic import ValidationError from pipefy_auth import _http from pipefy_auth.discovery import fetch_provider_metadata from pipefy_auth.locks import RefreshLockTimeout, file_lock, refresh_lock_path +from pipefy_auth.responses import ( + OAuthErrorResponse, + TokenResponse, + _format_validation_error, +) from pipefy_auth.storage import StoredSession, load_session, store_session _LEEWAY_S = 60 @@ -79,33 +85,20 @@ def _refresh_error_from(response: httpx.Response) -> RefreshError: """Render a non-200 refresh response as a structured ``RefreshError``. Only OAuth-standard ``error`` / ``error_description`` fields are surfaced; - raw bodies are never echoed (same threat model as the token-exchange scrub - in ``flow._format_token_error`` — a hostile IdP could echo submitted params - like ``refresh_token`` in error responses, and a ``[:N]`` window would be a - guaranteed leak channel). + raw bodies are never echoed (a hostile IdP could echo submitted params + like ``refresh_token`` in error responses, and a ``[:N]`` window would be + a guaranteed leak channel). """ - generic = f"Refresh failed (HTTP {response.status_code})" - try: - payload = response.json() - except ValueError: - return RefreshError(generic) - if not isinstance(payload, dict): - return RefreshError(generic) - error_value = payload.get("error") - error_code = error_value if isinstance(error_value, str) else None - description_value = payload.get("error_description") - description = description_value if isinstance(description_value, str) else None - if not error_code: - return RefreshError(generic) - if description: - return RefreshError( - f"{generic}: {error_code}: {description}", error_code=error_code - ) - return RefreshError(f"{generic}: {error_code}", error_code=error_code) + fallback = f"Refresh failed (HTTP {response.status_code})" + err = OAuthErrorResponse.from_response(response) + return RefreshError( + err.render(fallback=fallback, prefix="Refresh failed"), + error_code=err.error, + ) def _is_stale(session: StoredSession, leeway_s: int) -> bool: - expires_in = session.expires_in or 0 + expires_in = session.token.expires_in or 0 deadline = session.obtained_at + expires_in - leeway_s return time.time() >= deadline @@ -117,29 +110,38 @@ def _refresh_and_store( client_id: str, http_client: httpx.Client | None, ) -> StoredSession: - token_response = refresh_access_token( + prior = session.token + payload = refresh_access_token( issuer=issuer, client_id=client_id, - refresh_token=session.refresh_token, + refresh_token=prior.refresh_token, http_client=http_client, ) # Carry forward fields the IdP may omit from a refresh response so the # rotated session keeps its full shape — without ``expires_in`` the next # freshness check treats the token as already expired and refreshes again # on the very next call. - token_response.setdefault("refresh_token", session.refresh_token) - if session.expires_in is not None: - token_response.setdefault("expires_in", session.expires_in) - if session.refresh_expires_in is not None: - token_response.setdefault("refresh_expires_in", session.refresh_expires_in) - if session.scope is not None: - token_response.setdefault("scope", session.scope) - if session.id_token is not None: - token_response.setdefault("id_token", session.id_token) + payload.setdefault("refresh_token", prior.refresh_token) + if prior.expires_in is not None: + payload.setdefault("expires_in", prior.expires_in) + if prior.refresh_expires_in is not None: + payload.setdefault("refresh_expires_in", prior.refresh_expires_in) + if prior.scope is not None: + payload.setdefault("scope", prior.scope) + if prior.id_token is not None: + payload.setdefault("id_token", prior.id_token) + try: + new_token = TokenResponse.from_payload(payload) + except ValidationError as exc: + raise RefreshError( + f"Refresh response malformed: {_format_validation_error(exc)}" + ) from exc + except ValueError as exc: + raise RefreshError(f"Refresh response malformed: {exc}") from exc return store_session( issuer=session.issuer, client_id=session.client_id, - token_response=token_response, + token=new_token, ) diff --git a/packages/auth/src/pipefy_auth/resolver.py b/packages/auth/src/pipefy_auth/resolver.py index f8e9ee84..ac5fadb0 100644 --- a/packages/auth/src/pipefy_auth/resolver.py +++ b/packages/auth/src/pipefy_auth/resolver.py @@ -80,7 +80,7 @@ def _token() -> str: raise RuntimeError( "Stored Pipefy session was removed; run `pipefy auth login` again." ) - return session.access_token + return session.token.access_token def _force_refresh() -> str | None: try: @@ -91,7 +91,7 @@ def _force_refresh() -> str | None: ) except RefreshError: return None - return session.access_token if session is not None else None + return session.token.access_token if session is not None else None return RefreshableBearerAuth(token_provider=_token, force_refresh=_force_refresh) diff --git a/packages/auth/src/pipefy_auth/responses.py b/packages/auth/src/pipefy_auth/responses.py new file mode 100644 index 00000000..521322af --- /dev/null +++ b/packages/auth/src/pipefy_auth/responses.py @@ -0,0 +1,126 @@ +"""Typed OAuth response wrappers (RFC 6749 §5.1 success, §5.2 error; +RFC 8628 §3.5 error). + +``TokenResponse`` parses the token-endpoint success body. ``OAuthErrorResponse`` +parses the error envelope; the call site supplies the rendering prefix and +fallback so endpoint context stays out of the wire-shape model. +""" + +from __future__ import annotations + +from typing import Annotated + +import httpx +from pydantic import ( + BaseModel, + BeforeValidator, + ConfigDict, + Field, + StrictInt, + StrictStr, + ValidationError, +) + + +def _strip_or_none(value: object) -> str | None: + """Strip strings to ``None`` on empty; drop non-strings to ``None``. + + Non-string drop is load-bearing for :meth:`OAuthErrorResponse.from_response`: + callers invoke ``.render(...)`` inline without a try, so a non-string + ``error`` field must fall through to the fallback rather than raise. + """ + if not isinstance(value, str): + return None + stripped = value.strip() + return stripped or None + + +_OptionalStr = Annotated[str | None, BeforeValidator(_strip_or_none)] + + +class TokenResponse(BaseModel): + """Parsed OAuth 2.0 token-endpoint success response (RFC 6749 §5.1). + + Strict typing: ``StrictStr`` rejects bool/None coercion; ``StrictInt`` + rejects bool (``isinstance(True, int)`` is ``True`` in Python, so plain + ``int`` would accept a boolean lifetime). Unknown fields are ignored + because IdPs (Keycloak) routinely add proprietary keys like + ``not-before-policy`` / ``session_state``. + """ + + model_config = ConfigDict(extra="ignore", frozen=True) + + access_token: Annotated[StrictStr, Field(min_length=1)] + refresh_token: Annotated[StrictStr, Field(min_length=1)] + token_type: StrictStr = "Bearer" + expires_in: StrictInt | None = None + refresh_expires_in: StrictInt | None = None + scope: _OptionalStr = None + id_token: _OptionalStr = None + + @classmethod + def from_payload(cls, payload: dict[str, object]) -> "TokenResponse": + return cls.model_validate(payload) + + +class OAuthErrorResponse(BaseModel): + """Parsed OAuth error envelope (RFC 6749 §5.2 / RFC 8628 §3.5). + + ``error`` is ``None`` when the body wasn't OAuth-shaped (non-JSON, + non-dict, missing ``error``); callers render that case via :meth:`render`'s + ``fallback`` argument. + """ + + model_config = ConfigDict(extra="ignore", frozen=True) + + status_code: StrictInt + error: _OptionalStr = None + error_description: _OptionalStr = None + + @classmethod + def from_response(cls, response: httpx.Response) -> "OAuthErrorResponse": + try: + payload = response.json() + except ValueError: + payload = None + body = payload if isinstance(payload, dict) else {} + return cls.model_validate( + { + "status_code": response.status_code, + "error": body.get("error"), + "error_description": body.get("error_description"), + } + ) + + def render(self, *, fallback: str, prefix: str) -> str: + """Return a user-safe message; never echoes the raw body. + + Returns ``fallback`` when the response wasn't OAuth-shaped. ``prefix`` + names the failed action (e.g. ``"Token exchange failed"``). + """ + if not self.error: + return fallback + if self.error_description: + return f"{prefix}: {self.error}: {self.error_description}" + return f"{prefix}: {self.error}" + + +def _format_validation_error(exc: ValidationError) -> str: + """Render a pydantic ``ValidationError`` as a compact ``"loc: msg"`` list. + + Mirrors :func:`pipefy_mcp.tools.portal_tool_helpers.portal_element_validation_error` + so auth-facing messages match the MCP-tool shape. Avoids pydantic's verbose + default ``str(exc)`` (which includes doc URLs) on user-facing surfaces. + """ + clauses: list[str] = [] + for err in exc.errors(): + loc = ".".join(str(part) for part in err.get("loc", ())) + msg = str(err.get("msg", "")) + if loc and msg: + clauses.append(f"{loc}: {msg}") + elif msg: + clauses.append(msg) + return "; ".join(clauses) or "Response failed validation." + + +__all__ = ["OAuthErrorResponse", "TokenResponse"] diff --git a/packages/auth/src/pipefy_auth/revoke.py b/packages/auth/src/pipefy_auth/revoke.py index 47cc8e51..25d1e2a0 100644 --- a/packages/auth/src/pipefy_auth/revoke.py +++ b/packages/auth/src/pipefy_auth/revoke.py @@ -58,7 +58,7 @@ def revoke_session( # Keycloak returns 204; RFC treats any 2xx as success. No body echo on # failure — we just sent ``refresh_token`` in the POST body, so a ``[:N]`` # window would be a guaranteed leak channel under a hostile IdP (same - # threat model as ``flow._format_token_error``). + # threat model as ``OAuthErrorResponse.render``). if response.status_code // 100 != 2: raise RevocationError( f"Revocation endpoint returned HTTP {response.status_code}" diff --git a/packages/auth/src/pipefy_auth/storage.py b/packages/auth/src/pipefy_auth/storage.py index f7473bf1..a4959437 100644 --- a/packages/auth/src/pipefy_auth/storage.py +++ b/packages/auth/src/pipefy_auth/storage.py @@ -12,16 +12,27 @@ from __future__ import annotations -import json import time -from dataclasses import asdict, dataclass -from typing import Literal +from typing import Annotated, Any, Literal from urllib.parse import urlparse -from pipefy_infra.coerce import optional_int, optional_str +from pydantic import ( + BaseModel, + ConfigDict, + Field, + SerializerFunctionWrapHandler, + StrictInt, + StrictStr, + ValidationError, + model_serializer, + model_validator, +) + +from pipefy_auth.responses import TokenResponse _SERVICE = "pipefy" _KEYRING_FILENAME = "keyring.cfg" +_TOKEN_FIELDS: frozenset[str] = frozenset(TokenResponse.model_fields.keys()) def configure_keychain_backend(choice: Literal["auto", "file"]) -> None: @@ -53,20 +64,59 @@ class SessionDeleteError(RuntimeError): """Keychain backend rejected the delete (distinct from "entry absent").""" -@dataclass(frozen=True) -class StoredSession: - """Persisted session shape (JSON-serialised under one keychain key).""" +class StoredSession(BaseModel): + """Persisted session: identity + write timestamp + the token response. + + The token fields are bundled in :class:`TokenResponse` so there is one + source of truth for the OAuth wire shape. On-disk JSON destructures the + token attributes into the parent object (legacy flat shape) so existing + keychain entries continue to load. + + ``extra="forbid"`` because both writer and reader live in this codebase: + an unknown key on disk indicates corruption or hand-editing, not an IdP + extension. Unknown keys nested under ``token`` are still ignored + (see :class:`TokenResponse`). + """ - issuer: str - client_id: str - access_token: str - refresh_token: str - token_type: str - obtained_at: int - expires_in: int | None - refresh_expires_in: int | None - scope: str | None - id_token: str | None + model_config = ConfigDict(extra="forbid", frozen=True) + + issuer: Annotated[StrictStr, Field(min_length=1)] + client_id: Annotated[StrictStr, Field(min_length=1)] + obtained_at: StrictInt + token: TokenResponse + + @model_validator(mode="before") + @classmethod + def _accept_flat_blob(cls, data: Any) -> Any: + """Rebuild a nested ``token`` from a legacy flat on-disk shape. + + Pre-pydantic blobs destructured the token attributes into the parent + object. If we receive that shape (``"token"`` absent, token-shaped + keys present), re-nest before field validation. Already-nested input + passes through untouched. + """ + if not isinstance(data, dict): + return data + if "token" in data: + return data + if "access_token" not in data: + return data + nested: dict[str, Any] = {} + outer: dict[str, Any] = {} + for key, value in data.items(): + if key in _TOKEN_FIELDS: + nested[key] = value + else: + outer[key] = value + outer["token"] = nested + return outer + + @model_serializer(mode="wrap") + def _to_flat_blob(self, handler: SerializerFunctionWrapHandler) -> dict[str, Any]: + """Always serialize to the flat on-disk shape (legacy readers stay green).""" + nested = handler(self) + token = nested.pop("token", {}) + return {**nested, **token} def _issuer_host(issuer_url: str) -> str: @@ -85,7 +135,7 @@ def store_session( *, issuer: str, client_id: str, - token_response: dict[str, object], + token: TokenResponse, ) -> StoredSession: """Persist a token response in the OS keychain. Returns the stored shape. @@ -93,32 +143,17 @@ def store_session( KeyringError: When the keychain backend rejects the write. Caller should surface a user-facing message (e.g. headless Linux without a Secret Service daemon). - ValueError: When ``token_response`` is missing required fields. """ import keyring - try: - access_token = str(token_response["access_token"]) - refresh_token = str(token_response["refresh_token"]) - except KeyError as exc: - raise ValueError( - f"Token response is missing required field: {exc.args[0]!r}" - ) from exc - session = StoredSession( issuer=issuer, client_id=client_id, - access_token=access_token, - refresh_token=refresh_token, - token_type=str(token_response.get("token_type") or "Bearer"), obtained_at=int(time.time()), - expires_in=optional_int(token_response.get("expires_in")), - refresh_expires_in=optional_int(token_response.get("refresh_expires_in")), - scope=optional_str(token_response.get("scope")), - id_token=optional_str(token_response.get("id_token")), + token=token, ) keyring.set_password( - _SERVICE, keychain_key(issuer, client_id), json.dumps(asdict(session)) + _SERVICE, keychain_key(issuer, client_id), session.model_dump_json() ) return session @@ -135,12 +170,8 @@ def load_session(*, issuer: str, client_id: str) -> StoredSession | None: if not blob: return None try: - data = json.loads(blob) - except json.JSONDecodeError: - return None - try: - return StoredSession(**data) - except TypeError: + return StoredSession.model_validate_json(blob) + except ValidationError: return None diff --git a/packages/auth/tests/test_refresh.py b/packages/auth/tests/test_refresh.py index f0c4f922..62c135b0 100644 --- a/packages/auth/tests/test_refresh.py +++ b/packages/auth/tests/test_refresh.py @@ -9,6 +9,7 @@ from conftest import InMemoryKeyring from pipefy_auth import refresh, storage +from pipefy_auth.responses import TokenResponse _ISSUER = "https://signin.example.com/realms/pipefy" _CLIENT_ID = "pipefy-cli" @@ -66,13 +67,12 @@ def _seed_session( storage.store_session( issuer=_ISSUER, client_id=_CLIENT_ID, - token_response={ - "access_token": "OLD", - "refresh_token": "OLD_R", - "token_type": "Bearer", - "expires_in": expires_in, - "scope": "openid offline_access", - }, + token=TokenResponse( + access_token="OLD", + refresh_token="OLD_R", + expires_in=expires_in, + scope="openid offline_access", + ), ) @@ -103,8 +103,8 @@ def test_returns_session_unchanged_when_fresh( http_client=client, ) assert result is not None - assert result.access_token == "OLD" - assert result.refresh_token == "OLD_R" + assert result.token.access_token == "OLD" + assert result.token.refresh_token == "OLD_R" def test_refreshes_when_within_leeway( self, @@ -132,8 +132,8 @@ def test_refreshes_when_within_leeway( http_client=client, ) assert result is not None - assert result.access_token == "NEW_A" - assert result.refresh_token == "NEW_R" + assert result.token.access_token == "NEW_A" + assert result.token.refresh_token == "NEW_R" def test_persists_rotated_session( self, @@ -159,8 +159,8 @@ def test_persists_rotated_session( reloaded = storage.load_session(issuer=_ISSUER, client_id=_CLIENT_ID) assert reloaded is not None - assert reloaded.access_token == "NEW_A" - assert reloaded.refresh_token == "NEW_R" + assert reloaded.token.access_token == "NEW_A" + assert reloaded.token.refresh_token == "NEW_R" def test_carries_forward_omitted_lifetime_fields( self, @@ -187,8 +187,8 @@ def test_carries_forward_omitted_lifetime_fields( http_client=client, ) assert result is not None - assert result.expires_in == 300 # carried forward - assert result.scope == "openid offline_access" # carried forward + assert result.token.expires_in == 300 # carried forward + assert result.token.scope == "openid offline_access" # carried forward def test_falls_back_to_old_refresh_token_when_idp_does_not_rotate( self, @@ -210,7 +210,7 @@ def test_falls_back_to_old_refresh_token_when_idp_does_not_rotate( http_client=client, ) assert result is not None - assert result.refresh_token == "OLD_R" # unchanged + assert result.token.refresh_token == "OLD_R" # unchanged def test_obtained_at_updated_after_refresh( self, @@ -264,7 +264,7 @@ def test_custom_leeway_forces_earlier_refresh( http_client=client, ) assert result is not None - assert result.access_token == "REFRESHED_BECAUSE_LEEWAY" + assert result.token.access_token == "REFRESHED_BECAUSE_LEEWAY" def test_refresh_failure_does_not_delete_stored_session( self, @@ -312,8 +312,8 @@ def test_force_true_refreshes_even_when_fresh( http_client=client, ) assert result is not None - assert result.access_token == "FORCED" - assert result.refresh_token == "FORCED_R" + assert result.token.access_token == "FORCED" + assert result.token.refresh_token == "FORCED_R" def test_force_true_persists_rotated_session( self, @@ -339,8 +339,8 @@ def test_force_true_persists_rotated_session( ) reloaded = storage.load_session(issuer=_ISSUER, client_id=_CLIENT_ID) assert reloaded is not None - assert reloaded.access_token == "FORCED" - assert reloaded.refresh_token == "FORCED_R" + assert reloaded.token.access_token == "FORCED" + assert reloaded.token.refresh_token == "FORCED_R" def test_force_true_returns_none_when_no_session_stored( self, fake_keyring: InMemoryKeyring diff --git a/packages/auth/tests/test_refresh_concurrent.py b/packages/auth/tests/test_refresh_concurrent.py index 5c7692ed..fa894e7a 100644 --- a/packages/auth/tests/test_refresh_concurrent.py +++ b/packages/auth/tests/test_refresh_concurrent.py @@ -19,6 +19,7 @@ from pipefy_auth import refresh as refresh_module from pipefy_auth.locks import RefreshLockTimeout +from pipefy_auth.responses import TokenResponse from pipefy_auth.storage import StoredSession _ISSUER = "https://signin.example.com/realms/pipefy" @@ -29,14 +30,16 @@ def _stored(*, obtained_at: int, expires_in: int = 30) -> StoredSession: return StoredSession( issuer=_ISSUER, client_id=_CLIENT_ID, - access_token="OLD", - refresh_token="OLD_R", - token_type="Bearer", obtained_at=obtained_at, - expires_in=expires_in, - refresh_expires_in=None, - scope=None, - id_token=None, + token=TokenResponse( + access_token="OLD", + refresh_token="OLD_R", + token_type="Bearer", + expires_in=expires_in, + refresh_expires_in=None, + scope=None, + id_token=None, + ), ) @@ -44,14 +47,16 @@ def _fresh(*, obtained_at: int) -> StoredSession: return StoredSession( issuer=_ISSUER, client_id=_CLIENT_ID, - access_token="NEW", - refresh_token="NEW_R", - token_type="Bearer", obtained_at=obtained_at, - expires_in=300, - refresh_expires_in=None, - scope=None, - id_token=None, + token=TokenResponse( + access_token="NEW", + refresh_token="NEW_R", + token_type="Bearer", + expires_in=300, + refresh_expires_in=None, + scope=None, + id_token=None, + ), ) diff --git a/packages/auth/tests/test_resolver.py b/packages/auth/tests/test_resolver.py index fa29222f..447462d5 100644 --- a/packages/auth/tests/test_resolver.py +++ b/packages/auth/tests/test_resolver.py @@ -24,6 +24,7 @@ resolve_pipefy_auth, tier_for, ) +from pipefy_auth.responses import TokenResponse from pipefy_auth.storage import StoredSession @@ -31,14 +32,16 @@ def _stored_session() -> StoredSession: return StoredSession( issuer="https://issuer.test/realms/pipefy", client_id="pipefy-cli", - access_token="ACCESS", - refresh_token="REFRESH", - token_type="Bearer", obtained_at=int(time.time()), - expires_in=3600, - refresh_expires_in=None, - scope=None, - id_token=None, + token=TokenResponse( + access_token="ACCESS", + refresh_token="REFRESH", + token_type="Bearer", + expires_in=3600, + refresh_expires_in=None, + scope=None, + id_token=None, + ), ) @@ -240,14 +243,16 @@ def fake_ensure(**kwargs): return StoredSession( issuer=rotated.issuer, client_id=rotated.client_id, - access_token="ROTATED", - refresh_token="ROTATED_R", - token_type="Bearer", obtained_at=int(time.time()), - expires_in=3600, - refresh_expires_in=None, - scope=None, - id_token=None, + token=TokenResponse( + access_token="ROTATED", + refresh_token="ROTATED_R", + token_type="Bearer", + expires_in=3600, + refresh_expires_in=None, + scope=None, + id_token=None, + ), ) monkeypatch.setattr("pipefy_auth.resolver.ensure_fresh_session", fake_ensure) diff --git a/packages/auth/tests/test_responses.py b/packages/auth/tests/test_responses.py new file mode 100644 index 00000000..687d7c17 --- /dev/null +++ b/packages/auth/tests/test_responses.py @@ -0,0 +1,239 @@ +"""Unit coverage for ``TokenResponse`` and ``OAuthErrorResponse``.""" + +from __future__ import annotations + +import httpx +import pytest +from pydantic import ValidationError + +from pipefy_auth.responses import ( + OAuthErrorResponse, + TokenResponse, + _format_validation_error, +) + +# --------------------------------------------------------------------------- # +# TokenResponse # +# --------------------------------------------------------------------------- # + + +@pytest.mark.unit +class TestTokenResponseFromPayload: + def test_minimal_payload_accepts_defaults(self) -> None: + token = TokenResponse.from_payload({"access_token": "a", "refresh_token": "r"}) + assert token.access_token == "a" + assert token.refresh_token == "r" + assert token.token_type == "Bearer" + assert token.expires_in is None + assert token.scope is None + assert token.id_token is None + + def test_full_payload_round_trips_fields(self) -> None: + token = TokenResponse.from_payload( + { + "access_token": "a", + "refresh_token": "r", + "token_type": "Bearer", + "expires_in": 60, + "refresh_expires_in": 3600, + "scope": "openid email", + "id_token": "ID", + } + ) + assert token.expires_in == 60 + assert token.refresh_expires_in == 3600 + assert token.scope == "openid email" + assert token.id_token == "ID" + + def test_unknown_fields_are_ignored(self) -> None: + """Keycloak ``not-before-policy`` / ``session_state`` must not raise.""" + token = TokenResponse.from_payload( + { + "access_token": "a", + "refresh_token": "r", + "not-before-policy": 0, + "session_state": "abc", + } + ) + assert token.access_token == "a" + + @pytest.mark.parametrize("missing", ["access_token", "refresh_token"]) + def test_rejects_missing_required_field(self, missing: str) -> None: + payload = {"access_token": "a", "refresh_token": "r"} + del payload[missing] + with pytest.raises(ValidationError) as info: + TokenResponse.from_payload(payload) + assert any( + missing in str(part) for err in info.value.errors() for part in err["loc"] + ) + + @pytest.mark.parametrize("field", ["access_token", "refresh_token"]) + def test_rejects_empty_required_field(self, field: str) -> None: + payload = {"access_token": "a", "refresh_token": "r", field: ""} + with pytest.raises(ValidationError): + TokenResponse.from_payload(payload) + + @pytest.mark.parametrize("field", ["access_token", "refresh_token"]) + def test_rejects_null_required_field(self, field: str) -> None: + payload: dict[str, object] = { + "access_token": "a", + "refresh_token": "r", + field: None, + } + with pytest.raises(ValidationError): + TokenResponse.from_payload(payload) + + @pytest.mark.parametrize("field", ["expires_in", "refresh_expires_in"]) + def test_rejects_bool_in_int_fields(self, field: str) -> None: + """``isinstance(True, int)`` is True; StrictInt must reject this.""" + payload: dict[str, object] = { + "access_token": "a", + "refresh_token": "r", + field: True, + } + with pytest.raises(ValidationError): + TokenResponse.from_payload(payload) + + def test_accepts_null_optional_lifetime(self) -> None: + token = TokenResponse.from_payload( + {"access_token": "a", "refresh_token": "r", "expires_in": None} + ) + assert token.expires_in is None + + @pytest.mark.parametrize( + ("field", "value", "expected"), + [ + ("scope", " openid ", "openid"), + ("scope", "", None), + ("scope", " ", None), + ("id_token", " ID ", "ID"), + ("id_token", "", None), + ], + ) + def test_optional_strings_strip_and_coerce_empty_to_none( + self, field: str, value: str, expected: str | None + ) -> None: + token = TokenResponse.from_payload( + {"access_token": "a", "refresh_token": "r", field: value} + ) + assert getattr(token, field) == expected + + @pytest.mark.parametrize("field", ["scope", "id_token"]) + def test_non_string_optional_coerces_to_none(self, field: str) -> None: + """A numeric scope is a malformed wire value; drop to ``None`` rather than stringify.""" + token = TokenResponse.from_payload( + {"access_token": "a", "refresh_token": "r", field: 0} + ) + assert getattr(token, field) is None + + def test_is_frozen(self) -> None: + token = TokenResponse.from_payload({"access_token": "a", "refresh_token": "r"}) + with pytest.raises(ValidationError): + token.access_token = "different" # type: ignore[misc] + + +# --------------------------------------------------------------------------- # +# OAuthErrorResponse # +# --------------------------------------------------------------------------- # + + +def _error_response(status: int, body: object) -> httpx.Response: + """Build an httpx.Response with JSON or raw text content.""" + if isinstance(body, (dict, list)) or body is None: + return httpx.Response(status_code=status, json=body) + return httpx.Response(status_code=status, content=str(body).encode()) + + +@pytest.mark.unit +class TestOAuthErrorResponseFromResponse: + def test_parses_oauth_envelope(self) -> None: + err = OAuthErrorResponse.from_response( + _error_response( + 400, {"error": "invalid_grant", "error_description": "stale"} + ) + ) + assert err.status_code == 400 + assert err.error == "invalid_grant" + assert err.error_description == "stale" + + def test_non_json_body_yields_no_error_fields(self) -> None: + err = OAuthErrorResponse.from_response( + _error_response(502, "upstream down") + ) + assert err.error is None + assert err.error_description is None + + def test_non_dict_json_body_yields_no_error_fields(self) -> None: + err = OAuthErrorResponse.from_response(_error_response(400, ["x"])) + assert err.error is None + assert err.error_description is None + + def test_missing_error_field_yields_none(self) -> None: + err = OAuthErrorResponse.from_response(_error_response(400, {"other": "thing"})) + assert err.error is None + assert err.error_description is None + + def test_strips_and_coerces_blank_error_to_none(self) -> None: + err = OAuthErrorResponse.from_response( + _error_response(400, {"error": " ", "error_description": ""}) + ) + assert err.error is None + assert err.error_description is None + + def test_numeric_error_value_falls_through_to_none(self) -> None: + """A non-string ``error`` is not OAuth-shaped; drop to ``None`` so callers + render the fallback rather than crash on ValidationError.""" + err = OAuthErrorResponse.from_response(_error_response(400, {"error": 0})) + assert err.error is None + assert ( + err.render(fallback="Refresh failed (HTTP 400)", prefix="Refresh failed") + == "Refresh failed (HTTP 400)" + ) + + +@pytest.mark.unit +class TestOAuthErrorResponseRender: + def test_fallback_when_error_absent(self) -> None: + err = OAuthErrorResponse(status_code=500, error=None, error_description=None) + assert err.render(fallback="Generic failure", prefix="X") == "Generic failure" + + def test_prefix_and_error_when_description_absent(self) -> None: + err = OAuthErrorResponse( + status_code=400, error="invalid_grant", error_description=None + ) + assert ( + err.render(fallback="ignored", prefix="Refresh failed") + == "Refresh failed: invalid_grant" + ) + + def test_prefix_error_and_description(self) -> None: + err = OAuthErrorResponse( + status_code=400, + error="invalid_grant", + error_description="stale refresh token", + ) + assert ( + err.render(fallback="ignored", prefix="Refresh failed") + == "Refresh failed: invalid_grant: stale refresh token" + ) + + +# --------------------------------------------------------------------------- # +# format_validation_error # +# --------------------------------------------------------------------------- # + + +@pytest.mark.unit +class TestFormatValidationError: + def test_renders_loc_and_msg_per_error(self) -> None: + with pytest.raises(ValidationError) as info: + TokenResponse.from_payload({}) + rendered = _format_validation_error(info.value) + assert "access_token" in rendered + assert "refresh_token" in rendered + assert "; " in rendered + + def test_no_pydantic_doc_urls_in_output(self) -> None: + with pytest.raises(ValidationError) as info: + TokenResponse.from_payload({"access_token": "a", "refresh_token": ""}) + assert "https://" not in _format_validation_error(info.value) diff --git a/packages/auth/tests/test_storage_backend.py b/packages/auth/tests/test_storage_backend.py index 6738d579..2fa16027 100644 --- a/packages/auth/tests/test_storage_backend.py +++ b/packages/auth/tests/test_storage_backend.py @@ -1,15 +1,31 @@ -"""Coverage for ``configure_keychain_backend`` (env-driven keyring swap).""" +"""Coverage for ``configure_keychain_backend`` (env-driven keyring swap) +and the on-disk blob round-trip for ``StoredSession``. +""" from __future__ import annotations +import json from collections.abc import Iterator from pathlib import Path import keyring import pytest +from conftest import InMemoryKeyring from keyrings.alt.file import PlaintextKeyring +from pydantic import ValidationError -from pipefy_auth.storage import configure_keychain_backend +from pipefy_auth.responses import TokenResponse +from pipefy_auth.storage import ( + StoredSession, + configure_keychain_backend, + keychain_key, + load_session, + store_session, +) + +_SERVICE = "pipefy" +_ISSUER = "https://example.test/realms/pipefy" +_CLIENT_ID = "cli" @pytest.fixture @@ -78,3 +94,143 @@ def test_file_backend_round_trip_writes_under_config_dir( backing_file = config_home / "pipefy" / "keyring.cfg" assert backing_file.exists() assert keyring.get_password("pipefy-test", "user") == "secret-value" + + +# --------------------------------------------------------------------------- # +# StoredSession blob round-trip # +# --------------------------------------------------------------------------- # + + +def _token(access: str = "AT", refresh: str = "RT") -> TokenResponse: + return TokenResponse( + access_token=access, + refresh_token=refresh, + token_type="Bearer", + expires_in=300, + refresh_expires_in=3600, + scope="openid email", + id_token="ID", + ) + + +@pytest.mark.unit +def test_store_session_writes_flat_blob_and_round_trips( + fake_keyring: InMemoryKeyring, +) -> None: + """``store_session`` emits the flat shape; ``load_session`` reads it back.""" + stored = store_session(issuer=_ISSUER, client_id=_CLIENT_ID, token=_token()) + + raw = fake_keyring.get_password(_SERVICE, keychain_key(_ISSUER, _CLIENT_ID)) + assert raw is not None + blob = json.loads(raw) + assert "token" not in blob + assert blob["access_token"] == "AT" + assert blob["refresh_token"] == "RT" + assert blob["issuer"] == _ISSUER + + loaded = load_session(issuer=_ISSUER, client_id=_CLIENT_ID) + assert loaded == stored + + +@pytest.mark.unit +def test_load_session_accepts_legacy_flat_blob( + fake_keyring: InMemoryKeyring, +) -> None: + """Pre-pydantic keychain entries (flat, no ``token`` key) still load.""" + legacy = { + "issuer": _ISSUER, + "client_id": _CLIENT_ID, + "obtained_at": 1700000000, + "access_token": "AT", + "refresh_token": "RT", + "token_type": "Bearer", + "expires_in": 300, + "refresh_expires_in": 3600, + "scope": "openid email", + "id_token": "ID", + } + fake_keyring.set_password( + _SERVICE, keychain_key(_ISSUER, _CLIENT_ID), json.dumps(legacy) + ) + + loaded = load_session(issuer=_ISSUER, client_id=_CLIENT_ID) + assert loaded is not None + assert loaded.token.access_token == "AT" + assert loaded.token.refresh_token == "RT" + assert loaded.obtained_at == 1700000000 + + +@pytest.mark.unit +def test_load_session_accepts_nested_blob( + fake_keyring: InMemoryKeyring, +) -> None: + """Forward-compat: explicit nested ``token`` also loads.""" + nested = { + "issuer": _ISSUER, + "client_id": _CLIENT_ID, + "obtained_at": 1700000000, + "token": { + "access_token": "AT", + "refresh_token": "RT", + "token_type": "Bearer", + "expires_in": 300, + "refresh_expires_in": 3600, + "scope": "openid email", + "id_token": "ID", + }, + } + fake_keyring.set_password( + _SERVICE, keychain_key(_ISSUER, _CLIENT_ID), json.dumps(nested) + ) + + loaded = load_session(issuer=_ISSUER, client_id=_CLIENT_ID) + assert loaded is not None + assert loaded.token.access_token == "AT" + + +@pytest.mark.unit +def test_load_session_returns_none_for_corrupt_json( + fake_keyring: InMemoryKeyring, +) -> None: + fake_keyring.set_password(_SERVICE, keychain_key(_ISSUER, _CLIENT_ID), "{not json") + assert load_session(issuer=_ISSUER, client_id=_CLIENT_ID) is None + + +@pytest.mark.unit +def test_load_session_returns_none_when_required_field_missing( + fake_keyring: InMemoryKeyring, +) -> None: + """ValidationError on disk reads as 'no session' rather than crashing.""" + fake_keyring.set_password( + _SERVICE, + keychain_key(_ISSUER, _CLIENT_ID), + json.dumps({"issuer": _ISSUER, "client_id": _CLIENT_ID, "obtained_at": 1}), + ) + assert load_session(issuer=_ISSUER, client_id=_CLIENT_ID) is None + + +@pytest.mark.unit +def test_stored_session_rejects_bool_obtained_at() -> None: + """``StrictInt`` rejects bool to prevent ``True`` masquerading as ``1``.""" + with pytest.raises(ValidationError): + StoredSession( + issuer=_ISSUER, + client_id=_CLIENT_ID, + obtained_at=True, # type: ignore[arg-type] + token=_token(), + ) + + +@pytest.mark.unit +def test_stored_session_forbids_unknown_outer_fields() -> None: + """Unknown outer fields signal a corruption/hand-edit, not an IdP extension.""" + with pytest.raises(ValidationError): + StoredSession.model_validate( + { + "issuer": _ISSUER, + "client_id": _CLIENT_ID, + "obtained_at": 1, + "token": {"access_token": "a", "refresh_token": "r"}, + "unexpected": "x", + } + ) diff --git a/packages/cli/src/pipefy_cli/commands/auth.py b/packages/cli/src/pipefy_cli/commands/auth.py index 6d8cc060..146bd275 100644 --- a/packages/cli/src/pipefy_cli/commands/auth.py +++ b/packages/cli/src/pipefy_cli/commands/auth.py @@ -194,13 +194,8 @@ def _open(url: str) -> bool: store_session( issuer=result.issuer, client_id=client_id, - token_response=result.token_response, + token=result.token, ) - except ValueError as exc: - typer.echo( - f"Login succeeded but the token response was malformed: {exc}", err=True - ) - raise typer.Exit(1) from exc except (KeyringError, OSError) as exc: # ``OSError`` covers the file-backed PlaintextKeyring path: a read-only # ``config_dir()`` (e.g. CI runner without write perms, NFS mount @@ -369,9 +364,11 @@ def _populate_stored_session(report: AuthStatusReport, oidc: OidcClient) -> None # Best-effort expiry from the pre-refresh blob so users see *why*. stale = load_session(issuer=oidc.issuer_url, client_id=oidc.client_id) if stale is not None: - report.access_expires_at = _iso_expiry(stale.obtained_at, stale.expires_in) + report.access_expires_at = _iso_expiry( + stale.obtained_at, stale.token.expires_in + ) report.refresh_expires_at = _iso_expiry( - stale.obtained_at, stale.refresh_expires_in + stale.obtained_at, stale.token.refresh_expires_in ) raise _StatusExit( report=report, @@ -391,10 +388,10 @@ def _populate_stored_session(report: AuthStatusReport, oidc: OidcClient) -> None ) report.state = "active" report.access_expires_at = _iso_expiry( - fresh_session.obtained_at, fresh_session.expires_in + fresh_session.obtained_at, fresh_session.token.expires_in ) report.refresh_expires_at = _iso_expiry( - fresh_session.obtained_at, fresh_session.refresh_expires_in + fresh_session.obtained_at, fresh_session.token.refresh_expires_in ) @@ -498,7 +495,7 @@ def auth_logout(ctx: typer.Context) -> None: revoke_session( issuer=issuer, client_id=client_id, - refresh_token=session.refresh_token, + refresh_token=session.token.refresh_token, policy=DiscoveryPolicy(allow_insecure_urls=settings.allow_insecure_urls), ) except RevocationUnsupportedError: diff --git a/packages/cli/tests/test_auth.py b/packages/cli/tests/test_auth.py index 137f7f71..875fd3f9 100644 --- a/packages/cli/tests/test_auth.py +++ b/packages/cli/tests/test_auth.py @@ -13,6 +13,7 @@ ServiceAccount, StaticBearerAuth, StoredSession, + TokenResponse, ) from pipefy_sdk import PipefySettings @@ -203,14 +204,13 @@ def _fresh_stored_session(*, access_token: str = "SESSION_ACCESS") -> StoredSess return StoredSession( issuer=_ISSUER, client_id="pipefy-cli", - access_token=access_token, - refresh_token="REFRESH", - token_type="Bearer", obtained_at=int(time.time()), - expires_in=_FAR_FUTURE_EXPIRES_IN, - refresh_expires_in=None, - scope="openid offline_access", - id_token=None, + token=TokenResponse( + access_token=access_token, + refresh_token="REFRESH", + expires_in=_FAR_FUTURE_EXPIRES_IN, + scope="openid offline_access", + ), ) diff --git a/packages/cli/tests/test_auth_login.py b/packages/cli/tests/test_auth_login.py index c2c705d3..05324398 100644 --- a/packages/cli/tests/test_auth_login.py +++ b/packages/cli/tests/test_auth_login.py @@ -13,6 +13,7 @@ from conftest import InMemoryKeyring from pipefy_auth import discovery, flow, loopback, pkce, storage from pipefy_auth.discovery import ProviderMetadata +from pipefy_auth.responses import TokenResponse from pipefy_cli.main import app as cli_app @@ -280,30 +281,29 @@ def test_keychain_key_uses_host_and_client(self) -> None: assert key == "signin.pipefy.com|pipefy-cli" def test_store_then_load_roundtrip(self, fake_keyring: InMemoryKeyring) -> None: - token = { - "access_token": "AAA", - "refresh_token": "RRR", - "token_type": "Bearer", - "expires_in": 300, - "refresh_expires_in": 3600, - "scope": "openid email", - } + token = TokenResponse( + access_token="AAA", + refresh_token="RRR", + expires_in=300, + refresh_expires_in=3600, + scope="openid email", + ) before = int(time.time()) stored = storage.store_session( issuer="https://x.test/realms/foo", client_id="pipefy-cli", - token_response=token, + token=token, ) - assert stored.access_token == "AAA" - assert stored.refresh_token == "RRR" + assert stored.token.access_token == "AAA" + assert stored.token.refresh_token == "RRR" assert stored.obtained_at >= before loaded = storage.load_session( issuer="https://x.test/realms/foo", client_id="pipefy-cli" ) assert loaded is not None - assert loaded.refresh_token == "RRR" - assert loaded.scope == "openid email" + assert loaded.token.refresh_token == "RRR" + assert loaded.token.scope == "openid email" def test_load_returns_none_when_absent(self, fake_keyring: InMemoryKeyring) -> None: assert ( @@ -315,7 +315,7 @@ def test_delete_reports_presence(self, fake_keyring: InMemoryKeyring) -> None: storage.store_session( issuer="https://x.test/realms/foo", client_id="cid", - token_response={"access_token": "a", "refresh_token": "r"}, + token=TokenResponse(access_token="a", refresh_token="r"), ) assert storage.delete_session( issuer="https://x.test/realms/foo", client_id="cid" @@ -340,16 +340,6 @@ def _boom(service: str, username: str) -> None: with pytest.raises(storage.SessionDeleteError, match="backend locked"): storage.delete_session(issuer="https://x.test/realms/foo", client_id="cid") - def test_store_rejects_missing_required_field( - self, fake_keyring: InMemoryKeyring - ) -> None: - with pytest.raises(ValueError, match="refresh_token"): - storage.store_session( - issuer="https://x.test/realms/foo", - client_id="cid", - token_response={"access_token": "a"}, - ) - # --------------------------------------------------------------------------- # # Flow # @@ -402,7 +392,7 @@ def test_build_authorization_url_has_all_required_params(self) -> None: ): assert needle in url - def test_exchange_code_returns_token_dict(self) -> None: + def test_exchange_code_returns_token_response(self) -> None: token_payload = { "access_token": "AAA", "refresh_token": "RRR", @@ -430,7 +420,7 @@ def handler(request: httpx.Request) -> httpx.Response: code_verifier="ver1f1er", client=client, ) - assert result == token_payload + assert result == TokenResponse.from_payload(token_payload) def test_exchange_code_non_200_raises_login_error(self) -> None: meta = ProviderMetadata( @@ -603,12 +593,11 @@ def test_happy_path( def _fake_run_login(**_kwargs: object) -> flow.LoginResult: return flow.LoginResult( issuer="https://x.test/realms/foo", - token_response={ - "access_token": "AAA", - "refresh_token": "RRR", - "token_type": "Bearer", - "expires_in": 300, - }, + token=TokenResponse( + access_token="AAA", + refresh_token="RRR", + expires_in=300, + ), ) from pipefy_cli.commands import auth as auth_module @@ -623,7 +612,7 @@ def _fake_run_login(**_kwargs: object) -> flow.LoginResult: issuer="https://x.test/realms/foo", client_id="pipefy-cli" ) assert loaded is not None - assert loaded.refresh_token == "RRR" + assert loaded.token.refresh_token == "RRR" def test_masking_env_warning( self, @@ -643,7 +632,7 @@ def test_masking_env_warning( "run_login", lambda **_k: flow.LoginResult( issuer="https://x.test/realms/foo", - token_response={"access_token": "A", "refresh_token": "R"}, + token=TokenResponse(access_token="A", refresh_token="R"), ), ) @@ -679,7 +668,7 @@ def _spy_run_login(**kwargs: object) -> flow.LoginResult: on_url(fake_auth_url) return flow.LoginResult( issuer="https://x.test/realms/foo", - token_response={"access_token": "A", "refresh_token": "R"}, + token=TokenResponse(access_token="A", refresh_token="R"), ) monkeypatch.setattr(auth_module, "run_login", _spy_run_login) @@ -728,7 +717,7 @@ def _spy_run_login(**kwargs: object) -> flow.LoginResult: on_url(fake_auth_url) return flow.LoginResult( issuer="https://x.test/realms/foo", - token_response={"access_token": "A", "refresh_token": "R"}, + token=TokenResponse(access_token="A", refresh_token="R"), ) monkeypatch.setattr(auth_module, "run_login", _spy_run_login) @@ -782,7 +771,7 @@ def test_store_failure_default_backend_hints_at_secret_service( "run_login", lambda **_k: flow.LoginResult( issuer="https://x.test/realms/foo", - token_response={"access_token": "A", "refresh_token": "R"}, + token=TokenResponse(access_token="A", refresh_token="R"), ), ) @@ -816,7 +805,7 @@ def test_store_failure_file_backend_hints_at_config_dir( "run_login", lambda **_k: flow.LoginResult( issuer="https://x.test/realms/foo", - token_response={"access_token": "A", "refresh_token": "R"}, + token=TokenResponse(access_token="A", refresh_token="R"), ), ) monkeypatch.setattr( diff --git a/packages/cli/tests/test_auth_logout.py b/packages/cli/tests/test_auth_logout.py index 78639617..60c4f668 100644 --- a/packages/cli/tests/test_auth_logout.py +++ b/packages/cli/tests/test_auth_logout.py @@ -6,6 +6,7 @@ import pytest from conftest import InMemoryKeyring from pipefy_auth import revoke, storage +from pipefy_auth.responses import TokenResponse from pipefy_cli.commands import auth as auth_module from pipefy_cli.main import app as cli_app @@ -134,12 +135,11 @@ def _store_test_session(issuer: str = _ISSUER, client_id: str = "pipefy-cli") -> storage.store_session( issuer=issuer, client_id=client_id, - token_response={ - "access_token": "AAA", - "refresh_token": "RRR", - "token_type": "Bearer", - "expires_in": 300, - }, + token=TokenResponse( + access_token="AAA", + refresh_token="RRR", + expires_in=300, + ), ) diff --git a/packages/cli/tests/test_auth_status.py b/packages/cli/tests/test_auth_status.py index 723e1951..9a8869b9 100644 --- a/packages/cli/tests/test_auth_status.py +++ b/packages/cli/tests/test_auth_status.py @@ -11,6 +11,7 @@ import pytest from pipefy_auth import RefreshError, StoredSession, storage +from pipefy_auth.responses import TokenResponse from pipefy_cli.main import app @@ -34,14 +35,13 @@ def _seed_session( storage.store_session( issuer=_ISSUER, client_id=_CLIENT_ID, - token_response={ - "access_token": "AT", - "refresh_token": "RT", - "token_type": "Bearer", - "expires_in": expires_in, - "refresh_expires_in": refresh_expires_in, - "scope": "openid offline_access", - }, + token=TokenResponse( + access_token="AT", + refresh_token="RT", + expires_in=expires_in, + refresh_expires_in=refresh_expires_in, + scope="openid offline_access", + ), ) @@ -133,14 +133,16 @@ def test_status_stored_session_rotates_via_refresh( rotated = storage.StoredSession( issuer=_ISSUER, client_id=_CLIENT_ID, - access_token="ROTATED", - refresh_token="RT", - token_type="Bearer", obtained_at=int(time.time()), - expires_in=3600, - refresh_expires_in=2592000, - scope="openid offline_access", - id_token=None, + token=TokenResponse( + access_token="ROTATED", + refresh_token="RT", + token_type="Bearer", + expires_in=3600, + refresh_expires_in=2592000, + scope="openid offline_access", + id_token=None, + ), ) client = _mock_client_with_me() with _patch_fresh_session(rotated), _patch_command_client(client): diff --git a/packages/mcp/tests/core/test_container.py b/packages/mcp/tests/core/test_container.py index af13a063..658881da 100644 --- a/packages/mcp/tests/core/test_container.py +++ b/packages/mcp/tests/core/test_container.py @@ -8,6 +8,7 @@ RefreshableBearerAuth, RefreshError, StaticBearerAuth, + TokenResponse, ) from pipefy_auth.storage import StoredSession from pipefy_sdk import PipefyClient, PipefySettings @@ -44,14 +45,12 @@ def _fresh_stored_session() -> StoredSession: return StoredSession( issuer="https://signin.pipefy.com/realms/pipefy", client_id="pipefy-cli", - access_token="ACCESS", - refresh_token="REFRESH", - token_type="Bearer", obtained_at=int(time.time()), - expires_in=3600, - refresh_expires_in=None, - scope=None, - id_token=None, + token=TokenResponse( + access_token="ACCESS", + refresh_token="REFRESH", + expires_in=3600, + ), )