Repository navigation
[Security] Log the access tokens rejected by OidcTokenHandler at the debug level - #66710
Conversation
|
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. |
6df996e to
b437e90
Compare
b437e90 to
2d9e331
Compare
|
Thank you @lazerg. |
|
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 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. |
OidcTokenHandlerlogs every token it rejects at theerrorlevel, with the stack trace. An expired token, a wrong audience or a bad signature is a client mistake that ends in a 401, andAuthenticatorManageralready logs it as "Authenticator failed." at theinfolevel. 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
debuglevel, asDpopSenderConstraintdoes 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 theerrorlevel, since they happen incomputeDiscoveryKeys(), 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
debuglevel and as "Authenticator failed." atinfo. 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
errorcall 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.