Skip to content

Release 4.2.0#37

Merged
turegjorup merged 28 commits into
mainfrom
release/4.2.0
May 11, 2026
Merged

Release 4.2.0#37
turegjorup merged 28 commits into
mainfrom
release/4.2.0

Conversation

@turegjorup
Copy link
Copy Markdown
Contributor

Summary

Roll up the accumulated [Unreleased] changes since 4.1.0 into a tagged release.

MINOR per SemVer reading: two new optional configuration features are added; existing configs and call sites keep working unchanged. The bundle's composer.json constraint on itk-dev/openid-connect stays at ^4.0; the just-released library 4.1.2 resolves cleanly under it.

What's in 4.2.0

Added

  • Per-provider cache_duration option (integer seconds, default 86400) forwarded to the underlying library. Lets consumers tighten or extend the 24-hour default TTL for the cached OIDC discovery document and JWKS.
  • Per-provider http_client_options block (timeout, proxy, verify) forwarded to the Guzzle HTTP client used by league/oauth2-client. Closes the long-standing inability to bound HTTP requests to the IdP.

Fixed

  • CliLoginHelper (four catch sites in createToken / getUsername) and OpenIdLoginAuthenticator::validateClaims now preserve the originating exception via $previous. Previously the message was copied but the chain to the underlying PSR cache / upstream OIDC failure was lost, making log traversal impossible.

Changed

  • LoginController::login now maps unknown-provider failures to NotFoundHttpException (404) and IdP-unreachable / cache failures to ServiceUnavailableHttpException (503), with the cause chained via $previous. Previously these surfaced as a generic 500. Other ItkOpenIdConnectException subtypes (e.g. BadUrlException for a misconfigured metadata_url) intentionally still bubble as 500 — server-side configuration bugs that should fail loudly.
  • Expanded README note on Symfony's native OIDC support (7.3 features, comparison table, link to the upstream authorization-code-flow issue).
  • Documented architectural decision in docs/adr/001-marker-interface-exception-hierarchy.md: the bundle and library exception hierarchy will converge on a marker-interface contract in coordinated 5.0 majors. Status: Draft. Not a 4.2.0-shipping change in behaviour; the ADR is a forward-looking decision record.

Tooling / CI

  • Bumped actions/checkout v5 → v6 across all workflows.
  • Renamed PHP_EXEC variable to PHP in Taskfile.yml.
  • Added lint:composer task to Taskfile.yml.
  • test:coverage now runs via docker-compose with XDEBUG_MODE=coverage and prints text summary.
  • Fixed test:matrix:reset to include the ci profile when removing volumes.
  • Fixed test:run to remove stale composer.lock before composer update.

Backward compatibility

  • New config options are optional with defaults; existing configurations continue to validate and load.
  • LoginController HTTP-response status codes for two failure modes change (was 500, now 404 or 503). Consumers that monitor specifically for 500s from those failures will see fewer; those that monitor for 4xx/5xx in general are unaffected. The exception types thrown internally also change, but LoginController is invoked by Symfony's HTTP kernel, not by consumer code.
  • $previous chaining is strictly additive: consumers reading getPrevious() now see a non-null cause where they previously saw null.
  • Public exception classes, namespaces, and concrete types are unchanged.

Test plan

  • task lint:markdownCHANGELOG.md clean
  • task test — 56 tests, 141 assertions pass
  • task analyze:php — phpstan max, no errors
  • task lint:php — clean
  • CI matrix on PR

🤖 Generated with Claude Code

turegjorup and others added 28 commits March 20, 2026 11:52
Update all GitHub workflow files to use actions/checkout@v6.
Improve Taskfile with renamed PHP variable, new lint:composer task,
better test:coverage with XDEBUG_MODE, and fixes for test:run and
test:matrix:reset.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Bump actions/checkout to v6 and improve Taskfile
Document Symfony 7.3 OIDC additions (discovery, token introspection,
JWE) and add a comparison table clarifying which features still
require this bundle. Link upstream issue for authorization code flow.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Forwards the league/oauth2-client-whitelisted Guzzle options (timeout,
proxy, verify) through bundle configuration. Closes the long-standing
inability to bound HTTP requests to the IdP.

Additive only — existing config continues to work unchanged. README adds
a short note explaining why the stack uses Guzzle rather than Symfony
HttpClient (league/oauth2-client hard-types its httpClient as
GuzzleHttp\ClientInterface).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The badge pointed to a `pr.yaml` workflow that no longer exists, and the
link queried for a workflow named "Test & Code Style Review" that has
been renamed/split. Repointed to `php.yaml` (the workflow running tests,
phpstan, and php-cs-fixer) and pinned the badge to the `develop` branch.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Previously any failure during the OIDC authorization redirect bubbled
unhandled as a generic 500. Map to specific responses:

- InvalidProviderException        -> NotFoundHttpException (404)
- HttpException / JsonException /
  CacheException (metadata fetch)  -> ServiceUnavailableHttpException (503)

Cause chained via `previous`. Other ItkOpenIdConnectException subtypes
(config-time errors) still bubble as 500 — correctly indicates a
server-side configuration bug.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Drop the `?? []` fallback that masked the difference between an absent
key and an empty array. Per the schema (no defaultValue on the
arrayNode), an omitted input must produce no key at all.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous wording read as if league silently strips non-whitelisted
keys. The bundle config rejects them up front with
InvalidConfigurationException at container compile time.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Forwarded to the underlying library's `cacheDuration` constructor option;
controls the TTL of the cached OIDC discovery document and JWKS. Default
remains 86400 seconds (24h), matching the library default.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Expose per-provider cache_duration option
- Widen getAuthorizationUrl() catch from HttpException|JsonException|CacheException to the documented base ItkOpenIdConnectException. The catch list was an incomplete enumeration of subtypes; widening to the public base is the contract the upstream library advertises.
- Document in the @throws block that other ItkOpenIdConnectException subtypes raised during provider init (BadUrlException etc.) are config-time errors and intentionally bubble as 500 rather than be coerced to 503.
- Move $this->fail() out of the try block in the two new test cases so a future refactor that drops the assertion line cannot silently weaken the tests.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Map LoginController failures to specific HTTP errors
Five wrap sites (4 in CliLoginHelper, 1 in OpenIdLoginAuthenticator::validateClaims) copied the message from the caught exception but discarded the chain. Logs lost the originating PSR cache / upstream OIDC failure, making debug traversal impossible. Add `previous: $e` to each `new CacheException(...)` / `new ValidationException(...)`.

Test changes: convert the existing message-only assertions to a try/catch + early-return pattern so each test also asserts `getPrevious()` returns the original cause object. Five existing tests touched; test count and coverage unchanged.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Records the decision to migrate the bundle (and upstream library) to a marker-interface exception contract: cross-package marker interfaces, concretes extending SPL types, wrap-at-boundary discipline with `$previous` chained, and the controller HTTP-exception exit treated as a documented carve-out. Migration via coordinated 5.0 majors across `itk-dev/openid-connect` and this bundle, sequenced as four PRs. PR 0 (cause-chain bug fixes) already in flight as PR #35.

The ADR is self-contained: context, drivers, three options considered, decision, consequences, and references (PSR-18, Symfony's component markers and Psr18Client, Bloch's wrap-at-boundary principle).

Status: Draft — awaits team review.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add a short framing paragraph at the top of "Context" stating that the
marker-interface principles the ADR proposes were the intended design
all along; the implementation has drifted from intent on specific points
and this migration closes those gaps.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Preserve $previous chain in exception wraps
…tion

ADR 001: Adopt marker-interface exception hierarchy
Roll up the [Unreleased] section into 4.2.0 dated 2026-05-11. MINOR per SemVer — two new optional configuration features (`cache_duration` per-provider, `http_client_options.{timeout, proxy, verify}` per-provider) extend the bundle's surface without breaking existing configs. Plus PATCH-level fixes: LoginController now maps unknown-provider → 404 and IdP-unreachable / cache-failure → 503 instead of bubbling those as 500; `CliLoginHelper` and `OpenIdLoginAuthenticator::validateClaims` exception wraps now preserve `$previous` so logs can traverse to the originating PSR/upstream cause. `composer.json` constraint on `itk-dev/openid-connect` stays at `^4.0` (the just-released 4.1.2 resolves under it).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@turegjorup turegjorup self-assigned this May 11, 2026
@turegjorup turegjorup merged commit 7964acb into main May 11, 2026
15 checks passed
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (9e60580) to head (e236274).
⚠️ Report is 29 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##                main       #37   +/-   ##
===========================================
  Coverage     100.00%   100.00%           
- Complexity        56        60    +4     
===========================================
  Files              9         9           
  Lines            248       278   +30     
===========================================
+ Hits             248       278   +30     
Flag Coverage Δ
unittests 100.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

2 participants