From 90011f59584820465aa92b213a2a11531ccc20cf Mon Sep 17 00:00:00 2001
From: Vincent Chalamon <407859+vincentchalamon@users.noreply.github.com>
Date: Thu, 25 May 2023 15:13:40 +0200
Subject: [PATCH] [Security] Validate `aud` and `iss` claims on
OidcTokenHandler
---
.../AccessToken/OidcTokenHandlerFactory.php | 17 +++++++++---
.../Resources/config/schema/security-1.0.xsd | 12 ++++++++-
.../security_authenticator_access_token.php | 5 ++--
.../Tests/Functional/AccessTokenTest.php | 2 +-
.../app/AccessToken/config_oidc.yml | 1 +
.../AccessToken/Oidc/OidcTokenHandler.php | 12 ++++-----
.../AccessToken/Oidc/OidcTokenHandlerTest.php | 27 +++++++++----------
7 files changed, 48 insertions(+), 28 deletions(-)
diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/AccessToken/OidcTokenHandlerFactory.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/AccessToken/OidcTokenHandlerFactory.php
index 86d2783cb8d78..18a5ee6ac3eef 100644
--- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/AccessToken/OidcTokenHandlerFactory.php
+++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/AccessToken/OidcTokenHandlerFactory.php
@@ -28,8 +28,9 @@ class OidcTokenHandlerFactory implements TokenHandlerFactoryInterface
public function create(ContainerBuilder $container, string $id, array|string $config): void
{
$tokenHandlerDefinition = $container->setDefinition($id, (new ChildDefinition('security.access_token_handler.oidc'))
+ ->replaceArgument(2, $config['audience'])
+ ->replaceArgument(3, $config['issuers'])
->replaceArgument(4, $config['claim'])
- ->replaceArgument(5, $config['audience'])
);
if (!ContainerBuilder::willBeAvailable('web-token/jwt-core', Algorithm::class, ['symfony/security-bundle'])) {
@@ -39,11 +40,14 @@ public function create(ContainerBuilder $container, string $id, array|string $co
->addError('You cannot use the "oidc" token handler since "web-token/jwt-core" is not installed. Try running "web-token/jwt-core".');
}
+ // @see Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\SignatureAlgorithmFactory
+ // for supported algorithms
if (\in_array($config['algorithm'], ['ES256', 'ES384', 'ES512'], true)) {
$tokenHandlerDefinition->replaceArgument(0, new Reference('security.access_token_handler.oidc.signature.'.$config['algorithm']));
} else {
- $tokenHandlerDefinition->replaceArgument(0, (new ChildDefinition('security.access_token_handler.oidc.signature')))
- ->replaceArgument(0, $config['algorithm']);
+ $tokenHandlerDefinition->replaceArgument(0, (new ChildDefinition('security.access_token_handler.oidc.signature'))
+ ->replaceArgument(0, $config['algorithm'])
+ );
}
$tokenHandlerDefinition->replaceArgument(1, (new ChildDefinition('security.access_token_handler.oidc.jwk'))
@@ -68,7 +72,12 @@ public function addConfiguration(NodeBuilder $node): void
->end()
->scalarNode('audience')
->info('Audience set in the token, for validation purpose.')
- ->defaultNull()
+ ->isRequired()
+ ->end()
+ ->arrayNode('issuers')
+ ->info('Issuers allowed to generate the token, for validation purpose.')
+ ->isRequired()
+ ->prototype('scalar')->end()
->end()
->scalarNode('algorithm')
->info('Algorithm used to sign the token.')
diff --git a/src/Symfony/Bundle/SecurityBundle/Resources/config/schema/security-1.0.xsd b/src/Symfony/Bundle/SecurityBundle/Resources/config/schema/security-1.0.xsd
index 7798be2a28632..ab90f4b42606c 100644
--- a/src/Symfony/Bundle/SecurityBundle/Resources/config/schema/security-1.0.xsd
+++ b/src/Symfony/Bundle/SecurityBundle/Resources/config/schema/security-1.0.xsd
@@ -335,12 +335,22 @@
+
+
+
+
-
+
+
+
+
+
+
+
diff --git a/src/Symfony/Bundle/SecurityBundle/Resources/config/security_authenticator_access_token.php b/src/Symfony/Bundle/SecurityBundle/Resources/config/security_authenticator_access_token.php
index 32eac0e580c57..3254f332aace0 100644
--- a/src/Symfony/Bundle/SecurityBundle/Resources/config/security_authenticator_access_token.php
+++ b/src/Symfony/Bundle/SecurityBundle/Resources/config/security_authenticator_access_token.php
@@ -69,10 +69,11 @@
->args([
abstract_arg('signature algorithm'),
abstract_arg('signature key'),
+ abstract_arg('audience'),
+ abstract_arg('issuers'),
+ 'sub',
service('logger')->nullOnInvalid(),
service('clock'),
- 'sub',
- null,
])
->set('security.access_token_handler.oidc.jwk', JWK::class)
diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/AccessTokenTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/AccessTokenTest.php
index 8ca49ccc1430c..3deb91402165e 100644
--- a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/AccessTokenTest.php
+++ b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/AccessTokenTest.php
@@ -343,7 +343,7 @@ public function testOidcSuccess()
'iat' => $time,
'nbf' => $time,
'exp' => $time + 3600,
- 'iss' => 'https://www.example.com/',
+ 'iss' => 'https://www.example.com',
'aud' => 'Symfony OIDC',
'sub' => 'e21bf182-1538-406e-8ccb-e25a17aba39f',
'username' => 'dunglas',
diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/AccessToken/config_oidc.yml b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/AccessToken/config_oidc.yml
index 920bc81f1e788..dd770e4520e41 100644
--- a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/AccessToken/config_oidc.yml
+++ b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/AccessToken/config_oidc.yml
@@ -23,6 +23,7 @@ security:
oidc:
claim: 'username'
audience: 'Symfony OIDC'
+ issuers: [ 'https://www.example.com' ]
algorithm: 'ES256'
# tip: use https://mkjwk.org/ to generate a JWK
key: '{"kty":"EC","d":"iA_TV2zvftni_9aFAQwFO_9aypfJFCSpcCyevDvz220","crv":"P-256","x":"0QEAsI1wGI-dmYatdUZoWSRWggLEpyzopuhwk-YUnA4","y":"KYl-qyZ26HobuYwlQh-r0iHX61thfP82qqEku7i0woo"}'
diff --git a/src/Symfony/Component/Security/Http/AccessToken/Oidc/OidcTokenHandler.php b/src/Symfony/Component/Security/Http/AccessToken/Oidc/OidcTokenHandler.php
index c103236ed2021..0623a60016a0d 100644
--- a/src/Symfony/Component/Security/Http/AccessToken/Oidc/OidcTokenHandler.php
+++ b/src/Symfony/Component/Security/Http/AccessToken/Oidc/OidcTokenHandler.php
@@ -41,10 +41,11 @@ final class OidcTokenHandler implements AccessTokenHandlerInterface
public function __construct(
private Algorithm $signatureAlgorithm,
private JWK $jwk,
- private ?LoggerInterface $logger = null,
- private ClockInterface $clock = new Clock(),
+ private string $audience,
+ private array $issuers,
private string $claim = 'sub',
- private ?string $audience = null
+ private ?LoggerInterface $logger = null,
+ private ClockInterface $clock = new Clock()
) {
}
@@ -80,10 +81,9 @@ public function getUserBadgeFrom(string $accessToken): UserBadge
new Checker\IssuedAtChecker(0, false, $this->clock),
new Checker\NotBeforeChecker(0, false, $this->clock),
new Checker\ExpirationTimeChecker(0, false, $this->clock),
+ new Checker\AudienceChecker($this->audience),
+ new Checker\IssuerChecker($this->issuers),
];
- if ($this->audience) {
- $checkers[] = new Checker\AudienceChecker($this->audience);
- }
$claimCheckerManager = new ClaimCheckerManager($checkers);
// if this check fails, an InvalidClaimException is thrown
$claimCheckerManager->check($claims);
diff --git a/src/Symfony/Component/Security/Http/Tests/AccessToken/Oidc/OidcTokenHandlerTest.php b/src/Symfony/Component/Security/Http/Tests/AccessToken/Oidc/OidcTokenHandlerTest.php
index 2d35bfb8aede3..8c8d86284b59a 100644
--- a/src/Symfony/Component/Security/Http/Tests/AccessToken/Oidc/OidcTokenHandlerTest.php
+++ b/src/Symfony/Component/Security/Http/Tests/AccessToken/Oidc/OidcTokenHandlerTest.php
@@ -18,7 +18,6 @@
use Jose\Component\Signature\Serializer\CompactSerializer;
use PHPUnit\Framework\TestCase;
use Psr\Log\LoggerInterface;
-use Symfony\Component\Clock\Clock;
use Symfony\Component\Security\Core\Exception\BadCredentialsException;
use Symfony\Component\Security\Core\User\OidcUser;
use Symfony\Component\Security\Http\AccessToken\Oidc\OidcTokenHandler;
@@ -41,7 +40,7 @@ public function testGetsUserIdentifierFromSignedToken(string $claim, string $exp
'iat' => $time,
'nbf' => $time,
'exp' => $time + 3600,
- 'iss' => 'https://www.example.com/',
+ 'iss' => 'https://www.example.com',
'aud' => self::AUDIENCE,
'sub' => 'e21bf182-1538-406e-8ccb-e25a17aba39f',
'email' => 'foo@example.com',
@@ -55,10 +54,10 @@ public function testGetsUserIdentifierFromSignedToken(string $claim, string $exp
$userBadge = (new OidcTokenHandler(
new ES256(),
$this->getJWK(),
- $loggerMock,
- new Clock(),
+ self::AUDIENCE,
+ ['https://www.example.com'],
$claim,
- self::AUDIENCE
+ $loggerMock,
))->getUserBadgeFrom($token);
$actualUser = $userBadge->getUserLoader()();
@@ -89,10 +88,10 @@ public function testThrowsAnErrorIfTokenIsInvalid(string $token)
(new OidcTokenHandler(
new ES256(),
$this->getJWK(),
- $loggerMock,
- new Clock(),
+ self::AUDIENCE,
+ ['https://www.example.com'],
'sub',
- self::AUDIENCE
+ $loggerMock,
))->getUserBadgeFrom($token);
}
@@ -106,7 +105,7 @@ public static function getInvalidTokens(): iterable
'iat' => time() - 3600,
'nbf' => time() - 3600,
'exp' => time() - 3590,
- 'iss' => 'https://www.example.com/',
+ 'iss' => 'https://www.example.com',
'aud' => self::AUDIENCE,
'sub' => 'e21bf182-1538-406e-8ccb-e25a17aba39f',
'email' => 'foo@example.com',
@@ -118,7 +117,7 @@ public static function getInvalidTokens(): iterable
'iat' => time(),
'nbf' => time(),
'exp' => time() + 3590,
- 'iss' => 'https://www.example.com/',
+ 'iss' => 'https://www.example.com',
'aud' => 'invalid',
'sub' => 'e21bf182-1538-406e-8ccb-e25a17aba39f',
'email' => 'foo@example.com',
@@ -139,7 +138,7 @@ public function testThrowsAnErrorIfUserPropertyIsMissing()
'iat' => $time,
'nbf' => $time,
'exp' => $time + 3600,
- 'iss' => 'https://www.example.com/',
+ 'iss' => 'https://www.example.com',
'aud' => self::AUDIENCE,
'sub' => 'e21bf182-1538-406e-8ccb-e25a17aba39f',
];
@@ -148,10 +147,10 @@ public function testThrowsAnErrorIfUserPropertyIsMissing()
(new OidcTokenHandler(
new ES256(),
self::getJWK(),
- $loggerMock,
- new Clock(),
+ self::AUDIENCE,
+ ['https://www.example.com'],
'email',
- self::AUDIENCE
+ $loggerMock,
))->getUserBadgeFrom($token);
}