Release 4.2.0#37
Merged
Merged
Conversation
no message
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>
…roller-http-errors
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>
Codecov Report✅ All modified and coverable lines are covered by tests. 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
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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
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
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.jsonconstraint onitk-dev/openid-connectstays at^4.0; the just-released library4.1.2resolves cleanly under it.What's in 4.2.0
Added
cache_durationoption (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.http_client_optionsblock (timeout,proxy,verify) forwarded to the Guzzle HTTP client used byleague/oauth2-client. Closes the long-standing inability to bound HTTP requests to the IdP.Fixed
CliLoginHelper(four catch sites increateToken/getUsername) andOpenIdLoginAuthenticator::validateClaimsnow 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::loginnow maps unknown-provider failures toNotFoundHttpException(404) and IdP-unreachable / cache failures toServiceUnavailableHttpException(503), with the cause chained via$previous. Previously these surfaced as a generic 500. OtherItkOpenIdConnectExceptionsubtypes (e.g.BadUrlExceptionfor a misconfiguredmetadata_url) intentionally still bubble as 500 — server-side configuration bugs that should fail loudly.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
actions/checkoutv5 → v6 across all workflows.PHP_EXECvariable toPHPinTaskfile.yml.lint:composertask toTaskfile.yml.test:coveragenow runs via docker-compose withXDEBUG_MODE=coverageand prints text summary.test:matrix:resetto include theciprofile when removing volumes.test:runto remove stalecomposer.lockbeforecomposer update.Backward compatibility
LoginControllerHTTP-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, butLoginControlleris invoked by Symfony's HTTP kernel, not by consumer code.$previouschaining is strictly additive: consumers readinggetPrevious()now see a non-null cause where they previously sawnull.Test plan
task lint:markdown—CHANGELOG.mdcleantask test— 56 tests, 141 assertions passtask analyze:php— phpstan max, no errorstask lint:php— clean🤖 Generated with Claude Code