Skip to content

Add library-backed OAuth validation and managed interoperability tests - #7

Merged
kdubb merged 5 commits into
mainfrom
codex/oauth-conformance
Oct 6, 2026
Merged

kdubb merged 5 commits into
mainfrom
codex/oauth-conformance

Conversation

@kdubb

@kdubb kdubb commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

OAuth discovery and token responses previously permitted inconsistent parsing and contextual validation across runtimes. This change adds explicit validation, 123 wire fixtures and 34 matching HTTP classification fixtures, and managed interoperability tests while preserving existing public APIs and the public-client compatibility policy from #59 in sunday-kt.

Authlib 1.8.0 constructs protocol requests and authentication; Sunday keeps its borrowed HTTPX transport and strict response decoding. The adapter applies OAuth-required form encoding before Authlib Basic authentication. No HTTPX2 migration.

Native test infrastructure owns checksum/digest-pinned WireMock replay and Keycloak 26.2.5 live lifecycle. Replay is the default. On macOS CI, live tests select the official Java distribution without Docker discovery; other environments use the container. CI invokes standard test commands with replay for PRs and live on main.

Validation: Ruff, mypy, and 470 tests pass with 92.54% coverage. Managed live Keycloak acquisition/refresh passes through HTTPX using both the official Java distribution and pinned container. Failure tests cover cache integrity, readiness, cleanup, and zero Docker calls on macOS CI startup failure.

Acceptance follow-up: all five transports now assert exact HTTP error categories during acquisition and refresh; accepted wire cases assert token values and absolute expiry. Recognized discovery endpoint validation is aligned across runtimes. Native harness tests cover startup failure, readiness timeout, process cleanup, corrupt caches, and rejected downloads. Dependency footprint and license measurements are recorded in the evaluation report.

Local replay/full checks and live Keycloak acquisition/refresh pass using both the Java distribution and pinned container. Kotlin additionally prevents OkHttp from automatically replaying single-use token requests on 408 or immediate-retry 503 responses. No live Auth0 tenant was tested; Auth0-shaped fixtures are deterministic only. Current-head hosted checks and review remain the merge gates. This PR does not publish a beta.

Error classification tolerates empty advisory descriptions as a documented interoperability exception while rejecting wrong types. Fractional expiry values are checked before numeric rounding, with matching precision-boundary fixtures across runtimes. Python uses exact Decimal decoding before integer lifetime conversion. Startup failures retain sanitized diagnostics, and failed container removal is reported with retry state preserved. Confirmed auto-removal is accepted only after a successful Docker inventory query, preserving the original startup error.

Related implementation PRs: Kotlin #61, JavaScript #80, Python #7, Swift #73.

kdubb added 2 commits October 5, 2026 14:45
Use evaluated OAuth libraries behind existing transport APIs, enforce strict wire and contextual validation, and add aligned fixtures. Manage replay and live Keycloak lifecycle through native test infrastructure with macOS CI Java selection. Document library decisions and verification boundaries.
Cover empty authentication-method lists through acquisition and rotation. Supply otherwise-valid discovery metadata in rejection fixtures and require no authorization callback for every trust failure.
@kdubb
kdubb force-pushed the codex/oauth-conformance branch from 036abdc to 52e58d9 Compare October 5, 2026 21:46
Align recognized discovery endpoint validation, verify normalized tokens and common HTTP failure categories for acquisition and refresh, and exercise real process and artifact cleanup failures. Document resolved dependency footprint and live backend verification.
@kdubb
kdubb marked this pull request as ready for review October 5, 2026 22:45
@greptile-apps

greptile-apps Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

[High risk] Adds OAuth library dependency and test infrastructure.

The reviewed changes appear safe to merge.

What we checked:

  • Failed Docker queries retain cleanup state: No. A failed or timed-out query returns false, so close() keeps the container name and reports the cleanup failure.

Summary

Adds Authlib-backed OAuth request construction, strict response checks, shared fixtures, and managed WireMock/Keycloak tests.

  • The latest change confirms container absence before accepting a failed removal.
  • Cleanup keeps retry state when Docker fails or the container still exists.
  • All three supplied previous threads are addressed: empty descriptions are accepted, safe startup details survive cleanup, and automatic container removal no longer hides the startup error.
  • No new actionable issues were found. Tests were inspected, not run.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Close provider] --> B{Container removal succeeds?}
  B -- Yes --> E[Clear owned container name]
  B -- No --> C[Query all matching containers]
  C --> D{Query succeeds and owned name is absent?}
  D -- Yes --> E
  D -- No --> F[Keep name for retry]
  E --> G[Stop process and remove local files]
  F --> G
  G --> H{Cleanup failed?}
  H -- Yes --> I[Report cleanup failure]
  H -- No --> J[Finish cleanup]
Loading

Reviews (3) · Last reviewed commit: "Treat confirmed container auto-removal a..."

Comment thread src/sunday/httpx/_oauth_wire.py Outdated
Comment thread tests/oauth_support/provider.py Outdated
Preserve refresh error classification for empty advisory descriptions and validate expiry before numeric rounding. Align shared fixtures, repair reviewed test lifecycle behavior, and document protocol compatibility and dependency boundaries.
Comment thread tests/oauth_support/provider.py Outdated
Verify absence through a successful Docker inventory query after failed removal. Keep real cleanup failures retryable and preserve the original startup failure, with regression coverage for absence, daemon failures, timeouts, and existing containers.
@kdubb
kdubb merged commit 9fa9782 into main Oct 6, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant