Skip to content
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

set resourceOwner services directly without using tag #1874

Merged
merged 2 commits into from
Jan 10, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,12 @@ public function process(ContainerBuilder $container): void
$oauthUtilsDef = $container->getDefinition('hwi_oauth.security.oauth_utils');

foreach ($firewallNames as $firewallName => $_) {
$resourceOwnerMapRef = new Reference('hwi_oauth.resource_ownermap.'.$firewallName);
$resourceOwnerMapId = 'hwi_oauth.resource_ownermap.'.$firewallName;

$container->getDefinition($resourceOwnerMapId)
->setArgument('$locator', new Reference('hwi_oauth.resource_owners.locator'));

$resourceOwnerMapRef = new Reference($resourceOwnerMapId);

$locatorDef->addMethodCall('set', [$firewallName, $resourceOwnerMapRef]);
$oauthUtilsDef->addMethodCall('addResourceOwnerMap', [$resourceOwnerMapRef]);
Expand Down
18 changes: 11 additions & 7 deletions src/DependencyInjection/HWIOAuthExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ function service($class): ReferenceConfigurator
use Symfony\Component\Config\Definition\Processor;
use Symfony\Component\Config\FileLocator;
use Symfony\Component\DependencyInjection\Alias;
use Symfony\Component\DependencyInjection\Compiler\ServiceLocatorTagPass;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Definition;
use Symfony\Component\DependencyInjection\Exception\BadMethodCallException;
Expand Down Expand Up @@ -102,15 +103,20 @@ public function load(array $configs, ContainerBuilder $container): void

// setup services for all configured resource owners
$resourceOwners = [];
$resourceOwnerReferenceMap = [];
foreach ($config['resource_owners'] as $name => $options) {
$resourceOwners[$name] = $name;
$this->createResourceOwnerService($container, $name, $options);
$resourceOwnerReferenceMap[$name] = $this->createResourceOwnerService($container, $name, $options);

if (!$this->refreshTokenListenerEnabled) {
$this->refreshTokenListenerEnabled = $options['options']['refresh_on_expire'] ?? false;
}
}
$container->setParameter('hwi_oauth.resource_owners', $resourceOwners);
$container->setAlias(
'hwi_oauth.resource_owners.locator',
(string) ServiceLocatorTagPass::register($container, $resourceOwnerReferenceMap)
);

$this->createConnectIntegration($container, $config);
}
Expand All @@ -126,14 +132,11 @@ public function load(array $configs, ContainerBuilder $container): void
* @throws BadMethodCallException
* @throws InvalidArgumentException
*/
public function createResourceOwnerService(ContainerBuilder $container, string $name, array $options): void
public function createResourceOwnerService(ContainerBuilder $container, string $name, array $options): Reference
{
// alias services
if (isset($options['service'])) {
// set the appropriate name for aliased services, compiler pass depends on it
$container->setAlias('hwi_oauth.resource_owner.'.$name, new Alias($options['service'], true));

return;
return new Reference($options['service']);
}

$type = $options['type'];
Expand All @@ -156,9 +159,10 @@ public function createResourceOwnerService(ContainerBuilder $container, string $
$definition->setArgument('$options', $options);
$definition->setArgument('$name', $name);
$definition->setArgument('$storage', new Reference('hwi_oauth.storage.session'));
$definition->addTag('hwi_oauth.resource_owner', ['resource-name' => $name]);

$container->setDefinition('hwi_oauth.resource_owner.'.$name, $definition);

return new Reference('hwi_oauth.resource_owner.'.$name);
}

/**
Expand Down
3 changes: 1 addition & 2 deletions src/Resources/config/resource_owners.php
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,7 @@
->abstract()
->arg('$httpUtils', service('security.http_utils'))
->arg('$possibleResourceOwners', '%hwi_oauth.resource_owners%')
->arg('$resourceOwners', [])
->arg('$locator', tagged_locator('hwi_oauth.resource_owner', 'resource-name', 'getDefaultResourceNameName', 'getDefaultResourceNamePriority'));
->arg('$resourceOwners', []);

$services->set('hwi_oauth.resource_ownermap_locator', ResourceOwnerMapLocator::class);
};
18 changes: 18 additions & 0 deletions tests/App/ResourceOwner/CustomResourceOwner.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<?php

/*
* This file is part of the HWIOAuthBundle package.
*
* (c) Hardware Info <[email protected]>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace HWI\Bundle\OAuthBundle\Tests\App\ResourceOwner;

use HWI\Bundle\OAuthBundle\OAuth\ResourceOwner\GenericOAuth2ResourceOwner;

class CustomResourceOwner extends GenericOAuth2ResourceOwner
{
}
15 changes: 15 additions & 0 deletions tests/App/config/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,19 @@ services:
class: Symfony\Contracts\HttpClient\HttpClientInterface
public: true

app.custom_resource_owner:
class: HWI\Bundle\OAuthBundle\Tests\App\ResourceOwner\CustomResourceOwner
arguments:
$options: {
access_token_url: 'http://custom/token',
authorization_url: 'http://custom/auth',
infos_url: 'http://custom/info',
client_id: 'client_id',
client_secret: 'client_secret',
}
$name: 'custom'
$storage: '@hwi_oauth.storage.session'

doctrine:
dbal:
driver: pdo_sqlite
Expand Down Expand Up @@ -61,6 +74,8 @@ hwi_oauth:
scope: 'scope=mail-r sdct-r'
options:
refresh_on_expire: true
custom:
service: 'app.custom_resource_owner'

twig:
strict_variables: '%kernel.debug%'
Expand Down
3 changes: 3 additions & 0 deletions tests/App/config/routing.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,6 @@ google_login:

yahoo_login:
path: /check-login/yahoo

custom_login:
path: /check-login/custom
1 change: 1 addition & 0 deletions tests/App/config/security_v4.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ security:
resource_owners:
google: '/check-login/google'
yahoo: '/check-login/yahoo'
custom: '/check-login/custom'
login_path: /login
use_forward: false
failure_path: /login
Expand Down
1 change: 1 addition & 0 deletions tests/App/config/security_v5.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ security:
resource_owners:
google: '/check-login/google'
yahoo: '/check-login/yahoo'
custom: '/check-login/custom'
login_path: /login
use_forward: false
failure_path: /login
Expand Down
1 change: 1 addition & 0 deletions tests/App/config/security_v5_bc.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ security:
resource_owners:
google: '/check-login/google'
yahoo: '/check-login/yahoo'
custom: '/check-login/custom'
login_path: /login
use_forward: false
failure_path: /login
Expand Down
1 change: 1 addition & 0 deletions tests/App/config/security_v6.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ security:
resource_owners:
google: '/check-login/google'
yahoo: '/check-login/yahoo'
custom: '/check-login/custom'
login_path: /login
use_forward: false
failure_path: /login
Expand Down
119 changes: 23 additions & 96 deletions tests/DependencyInjection/HWIOAuthExtensionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,10 @@
use PHPUnit\Framework\TestCase;
use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait;
use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException;
use Symfony\Component\DependencyInjection\Argument\ServiceClosureArgument;
use Symfony\Component\DependencyInjection\ChildDefinition;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Reference;
use Symfony\Component\Yaml\Parser;

/**
Expand Down Expand Up @@ -464,10 +466,26 @@ public function provideInvalidData(): array
];
}

public function testRegistersResourceOwnerServiceLocator(): void
{
$this->createEmptyConfiguration();

$this->assertTrue($this->containerBuilder->hasAlias('hwi_oauth.resource_owners.locator'));
$locatorDefinition = $this->containerBuilder->findDefinition('hwi_oauth.resource_owners.locator');

$this->assertEquals(
[
'any_name' => new ServiceClosureArgument(new Reference('hwi_oauth.resource_owner.any_name')),
'some_service' => new ServiceClosureArgument(new Reference('hwi_oauth.abstract_resource_owner.generic')),
],
$locatorDefinition->getArgument(0)
);
}

public function testCreateResourceOwnerService(): void
{
$extension = new HWIOAuthExtension();
$extension->createResourceOwnerService($this->containerBuilder, 'my_github', [
$reference = $extension->createResourceOwnerService($this->containerBuilder, 'my_github', [
'type' => 'github',
'client_id' => '42',
'client_secret' => 'foo',
Expand All @@ -476,6 +494,8 @@ public function testCreateResourceOwnerService(): void
/** @var array<string, ChildDefinition> $definitions */
$definitions = $this->containerBuilder->getDefinitions();

$this->assertSame('hwi_oauth.resource_owner.my_github', (string) $reference);

$this->assertArrayHasKey('hwi_oauth.resource_owner.my_github', $definitions);
$this->assertEquals('%hwi_oauth.resource_owner.github.class%', $definitions['hwi_oauth.resource_owner.my_github']->getClass());

Expand All @@ -488,13 +508,11 @@ public function testCreateResourceOwnerService(): void
public function testCreateResourceOwnerServiceWithService(): void
{
$extension = new HWIOAuthExtension();
$extension->createResourceOwnerService($this->containerBuilder, 'external_ressource_owner', [
$reference = $extension->createResourceOwnerService($this->containerBuilder, 'external_ressource_owner', [
'service' => 'my.service',
]);

$aliases = $this->containerBuilder->getAliases();
$this->assertArrayHasKey('hwi_oauth.resource_owner.external_ressource_owner', $aliases);
$this->assertEquals('my.service', $aliases['hwi_oauth.resource_owner.external_ressource_owner']);
$this->assertSame('my.service', (string) $reference);
}

public function testCreateResourceOwnerServiceWithWrongClass(): void
Expand Down Expand Up @@ -540,13 +558,6 @@ protected function createEmptyConfiguration(): void
$loader->load([$config], $this->containerBuilder);
}

protected function createFullConfiguration(): void
stloyd marked this conversation as resolved.
Show resolved Hide resolved
{
$config = $this->getFullConfig();
$loader = new HWIOAuthExtension();
$loader->load([$config], $this->containerBuilder);
}

protected function getEmptyConfig(): array
{
$yaml = <<<EOF
Expand All @@ -563,90 +574,6 @@ protected function getEmptyConfig(): array
return $parser->parse($yaml);
}

protected function getFullConfig(): array
{
$yaml = <<<EOF
resource_owners:
github:
type: github
client_id: client_id
client_secret: client_secret
scope: ""

google:
type: google
client_id: client_id
client_secret: client_secret
scope: ""
user_response_class: \Our\Custom\Response\Class
paths:
email: email
profilepicture: picture

facebook:
type: facebook
client_id: client_id
client_secret: client_secret
scope: ""
paths:
nickname: [email, id]

my_custom_oauth2:
type: oauth2
client_id: client_id
client_secret: client_secret
access_token_url: https://path.to/oauth/v2/token
authorization_url: https://path.to/oauth/v2/authorize
infos_url: https://path.to/api/user
scope: ""
user_response_class: HWI\Bundle\OAuthBundle\OAuth\Response\AdvancedPathUserResponse
paths:
identifier: id
nickname: username
realname: username
email: email

my_custom_oauth1:
type: oauth1
client_id: client_id
client_secret: client_secret
request_token_url: https://path.to/oauth/v1/requestToken
access_token_url: https://path.to/oauth/v1/token
authorization_url: https://path.to/oauth/v1/authorize
infos_url: https://path.to/api/user
realm: ""
user_response_class: HWI\Bundle\OAuthBundle\OAuth\Response\PathUserResponse
paths:
identifier: id
nickname: username
realname: username

fosub:
username_iterations: 30

properties:
github: githubId
google: googleId
facebook: facebookId
my_custom_provider: customId

connect:
registration_form_handler: my_registration_form_handler
registration_form: my_registration_form
account_connector: my_link_provider

http_client:
timeout: 5
verify_peer: true
ignore_errors: true
max_redirects: 5

templating_engine: "php"
EOF;

return (new Parser())->parse($yaml);
}

private function assertAlias(string $value, string $key): void
{
$this->assertEquals($value, (string) $this->containerBuilder->getAlias($key), sprintf('%s alias is correct', $key));
Expand Down
4 changes: 3 additions & 1 deletion tests/Functional/Controller/LoginControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@ public function testLoginPage(): void
$response = $client->getResponse();

$this->assertSame(200, $response->getStatusCode(), $response->getContent());
$this->assertSame('google', $crawler->filter('a')->text(), $response->getContent());
$this->assertSame('google', $crawler->filter('a:nth-child(1)')->text(), $response->getContent());
$this->assertSame('yahoo', $crawler->filter('a:nth-child(3)')->text(), $response->getContent());
$this->assertSame('custom', $crawler->filter('a:nth-child(5)')->text(), $response->getContent());
stloyd marked this conversation as resolved.
Show resolved Hide resolved
}

public function testRedirectingToRegistrationFormWithError(): void
Expand Down