feat: handle provider error callbacks

[turegjorup]

A provider that refuses an authorization request no longer sends the user into a redirect loop.

First of three PRs toward 6.1. The next covers configurable scopes, min(0) on the duration options, InteractiveAuthenticatorInterface and a friendlier stateless-firewall failure; PKCE trails whenever itk-dev/openid-connect ships S256.

Changes

• supports() accepts a callback carrying state and either code or error. The path check is untouched, so ADR 003 holds and Constrain supports() to the configured redirect URI path #63 does not reopen.
• validateClaims() restructured around four ordering rules: all three one-time session values consumed up front; state checked before anything else in the URL is read; the error branch above the nonce and code steps; getProvider() moved below the state check.
• New ProviderErrorException, extending AuthenticationFailedException and implementing HttpExceptionInterface. Carries getError(), getErrorDescription(), getStatusCode() and an ACCESS_DENIED constant.
• onAuthenticationFailure() rethrows a provider error unwrapped; everything else is wrapped exactly as before.
• State compared with hash_equals(), behind an explicit guard against an empty or missing stored value.
• A private sanitizer for provider-supplied text: control-character runs collapsed, invalid UTF-8 dropped, capped at 200 characters.
• ADR 004, UPGRADE-6.1.md, README consumer section, changelog.

Why

RFC 6749 §4.1.2.1 gives an authorization request two answers. supports() only recognised one of them, so a refusal — cancelled consent screen, expired provider session, tenant policy — was not a callback at all. The firewall answered it, the entry point asked the provider again, and the provider refused again.

Captured in production against Azure AD B2C: dozens of rounds between /openidconnect/login/azure_az and /openid-connect/generic, the browser never settling. Nothing was logged, because no failing callback existed to log, and the one-time session values were never consumed, so each round replayed a session already half spent.

This is the ADR 002 outage shape reached through the one door that decision did not close. ADR 002 stopped a failing callback being retried; it did not help a request never recognised as a callback.

Decisions worth reviewing

• A refusal is a 4xx, not the ADR 002 500. access_denied means the user clicked Cancel; answering that with a 500 pages someone for a normal outcome. getStatusCode() maps refusals to 403, server_error/temporarily_unavailable to 503, everything else to 500. ADR 002 is unaffected either way — the security ExceptionListener keys on AuthenticationException, not on HTTP status, and the exception carries an empty cause chain.
• No PHPStan rule change was needed for that. ThrownExceptionImplementsBundleMarker returns [] as soon as the thrown class implements the bundle marker, before it consults the file path; the /src/Controller/ carve-out only adds HttpExceptionInterface where the marker is absent. A class implementing both passes unchanged. Flagging it because CLAUDE.md's prose reads as if the carve-out were exclusive.
• A new concrete type rather than reusing ValidationException. Nothing of ours failed validation. A consumer wants to tell "you cancelled" from "we could not verify that" — different page, different level — and the only alternative is matching on message text. It extends AuthenticationFailedException so every 6.0 catch keeps matching, which is what makes this a minor.
• supports() tests for the parameter's presence, not a usable value. ?state=…&error= has to be recognised: if it is not, the entry point mints a fresh state and the next refusal arrives with a state that matches, making the loop indistinguishable from a first attempt. testAnEmptyErrorIsStillACallback is the guard.
• getProvider() moved below the state check. A refusal has no use for discovery, an HTTP client and a cache pool. Behaviour change to note: a forged callback naming an unconfigured provider is now reported as an invalid state at warning rather than an unconfigured provider at error, so an anonymous caller can no longer produce error-level records on the callback path.
• Both README authenticator examples now chain the cause. They threw CustomUserMessageAuthenticationException($exception->getMessage()) with no previous, which discards the status and the type. The feature would not have worked for anyone following the documented shape.

Tests

331 tests, 838 assertions. New coverage:

• tests/Security/ProviderErrorCallbackDoesNotLoopTest.php — six kernel tests through a real firewall with the entry point wired. The load-bearing one asserts the Location header is null: a redirect there is the loop.
• tests/Security/OpenIdLoginAuthenticatorTest.php — the error shape in supports(), the refusal path, and the negative assertions that a forged state keeps the sender's text out of the log entirely.
• tests/Exception/ProviderErrorExceptionTest.php — status mapping including an unknown vendor code for the default arm.

Check	Result
PHPUnit	331 tests, 838 assertions, OK
PHPStan max	No errors
php-cs-fixer, markdownlint	Clean
Infection	450 killed, 1 escaped
Matrix, all 6 combinations	331 pass on each

Two caveats, both pre-existing and neither on this branch. The escaped mutant is in ItkDevOpenIdConnectExtension.php:114, untouched here, so the suite was not at 100% MSI before this either. And task test:matrix fails wholesale on my machine: composer's parallel extraction races in the matrix containers, and the Taskfile's install-then-test split across two docker compose run invocations does not survive it. I ran all six by doing install and phpunit in one invocation. 8.4-lowest and 8.5-lowest report 44 deprecations, the dev-tooling downgrade artefact the workflow already documents at .github/workflows/php.yaml:47.

Not done

No devops_itksites validation yet, unlike #66. Worth doing before merge given that is where the loop was captured — installing from disk and confirming a cancelled consent screen stops on a 403 instead of ping-ponging. Say if you want that before review or after.

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (482c4a7) to head (e1403fd).

Additional details and impacted files

@@             Coverage Diff             @@
##             develop       #76   +/-   ##
===========================================
  Coverage     100.00%   100.00%           
- Complexity       177       195   +18     
===========================================
  Files             14        15    +1     
  Lines            732       774   +42     
===========================================
+ Hits             732       774   +42     

Flag	Coverage Δ
unittests	100.00% <100.00%> (ø)

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

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.