refactor(auth): consolidate OAuth response parsing#258
Merged
Conversation
5 tasks
gbrlcustodio
force-pushed
the
refactor/infra-coerce
branch
from
5c8db27 to
638baa3
Compare
gbrlcustodio
force-pushed
the
refactor/auth-responses
branch
from
f04f589 to
325d6f1
Compare
gbrlcustodio
force-pushed
the
refactor/infra-coerce
branch
from
638baa3 to
d9c5402
Compare
gbrlcustodio
force-pushed
the
refactor/auth-responses
branch
from
325d6f1 to
285500e
Compare
gbrlcustodio
force-pushed
the
refactor/infra-coerce
branch
from
d9c5402 to
53fa1a9
Compare
gbrlcustodio
force-pushed
the
refactor/auth-responses
branch
2 times, most recently
from
bb19834 to
b3ab7c0
Compare
Give the OAuth wire shapes typed domain objects instead of raw dicts threaded through call sites, eliminating three near-duplicate `_format_*_error` free functions and centralizing the RFC 6749 / RFC 8628 parsing in one place. * New `responses` module exports `TokenResponse` (RFC 6749 §5.1) and `OAuthErrorResponse` (§5.2 / RFC 8628 §3.5). `TokenResponse` replaces `dict[str, object]` across `exchange_code` and `store_session`; `OAuthErrorResponse` replaces the duplicated error-parsing helpers in `flow.py` and `refresh.py`. Both factories use the shared `pipefy_infra.coerce` helpers internally. * `StoredSession` now bundles a `TokenResponse` so the keychain blob reads from one source of truth. On-disk JSON keeps the legacy flat shape via destructure-on-write / rebuild-on-load, so existing keychain entries continue to load without forcing a re-login. * `refresh._refresh_error_from` collapses to `OAuthErrorResponse.from_response(response)`, and `_refresh_and_store` wraps the `TokenResponse.from_payload` `ValueError` as `RefreshError` so a malformed refresh body surfaces with the right exception class. * CLI `auth login` / `auth status` / `auth logout` access token fields via `session.token.<field>` (attribute) instead of `session.<field>` (flat). The post-store `ValueError` catch drops out because malformed payloads now fail earlier in `TokenResponse.from_payload`. Test surface: refresh, resolver, storage, and CLI auth tests migrated to the bundled `TokenResponse` shape. Auth + CLI + MCP suites pass.
gbrlcustodio
force-pushed
the
refactor/auth-responses
branch
from
b3ab7c0 to
01e1cd9
Compare
Replace the hand-rolled `_required_str` / `optional_*` validation in the
three frozen dataclasses with `BaseModel` subclasses, aligning with the
codebase's established pattern for parsing untrusted wire data (SDK
input models, MCP tool inputs, every `Settings` layer).
* `TokenResponse`, `OAuthErrorResponse`, `StoredSession` are now
`BaseModel` with `ConfigDict(extra="ignore", frozen=True)` (or
`extra="forbid"` for `StoredSession`, since both writer and reader
live in this repo and an unknown outer key signals corruption).
Required strings use `StrictStr` + `Field(min_length=1)`; lifetime
fields use `StrictInt | None` so a bool can't slip in as `1`.
* `StoredSession` preserves the legacy on-disk shape via
`@model_validator(mode="before") _accept_flat_blob` (re-nests
destructured token fields back under `token` before validation) plus
`@model_serializer(mode="wrap") _to_flat_blob` (always emits the flat
shape). Existing keychain entries continue to load without forcing a
re-login. `_session_to_blob` / `_session_from_blob` / `_required_str`
are deleted; `store_session` / `load_session` collapse to
`model_dump_json` / `model_validate_json`.
* `flow.py:exchange_code` and `refresh.py:_refresh_and_store` gain
`ValidationError` branches that render via a shared
`_format_validation_error` helper, mirroring the MCP house style
(`pipefy_mcp.tools.portal_tool_helpers.portal_element_validation_error`).
Pydantic's verbose default `str(exc)` with doc URLs never reaches
end users.
* `_strip_or_none` for optional string fields (`scope`, `id_token`,
`error`, `error_description`) drops non-strings to `None` rather
than rejecting them. Load-bearing for the inline call sites at
`flow.py:96` and `refresh.py:_refresh_error_from`, which invoke
`OAuthErrorResponse.from_response(response).render(...)` without a
try: a malformed `{"error": 0}` body must fall through to the
fallback rather than raise `ValidationError` into an
error-rendering path.
Test surface: new `test_responses.py` covers missing/empty/bool/null
required fields, `_strip_or_none` behavior on optional strings,
`from_response` over non-JSON / non-dict / numeric-error bodies, the
three `render()` branches, and the `_format_validation_error` output
shape. `test_storage_backend.py` adds legacy-flat + nested-shape
round-trips, corrupt JSON, missing required, and forbid-extras.
Auth (152) + CLI (303) + cross-package (2764 total) suites pass.
4 tasks
gbrlcustodio
added a commit
that referenced
this pull request
Jun 1, 2026
Move DeviceAuthorization from a frozen dataclass in device.py to a Pydantic BaseModel in responses.py, matching the TokenResponse / OAuthErrorResponse pattern established in #258. - StrictInt on expires_in and interval (rejects bool, str, float) - _OptionalStr (BeforeValidator strip-or-none) on verification_uri_complete - field_validator clamps non-positive interval to the RFC 8628 §3.5 default - device.py routes both from_payload failures through _format_validation_error for consistent LoginError shape with flow.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Give the OAuth wire shapes typed domain objects instead of raw dicts threaded through call sites, and parse them through Pydantic v2 models rather than hand-rolled
_required_str/optional_*validation. Centralizes RFC 6749 / RFC 8628 parsing in one place and matches the codebase pattern used everywhere else for untrusted wire data (SDK input models, MCP tool inputs, everySettingslayer).Why split out
Carved out of #154 as the second foundational layer. Depended on #257 (infra coerce helpers), which has landed. The device login strategy (PR #154, to be re-pointed) sits on top of this.
Changes
Typed OAuth wrappers (01e1cd9)
pipefy_auth.responsesmodule:TokenResponse(RFC 6749 §5.1) with factoryfrom_payload(dict).OAuthErrorResponse(RFC 6749 §5.2 / RFC 8628 §3.5) with factoryfrom_response(httpx.Response)and.render(prefix, fallback)for safe user-facing messages.StoredSessionbundles aTokenResponse. On-disk JSON keeps the legacy flat shape via destructure-on-write / rebuild-on-load, so existing keychain entries continue to load without re-login.refresh.pydrops the bespoke_refresh_error_fromforOAuthErrorResponse.from_response._refresh_and_storewraps malformed payloads asRefreshError.auth login/auth status/auth logoutaccess token fields viasession.token.<field>instead ofsession.<field>.Pydantic v2 models (88a3252)
TokenResponse,OAuthErrorResponse,StoredSessionare nowBaseModelsubclasses. Required strings useStrictStr+Field(min_length=1); lifetime fields useStrictInt | Noneso a bool cannot slip in as1. Unknown fields are ignored on the wire-facing models (Keycloak addsnot-before-policy,session_state) and forbidden onStoredSession(corruption signal: we own both writer and reader).StoredSessionkeeps the legacy on-disk shape via@model_validator(mode="before") _accept_flat_blob(re-nests destructured token fields back undertoken) plus@model_serializer(mode="wrap") _to_flat_blob(always emits the flat shape)._session_to_blob/_session_from_blob/_required_strare deleted;store_session/load_sessioncollapse tomodel_dump_json/model_validate_json.flow.py:exchange_codeandrefresh.py:_refresh_and_storegetValidationErrorbranches that render via a shared_format_validation_errorhelper, mirroring the MCP-tool message shape (pipefy_mcp.tools.portal_tool_helpers.portal_element_validation_error). Pydantic's verbose defaultstr(exc)with doc URLs never reaches end users.scope,id_token,error,error_description) route through_strip_or_none, which drops non-strings toNone. Load-bearing for the inline call sites that invokeOAuthErrorResponse.from_response(response).render(...)without a try: a malformed{"error": 0}body falls through to the fallback rather than crash the error-rendering path.Test plan
uv run pytest packages/auth/tests/: 152 passeduv run pytest packages/cli/tests/: 303 passed, 13 skippeduv run pytest packages/mcp/tests/: passesPIPEFY_KEYCHAIN_BACKEND=file, sandboxedXDG_CONFIG_HOME): legacy flat blob loads, nested-shape blob loads, fresh write produces flat on-disk shape,OAuthErrorResponse.from_response({"error": 0})renders the fallback without raising,pipefy auth status --jsonreadsauth_source: "stored-session"end-to-end.