Release 6.1.0 - #82
Merged
Merged
Conversation
RFC 6749 §4.1.2.1: a provider that refuses redirects back with `error` and `state` and no `code`. supports() required a code, so that request was not a callback: the firewall answered it, the entry point asked again, and the provider refused again. Captured in production against Azure AD B2C as dozens of rounds, nothing logged, the one-time session values never spent. supports() now accepts `state` with either `code` or `error`, on the callback path only, so ADR 003 and issue #63 are untouched. validateClaims() consumes all three one-time values up front, checks state before reading anything else the URL carries, and throws ProviderErrorException with the provider's error code as an accessor. ProviderErrorException extends AuthenticationFailedException, so existing catches keep matching, and implements HttpExceptionInterface so a refusal is a 403 rather than a 500. onAuthenticationFailure() rethrows it unwrapped. State is now compared with hash_equals(), behind an explicit guard against an empty or missing stored value. See ADR 004.
…lbacks feat: handle provider error callbacks
Wires RFC 7636 S256 PKCE through the login flow on itk-dev/openid-connect 5.1. The login route generates a verifier, keeps it in the session, and sends only its challenge; the authenticator redeems the code with it. On by default — RFC 6749 §3.1 makes an authorization server ignore parameters it does not know — with `pkce: false` per provider for one that rejects them instead. The verifier joins the one-time session values consumed on every callback, so it cannot be redeemed against a code it does not belong to. getProvider() now builds a fresh provider each call. AbstractProvider assigns $this->state on every getAuthorizationUrl(), so a memoized instance carried one request's state into the next — nothing reads it today, but that stops being true the moment a process outlives a request. The HTTP client is cached per provider instead, so the connection pool still survives; it is built with league's own option filter, which keeps `verify` reachable only alongside a proxy.
feat: send a PKCE challenge, and stop holding providers between requests
Scopes are a per-provider option, defaulting to the openid/email/profile the bundle has always asked for. A list or a space-separated string, so the value can come from an environment variable. `openid` is required: OIDC Core §3.1.2.1 defines an authentication request as one that asks for it, and without it the provider returns no ID token. `leeway` and `cache_duration` reject a negative value at compile time. A negative leeway used to fail at the first login that needed it; a negative cache duration passed through unnoticed. The authenticator implements InteractiveAuthenticatorInterface, so a completed login dispatches security.interactive_login and remember-me treats the token as one a user asked for. A firewall declared `stateless: true` now throws StatelessFirewallException naming the setting, rather than surfacing Symfony's SessionNotFoundException as an unexplained 500.
feat: configurable scopes, config bounds, and two clearer failures
docs: state the bundle's worker-mode contract
Igor reports shared mutable state, which is not the same thing as a leak. The four values it finds here are shared on purpose — Guzzle clients and their connection pools, the derived callback paths, the authenticator's logger, and Symfony's own lazy init of the bundle extension — and igor-baseline.json records each with a written reason. What the gate protects is the difference: state that appears without one. That is the case worth catching. The provider memoization removed in the PKCE work was exactly this shape, and nothing but review would have stopped it coming back. IGOR_VERSION pins the binary the composer bootstrapper fetches, in the Taskfile and the workflow. Igor is pre-1.0 and its rules move between releases, which would move the baseline underneath us.
ci: gate worker-mode state leaks with Igor
docs: developing against a mock identity provider
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #82 +/- ##
=============================================
- Coverage 100.00% 99.52% -0.48%
- Complexity 177 205 +28
=============================================
Files 14 15 +1
Lines 732 834 +102
=============================================
+ Hits 732 830 +98
- Misses 0 4 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Collapse the unreleased entries into a 6.1.0 section stating what changed, and add the compare link. Rationale, measurements and history stay in the commits and PRs where they belong.
turegjorup
force-pushed
the
release/6.1.0
branch
from
August 26, 2026 14:20
1e6a3c8 to
1577a2f
Compare
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
2 participants
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.
Hardening release. Everything here is additive or a fix; nothing is required of a consumer.
See UPGRADE-6.1.md and the changelog.
What is in it
A refused login no longer loops. RFC 6749 §4.1.2.1 sends a refusal back as
errorwith nocode, whichsupports()did not recognise as a callback: the firewall answered it, the entry point asked again, and the provider refused again. Captured in production against Azure AD B2C as dozens of rounds with nothing logged. Now it ends in a page that says so, at a status matching the cause — 403 where the user declined, 503 where the provider reports its own trouble, 500 otherwise. ADR 004.PKCE (RFC 7636, S256), on by default. Verified against the real Azure AD B2C tenant on the
devops_itksites6.0 branch:code_challenge_method=S256, 43-char challenge matching S256 of the 128-char verifier, and the verifier never leaving the session.Configurable scopes, defaulting to what the bundle has always asked for, accepting a space-separated string so the value can come from an environment variable, and rejecting a list without
openidat compile time.Worker-mode readiness.
getProvider()no longer memoizes —league/oauth2-clientwrites the authorization request'sstateonto the provider, so a held instance carried one request's state into the next. The Guzzle client is kept per provider instead, so connection reuse survives. CI gates this with Igor against a baseline that gives a written reason for each value the bundle shares on purpose.Plus
hash_equals()on the state comparison behind an explicit guard,min(0)onleewayandcache_duration,InteractiveAuthenticatorInterface, andStatelessFirewallExceptionnaming a firewall that can never complete a login.Requires
itk-dev/openid-connect^5.1That release enforces
allowHttpon every endpoint the discovery document announces rather than only the metadata URL, requiresexpandiaton the ID token, and changes the JWKS cache key. Read its changelog before deploying — a deployment talking to a plain-http provider withoutallow_httpwill stop working.Verified
The one escaped mutant is the long-standing one in
ItkDevOpenIdConnectExtension.php:114, untouched by this release.Known, and not in this release
authenticate()does not chain the bundle exception into theAuthenticationExceptionit raises loses the 403:causeOutsideSecurity()finds nothing and the refusal arrives as a plainAuthenticationFailedException. The README and upgrade guide both say to passprevious:.devops_itksitesis one such consumer today — raised there rather than worked around here.OpenIdConfigurationProvider::generateState()in the library still assigns$this->state. Harmless now that the bundle holds no provider, but a consumer holding one inherits it. Worth an issue upstream.