Skip to content

Commit af57823

Browse files
authored
Merge pull request #41 from itk-dev/release/5.0.0
release: 5.0.0
2 parents 7964acb + 314b137 commit af57823

33 files changed

Lines changed: 843 additions & 93 deletions

.github/workflows/composer.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,4 +80,5 @@ jobs:
8080
docker network create frontend
8181
8282
- run: |
83+
docker compose run --rm phpfpm composer install
8384
docker compose run --rm phpfpm composer audit

.gitignore

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,3 +18,6 @@ var
1818
.php-cs-fixer.cache
1919
.phpunit.cache
2020
unit.xml
21+
22+
# Local Claude Code instructions (not part of the published package)
23+
/CLAUDE.md

.markdownlintignore

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,3 +10,5 @@ web/*.md
1010
web/core/
1111
web/libraries/
1212
web/*/contrib/
13+
# Claude Code guidance — verbose narrative prose, intentionally long lines.
14+
CLAUDE.md

CHANGELOG.md

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,59 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
## [5.0.0] - 2026-06-02
11+
12+
### Changed (BREAKING)
13+
14+
- **Exception hierarchy reworked.** Every exception thrown from a public
15+
method now implements `OpenIdConnectBundleExceptionInterface` (which
16+
extends `\ItkDev\OpenIdConnect\Exception\OpenIdConnectExceptionInterface`
17+
from the upstream library). Concrete exceptions now extend the SPL type
18+
that best describes the failure category (`\RuntimeException`,
19+
`\InvalidArgumentException`); they no longer extend
20+
`ItkOpenIdConnectBundleException`. Consumers catching the abstract base
21+
must migrate to `OpenIdConnectBundleExceptionInterface` — the abstract
22+
class is kept for this release as a documented alias and is
23+
`@deprecated`, but `catch (ItkOpenIdConnectBundleException $e)` blocks
24+
will no longer match any concrete thrown by the bundle.
25+
- Bumped `itk-dev/openid-connect` requirement to `^5.0` for the matching
26+
upstream contract.
27+
- `OpenIdLoginAuthenticator::validateClaims` now catches on the marker
28+
interface (`OpenIdConnectExceptionInterface`) instead of the deprecated
29+
upstream abstract. The `$previous`-chain behaviour is preserved.
30+
- `LoginController::login` catches on the marker interface before mapping
31+
to `ServiceUnavailableHttpException`. No consumer-visible behaviour
32+
change.
33+
34+
### Added
35+
36+
- `ItkDev\OpenIdConnectBundle\Exception\OpenIdConnectBundleExceptionInterface`
37+
marker for catching all bundle-thrown OIDC failures.
38+
- Custom PHPStan rules (`ThrownExceptionImplementsBundleMarker`,
39+
`WrappedExceptionChainsPrevious`) that lock the exception contract on every
40+
CI run — thrown exceptions must implement the marker (with documented
41+
controller/authenticator carve-outs), and wraps inside a catch must chain
42+
the caught cause as `$previous`.
43+
- `UPGRADE-5.0.md` migration guide for consumers.
44+
45+
### Changed
46+
47+
- Hardened static analysis. PHPStan now analyses `tests/` in addition to
48+
`src/`, runs the strict, deprecation, PHPUnit and Symfony rule packs, and
49+
requires a comment on every ignore (`reportIgnoresWithoutComments`). Pinned
50+
`phpstan/phpstan` to `^2.1.41`. No public-API or behavioural change.
51+
52+
### Deprecated
53+
54+
- `ItkDev\OpenIdConnectBundle\Exception\ItkOpenIdConnectBundleException`
55+
abstract class (catch `OpenIdConnectBundleExceptionInterface` instead).
56+
Will be removed in 6.0.
57+
58+
### Fixed
59+
60+
- Tests build real `Request` instances instead of stubbing `Request`, which
61+
fails under Symfony 8.1 (where `InputBag` is `final`) with recent PHPUnit.
62+
1063
## [4.2.0] - 2026-05-11
1164

1265
### Added
@@ -154,7 +207,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
154207
`itk-dev/openid-connect` 1.0.0 to 2.1.0
155208
- OpenId Connect Bundle: Added CLI login feature.
156209

157-
[unreleased]: https://github.com/itk-dev/openid-connect-bundle/compare/4.2.0...HEAD
210+
[unreleased]: https://github.com/itk-dev/openid-connect-bundle/compare/5.0.0...HEAD
211+
[5.0.0]: https://github.com/itk-dev/openid-connect-bundle/compare/4.2.0...5.0.0
158212
[4.2.0]: https://github.com/itk-dev/openid-connect-bundle/compare/4.1.0...4.2.0
159213
[4.1.0]: https://github.com/itk-dev/openid-connect-bundle/compare/4.0.1...4.1.0
160214
[4.0.1]: https://github.com/itk-dev/openid-connect-bundle/compare/4.0.0...4.0.1

UPGRADE-5.0.md

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
# Upgrading from 4.x to 5.0
2+
3+
5.0 reworks the bundle's exception hierarchy onto a marker interface and
4+
bumps the `itk-dev/openid-connect` requirement to `^5.0`. Runtime behaviour
5+
is unchanged — this document covers the consumer-visible API changes you'll
6+
need to adjust catch blocks for.
7+
8+
See the architecture decision in
9+
[docs/adr/001-marker-interface-exception-hierarchy.md](docs/adr/001-marker-interface-exception-hierarchy.md).
10+
11+
## Catch the marker interface, not the abstract
12+
13+
Concrete bundle exceptions no longer extend
14+
`\ItkDev\OpenIdConnectBundle\Exception\ItkOpenIdConnectBundleException`.
15+
Existing catches against the abstract will not match anything thrown by
16+
5.0+ code:
17+
18+
```diff
19+
- } catch (\ItkDev\OpenIdConnectBundle\Exception\ItkOpenIdConnectBundleException $e) {
20+
+ } catch (\ItkDev\OpenIdConnectBundle\Exception\OpenIdConnectBundleExceptionInterface $e) {
21+
```
22+
23+
The abstract class is kept through the 5.x line as a `@deprecated` alias
24+
that still implements the marker; removal is scheduled for 6.0.
25+
26+
`OpenIdConnectBundleExceptionInterface` **extends** the upstream library's
27+
`\ItkDev\OpenIdConnect\Exception\OpenIdConnectExceptionInterface`, so a
28+
single catch handles failures from both packages:
29+
30+
```php
31+
try {
32+
$claims = $this->validateClaims($request);
33+
} catch (\ItkDev\OpenIdConnect\Exception\OpenIdConnectExceptionInterface $e) {
34+
// catches both library- and bundle-thrown OIDC failures
35+
}
36+
```
37+
38+
## Catch on SPL types still works
39+
40+
Each concrete now extends the SPL type that best fits its failure category,
41+
so catches at the SPL level continue to match:
42+
43+
| Concrete | Parent | When it fires |
44+
| --- | --- | --- |
45+
| `InvalidProviderException` | `\InvalidArgumentException` | Unknown provider key requested from the manager |
46+
| `UsernameDoesNotExistException` | `\InvalidArgumentException` | CLI login resolved a token to no username |
47+
| `CacheException` | `\RuntimeException` | PSR-6 cache layer failed during CLI login token handling |
48+
| `TokenNotFoundException` | `\RuntimeException` | CLI login token not found in the cache |
49+
| `UserDoesNotExistException` | `\RuntimeException` | Configured user provider has no matching user |
50+
51+
For example, code catching the raw `\InvalidArgumentException` around a
52+
provider lookup keeps working, because `InvalidProviderException` still
53+
extends it:
54+
55+
```php
56+
try {
57+
$provider = $providerManager->getProvider($key);
58+
} catch (\InvalidArgumentException $e) { // still catches in 5.0
59+
// ...
60+
}
61+
```
62+
63+
## Update the dependency constraint
64+
65+
If your application pins the bundle, bump it to `^5.0`. The bundle now
66+
requires `itk-dev/openid-connect:^5.0`; review that library's
67+
[UPGRADE-5.0.md](https://github.com/itk-dev/openid-connect/blob/main/UPGRADE-5.0.md)
68+
for its own contract changes (it reworks the same exception hierarchy and
69+
tightens several IdP-payload validations).
70+
71+
## Framework-boundary exceptions are unchanged
72+
73+
`LoginController` still ships Symfony `HttpException` subclasses
74+
(`NotFoundHttpException` for an unknown provider → 404,
75+
`ServiceUnavailableHttpException` for an unreachable IdP / cache failure →
76+
503), and authenticators still throw `AuthenticationException`. These are a
77+
documented carve-out from the wrap-at-boundary rule and did not change in
78+
5.0; the OIDC cause remains chained via `$previous`.

composer.json

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
"ext-json": "*",
1919
"ext-openssl": "*",
2020
"doctrine/orm": "^2.8 || ^3.0",
21-
"itk-dev/openid-connect": "^4.0",
21+
"itk-dev/openid-connect": "^5.0",
2222
"symfony/cache": "^6.4 || ^7.0 || ^8.0",
2323
"symfony/framework-bundle": "^6.4.13 || ^7.0 || ^8.0",
2424
"symfony/security-bundle": "^6.4.13 || ^7.0 || ^8.0",
@@ -28,7 +28,11 @@
2828
"require-dev": {
2929
"ergebnis/composer-normalize": "^2.28",
3030
"friendsofphp/php-cs-fixer": "^3.11",
31-
"phpstan/phpstan": "^2.1",
31+
"phpstan/phpstan": "^2.1.41",
32+
"phpstan/phpstan-deprecation-rules": "^2.0",
33+
"phpstan/phpstan-phpunit": "^2.0",
34+
"phpstan/phpstan-strict-rules": "^2.0",
35+
"phpstan/phpstan-symfony": "^2.0",
3236
"phpunit/phpunit": "^12.0",
3337
"rector/rector": "^2.0",
3438
"symfony/runtime": "^6.4.13 || ^7.0 || ^8.0"
@@ -40,6 +44,7 @@
4044
},
4145
"autoload-dev": {
4246
"psr-4": {
47+
"ItkDev\\OpenIdConnectBundle\\PHPStan\\": "phpstan/",
4348
"ItkDev\\OpenIdConnectBundle\\Tests\\": "tests/"
4449
}
4550
},

phpstan.neon

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,65 @@
1+
includes:
2+
- vendor/phpstan/phpstan-strict-rules/rules.neon
3+
- vendor/phpstan/phpstan-deprecation-rules/rules.neon
4+
- vendor/phpstan/phpstan-phpunit/extension.neon
5+
- vendor/phpstan/phpstan-phpunit/rules.neon
6+
- vendor/phpstan/phpstan-symfony/extension.neon
7+
- vendor/phpstan/phpstan-symfony/rules.neon
8+
- phpstan/exception-contract.neon
9+
110
parameters:
211
level: max
312
paths:
413
- src
14+
- tests
15+
- phpstan/Rule
16+
reportIgnoresWithoutComments: true
17+
18+
ignoreErrors:
19+
# PHPUnit declares assertions as static methods on `Assert`, but the
20+
# pervasive idiom is `$this->assertX()`. phpstan-phpunit handles type
21+
# narrowing for these but doesn't override strict-rules' judgment that
22+
# this is a dynamic call to a static method.
23+
-
24+
identifier: staticMethod.dynamicCall
25+
path: tests/*
26+
27+
# Test fixture/helper PHPDoc completeness — out of scope for the static
28+
# analysis hardening; can be tightened as a follow-up.
29+
-
30+
identifier: missingType.generics
31+
path: tests/*
32+
-
33+
identifier: missingType.iterableValue
34+
path: tests/*
35+
-
36+
identifier: missingType.return
37+
path: tests/*
38+
39+
# Symfony's Processor returns an untyped array; phpstan-symfony narrows
40+
# many other Symfony APIs but does not type processConfiguration's result
41+
# from a Configuration definition. The tests probe specific keys precisely
42+
# *because* the production code does.
43+
-
44+
identifier: argument.type
45+
path: tests/DependencyInjection/ConfigurationTest.php
46+
-
47+
identifier: offsetAccess.nonOffsetAccessible
48+
path: tests/DependencyInjection/ConfigurationTest.php
49+
50+
# `validateClaims` throws `ValidationException`, but the throw happens behind
51+
# the abstract `authenticate()` the test fixture overrides, so PHPStan's
52+
# throw-type analysis can't see it reach the catch. The test deliberately
53+
# asserts the wrap behaviour by catching the concrete type.
54+
-
55+
identifier: catch.neverThrown
56+
path: tests/Security/OpenIdLoginAuthenticatorTest.php
57+
58+
# Manager tests assert `getProvider()` returns an `OpenIdConfigurationProvider`
59+
# as a "didn't throw on construction" smoke check. The return type already
60+
# guarantees the class, so phpstan-phpunit flags the assertion as redundant.
61+
# A follow-up should replace these with behavioural assertions on the
62+
# constructed provider; until then the assertion documents intent.
63+
-
64+
identifier: method.alreadyNarrowedType
65+
path: tests/Security/OpenIdConfigurationProviderManagerTest.php

phpstan/README.md

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
# PHPStan exception-contract rules
2+
3+
Two custom PHPStan rules lock the exception contract documented in
4+
[ADR 001](../docs/adr/001-marker-interface-exception-hierarchy.md). They run
5+
automatically as part of `task analyze:php` (and therefore `task pr:actions`).
6+
7+
## `ThrownExceptionImplementsBundleMarker`
8+
9+
Every literal `throw new X(...)` site under `src/` must throw an exception
10+
that implements:
11+
12+
- `ItkDev\OpenIdConnectBundle\Exception\OpenIdConnectBundleExceptionInterface`, or
13+
- `ItkDev\OpenIdConnect\Exception\OpenIdConnectExceptionInterface` (the upstream marker).
14+
15+
Two framework-boundary carve-outs apply, matching ADR 001:
16+
17+
| Path | Additionally allowed | Why |
18+
| ----------------------------------- | ----------------------------------------------------------------------- | ------------------------------------------------------------------ |
19+
| `src/Controller/*` | `Symfony\Component\HttpKernel\Exception\HttpExceptionInterface` | Symfony's kernel renders the HTTP status off these. |
20+
| `src/Security/*Authenticator.php` | `Symfony\Component\Security\Core\Exception\AuthenticationException` | Symfony Security catches these to render the auth-failure response.|
21+
22+
Escape hatch for an intentional outlier:
23+
24+
```php
25+
// @phpstan-ignore throw.notMarkerInterface
26+
throw new MyOddException(...);
27+
```
28+
29+
Pair the annotation with a one-line justification comment.
30+
31+
## `WrappedExceptionChainsPrevious`
32+
33+
Any `throw new Y(...)` inside a `catch (... $e)` block must pass `$e`
34+
as the new exception's `$previous`, either as a named argument
35+
(`previous: $e`) or as a direct positional argument value. This preserves
36+
`getPrevious()` traversal for logs and debugging — the lesson from the
37+
4.1 → 4.2 bug-fix PR that surfaced four `CliLoginHelper` wrap sites
38+
discarding the original cause.
39+
40+
The check is intentionally lenient: it accepts any positional argument
41+
whose value is the catch variable, not just the 3rd slot. This covers
42+
Symfony's HTTP exception subclasses (which bake the status into the
43+
class and place `$previous` at index 1) without needing constructor
44+
reflection.
45+
46+
Escape hatch when the rethrow is intentionally unrelated to the cause:
47+
48+
```php
49+
} catch (\Throwable $e) {
50+
// @phpstan-ignore throw.unchainedPrevious
51+
throw new UnrelatedDomainException('...');
52+
}
53+
```
54+
55+
Annotate with a justification.
56+
57+
## Verifying the rules
58+
59+
The rules are exercised on every CI run because PHPStan analyses `src/`
60+
and `tests/`. If a refactor produces a contract violation, the build
61+
breaks — which is the point. To verify a rule manually after editing,
62+
clear the cache and re-run:
63+
64+
```shell
65+
docker compose exec phpfpm vendor/bin/phpstan clear-result-cache
66+
task analyze:php
67+
```
68+
69+
## Why not php-cs-fixer
70+
71+
php-cs-fixer's rule model is syntactic. Exception-type enforcement is
72+
a type-level concern — what a thrown class extends or implements — and
73+
PHPStan has the reflection machinery to express it. Future work could
74+
add a php-cs-fixer rule for the narrower "no `throw new \Exception(…)`
75+
anywhere" check, but the inheritance graph belongs to PHPStan.

0 commit comments

Comments
 (0)