Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 90011f5

Browse filesBrowse files
vincentchalamonnicolas-grekas
vincentchalamon
authored and
nicolas-grekas
committed
[Security] Validate aud and iss claims on OidcTokenHandler
1 parent 2654e7b commit 90011f5
Copy full SHA for 90011f5

File tree

7 files changed

+48
-28
lines changed
Filter options

7 files changed

+48
-28
lines changed

‎src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/AccessToken/OidcTokenHandlerFactory.php

Copy file name to clipboardExpand all lines: src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/AccessToken/OidcTokenHandlerFactory.php
+13-4Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,9 @@ class OidcTokenHandlerFactory implements TokenHandlerFactoryInterface
2828
public function create(ContainerBuilder $container, string $id, array|string $config): void
2929
{
3030
$tokenHandlerDefinition = $container->setDefinition($id, (new ChildDefinition('security.access_token_handler.oidc'))
31+
->replaceArgument(2, $config['audience'])
32+
->replaceArgument(3, $config['issuers'])
3133
->replaceArgument(4, $config['claim'])
32-
->replaceArgument(5, $config['audience'])
3334
);
3435

3536
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
3940
->addError('You cannot use the "oidc" token handler since "web-token/jwt-core" is not installed. Try running "web-token/jwt-core".');
4041
}
4142

43+
// @see Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\SignatureAlgorithmFactory
44+
// for supported algorithms
4245
if (\in_array($config['algorithm'], ['ES256', 'ES384', 'ES512'], true)) {
4346
$tokenHandlerDefinition->replaceArgument(0, new Reference('security.access_token_handler.oidc.signature.'.$config['algorithm']));
4447
} else {
45-
$tokenHandlerDefinition->replaceArgument(0, (new ChildDefinition('security.access_token_handler.oidc.signature')))
46-
->replaceArgument(0, $config['algorithm']);
48+
$tokenHandlerDefinition->replaceArgument(0, (new ChildDefinition('security.access_token_handler.oidc.signature'))
49+
->replaceArgument(0, $config['algorithm'])
50+
);
4751
}
4852

4953
$tokenHandlerDefinition->replaceArgument(1, (new ChildDefinition('security.access_token_handler.oidc.jwk'))
@@ -68,7 +72,12 @@ public function addConfiguration(NodeBuilder $node): void
6872
->end()
6973
->scalarNode('audience')
7074
->info('Audience set in the token, for validation purpose.')
71-
->defaultNull()
75+
->isRequired()
76+
->end()
77+
->arrayNode('issuers')
78+
->info('Issuers allowed to generate the token, for validation purpose.')
79+
->isRequired()
80+
->prototype('scalar')->end()
7281
->end()
7382
->scalarNode('algorithm')
7483
->info('Algorithm used to sign the token.')

‎src/Symfony/Bundle/SecurityBundle/Resources/config/schema/security-1.0.xsd

Copy file name to clipboardExpand all lines: src/Symfony/Bundle/SecurityBundle/Resources/config/schema/security-1.0.xsd
+11-1Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -335,12 +335,22 @@
335335
</xsd:complexType>
336336

337337
<xsd:complexType name="oidc">
338+
<xsd:choice maxOccurs="unbounded">
339+
<xsd:element name="issuers" type="oidc_issuers" minOccurs="0" maxOccurs="1" />
340+
<xsd:element name="issuer" type="password_hasher" minOccurs="0" maxOccurs="unbounded" />
341+
</xsd:choice>
338342
<xsd:attribute name="claim" type="xsd:string" />
339-
<xsd:attribute name="audience" type="xsd:string" />
343+
<xsd:attribute name="audience" type="xsd:string" use="required" />
340344
<xsd:attribute name="algorithm" type="xsd:string" use="required" />
341345
<xsd:attribute name="key" type="xsd:string" use="required" />
342346
</xsd:complexType>
343347

348+
<xsd:complexType name="oidc_issuers">
349+
<xsd:sequence>
350+
<xsd:element name="issuer" type="xsd:string" minOccurs="1" maxOccurs="unbounded" />
351+
</xsd:sequence>
352+
</xsd:complexType>
353+
344354
<xsd:complexType name="login_throttling">
345355
<xsd:attribute name="limiter" type="xsd:string" />
346356
<xsd:attribute name="max-attempts" type="xsd:integer" />

‎src/Symfony/Bundle/SecurityBundle/Resources/config/security_authenticator_access_token.php

Copy file name to clipboardExpand all lines: src/Symfony/Bundle/SecurityBundle/Resources/config/security_authenticator_access_token.php
+3-2Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,10 +69,11 @@
6969
->args([
7070
abstract_arg('signature algorithm'),
7171
abstract_arg('signature key'),
72+
abstract_arg('audience'),
73+
abstract_arg('issuers'),
74+
'sub',
7275
service('logger')->nullOnInvalid(),
7376
service('clock'),
74-
'sub',
75-
null,
7677
])
7778

7879
->set('security.access_token_handler.oidc.jwk', JWK::class)

‎src/Symfony/Bundle/SecurityBundle/Tests/Functional/AccessTokenTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Bundle/SecurityBundle/Tests/Functional/AccessTokenTest.php
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -343,7 +343,7 @@ public function testOidcSuccess()
343343
'iat' => $time,
344344
'nbf' => $time,
345345
'exp' => $time + 3600,
346-
'iss' => 'https://www.example.com/',
346+
'iss' => 'https://www.example.com',
347347
'aud' => 'Symfony OIDC',
348348
'sub' => 'e21bf182-1538-406e-8ccb-e25a17aba39f',
349349
'username' => 'dunglas',

‎src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/AccessToken/config_oidc.yml

Copy file name to clipboardExpand all lines: src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/AccessToken/config_oidc.yml
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ security:
2323
oidc:
2424
claim: 'username'
2525
audience: 'Symfony OIDC'
26+
issuers: [ 'https://www.example.com' ]
2627
algorithm: 'ES256'
2728
# tip: use https://mkjwk.org/ to generate a JWK
2829
key: '{"kty":"EC","d":"iA_TV2zvftni_9aFAQwFO_9aypfJFCSpcCyevDvz220","crv":"P-256","x":"0QEAsI1wGI-dmYatdUZoWSRWggLEpyzopuhwk-YUnA4","y":"KYl-qyZ26HobuYwlQh-r0iHX61thfP82qqEku7i0woo"}'

‎src/Symfony/Component/Security/Http/AccessToken/Oidc/OidcTokenHandler.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Http/AccessToken/Oidc/OidcTokenHandler.php
+6-6Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,10 +41,11 @@ final class OidcTokenHandler implements AccessTokenHandlerInterface
4141
public function __construct(
4242
private Algorithm $signatureAlgorithm,
4343
private JWK $jwk,
44-
private ?LoggerInterface $logger = null,
45-
private ClockInterface $clock = new Clock(),
44+
private string $audience,
45+
private array $issuers,
4646
private string $claim = 'sub',
47-
private ?string $audience = null
47+
private ?LoggerInterface $logger = null,
48+
private ClockInterface $clock = new Clock()
4849
) {
4950
}
5051

@@ -80,10 +81,9 @@ public function getUserBadgeFrom(string $accessToken): UserBadge
8081
new Checker\IssuedAtChecker(0, false, $this->clock),
8182
new Checker\NotBeforeChecker(0, false, $this->clock),
8283
new Checker\ExpirationTimeChecker(0, false, $this->clock),
84+
new Checker\AudienceChecker($this->audience),
85+
new Checker\IssuerChecker($this->issuers),
8386
];
84-
if ($this->audience) {
85-
$checkers[] = new Checker\AudienceChecker($this->audience);
86-
}
8787
$claimCheckerManager = new ClaimCheckerManager($checkers);
8888
// if this check fails, an InvalidClaimException is thrown
8989
$claimCheckerManager->check($claims);

‎src/Symfony/Component/Security/Http/Tests/AccessToken/Oidc/OidcTokenHandlerTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Http/Tests/AccessToken/Oidc/OidcTokenHandlerTest.php
+13-14Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
use Jose\Component\Signature\Serializer\CompactSerializer;
1919
use PHPUnit\Framework\TestCase;
2020
use Psr\Log\LoggerInterface;
21-
use Symfony\Component\Clock\Clock;
2221
use Symfony\Component\Security\Core\Exception\BadCredentialsException;
2322
use Symfony\Component\Security\Core\User\OidcUser;
2423
use Symfony\Component\Security\Http\AccessToken\Oidc\OidcTokenHandler;
@@ -41,7 +40,7 @@ public function testGetsUserIdentifierFromSignedToken(string $claim, string $exp
4140
'iat' => $time,
4241
'nbf' => $time,
4342
'exp' => $time + 3600,
44-
'iss' => 'https://www.example.com/',
43+
'iss' => 'https://www.example.com',
4544
'aud' => self::AUDIENCE,
4645
'sub' => 'e21bf182-1538-406e-8ccb-e25a17aba39f',
4746
'email' => 'foo@example.com',
@@ -55,10 +54,10 @@ public function testGetsUserIdentifierFromSignedToken(string $claim, string $exp
5554
$userBadge = (new OidcTokenHandler(
5655
new ES256(),
5756
$this->getJWK(),
58-
$loggerMock,
59-
new Clock(),
57+
self::AUDIENCE,
58+
['https://www.example.com'],
6059
$claim,
61-
self::AUDIENCE
60+
$loggerMock,
6261
))->getUserBadgeFrom($token);
6362
$actualUser = $userBadge->getUserLoader()();
6463

@@ -89,10 +88,10 @@ public function testThrowsAnErrorIfTokenIsInvalid(string $token)
8988
(new OidcTokenHandler(
9089
new ES256(),
9190
$this->getJWK(),
92-
$loggerMock,
93-
new Clock(),
91+
self::AUDIENCE,
92+
['https://www.example.com'],
9493
'sub',
95-
self::AUDIENCE
94+
$loggerMock,
9695
))->getUserBadgeFrom($token);
9796
}
9897

@@ -106,7 +105,7 @@ public static function getInvalidTokens(): iterable
106105
'iat' => time() - 3600,
107106
'nbf' => time() - 3600,
108107
'exp' => time() - 3590,
109-
'iss' => 'https://www.example.com/',
108+
'iss' => 'https://www.example.com',
110109
'aud' => self::AUDIENCE,
111110
'sub' => 'e21bf182-1538-406e-8ccb-e25a17aba39f',
112111
'email' => 'foo@example.com',
@@ -118,7 +117,7 @@ public static function getInvalidTokens(): iterable
118117
'iat' => time(),
119118
'nbf' => time(),
120119
'exp' => time() + 3590,
121-
'iss' => 'https://www.example.com/',
120+
'iss' => 'https://www.example.com',
122121
'aud' => 'invalid',
123122
'sub' => 'e21bf182-1538-406e-8ccb-e25a17aba39f',
124123
'email' => 'foo@example.com',
@@ -139,7 +138,7 @@ public function testThrowsAnErrorIfUserPropertyIsMissing()
139138
'iat' => $time,
140139
'nbf' => $time,
141140
'exp' => $time + 3600,
142-
'iss' => 'https://www.example.com/',
141+
'iss' => 'https://www.example.com',
143142
'aud' => self::AUDIENCE,
144143
'sub' => 'e21bf182-1538-406e-8ccb-e25a17aba39f',
145144
];
@@ -148,10 +147,10 @@ public function testThrowsAnErrorIfUserPropertyIsMissing()
148147
(new OidcTokenHandler(
149148
new ES256(),
150149
self::getJWK(),
151-
$loggerMock,
152-
new Clock(),
150+
self::AUDIENCE,
151+
['https://www.example.com'],
153152
'email',
154-
self::AUDIENCE
153+
$loggerMock,
155154
))->getUserBadgeFrom($token);
156155
}
157156

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.