Skip to content

[Security] Log the access tokens rejected by OidcTokenHandler at the debug level - #66710

Merged
nicolas-grekas merged 1 commit into
symfony:8.2from
lazerg:fix/issue-66706-oidc-token-log-level
Oct 9, 2026
Merged

nicolas-grekas merged 1 commit into
symfony:8.2from
lazerg:fix/issue-66706-oidc-token-log-level

Conversation

@lazerg

@lazerg lazerg commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor
Q A
Branch? 8.2
Bug fix? no
New feature? no
Deprecations? no
Issues Fix #66706
License MIT

OidcTokenHandler logs every token it rejects at the error level, with the stack trace. An expired token, a wrong audience or a bad signature is a client mistake that ends in a 401, and AuthenticatorManager already logs it as "Authenticator failed." at the info level. With the Monolog recipe (fingers_crossed, action_level: error), each expired token flushes the whole log buffer of the request.

This logs these rejections at the debug level, as DpopSenderConstraint does for a refused proof. The same goes for a token that cannot be decrypted when encryption is required, since a client sending a plain token there is the same kind of mistake. Errors while fetching the discovery document or the JWKS keep the error level, since they happen in computeDiscoveryKeys(), outside of this catch block.

A wrong key or a stale JWKS also shows up as a bad signature, so after this change it is only visible at the debug level and as "Authenticator failed." at info. The handler cannot tell it apart from a forged token.

I sent this to 8.2 because it changes the log level of existing code, like #66391 and #53201.

The tests that expected one error call now expect none, and a new test covers a plain token sent when encryption is required. They fail on 8.2 and pass with the change. ./phpunit src/Symfony/Component/Security/Http: OK, 1523 tests, 15 skipped.

@lazerg

lazerg commented Oct 8, 2026

Copy link
Copy Markdown
Contributor Author

The red jobs fail on KeyManagement and the Doctrine bridge (new DBAL deprecations). The same jobs fail on the 8.2 push of the base commit 58a5249, so this change does not cause them.

@lazerg
lazerg force-pushed the fix/issue-66706-oidc-token-log-level branch from 6df996e to b437e90 Compare October 8, 2026 18:10
@nicolas-grekas

Copy link
Copy Markdown
Member

Thank you @lazerg.

@nicolas-grekas
nicolas-grekas merged commit c0d7f14 into symfony:8.2 Oct 9, 2026
13 checks passed
@snortlin

snortlin commented Oct 9, 2026

Copy link
Copy Markdown

Thanks for fixing this so quickly!

Would you consider backporting this to Symfony 7.4 LTS as a bug fix?

Expired access tokens are an expected authentication condition, not an application error. With the default Monolog fingers_crossed configuration, this causes unnecessary log buffer flushing in production.

Since the change only corrects the log severity without affecting authentication behavior or the public API, I think it could reasonably be considered a bug fix rather than a behavioral change.

That said, I understand the reasoning behind targeting 8.2, and I'll leave the final decision to the maintainers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Security] OidcTokenHandler logs expired access tokens as errors

4 participants