-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Security] Add clock dependency to OidcTokenHandler #50477
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
use Psr\Log\LoggerInterface; | ||
use Symfony\Component\Clock\NativeClock; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NativeClock should never be used directly since 6.3
1eb6720
to
2349d1b
Compare
->setArguments([$config['signature']['algorithm']]); | ||
$container->register('security.access_token_handler.oidc.jwk', JWK::class) | ||
->setFactory([JWK::class, 'createFromJson']) | ||
->setArguments([$config['signature']['key']]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This fixes the wiring. Before this change, all firewalls would share a single jwk+algo definition, since varying key/algo were registered under the same id
->isRequired() | ||
->children() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removing one unneeded nesting level in the config /cc @vincentchalamon for the doc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -49,7 +49,7 @@ public function getUserBadgeFrom(string $accessToken): UserBadge | |||
|
|||
// UserLoader argument can be overridden by a UserProvider on AccessTokenAuthenticator::authenticate | |||
return new UserBadge($claims[$this->claim], fn () => $this->createUser($claims), $claims); | |||
} catch (\Throwable $e) { | |||
} catch (\Exception $e) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should never catch Throwable
unless reasons which should be explicit/obvious
2349d1b
to
8bee7b2
Compare
From "web-token/jwt-checker": The parameter "$clock" will become mandatory in 4.0.0. Please set a valid PSR Clock implementation instead of "null".
8bee7b2
to
5394130
Compare
From "web-token/jwt-checker":
The parameter "$clock" will become mandatory in 4.0.0. Please set a valid PSR Clock implementation instead of "null".
Also fixing some other issues found meanwhile.