Skip to content

Commit 23c9e17

Browse files
vincentchalamonnicolas-grekas
vincentchalamon
authored and
nicolas-grekas
committed
[SecurityBundle] Fix configuring OIDC user info token handler client
1 parent 5bab536 commit 23c9e17

File tree

4 files changed

+68
-32
lines changed

4 files changed

+68
-32
lines changed

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

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
use Symfony\Component\DependencyInjection\ChildDefinition;
1616
use Symfony\Component\DependencyInjection\ContainerBuilder;
1717
use Symfony\Component\DependencyInjection\Reference;
18-
use Symfony\Component\HttpClient\HttpClient;
18+
use Symfony\Contracts\HttpClient\HttpClientInterface;
1919

2020
/**
2121
* Configures a token handler for an OIDC server.
@@ -26,24 +26,20 @@ class OidcUserInfoTokenHandlerFactory implements TokenHandlerFactoryInterface
2626
{
2727
public function create(ContainerBuilder $container, string $id, array|string $config): void
2828
{
29-
$tokenHandlerDefinition = $container->setDefinition($id, new ChildDefinition('security.access_token_handler.oidc_user_info'));
30-
$tokenHandlerDefinition->replaceArgument(2, $config['claim']);
29+
$clientDefinition = (new ChildDefinition('security.access_token_handler.oidc_user_info.http_client'))
30+
->replaceArgument(0, ['base_uri' => $config['base_uri']]);
3131

32-
// Create the client service
33-
if (!isset($config['client']['id'])) {
34-
$clientDefinitionId = 'http_client.security.access_token_handler.oidc_user_info';
35-
if (!ContainerBuilder::willBeAvailable('symfony/http-client', HttpClient::class, ['symfony/security-bundle'])) {
36-
$container->register($clientDefinitionId, 'stdClass')
37-
->addError('You cannot use the "oidc_user_info" token handler since the HttpClient component is not installed. Try running "composer require symfony/http-client".');
38-
} else {
39-
$container->register($clientDefinitionId, HttpClient::class)
40-
->setFactory([HttpClient::class, 'create'])
41-
->setArguments([$config['client']])
42-
->addTag('http_client.client');
43-
}
32+
if (isset($config['client'])) {
33+
$clientDefinition->setFactory([new Reference($config['client']), 'withOptions']);
34+
} elseif (!ContainerBuilder::willBeAvailable('symfony/http-client', HttpClientInterface::class, ['symfony/security-bundle'])) {
35+
$clientDefinition
36+
->setFactory(null)
37+
->addError('You cannot use the "oidc_user_info" token handler since the HttpClient component is not installed. Try running "composer require symfony/http-client".');
4438
}
4539

46-
$tokenHandlerDefinition->replaceArgument(0, new Reference($config['client']['id'] ?? $clientDefinitionId));
40+
$container->setDefinition($id, new ChildDefinition('security.access_token_handler.oidc_user_info'))
41+
->replaceArgument(0, $clientDefinition)
42+
->replaceArgument(2, $config['claim']);
4743
}
4844

4945
public function getKey(): string
@@ -56,19 +52,23 @@ public function addConfiguration(NodeBuilder $node): void
5652
$node
5753
->arrayNode($this->getKey())
5854
->fixXmlConfig($this->getKey())
55+
->beforeNormalization()
56+
->ifString()
57+
->then(static fn ($v) => ['claim' => 'sub', 'base_uri' => $v])
58+
->end()
5959
->children()
60+
->scalarNode('base_uri')
61+
->info('Base URI of the userinfo endpoint on the OIDC server.')
62+
->isRequired()
63+
->cannotBeEmpty()
64+
->end()
6065
->scalarNode('claim')
61-
->info('Claim which contains the user identifier (e.g.: sub, email..).')
66+
->info('Claim which contains the user identifier (e.g. sub, email, etc.).')
6267
->defaultValue('sub')
68+
->cannotBeEmpty()
6369
->end()
64-
->arrayNode('client')
65-
->info('HttpClient to call the OIDC server.')
66-
->isRequired()
67-
->beforeNormalization()
68-
->ifString()
69-
->then(static function ($v): array { return ['id' => $v]; })
70-
->end()
71-
->prototype('scalar')->end()
70+
->scalarNode('client')
71+
->info('HttpClient service id to use to call the OIDC server.')
7272
->end()
7373
->end()
7474
->end()

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
use Symfony\Component\Security\Http\AccessToken\Oidc\OidcUserInfoTokenHandler;
1919
use Symfony\Component\Security\Http\AccessToken\QueryAccessTokenExtractor;
2020
use Symfony\Component\Security\Http\Authenticator\AccessTokenAuthenticator;
21+
use Symfony\Contracts\HttpClient\HttpClientInterface;
2122

2223
return static function (ContainerConfigurator $container) {
2324
$container->services()
@@ -44,12 +45,17 @@
4445
])
4546

4647
// OIDC
48+
->set('security.access_token_handler.oidc_user_info.http_client', HttpClientInterface::class)
49+
->abstract()
50+
->factory([service('http_client'), 'withOptions'])
51+
->args([abstract_arg('http client options')])
52+
4753
->set('security.access_token_handler.oidc_user_info', OidcUserInfoTokenHandler::class)
4854
->abstract()
4955
->args([
5056
abstract_arg('http client'),
5157
service('logger')->nullOnInvalid(),
52-
'sub',
58+
abstract_arg('claim'),
5359
])
5460

5561
->set('security.access_token_handler.oidc', OidcTokenHandler::class)

‎src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Security/Factory/AccessTokenFactoryTest.php‎

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,9 @@
1818
use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\AccessTokenFactory;
1919
use Symfony\Component\Config\Definition\Builder\ArrayNodeDefinition;
2020
use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException;
21+
use Symfony\Component\DependencyInjection\ChildDefinition;
2122
use Symfony\Component\DependencyInjection\ContainerBuilder;
23+
use Symfony\Component\DependencyInjection\Reference;
2224

2325
class AccessTokenFactoryTest extends TestCase
2426
{
@@ -76,7 +78,12 @@ public function testOidcUserInfoTokenHandlerConfigurationWithExistingClient()
7678
{
7779
$container = new ContainerBuilder();
7880
$config = [
79-
'token_handler' => ['oidc_user_info' => ['client' => 'oidc.client']],
81+
'token_handler' => [
82+
'oidc_user_info' => [
83+
'base_uri' => 'https://www.example.com/realms/demo/protocol/openid-connect/userinfo',
84+
'client' => 'oidc.client',
85+
],
86+
],
8087
];
8188

8289
$factory = new AccessTokenFactory($this->createTokenHandlerFactories());
@@ -86,14 +93,24 @@ public function testOidcUserInfoTokenHandlerConfigurationWithExistingClient()
8693

8794
$this->assertTrue($container->hasDefinition('security.authenticator.access_token.firewall1'));
8895
$this->assertTrue($container->hasDefinition('security.access_token_handler.firewall1'));
89-
$this->assertFalse($container->hasDefinition('http_client.security.access_token_handler.oidc_user_info'));
96+
97+
$expected = [
98+
'index_0' => (new ChildDefinition('security.access_token_handler.oidc_user_info.http_client'))
99+
->setFactory([new Reference('oidc.client'), 'withOptions'])
100+
->replaceArgument(0, ['base_uri' => 'https://www.example.com/realms/demo/protocol/openid-connect/userinfo']),
101+
'index_2' => 'sub',
102+
];
103+
$this->assertEquals($expected, $container->getDefinition('security.access_token_handler.firewall1')->getArguments());
90104
}
91105

92-
public function testOidcUserInfoTokenHandlerConfigurationWithClientCreation()
106+
/**
107+
* @dataProvider getOidcUserInfoConfiguration
108+
*/
109+
public function testOidcUserInfoTokenHandlerConfigurationWithBaseUri(array|string $configuration)
93110
{
94111
$container = new ContainerBuilder();
95112
$config = [
96-
'token_handler' => ['oidc_user_info' => ['client' => ['base_uri' => 'https://www.example.com/realms/demo/protocol/openid-connect/userinfo']]],
113+
'token_handler' => ['oidc_user_info' => $configuration],
97114
];
98115

99116
$factory = new AccessTokenFactory($this->createTokenHandlerFactories());
@@ -103,7 +120,19 @@ public function testOidcUserInfoTokenHandlerConfigurationWithClientCreation()
103120

104121
$this->assertTrue($container->hasDefinition('security.authenticator.access_token.firewall1'));
105122
$this->assertTrue($container->hasDefinition('security.access_token_handler.firewall1'));
106-
$this->assertTrue($container->hasDefinition('http_client.security.access_token_handler.oidc_user_info'));
123+
124+
$expected = [
125+
'index_0' => (new ChildDefinition('security.access_token_handler.oidc_user_info.http_client'))
126+
->replaceArgument(0, ['base_uri' => 'https://www.example.com/realms/demo/protocol/openid-connect/userinfo']),
127+
'index_2' => 'sub',
128+
];
129+
$this->assertEquals($expected, $container->getDefinition('security.access_token_handler.firewall1')->getArguments());
130+
}
131+
132+
public static function getOidcUserInfoConfiguration(): iterable
133+
{
134+
yield [['base_uri' => 'https://www.example.com/realms/demo/protocol/openid-connect/userinfo']];
135+
yield ['https://www.example.com/realms/demo/protocol/openid-connect/userinfo'];
107136
}
108137

109138
public function testMultipleTokenHandlersSet()
@@ -114,7 +143,7 @@ public function testMultipleTokenHandlersSet()
114143
$config = [
115144
'token_handler' => [
116145
'id' => 'in_memory_token_handler_service_id',
117-
'oidc_user_info' => ['client' => 'oidc.client'],
146+
'oidc_user_info' => 'https://www.example.com/realms/demo/protocol/openid-connect/userinfo',
118147
],
119148
];
120149

‎src/Symfony/Bundle/SecurityBundle/composer.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@
6060
"symfony/browser-kit": "<5.4",
6161
"symfony/console": "<5.4",
6262
"symfony/framework-bundle": "<5.4",
63+
"symfony/http-client": "<5.4",
6364
"symfony/ldap": "<5.4",
6465
"symfony/twig-bundle": "<5.4"
6566
},

0 commit comments

Comments
 (0)