8000 [Security] Validate `aud` and `iss` claims on OidcTokenHandler by vincentchalamon · Pull Request #50432 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Security] Validate aud and iss claims on OidcTokenHandler #50432

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
May 30, 2023
Merged

[Security] Validate aud and iss claims on OidcTokenHandler #50432

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 13 additions & 4 deletions ...undle/SecurityBundle/DependencyInjection/Security/AccessToken/OidcTokenHandlerFactory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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'])) {
Expand All @@ -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'))
Expand All @@ -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.')
Expand Down
12 changes: 11 additions & 1 deletion src/Symfony/Bundle/SecurityBundle/Resources/config/schema/security-1.0.xsd
View file
Open in desktop
10000
Original file line numberDiff line number Diff line change
Expand Up @@ -335,12 +335,22 @@
</xsd:complexType>

<xsd:complexType name="oidc">
<xsd:choice maxOccurs="unbounded">
<xsd:element name="issuers" type="oidc_issuers" minOccurs="0" maxOccurs="1" />
<xsd:element name="issuer" type="password_hasher" minOccurs="0" maxOccurs="unbounded" />
</xsd:choice>
<xsd:attribute name="claim" type="xsd:string" />
<xsd:attribute name="audience" type="xsd:string" />
<xsd:attribute name="audience" type="xsd:string" use="required" />
<xsd:attribute name="algorithm" type="xsd:string" use="required" />
<xsd:attribute name="key" type="xsd:string" use="required" />
</xsd:complexType>

<xsd:complexType name="oidc_issuers">
<xsd:sequence>
<xsd:element name="issuer" type="xsd:string" minOccurs="1" maxOccurs="unbounded" />
</xsd:sequence>
</xsd:complexType>

<xsd:complexType name="login_throttling">
<xsd:attribute name="limiter" type="xsd:string" />
<xsd:attribute name="max-attempts" type="xsd:integer" />
Expand Down
5 changes: 3 additions & 2 deletions src/Symfony/Bundle/SecurityBundle/Resources/config/security_authenticator_access_token.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Bundle/SecurityBundle/Tests/Functional/AccessTokenTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
1 change: 1 addition & 0 deletions src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/AccessToken/config_oidc.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"}'
Expand Down
12 changes: 6 additions & 6 deletions src/Symfony/Component/Security/Http/AccessToken/Oidc/OidcTokenHandler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
) {
}

Expand Down Expand Up @@ -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);
Expand Down
27 changes: 13 additions & 14 deletions src/Symfony/Component/Security/Http/Tests/AccessToken/Oidc/OidcTokenHandlerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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',
Expand All @@ -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()();

Expand Down Expand Up @@ -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);
}

Expand All @@ -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',
Expand All @@ -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',
Expand All @@ -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',
];
Expand All @@ -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);
}

Expand Down
0