diff --git a/.github/workflows/changelog.yaml b/.github/workflows/changelog.yaml
index fead572..63638c2 100644
--- a/.github/workflows/changelog.yaml
+++ b/.github/workflows/changelog.yaml
@@ -18,7 +18,7 @@ jobs:
fail-fast: false
steps:
- name: Checkout
- uses: actions/checkout@v5
+ uses: actions/checkout@v6
with:
fetch-depth: 2
diff --git a/.github/workflows/composer.yaml b/.github/workflows/composer.yaml
index eebafe7..965d42a 100644
--- a/.github/workflows/composer.yaml
+++ b/.github/workflows/composer.yaml
@@ -44,7 +44,7 @@ jobs:
matrix:
prefer: [prefer-lowest, prefer-stable]
steps:
- - uses: actions/checkout@v5
+ - uses: actions/checkout@v6
- name: Create docker network
run: |
@@ -58,7 +58,7 @@ jobs:
strategy:
fail-fast: false
steps:
- - uses: actions/checkout@v5
+ - uses: actions/checkout@v6
- name: Create docker network
run: |
@@ -73,7 +73,7 @@ jobs:
strategy:
fail-fast: false
steps:
- - uses: actions/checkout@v5
+ - uses: actions/checkout@v6
- name: Create docker network
run: |
diff --git a/.github/workflows/github_build_release.yml b/.github/workflows/github_build_release.yml
index 0751011..b2c083f 100644
--- a/.github/workflows/github_build_release.yml
+++ b/.github/workflows/github_build_release.yml
@@ -16,7 +16,7 @@ jobs:
APP_ENV: prod
steps:
- name: Checkout
- uses: actions/checkout@v5
+ uses: actions/checkout@v6
- name: Create a release in GitHub
run: gh release create ${{ github.ref_name }} --verify-tag --generate-notes
diff --git a/.github/workflows/markdown.yaml b/.github/workflows/markdown.yaml
index ae83163..8f0fc25 100644
--- a/.github/workflows/markdown.yaml
+++ b/.github/workflows/markdown.yaml
@@ -34,7 +34,7 @@ jobs:
fail-fast: false
steps:
- name: Checkout
- uses: actions/checkout@v5
+ uses: actions/checkout@v6
- name: Create docker network
run: |
diff --git a/.github/workflows/php.yaml b/.github/workflows/php.yaml
index 39a0643..ec59121 100644
--- a/.github/workflows/php.yaml
+++ b/.github/workflows/php.yaml
@@ -15,7 +15,7 @@ jobs:
name: PHP - Check Coding Standards
runs-on: ubuntu-latest
steps:
- - uses: actions/checkout@v5
+ - uses: actions/checkout@v6
- name: Create docker network
run: |
@@ -29,7 +29,7 @@ jobs:
name: PHPStan
runs-on: ubuntu-latest
steps:
- - uses: actions/checkout@v5
+ - uses: actions/checkout@v6
- name: Create docker network
run: |
@@ -65,7 +65,7 @@ jobs:
php: "8.5"
prefer: prefer-stable
steps:
- - uses: actions/checkout@v5
+ - uses: actions/checkout@v6
- name: Create docker network
run: |
diff --git a/.github/workflows/yaml.yaml b/.github/workflows/yaml.yaml
index 631e525..299d4e1 100644
--- a/.github/workflows/yaml.yaml
+++ b/.github/workflows/yaml.yaml
@@ -31,7 +31,7 @@ jobs:
yaml-lint:
runs-on: ubuntu-latest
steps:
- - uses: actions/checkout@v5
+ - uses: actions/checkout@v6
- name: Create docker network
run: |
diff --git a/CHANGELOG.md b/CHANGELOG.md
index b43bf5d..2b6d219 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,6 +7,37 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
## [Unreleased]
+## [4.2.0] - 2026-05-11
+
+### Added
+
+- Per-provider `cache_duration` option (seconds) forwarded to the
+ underlying library; lets consumers tighten or extend the 24h default
+ TTL for the cached OIDC discovery document and JWKS
+- Per-provider `http_client_options` block (`timeout`, `proxy`, `verify`)
+ forwarded to the underlying Guzzle HTTP client used by league/oauth2-client.
+ Closes the long-standing inability to bound HTTP requests to the IdP.
+
+### Fixed
+
+- Preserve original cause via `$previous` in `CliLoginHelper` and
+ `OpenIdLoginAuthenticator::validateClaims` exception wraps. Previously
+ the message was copied but the chain to the originating PSR cache or
+ upstream OIDC failure was lost, making logs harder to debug.
+
+### Changed
+
+- Mapped LoginController failures to 404 (unknown provider) or 503
+ (upstream/cache) instead of a generic 500; cause chained via `previous`
+- Expanded README note on Symfony native OIDC support (7.3 features,
+ comparison table, link to upstream authorization-code-flow issue)
+- Bumped actions/checkout from v5 to v6 in all GitHub workflows
+- Renamed PHP_EXEC variable to PHP in Taskfile
+- Added lint:composer task to Taskfile
+- Improved test:coverage to enable XDEBUG_MODE and add text output
+- Fixed test:matrix:reset to include CI profile when removing volumes
+- Fixed test:run to remove stale composer.lock before updating dependencies
+
## [4.1.0] - 2026-03-20
### Added
@@ -123,7 +154,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
`itk-dev/openid-connect` 1.0.0 to 2.1.0
- OpenId Connect Bundle: Added CLI login feature.
-[unreleased]: https://github.com/itk-dev/openid-connect-bundle/compare/4.1.0...HEAD
+[unreleased]: https://github.com/itk-dev/openid-connect-bundle/compare/4.2.0...HEAD
+[4.2.0]: https://github.com/itk-dev/openid-connect-bundle/compare/4.1.0...4.2.0
[4.1.0]: https://github.com/itk-dev/openid-connect-bundle/compare/4.0.1...4.1.0
[4.0.1]: https://github.com/itk-dev/openid-connect-bundle/compare/4.0.0...4.0.1
[4.0.0]: https://github.com/itk-dev/openid-connect-bundle/compare/3.1.0...4.0.0
diff --git a/README.md b/README.md
index d7e7dd6..4892b13 100644
--- a/README.md
+++ b/README.md
@@ -3,7 +3,7 @@
[](https://github.com/itk-dev/openid-connect-bundle)
[](https://packagist.org/packages/itk-dev/openid-connect-bundle)
[](https://www.php.net/downloads)
-[](https://github.com/itk-dev/openid-connect-bundle/actions?query=workflow%3A%22Test+%26+Code+Style+Review%22)
+[](https://github.com/itk-dev/openid-connect-bundle/actions/workflows/php.yaml?query=branch%3Adevelop)
[](https://codecov.io/gh/itk-dev/openid-connect-bundle)
[](https://github.com/itk-dev/openid-connect-bundle/blob/master/LICENSE.md)
[](https://packagist.org/packages/itk-dev/openid-connect-bundle/stats)
@@ -17,16 +17,48 @@ Symfony bundle for authorization via OpenID Connect.
> Since this bundle was created Symfony has added [support for OpenID Connect](https://symfony.com/blog/new-in-symfony-6-3-openid-connect-token-handler)
> as documented in ["Using OpenID Connect (OIDC)"](https://symfony.com/doc/current/security/access_token.html#using-openid-connect-oidc).
>
-> As of Symfony 7.4 (March 2026), Symfony's native OIDC support has matured:
+> Symfony's native OIDC support has improved significantly in recent releases:
>
-> * [OIDC discovery](https://github.com/symfony/symfony/pull/54932) was added in Symfony 7.3, removing the need
-> for manual keyset configuration.
-> * Multiple providers are supported via multiple `base_uri` and `issuers` entries in the discovery config.
+> * [OIDC discovery](https://github.com/symfony/symfony/pull/54932) was added in
+> Symfony 7.3 (May 2025), removing the need for manual keyset configuration.
+> Keys are fetched and cached automatically from the provider's
+> `.well-known/openid-configuration` endpoint.
+> * [OAuth2 Token Introspection](https://symfony.com/blog/new-in-symfony-7-3-security-improvements)
+> (RFC 7662) support was added in Symfony 7.3, useful when access tokens are
+> opaque (not JWTs).
+> * [JWE (encrypted token) support](https://github.com/symfony/symfony/pull/57721)
+> was added in Symfony 7.3 for OIDC token handlers.
>
-> However, Symfony's native OIDC support is designed for **bearer token validation** (API authentication) only.
-> It does not implement the **authorization code flow** (browser-based login with redirect to the IdP and callback
-> handling), which is the primary use case of this bundle. If your application needs browser-based OIDC login,
-> this bundle is still required.
+> However, Symfony's native OIDC support is designed for **stateless bearer
+> token validation** (the `access_token` authenticator) only. It validates tokens
+> that are already present on the request (e.g. in an `Authorization: Bearer`
+> header).
+>
+> It does **not** implement the **authorization code flow** — the browser-based
+> login where the application redirects to the IdP, handles the callback with an
+> authorization code, exchanges it for tokens, and establishes a session. This
+> is tracked upstream in [symfony/symfony#50896](https://github.com/symfony/symfony/issues/50896).
+>
+> This means the following features of this bundle have no native Symfony
+> equivalent:
+>
+> | Feature | This bundle | Symfony native |
+> |--------------------------------|:-----------:|:--------------:|
+> | Authorization code flow | ✅ | ❌ |
+> | Session-based browser login | ✅ | ❌ |
+> | Multiple named OIDC providers | ✅ | ❌ ¹ |
+> | CLI login tokens | ✅ | ❌ |
+> | OIDC discovery | ✅ | ✅ |
+> | Bearer token validation (API) | ❌ | ✅ |
+> | OAuth2 token introspection | ❌ | ✅ |
+>
+> ¹ Symfony's `access_token` handler accepts multiple `issuers` for token
+> validation, but this is not the same as this bundle's named provider model
+> with distinct client credentials, redirect URIs, and selectable login routes
+> per provider.
+>
+> If your application needs browser-based OIDC login, this bundle is still
+> required.
## Installation
@@ -76,6 +108,9 @@ itkdev_openid_connect:
# Optional: Specify leeway (seconds) to account for clock skew between provider and hosting
# Defaults to 10
leeway: '%env(int:ADMIN_OIDC_LEEWAY)%'
+ # Optional: Cache duration (seconds) for the OIDC discovery document and JWKS
+ # Defaults to 86400 (24 hours)
+ cache_duration: '%env(int:ADMIN_OIDC_CACHE_DURATION)%'
# Optional: Allow (non-secure) http requests (used for mocking a IdP). NOT RECOMMENDED FOR PRODUCTION.
# Defaults to false
allow_http: '%env(bool:ADMIN_OIDC_ALLOW_HTTP)%'
@@ -101,6 +136,7 @@ ADMIN_OIDC_CLIENT_ID=ADMIN_APP_CLIENT_ID
ADMIN_OIDC_CLIENT_SECRET=ADMIN_APP_CLIENT_SECRET
ADMIN_OIDC_REDIRECT_URI=ADMIN_APP_REDIRECT_URI
ADMIN_OIDC_LEEWAY=30
+ADMIN_OIDC_CACHE_DURATION=86400
ADMIN_OIDC_ALLOW_HTTP=false
# "user" open id connect configuration variables
@@ -115,6 +151,42 @@ OIDC_CLI_LOGIN_ROUTE=OIDC_CLI_LOGIN_ROUTE
Set the actual values your `env.local` file to ensure they are not committed to Git.
+#### Configuring the HTTP client
+
+Each provider accepts an optional `http_client_options` block that is forwarded
+to the underlying Guzzle HTTP client used by `league/oauth2-client`. This is
+useful for setting a request timeout so a slow IdP cannot block worker
+processes indefinitely.
+
+```yaml
+itkdev_openid_connect:
+ openid_providers:
+ user:
+ options:
+ # ... existing keys ...
+ # @see https://docs.guzzlephp.org/en/stable/request-options.html
+ http_client_options:
+ # Float describing the total timeout of the request in seconds. Use 0 to wait indefinitely (the default behavior).
+ timeout: 5.0
+ # Pass a string to specify an HTTP proxy, or an array to specify different proxies for different protocols. (Default: none)
+ proxy: "%env(string:HTTP_PROXY)%"
+ # Describes the SSL certificate verification behavior of a request. (Default: true)
+ verify: true
+```
+
+The bundle accepts only `timeout`, `proxy`, and `verify` under
+`http_client_options` — these are the keys `league/oauth2-client` forwards to
+Guzzle (`verify` is consulted only when `proxy` is set). Any other key causes
+an `InvalidConfigurationException` at container compile time.
+
+> **Why Guzzle and not Symfony HttpClient?**
+> `league/oauth2-client`, which the underlying `itk-dev/openid-connect`
+> library extends, hard-types its HTTP client as `GuzzleHttp\ClientInterface`.
+> Symfony HttpClient implements PSR-18 / HTTPlug, not Guzzle's interface, and
+> no maintained adapter goes Symfony → Guzzle. Configure Guzzle via the
+> options above; full transport replacement is not currently possible without
+> a custom adapter we are not yet shipping.
+
In `/config/routes/` you need a similar `itkdev_openid_connect.yaml` file for
configuring the routing
diff --git a/Taskfile.yml b/Taskfile.yml
index ba09624..0eb711d 100644
--- a/Taskfile.yml
+++ b/Taskfile.yml
@@ -4,8 +4,8 @@ version: "3"
vars:
DOCKER_COMPOSE: "docker compose"
- PHP_EXEC: "{{.DOCKER_COMPOSE}} exec phpfpm"
- COMPOSER: "{{.PHP_EXEC}} composer"
+ PHP: "{{.DOCKER_COMPOSE}} exec phpfpm"
+ COMPOSER: "{{.PHP}} composer"
tasks:
default:
@@ -74,7 +74,7 @@ tasks:
composer:check:
desc: Validate and audit composer
cmds:
- - "{{.PHP_EXEC}} composer validate --strict"
+ - "{{.PHP}} composer validate --strict"
- "{{.COMPOSER}} normalize --dry-run"
- "{{.COMPOSER}} audit"
@@ -84,18 +84,26 @@ tasks:
desc: Run all linters
cmds:
- task: lint:php
+ - task: lint:composer
- task: lint:markdown
- task: lint:yaml
lint:php:
desc: Check PHP coding standards
cmds:
- - "{{.PHP_EXEC}} vendor/bin/php-cs-fixer fix --dry-run --diff"
+ - "{{.PHP}} vendor/bin/php-cs-fixer fix --dry-run --diff"
lint:php:fix:
desc: Fix PHP coding standards
cmds:
- - "{{.PHP_EXEC}} vendor/bin/php-cs-fixer fix"
+ - "{{.PHP}} vendor/bin/php-cs-fixer fix"
+
+ lint:composer:
+ desc: Validate and audit composer
+ cmds:
+ - "{{.PHP}} composer validate --strict"
+ - "{{.COMPOSER}} normalize --dry-run"
+ - "{{.COMPOSER}} audit"
lint:markdown:
desc: Lint markdown files
@@ -122,19 +130,19 @@ tasks:
analyze:php:
desc: Run PHPStan static analysis
cmds:
- - "{{.PHP_EXEC}} vendor/bin/phpstan"
+ - "{{.PHP}} vendor/bin/phpstan"
# Testing
test:
desc: Run tests
cmds:
- - "{{.PHP_EXEC}} vendor/bin/phpunit"
+ - "{{.PHP}} vendor/bin/phpunit"
test:coverage:
desc: Run tests with coverage
cmds:
- - "{{.PHP_EXEC}} vendor/bin/phpunit --coverage-clover=coverage/unit.xml"
+ - "{{.DOCKER_COMPOSE}} exec -e XDEBUG_MODE=coverage phpfpm vendor/bin/phpunit --coverage-text --coverage-clover=coverage/unit.xml"
test:run:
desc: "Run tests for a PHP version and dependency set (e.g. task test:run PHP=8.4 DEPS=lowest)"
@@ -154,6 +162,7 @@ tasks:
cp /app/vendor/.composer.lock /app/composer.lock
composer install -q
else
+ rm -f /app/composer.lock
composer update -q --{{.PREFER}}
cp /app/composer.lock /app/vendor/.composer.lock
fi'
@@ -162,7 +171,7 @@ tasks:
test:matrix:reset:
desc: Remove cached vendor volumes to force a fresh dependency resolve
cmds:
- - "{{.DOCKER_COMPOSE}} down --volumes"
+ - "{{.DOCKER_COMPOSE}} --profile ci down --volumes"
test:matrix:
desc: Run tests across all PHP versions (mirrors CI matrix)
diff --git a/docs/adr/001-marker-interface-exception-hierarchy.md b/docs/adr/001-marker-interface-exception-hierarchy.md
new file mode 100644
index 0000000..6d0dbef
--- /dev/null
+++ b/docs/adr/001-marker-interface-exception-hierarchy.md
@@ -0,0 +1,370 @@
+# 001: Adopt marker-interface exception hierarchy across library and bundle
+
+- **Created By:** Ture Gjørup
+- **Date:** 2026-05-11
+- **Decision Maker:** Ture Gjørup (draft — awaits team review)
+- **Stakeholders:** Bundle consumers (downstream apps catching OIDC exceptions);
+ `itk-dev/openid-connect` library maintainers;
+ `itk-dev/openid-connect-bundle` maintainers
+- **Status:** Draft
+
+## Context
+
+The principles this ADR proposes — cross-package marker interfaces,
+concrete exceptions extending SPL types, wrap-at-boundary discipline with
+`$previous` chained — are the intended design of the bundle's and
+library's exception layer. This is not a change in design direction. It
+is a course correction: the implementation has drifted from the intent on
+several specific points, and the migration below closes those gaps and
+adds the static-analysis guardrails needed to keep the implementation
+aligned going forward.
+
+This bundle and its upstream `itk-dev/openid-connect` library expose
+exception types that consuming applications catch in production code. The
+shape of that exception hierarchy is part of the public API: a consumer
+deciding whether to retry, re-authenticate, or fail hard has to be able to
+distinguish failure modes by `catch` type. Today, the hierarchy makes
+that decision harder than it needs to be.
+
+**Library — `itk-dev/openid-connect`.**
+The base type is the abstract class
+`\ItkDev\OpenIdConnect\Exception\ItkOpenIdConnectException`, which extends
+`\Exception` directly. Every concrete exception (`CacheException`,
+`HttpException`, `JsonException`, `ValidationException`, `ClaimsException`,
+`CodeException`, `BadUrlException`, `IllegalSchemeException`,
+`MissingParameterException`, `NegativeLeewayException`,
+`NegativeCacheDurationException`, `DecodeException`, `KeyException`)
+extends that abstract.
+
+**Bundle — `itk-dev/openid-connect-bundle`.**
+The base type is a separate abstract class
+`\ItkDev\OpenIdConnectBundle\Exception\ItkOpenIdConnectBundleException`,
+also extending `\Exception` directly. Concrete bundle exceptions
+(`CacheException`, `InvalidProviderException`, `TokenNotFoundException`,
+`UsernameDoesNotExistException`, `UserDoesNotExistException`) extend that
+abstract. **There is no inheritance link between the bundle's base and
+the library's** — they are two parallel trees that happen to share a
+domain.
+
+This produces three consumer-visible problems:
+
+1. **No cross-package catch.** A downstream application that needs to
+ handle "any OIDC failure" — for example, to render a generic
+ re-authentication page — cannot write a single `catch` block.
+ `catch (ItkOpenIdConnectBundleException $e)` misses everything the
+ library throws (`HttpException` from the IdP discovery fetch,
+ `ClaimsException` from token validation, …). `catch (\Exception $e)`
+ is too wide and matches unrelated failures. The only correct option is
+ a list of every concrete, which rots as the libraries evolve.
+
+2. **No SPL-typed semantics.** The concretes all extend the bundle/library
+ abstract, which extends `\Exception`. A generic retry decorator that
+ does `catch (\RuntimeException $e)` to retry transient failures, or a
+ strict input validator that catches `\InvalidArgumentException` — both
+ common idioms — does not match anything in the OIDC trees, because the
+ trees skip the SPL middle layer.
+
+3. **Lost cause chain.** Eight wrap sites across the bundle and library
+ catch a dependency exception, copy its message into a new bundle/library
+ exception, and discard `$previous`. Log traversal stops at the wrapper
+ — the originating Guzzle / PSR-cache / lcobucci-JWT exception is
+ invisible. Concretely: four sites in
+ `src/Util/CliLoginHelper.php` (PSR cache wraps),
+ `src/Security/OpenIdLoginAuthenticator.php` line 75 (library exception
+ wrap), and three sites in the upstream library's
+ `OpenIdConfigurationProvider` (JWKS fetch, JWT validation, JSON
+ resource fetch).
+
+There are two related defects that the same migration should fix:
+
+- `src/Security/OpenIdLoginAuthenticator::validateClaims` calls
+ `$this->providerManager->getProvider($providerKey)` (line 49) without a
+ surrounding `try`/`catch`. The upstream `@throws` declares
+ `ItkOpenIdConnectException`, which escapes uncaught from a `protected`
+ method on a class designed to be subclassed.
+- Upstream `OpenIdConfigurationProvider::__construct` throws a raw
+ `\InvalidArgumentException` (not within the library's hierarchy);
+ `checkResponse` lets
+ `League\OAuth2\Client\Provider\Exception\IdentityProviderException`
+ escape unwrapped through `getIdToken`. Both are boundary leaks — a
+ dependency's type surfaces from a public method of ours.
+
+The gap surfaced concretely during review of
+[PR #33](https://github.com/itk-dev/openid-connect-bundle/pull/33)
+(LoginController HTTP error mapping): the reviewer flagged that the
+controller's `catch` list was enumerating subtypes incompletely.
+Widening to the documented base addressed the immediate question, but the
+underlying contract problem — that the base isn't part of a usable
+cross-package hierarchy — remained.
+
+### Drivers
+
+- **Functional:**
+ - A consumer must be able to catch "any OIDC failure" with a single
+ `catch` line. This is the conventional Symfony / PSR idiom: every
+ Symfony component ships an `ExceptionInterface` marker
+ (`Symfony\Component\Security\Core\Exception\ExceptionInterface`,
+ `Symfony\Component\Console\Exception\ExceptionInterface`, etc.);
+ PSR-18 ships
+ `Psr\Http\Client\ClientExceptionInterface`.
+ - The `$previous` chain is consumer-visible debugging information and
+ must be preserved. Symfony's `Psr18Client` is the canonical wrap-at-
+ boundary reference: it translates `TransportExceptionInterface` into
+ PSR-18 exceptions while passing the original as `$previous`.
+ - The HTTP controller already exits through Symfony's `HttpException`
+ subclasses (per PR #33). The contract needs to declare this as a
+ deliberate carve-out rather than treat it as a violation.
+
+- **Non-functional:**
+ - Maintainability: the current convention is informal — nothing
+ prevents a future throw site from regressing (a new exception that
+ doesn't fit the tree, a wrap site that drops `$previous`). Static
+ analysis can lock the contract on every CI run.
+ - Migration cost: any change to the public exception hierarchy is
+ consumer-visible. The migration's cost is dominated by downstream
+ apps mechanically rewriting `catch` blocks; the bundle's internal
+ refactor is small.
+ - Release coordination: the bundle depends on the library
+ (`"itk-dev/openid-connect": "^4.0"` in `composer.json`). A
+ contract-level change to the library forces a coordinated MAJOR cut
+ on both packages — composer cannot resolve the new bundle against
+ the old library.
+ - Exception granularity: PSR-18's meta document
+ ([psr-18 meta](https://www.php-fig.org/psr/psr-18/meta/)) explicitly
+ rejected adding subtypes like `TimeOutException` because consumers
+ would handle them identically. Target: three to five concrete
+ failure categories per package, not one per RFC section.
+
+### Options Considered
+
+1. **Marker interface + SPL re-parenting (proposed).** Introduce two
+ marker interfaces:
+ `\ItkDev\OpenIdConnect\Exception\OpenIdConnectExceptionInterface`
+ (in the library) and
+ `\ItkDev\OpenIdConnectBundle\Exception\OpenIdConnectBundleExceptionInterface`
+ (in the bundle, extending the library's). Re-parent every concrete to
+ its SPL base (`\RuntimeException` for transient/network/data,
+ `\LogicException` for programmer/config bugs,
+ `\InvalidArgumentException` for invalid input). Each concrete
+ implements the appropriate marker. Keep the existing abstract base
+ classes as `@deprecated` aliases that implement the marker, so
+ `catch (ItkOpenIdConnectBundleException $e)` keeps working through
+ the 5.x line. Coordinated 5.0 majors on both packages. Custom PHPStan
+ rules in the bundle lock the contract on every CI run.
+ - **Pros:** Single `catch` line covers both packages. SPL parents
+ give the rare consumer who catches on `\RuntimeException` correct
+ semantics. PHPStan-enforced so the contract cannot silently rot.
+ `$previous` chain becomes a tested invariant. The
+ marker-interface idiom matches Symfony and PSR conventions.
+ - **Cons:** Two coordinated MAJOR releases. Consumers must migrate
+ `catch` blocks at upgrade time (mechanical: catch on the marker
+ interface rather than the abstract; class names of concretes
+ unchanged). ~200 lines of in-tree PHPStan rule code maintained
+ alongside production code.
+
+2. **Marker interface only, no SPL re-parenting.** Add the marker
+ interface and have the existing abstract base classes implement it.
+ Concrete exceptions keep their current parent (the abstract).
+ Additive — no MAJOR bump needed.
+ - **Pros:** No BC break, no MAJOR cut, consumers can adopt at their
+ own pace. The cross-package catch problem is solved (the most
+ visible defect).
+ - **Cons:** Skips the SPL hierarchy entirely. The
+ catch-on-`\RuntimeException` idiom never becomes available. Future
+ maintainers see the abstract being both "deprecated, prefer the
+ marker" and "still the only path to concrete subtypes" — confusing.
+ The implementation is half a design.
+
+3. **Status quo + targeted bug fixes only.** Keep the current abstract-
+ class convention, but fix the discrete defects (`$previous` chains,
+ the missing `getProvider` catch in `validateClaims`, the library
+ boundary leaks). No public API change.
+ - **Pros:** Lowest-cost option. PATCH-level releases only. No
+ consumer migration. The most visible bugs go away.
+ - **Cons:** The cross-package catch problem remains unfixable
+ without a public API change. The catch-on-SPL idiom remains
+ unavailable. Future maintainers inherit the same gap and the
+ question recurs the next time someone writes a consumer that
+ needs to handle OIDC failures uniformly.
+
+## Decision
+
+**Adopt Option 1.** Migrate to the marker-interface contract via
+coordinated 5.0 majors on the library and bundle, sequenced as four PRs:
+
+- **PR 0** — `$previous` chaining bug fixes (4.x patch). Bundle repo,
+ branch `fix/exception-cause-chain`,
+ [PR #35](https://github.com/itk-dev/openid-connect-bundle/pull/35). No
+ dependencies; ships on the 4.x line as a `### Fixed` bullet.
+- **PR 1** — Library marker interface + SPL re-parenting + boundary
+ wraps. Library repo, branch `feature/exception-contract-5.0`, tag
+ `5.0.0`. No dependencies.
+- **PR 2** — Bundle marker interface + re-parent + composer bump + a new
+ carve-out subsection that documents the controller HTTP-exception
+ exit. Bundle repo, branch `feature/exception-contract-5.0`, tag
+ `5.0.0`. Depends on PR 1 tagged so composer can resolve `^5.0`.
+- **PR 3** — Custom PHPStan rules locking the contract. Bundled inside
+ PR 2 so a future revert of the migration also reverts the lock-in
+ (intentional friction; one cannot land without the other).
+
+PR 0 ships as part of the in-flight 4.2.0 release. PR 2's `[Unreleased]`
+bullets are reserved for the section opened after 4.2 is cut. The
+bundle's `composer.json` constraint moves from
+`"itk-dev/openid-connect": "^4.0"` to `"^5.0"` in PR 2.
+
+### Concrete-class parent mapping
+
+Library concretes (PR 1):
+
+- `\LogicException` — `BadUrlException`, `IllegalSchemeException`,
+ `MissingParameterException`, and a new `ConfigurationException` that
+ replaces the raw `\InvalidArgumentException` currently thrown by
+ `OpenIdConfigurationProvider::__construct`.
+- `\InvalidArgumentException` — `NegativeLeewayException`,
+ `NegativeCacheDurationException`.
+- `\RuntimeException` — `CacheException`, `HttpException` (also keeps
+ `Psr\Http\Client\ClientExceptionInterface`, so PSR-18-savvy consumers
+ can still catch on the standard PSR interface),
+ `JsonException`, `DecodeException`, `KeyException`, `CodeException`,
+ `ValidationException`, `ClaimsException`.
+
+Bundle concretes (PR 2):
+
+- `\InvalidArgumentException` — `InvalidProviderException`,
+ `UsernameDoesNotExistException`.
+- `\RuntimeException` — `CacheException`, `TokenNotFoundException`,
+ `UserDoesNotExistException`.
+
+### Boundary fixes (in scope for PR 1 and PR 2)
+
+- `OpenIdLoginAuthenticator::validateClaims` line 49 — wrap
+ `getProvider()` in `try`/`catch (OpenIdConnectExceptionInterface)`
+ and surface as a bundle-typed exception with `$previous` chained.
+- `OpenIdConfigurationProvider::__construct` — wrap raw
+ `\InvalidArgumentException` into the new `ConfigurationException`.
+- `OpenIdConfigurationProvider::checkResponse` — wrap
+ `League\OAuth2\Client\Provider\Exception\IdentityProviderException`
+ into `CodeException` with `$previous` at the public method that
+ triggers it (`getIdToken`).
+- All eight `$previous`-discarding wrap sites named in Context — pass
+ the caught exception as `$previous`. PR 0 fixes the bundle's five
+ sites already.
+
+### Controller carve-out
+
+The `LoginController::login` method exits through Symfony's
+`NotFoundHttpException` / `ServiceUnavailableHttpException` (added in
+PR #33) rather than through the bundle marker. The Symfony HTTP kernel
+reads the status code off these specific concretes to render the
+response; wrapping them into bundle types would require an exception
+listener to unwrap and re-throw, with no benefit to a catching consumer
+(the controller's caller is `HttpKernel`, not user code). The OIDC
+cause is preserved via `$previous` so logs still carry the underlying
+failure.
+
+This carve-out is scoped to `src/Controller/` and encoded in the custom
+PHPStan rule `ThrownExceptionImplementsBundleMarker` (see below).
+
+### Static-analysis lock-in (PR 3)
+
+Added to `require-dev`:
+
+- `phpstan/phpstan-strict-rules` — catches general exception-hygiene
+ issues (empty catches, throws from void methods).
+- `phpstan/phpstan-deprecation-rules` — flags any in-tree code still
+ using the deprecated abstract bases, so the team is reminded if a
+ future throw site picks up the wrong parent.
+
+Custom rules registered via `phpstan/exception-contract.neon`:
+
+- **`ThrownExceptionImplementsBundleMarker`** — every `throw new X(...)`
+ node in `src/` (excluding `src/Controller/`) must throw a class that
+ implements either bundle marker or library marker. The Controller
+ exception accepts any class implementing
+ `\Symfony\Component\HttpKernel\Exception\HttpExceptionInterface`.
+- **`WrappedExceptionChainsPrevious`** — every `throw new Y(...)` inside
+ a `catch (\Throwable $e)` block must pass `$e` as `$previous`. Escape
+ hatch via per-line `@phpstan-ignore` annotation with a
+ justification comment.
+
+The PHPStan `paths` setting is extended to include `tests/`, so
+`@throws` drift in test stubs and test exception expectations gets
+caught too.
+
+## Consequences
+
+### Positive
+
+- Consumers can write
+ `catch (OpenIdConnectBundleExceptionInterface $e)` (or the library
+ marker) to handle all OIDC failures with one block, instead of
+ enumerating concretes or catching `\Throwable`.
+- Generic retry / validator code that catches on `\RuntimeException` or
+ `\InvalidArgumentException` now matches the appropriate OIDC failures.
+ Today such code is invisible to bundle/library exceptions.
+- `$previous` chain becomes a tested invariant; log traversal across the
+ bundle → library → dependency boundary works without consumers having
+ to unwrap manually.
+- The contract is mechanically enforced from CI day one. A future throw
+ site that picks up a non-marker exception, or a future wrap site that
+ drops `$previous`, fails CI before review can miss it.
+- The controller HTTP-exception carve-out is explicit, documented, and
+ statically scoped — no longer an undeclared deviation.
+
+### Negative / Trade-offs
+
+- Two coordinated MAJOR releases. Consumers cannot install bundle 5.0
+ against library ≤ 4.x — the `composer.json` constraint enforces the
+ new contract.
+- Consumer catch-block migration: anyone catching
+ `ItkOpenIdConnectBundleException` or `ItkOpenIdConnectException` (the
+ abstract bases) must switch to the marker interface. Concrete class
+ names and namespaces stay stable so the migration is mechanical, not
+ semantic. Upgrade notes accompany the 5.0 release.
+- A `CacheException` re-parented to `\RuntimeException` may now match a
+ downstream `catch (\RuntimeException $e)` that previously did not.
+ Reviewed during planning: this is the desired semantic outcome, and is
+ called out in the upgrade notes.
+- ~200 lines of in-tree PHPStan rule code (`phpstan/Rule/`) plus rule
+ fixtures (`phpstan/tests/`) maintained alongside the production code.
+ Cost is one-time write + occasional adjustment when PHPStan's rule
+ API evolves.
+- The deprecated abstract base classes stay in the codebase through
+ 5.x; removal deferred to 6.0. Cost is two empty class files surviving
+ one major.
+
+### Follow-up Actions
+
+- [x] PR 0 — `$previous`-chain bug fixes
+ ([PR #35](https://github.com/itk-dev/openid-connect-bundle/pull/35))
+- [ ] PR 1 — upstream library 5.0
+ (`itk-dev/openid-connect`, separate repo)
+- [ ] PR 2 — bundle 5.0 contract migration + carve-out subsection added
+ to the project's contributor docs (depends on PR 1 tagged)
+- [ ] PR 3 — custom PHPStan rules (bundled inside PR 2)
+- [ ] `UPGRADE-5.0.md` (or equivalent README section) listing the
+ consumer `catch`-migration steps
+- [ ] Open follow-up to revisit removing the deprecated abstract base
+ classes in 6.0
+
+## References
+
+- PSR-18 HTTP Client — `ClientExceptionInterface` /
+ `NetworkExceptionInterface` / `RequestExceptionInterface`:
+
+- PSR-18 meta document — design rationale for not subtyping by failure
+ mode:
+- Symfony Security Core exception marker —
+ `Symfony\Component\Security\Core\Exception\ExceptionInterface`:
+
+- Symfony Console exception marker —
+ `Symfony\Component\Console\Exception\ExceptionInterface`:
+
+- Symfony `Psr18Client` — reference implementation of wrap-at-boundary
+ translating `TransportExceptionInterface` into PSR-18 exceptions
+ while preserving `$previous`:
+
+- Bloch, *Effective Java*, Item 73: "Throw exceptions appropriate to
+ the abstraction" — the principle behind wrap-at-boundary.
+- Keep a Changelog:
+- Semantic Versioning 2.0.0:
diff --git a/docs/adr/README.md b/docs/adr/README.md
new file mode 100644
index 0000000..d766d1a
--- /dev/null
+++ b/docs/adr/README.md
@@ -0,0 +1,14 @@
+# Architecture Decision Records
+
+This directory contains Architecture Decision Records (ADRs) for
+`itk-dev/openid-connect-bundle`.
+
+ADRs capture significant architectural or design decisions along with the
+context that drove them and the consequences we accept. See
+[adr.github.io](https://adr.github.io/) for background.
+
+## Index
+
+- **[001 — Adopt marker-interface exception hierarchy across library and
+ bundle](001-marker-interface-exception-hierarchy.md)** — Draft —
+ 2026-05-11
diff --git a/src/Controller/LoginController.php b/src/Controller/LoginController.php
index 576cb05..af536d8 100644
--- a/src/Controller/LoginController.php
+++ b/src/Controller/LoginController.php
@@ -9,6 +9,8 @@
use Symfony\Component\HttpFoundation\RedirectResponse;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Session\SessionInterface;
+use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
+use Symfony\Component\HttpKernel\Exception\ServiceUnavailableHttpException;
/**
* Login Controller class.
@@ -23,11 +25,18 @@ public function __construct(
/**
* Login method redirecting to authorizer.
*
- * @throws ItkOpenIdConnectException|InvalidProviderException
+ * @throws NotFoundHttpException Provider key not configured (404)
+ * @throws ServiceUnavailableHttpException IdP unreachable, returned a non-200, served malformed JSON, or local cache failed (503)
+ * @throws ItkOpenIdConnectException Other provider-init failures (e.g. BadUrlException for a misconfigured metadata_url) — server-side configuration bugs that intentionally bubble as 500
+ * @throws \InvalidArgumentException Declared by league\AbstractProvider::getAuthorizationUrl for missing scope/state. Unreachable in this flow (state always provided, getDefaultScopes() implemented in upstream OpenIdConfigurationProvider). Bubbles as 500 if it ever fires — programmer error.
*/
public function login(Request $request, SessionInterface $session, string $providerKey): RedirectResponse
{
- $provider = $this->providerManager->getProvider($providerKey);
+ try {
+ $provider = $this->providerManager->getProvider($providerKey);
+ } catch (InvalidProviderException $e) {
+ throw new NotFoundHttpException(sprintf('Unknown OIDC provider "%s"', $providerKey), $e);
+ }
$nonce = $provider->generateNonce();
$state = $provider->generateState();
@@ -37,12 +46,19 @@ public function login(Request $request, SessionInterface $session, string $provi
$session->set('oauth2state', $state);
$session->set('oauth2nonce', $nonce);
- $authUrl = $provider->getAuthorizationUrl([
- 'state' => $state,
- 'nonce' => $nonce,
- 'response_type' => 'code',
- 'scope' => 'openid email profile',
- ]);
+ try {
+ $authUrl = $provider->getAuthorizationUrl([
+ 'state' => $state,
+ 'nonce' => $nonce,
+ 'response_type' => 'code',
+ 'scope' => 'openid email profile',
+ ]);
+ } catch (ItkOpenIdConnectException $e) {
+ // Building the authorization URL fetches the IdP's discovery
+ // document. Surface upstream/transport/cache failures as 503 with
+ // the cause chained, rather than an unhandled 500.
+ throw new ServiceUnavailableHttpException(null, sprintf('Cannot reach OIDC provider "%s"', $providerKey), $e);
+ }
return new RedirectResponse($authUrl);
}
diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php
index a099b77..a6bd2a4 100644
--- a/src/DependencyInjection/Configuration.php
+++ b/src/DependencyInjection/Configuration.php
@@ -65,6 +65,10 @@ public function getConfigTreeBuilder(): TreeBuilder
->info('Leeway in seconds to account for clock skew between server and provider')
->defaultValue(10)
->end()
+ ->integerNode('cache_duration')
+ ->info('Cache duration in seconds for the OIDC discovery document and JWKS (default: 86400 — 24 hours)')
+ ->defaultValue(86400)
+ ->end()
->scalarNode('redirect_uri')
->info('Redirect URI registered at identity provider')
->cannotBeEmpty()
@@ -80,6 +84,24 @@ public function getConfigTreeBuilder(): TreeBuilder
->info('Whether to allow http or not (default: false)')
->defaultValue(false)
->end()
+ // Uses Guzzle under the hood through itk-dev/openid-connect -> league/oauth2-client -> guzzlehttp/guzzle
+ ->arrayNode('http_client_options')
+ ->info('Options forwarded to the underlying Guzzle HTTP client. league/oauth2-client only forwards: timeout, proxy, verify (verify is only consulted when proxy is set).')
+ ->children()
+ // @see https://docs.guzzlephp.org/en/stable/request-options.html#timeout
+ ->floatNode('timeout')
+ ->info('Total request timeout in seconds')
+ ->end()
+ // @see https://docs.guzzlephp.org/en/stable/request-options.html#proxy
+ ->scalarNode('proxy')
+ ->info('HTTP proxy URI')
+ ->end()
+ // @see https://docs.guzzlephp.org/en/stable/request-options.html#verify
+ ->booleanNode('verify')
+ ->info('Verify TLS certificates (only consulted by Guzzle when proxy is set)')
+ ->end()
+ ->end()
+ ->end()
->end()
->validate()
->ifTrue(static fn (array $v) => isset($v['redirect_uri'], $v['redirect_route']))
diff --git a/src/Security/OpenIdConfigurationProviderManager.php b/src/Security/OpenIdConfigurationProviderManager.php
index 589d950..b431210 100644
--- a/src/Security/OpenIdConfigurationProviderManager.php
+++ b/src/Security/OpenIdConfigurationProviderManager.php
@@ -24,7 +24,13 @@ class OpenIdConfigurationProviderManager
* redirect_route?: string,
* redirect_route_parameters?: array,
* leeway?: int,
+ * cache_duration?: int,
* allow_http?: bool,
+ * http_client_options?: array{
+ * timeout?: float,
+ * proxy?: string,
+ * verify?: bool,
+ * },
* }>,
* } $config
*/
@@ -74,10 +80,18 @@ public function getProvider(string $key): OpenIdConfigurationProvider
$providerOptions['leeway'] = $options['leeway'];
}
+ if (isset($options['cache_duration'])) {
+ $providerOptions['cacheDuration'] = $options['cache_duration'];
+ }
+
if (isset($options['allow_http'])) {
$providerOptions['allowHttp'] = $options['allow_http'];
}
+ if (!empty($options['http_client_options'])) {
+ $providerOptions += $options['http_client_options'];
+ }
+
$this->providers[$key] = new OpenIdConfigurationProvider($providerOptions);
}
diff --git a/src/Security/OpenIdLoginAuthenticator.php b/src/Security/OpenIdLoginAuthenticator.php
index 2379900..5df3488 100644
--- a/src/Security/OpenIdLoginAuthenticator.php
+++ b/src/Security/OpenIdLoginAuthenticator.php
@@ -72,7 +72,7 @@ protected function validateClaims(Request $request): array
// Authentication successful
} catch (ItkOpenIdConnectException $exception) {
// Handle failed authentication
- throw new ValidationException($exception->getMessage());
+ throw new ValidationException($exception->getMessage(), previous: $exception);
}
/** @var array $claimsArray */
diff --git a/src/Util/CliLoginHelper.php b/src/Util/CliLoginHelper.php
index 333dbfd..2c469c8 100644
--- a/src/Util/CliLoginHelper.php
+++ b/src/Util/CliLoginHelper.php
@@ -34,7 +34,7 @@ public function createToken(string $username): string
try {
$revCacheItem = $this->cache->getItem($encodedUsername);
} catch (InvalidArgumentException $e) {
- throw new CacheException($e->getMessage());
+ throw new CacheException($e->getMessage(), previous: $e);
}
if ($revCacheItem->isHit()) {
@@ -53,7 +53,7 @@ public function createToken(string $username): string
try {
$cacheItem = $this->cache->getItem($token);
} catch (InvalidArgumentException $e) {
- throw new CacheException($e->getMessage());
+ throw new CacheException($e->getMessage(), previous: $e);
}
$cacheItem->set($encodedUsername);
@@ -73,7 +73,7 @@ public function getUsername(string $token): ?string
try {
$usernameItem = $this->cache->getItem($token);
} catch (InvalidArgumentException $e) {
- throw new CacheException($e->getMessage());
+ throw new CacheException($e->getMessage(), previous: $e);
}
if (!$usernameItem->isHit()) {
@@ -91,7 +91,7 @@ public function getUsername(string $token): ?string
$this->cache->deleteItem($token);
$this->cache->deleteItem($username);
} catch (InvalidArgumentException $e) {
- throw new CacheException($e->getMessage());
+ throw new CacheException($e->getMessage(), previous: $e);
}
return $this->decodeKey($username);
diff --git a/tests/Controller/LoginControllerTest.php b/tests/Controller/LoginControllerTest.php
index 7305e74..a873b9c 100644
--- a/tests/Controller/LoginControllerTest.php
+++ b/tests/Controller/LoginControllerTest.php
@@ -2,23 +2,26 @@
namespace ItkDev\OpenIdConnectBundle\Tests\Controller;
+use ItkDev\OpenIdConnect\Exception\CacheException;
+use ItkDev\OpenIdConnect\Exception\HttpException;
+use ItkDev\OpenIdConnect\Exception\JsonException;
use ItkDev\OpenIdConnect\Security\OpenIdConfigurationProvider;
use ItkDev\OpenIdConnectBundle\Controller\LoginController;
+use ItkDev\OpenIdConnectBundle\Exception\InvalidProviderException;
use ItkDev\OpenIdConnectBundle\Security\OpenIdConfigurationProviderManager;
+use PHPUnit\Framework\Attributes\DataProvider;
use PHPUnit\Framework\TestCase;
use Symfony\Component\HttpFoundation\InputBag;
use Symfony\Component\HttpFoundation\RedirectResponse;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Session\SessionInterface;
+use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
+use Symfony\Component\HttpKernel\Exception\ServiceUnavailableHttpException;
class LoginControllerTest extends TestCase
{
- private LoginController $loginController;
-
- public function setUp(): void
+ public function testLogin(): void
{
- parent::setUp();
-
$mockProvider = $this->createMock(OpenIdConfigurationProvider::class);
$mockProvider
->expects($this->exactly(1))
@@ -34,18 +37,8 @@ public function setUp(): void
->with(['state' => 'abcd', 'nonce' => '1234', 'response_type' => 'code', 'scope' => 'openid email profile'])
->willReturn('https://test.com');
- $mockProviderManager = $this->createMock(OpenIdConfigurationProviderManager::class);
- $mockProviderManager
- ->expects($this->once())
- ->method('getProvider')
- ->with('test')
- ->willReturn($mockProvider);
+ $controller = $this->createController($mockProvider);
- $this->loginController = new LoginController($mockProviderManager);
- }
-
- public function testLogin(): void
- {
$stubRequest = $this->createStub(Request::class);
$stubRequest->query = new InputBag(['provider' => 'test']);
$mockSession = $this->createMock(SessionInterface::class);
@@ -67,8 +60,77 @@ public function testLogin(): void
}
});
- $response = $this->loginController->login($stubRequest, $mockSession, 'test');
+ $response = $controller->login($stubRequest, $mockSession, 'test');
$this->assertInstanceOf(RedirectResponse::class, $response);
$this->assertSame('https://test.com', $response->getTargetUrl());
}
+
+ public function testUnknownProviderKeyMapsTo404(): void
+ {
+ $cause = new InvalidProviderException('Invalid provider: bogus');
+
+ $mockProviderManager = $this->createMock(OpenIdConfigurationProviderManager::class);
+ $mockProviderManager
+ ->expects($this->once())
+ ->method('getProvider')
+ ->with('bogus')
+ ->willThrowException($cause);
+
+ $controller = new LoginController($mockProviderManager);
+
+ try {
+ $controller->login($this->createStub(Request::class), $this->createStub(SessionInterface::class), 'bogus');
+ } catch (NotFoundHttpException $thrown) {
+ $this->assertSame(404, $thrown->getStatusCode());
+ $this->assertStringContainsString('bogus', $thrown->getMessage());
+ $this->assertSame($cause, $thrown->getPrevious(), 'Original exception must be chained');
+
+ return;
+ }
+ $this->fail('Expected NotFoundHttpException');
+ }
+
+ /**
+ * @return iterable
+ */
+ public static function upstreamFailureProvider(): iterable
+ {
+ yield 'metadata HTTP timeout' => [new HttpException('Connection timed out')];
+ yield 'metadata malformed JSON' => [new JsonException('Syntax error')];
+ yield 'cache layer failure' => [new CacheException('Cache backend unreachable')];
+ }
+
+ #[DataProvider('upstreamFailureProvider')]
+ public function testUpstreamFailureMapsTo503(\Throwable $cause): void
+ {
+ $stubProvider = $this->createStub(OpenIdConfigurationProvider::class);
+ $stubProvider->method('generateNonce')->willReturn('1234');
+ $stubProvider->method('generateState')->willReturn('abcd');
+ $stubProvider->method('getAuthorizationUrl')->willThrowException($cause);
+
+ $controller = $this->createController($stubProvider);
+
+ try {
+ $controller->login($this->createStub(Request::class), $this->createStub(SessionInterface::class), 'test');
+ } catch (ServiceUnavailableHttpException $thrown) {
+ $this->assertSame(503, $thrown->getStatusCode());
+ $this->assertStringContainsString('test', $thrown->getMessage());
+ $this->assertSame($cause, $thrown->getPrevious(), 'Original exception must be chained');
+
+ return;
+ }
+ $this->fail('Expected ServiceUnavailableHttpException');
+ }
+
+ private function createController(OpenIdConfigurationProvider $provider): LoginController
+ {
+ $mockProviderManager = $this->createMock(OpenIdConfigurationProviderManager::class);
+ $mockProviderManager
+ ->expects($this->once())
+ ->method('getProvider')
+ ->with('test')
+ ->willReturn($provider);
+
+ return new LoginController($mockProviderManager);
+ }
}
diff --git a/tests/DependencyInjection/ConfigurationTest.php b/tests/DependencyInjection/ConfigurationTest.php
index 385ee00..b6ef9e0 100644
--- a/tests/DependencyInjection/ConfigurationTest.php
+++ b/tests/DependencyInjection/ConfigurationTest.php
@@ -56,6 +56,7 @@ public function testMinimalConfig(): void
$this->assertSame('my_id', $provider['client_id']);
$this->assertSame('my_secret', $provider['client_secret']);
$this->assertSame(10, $provider['leeway']);
+ $this->assertSame(86400, $provider['cache_duration']);
$this->assertFalse($provider['allow_http']);
}
@@ -64,6 +65,7 @@ public function testFullConfig(): void
$input = $this->getMinimalConfig();
$input['user_provider'] = 'my_user_provider';
$input['openid_providers']['provider1']['options']['leeway'] = 30;
+ $input['openid_providers']['provider1']['options']['cache_duration'] = 3600;
$input['openid_providers']['provider1']['options']['redirect_uri'] = 'https://app.com/callback';
$input['openid_providers']['provider1']['options']['allow_http'] = true;
@@ -76,6 +78,7 @@ public function testFullConfig(): void
$provider = $config['openid_providers']['provider1']['options'];
$this->assertSame(30, $provider['leeway']);
+ $this->assertSame(3600, $provider['cache_duration']);
$this->assertSame('https://app.com/callback', $provider['redirect_uri']);
$this->assertTrue($provider['allow_http']);
}
@@ -109,6 +112,55 @@ public function testBothRedirectUriAndRouteThrows(): void
);
}
+ public function testHttpClientOptionsAccepted(): void
+ {
+ $input = $this->getMinimalConfig();
+ $input['openid_providers']['provider1']['options']['http_client_options'] = [
+ 'timeout' => 2.5,
+ 'proxy' => 'http://proxy:8080',
+ 'verify' => true,
+ ];
+
+ $config = $this->processor->processConfiguration(
+ $this->configuration,
+ [$input]
+ );
+
+ $httpClientOptions = $config['openid_providers']['provider1']['options']['http_client_options'];
+ $this->assertSame(2.5, $httpClientOptions['timeout']);
+ $this->assertSame('http://proxy:8080', $httpClientOptions['proxy']);
+ $this->assertTrue($httpClientOptions['verify']);
+ }
+
+ public function testHttpClientOptionsAbsentByDefault(): void
+ {
+ $config = $this->processor->processConfiguration(
+ $this->configuration,
+ [$this->getMinimalConfig()]
+ );
+
+ $providerOptions = $config['openid_providers']['provider1']['options'];
+ // The block has no default value, so an omitted input must produce no
+ // http_client_options key in the processed config — otherwise an empty
+ // array would still be merged into the provider options.
+ $this->assertArrayNotHasKey('http_client_options', $providerOptions);
+ }
+
+ public function testHttpClientOptionsRejectsUnknownKey(): void
+ {
+ $input = $this->getMinimalConfig();
+ $input['openid_providers']['provider1']['options']['http_client_options'] = [
+ 'foo' => 1,
+ ];
+
+ $this->expectException(InvalidConfigurationException::class);
+
+ $this->processor->processConfiguration(
+ $this->configuration,
+ [$input]
+ );
+ }
+
public function testMultipleProviders(): void
{
$input = $this->getMinimalConfig();
diff --git a/tests/Security/OpenIdConfigurationProviderManagerTest.php b/tests/Security/OpenIdConfigurationProviderManagerTest.php
index 94c3837..10840cc 100644
--- a/tests/Security/OpenIdConfigurationProviderManagerTest.php
+++ b/tests/Security/OpenIdConfigurationProviderManagerTest.php
@@ -2,6 +2,7 @@
namespace ItkDev\OpenIdConnectBundle\Tests\Security;
+use GuzzleHttp\Client as GuzzleClient;
use ItkDev\OpenIdConnect\Security\OpenIdConfigurationProvider;
use ItkDev\OpenIdConnectBundle\Exception\InvalidProviderException;
use ItkDev\OpenIdConnectBundle\Security\OpenIdConfigurationProviderManager;
@@ -106,6 +107,19 @@ public function testGetProviderWithLeeway(): void
$this->assertInstanceOf(OpenIdConfigurationProvider::class, $provider);
}
+ public function testGetProviderWithCacheDuration(): void
+ {
+ $manager = $this->createManager([
+ 'test' => $this->getBaseProviderConfig() + [
+ 'redirect_uri' => 'https://app.com/callback',
+ 'cache_duration' => 3600,
+ ],
+ ]);
+
+ $provider = $manager->getProvider('test');
+ $this->assertInstanceOf(OpenIdConfigurationProvider::class, $provider);
+ }
+
public function testGetProviderWithAllowHttp(): void
{
$manager = $this->createManager([
@@ -119,6 +133,60 @@ public function testGetProviderWithAllowHttp(): void
$this->assertInstanceOf(OpenIdConfigurationProvider::class, $provider);
}
+ /**
+ * Read a Guzzle 7 client option.
+ *
+ * Guzzle's getConfig() carries a @deprecated tag for the planned v8 removal,
+ * but it remains the only public way to introspect a Client's effective
+ * config in v7 — which is what league/oauth2-client mandates. The tests
+ * below assert effective config, so we intentionally call the deprecated
+ * accessor and silence the single phpstan diagnostic it produces.
+ */
+ private function getGuzzleConfig(GuzzleClient $client, string $option): mixed
+ {
+ // @phpstan-ignore method.deprecated (see docblock above)
+ return $client->getConfig($option);
+ }
+
+ public function testGetProviderForwardsHttpClientOptions(): void
+ {
+ $manager = $this->createManager([
+ 'test' => $this->getBaseProviderConfig() + [
+ 'redirect_uri' => 'https://app.com/callback',
+ 'http_client_options' => [
+ 'timeout' => 1.5,
+ 'proxy' => 'http://proxy:8080',
+ 'verify' => false,
+ ],
+ ],
+ ]);
+
+ $provider = $manager->getProvider('test');
+ $httpClient = $provider->getHttpClient();
+
+ $this->assertInstanceOf(GuzzleClient::class, $httpClient);
+ $this->assertSame(1.5, $this->getGuzzleConfig($httpClient, 'timeout'));
+ $this->assertSame('http://proxy:8080', $this->getGuzzleConfig($httpClient, 'proxy'));
+ // verify is only forwarded by league when proxy is set.
+ $this->assertFalse($this->getGuzzleConfig($httpClient, 'verify'));
+ }
+
+ public function testGetProviderWithoutHttpClientOptionsLeavesGuzzleDefaults(): void
+ {
+ $manager = $this->createManager([
+ 'test' => $this->getBaseProviderConfig() + [
+ 'redirect_uri' => 'https://app.com/callback',
+ ],
+ ]);
+
+ $provider = $manager->getProvider('test');
+ $httpClient = $provider->getHttpClient();
+
+ $this->assertInstanceOf(GuzzleClient::class, $httpClient);
+ // No timeout configured ⇒ Guzzle's getConfig returns null. Asserts no leak from our pass-through.
+ $this->assertNull($this->getGuzzleConfig($httpClient, 'timeout'));
+ }
+
public function testGetProviderCachesInstance(): void
{
$manager = $this->createManager([
diff --git a/tests/Security/OpenIdLoginAuthenticatorTest.php b/tests/Security/OpenIdLoginAuthenticatorTest.php
index 238b468..29f0a02 100644
--- a/tests/Security/OpenIdLoginAuthenticatorTest.php
+++ b/tests/Security/OpenIdLoginAuthenticatorTest.php
@@ -87,8 +87,9 @@ public function testValidateClaimsMissingCode(): void
public function testValidateClaimsCodeDoesNotValidate(): void
{
+ $cause = new ClaimsException('test message');
$stubProvider = $this->createStub(OpenIdConfigurationProvider::class);
- $stubProvider->method('validateIdToken')->willThrowException(new ClaimsException('test message'));
+ $stubProvider->method('validateIdToken')->willThrowException($cause);
$this->stubProviderManager->method('getProvider')->willReturn($stubProvider);
$request = $this->createStub(Request::class);
@@ -96,9 +97,15 @@ public function testValidateClaimsCodeDoesNotValidate(): void
$this->setupStubSessionOnRequest($request);
- $this->expectException(ValidationException::class);
- $this->expectExceptionMessage('test message');
- $this->authenticator->authenticate($request);
+ try {
+ $this->authenticator->authenticate($request);
+ } catch (ValidationException $thrown) {
+ $this->assertSame('test message', $thrown->getMessage());
+ $this->assertSame($cause, $thrown->getPrevious(), 'Original cause must be chained');
+
+ return;
+ }
+ $this->fail('Expected ValidationException');
}
public function testValidateClaimsSuccess(): void
diff --git a/tests/Util/CliLoginHelperTest.php b/tests/Util/CliLoginHelperTest.php
index 8c05943..919b50c 100644
--- a/tests/Util/CliLoginHelperTest.php
+++ b/tests/Util/CliLoginHelperTest.php
@@ -83,16 +83,21 @@ public function testCreateTokenAndGetUsername(): void
public function testCreateTokenThrowsCacheExceptionOnGetItem(): void
{
+ $cause = new TestInvalidArgumentException('Cache error');
$stubCache = $this->createStub(CacheItemPoolInterface::class);
- $stubCache->method('getItem')
- ->willThrowException(new TestInvalidArgumentException('Cache error'));
+ $stubCache->method('getItem')->willThrowException($cause);
$cliHelper = new CliLoginHelper($stubCache);
- $this->expectException(CacheException::class);
- $this->expectExceptionMessage('Cache error');
+ try {
+ $cliHelper->createToken('test_user');
+ } catch (CacheException $thrown) {
+ $this->assertSame('Cache error', $thrown->getMessage());
+ $this->assertSame($cause, $thrown->getPrevious(), 'Original cause must be chained');
- $cliHelper->createToken('test_user');
+ return;
+ }
+ $this->fail('Expected CacheException');
}
public function testCreateTokenThrowsCacheExceptionOnSecondGetItem(): void
@@ -101,38 +106,49 @@ public function testCreateTokenThrowsCacheExceptionOnSecondGetItem(): void
$stubCacheItem->method('isHit')->willReturn(false);
$stubCacheItem->method('get')->willReturn(null);
+ $cause = new TestInvalidArgumentException('Second cache error');
$stubCache = $this->createStub(CacheItemPoolInterface::class);
$callCount = 0;
$stubCache->method('getItem')
- ->willReturnCallback(function () use ($stubCacheItem, &$callCount) {
+ ->willReturnCallback(function () use ($stubCacheItem, $cause, &$callCount) {
++$callCount;
if (1 === $callCount) {
return $stubCacheItem;
}
- throw new TestInvalidArgumentException('Second cache error');
+ throw $cause;
});
$stubCache->method('save')->willReturn(true);
$cliHelper = new CliLoginHelper($stubCache);
- $this->expectException(CacheException::class);
- $this->expectExceptionMessage('Second cache error');
+ try {
+ $cliHelper->createToken('another_user');
+ } catch (CacheException $thrown) {
+ $this->assertSame('Second cache error', $thrown->getMessage());
+ $this->assertSame($cause, $thrown->getPrevious(), 'Original cause must be chained');
- $cliHelper->createToken('another_user');
+ return;
+ }
+ $this->fail('Expected CacheException');
}
public function testGetUsernameThrowsCacheExceptionOnGetItem(): void
{
+ $cause = new TestInvalidArgumentException('Cache error');
$stubCache = $this->createStub(CacheItemPoolInterface::class);
- $stubCache->method('getItem')
- ->willThrowException(new TestInvalidArgumentException('Cache error'));
+ $stubCache->method('getItem')->willThrowException($cause);
$cliHelper = new CliLoginHelper($stubCache);
- $this->expectException(CacheException::class);
- $this->expectExceptionMessage('Cache error');
+ try {
+ $cliHelper->getUsername('some-token');
+ } catch (CacheException $thrown) {
+ $this->assertSame('Cache error', $thrown->getMessage());
+ $this->assertSame($cause, $thrown->getPrevious(), 'Original cause must be chained');
- $cliHelper->getUsername('some-token');
+ return;
+ }
+ $this->fail('Expected CacheException');
}
public function testCreateTokenThrowsCacheExceptionOnNonStringCachedToken(): void
@@ -175,16 +191,21 @@ public function testGetUsernameThrowsCacheExceptionOnDeleteItem(): void
$stubCacheItem->method('isHit')->willReturn(true);
$stubCacheItem->method('get')->willReturn('encoded_username');
+ $cause = new TestInvalidArgumentException('Delete error');
$stubCache = $this->createStub(CacheItemPoolInterface::class);
$stubCache->method('getItem')->willReturn($stubCacheItem);
- $stubCache->method('deleteItem')
- ->willThrowException(new TestInvalidArgumentException('Delete error'));
+ $stubCache->method('deleteItem')->willThrowException($cause);
$cliHelper = new CliLoginHelper($stubCache);
- $this->expectException(CacheException::class);
- $this->expectExceptionMessage('Delete error');
+ try {
+ $cliHelper->getUsername('some-token');
+ } catch (CacheException $thrown) {
+ $this->assertSame('Delete error', $thrown->getMessage());
+ $this->assertSame($cause, $thrown->getPrevious(), 'Original cause must be chained');
- $cliHelper->getUsername('some-token');
+ return;
+ }
+ $this->fail('Expected CacheException');
}
}