Skip to content

Commit

Permalink
Merge pull request #1863 from stloyd/maintain/imprv-code-quality
Browse files Browse the repository at this point in the history
Maintain | Improved code quality by adding hint & return types
  • Loading branch information
stloyd committed Dec 24, 2021
2 parents 030b72a + 2b90b51 commit 3ae855b
Show file tree
Hide file tree
Showing 15 changed files with 108 additions and 195 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ final class ResourceOwnerMapCompilerPass implements CompilerPassInterface
/**
* {@inheritdoc}
*/
public function process(ContainerBuilder $container)
public function process(ContainerBuilder $container): void
{
$def = $container->getDefinition('hwi_oauth.resource_ownermap_locator');

Expand Down
26 changes: 8 additions & 18 deletions src/DependencyInjection/Configuration.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@ final class Configuration implements ConfigurationInterface
* Array of supported resource owners, indentation is intentional to easily notice
* which resource is of which type.
*
* @var array
* @var array<string, array<int, string>>
*/
private static $resourceOwners = [
private static array $resourceOwners = [
'oauth2' => [
'amazon',
'apple',
Expand Down Expand Up @@ -100,19 +100,15 @@ final class Configuration implements ConfigurationInterface

/**
* Return the type (OAuth1 or OAuth2) of given resource owner.
*
* @param string $resourceOwner
*
* @return string
*/
public static function getResourceOwnerType($resourceOwner)
public static function getResourceOwnerType(string $resourceOwner): string
{
$resourceOwner = strtolower($resourceOwner);
if ('oauth1' === $resourceOwner || 'oauth2' === $resourceOwner) {
return $resourceOwner;
}

if (\in_array($resourceOwner, static::$resourceOwners['oauth1'], true)) {
if (\in_array($resourceOwner, self::$resourceOwners['oauth1'], true)) {
return 'oauth1';
}

Expand All @@ -121,31 +117,25 @@ public static function getResourceOwnerType($resourceOwner)

/**
* Checks that given resource owner is supported by this bundle.
*
* @param string $resourceOwner
*
* @return bool
*/
public static function isResourceOwnerSupported($resourceOwner)
public static function isResourceOwnerSupported(string $resourceOwner): bool
{
$resourceOwner = strtolower($resourceOwner);
if ('oauth1' === $resourceOwner || 'oauth2' === $resourceOwner) {
return true;
}

if (\in_array($resourceOwner, static::$resourceOwners['oauth1'], true)) {
if (\in_array($resourceOwner, self::$resourceOwners['oauth1'], true)) {
return true;
}

return \in_array($resourceOwner, static::$resourceOwners['oauth2'], true);
return \in_array($resourceOwner, self::$resourceOwners['oauth2'], true);
}

/**
* Generates the configuration tree builder.
*
* @return TreeBuilder $builder The tree builder
*/
public function getConfigTreeBuilder()
public function getConfigTreeBuilder(): TreeBuilder
{
$builder = new TreeBuilder('hwi_oauth');

Expand Down
2 changes: 1 addition & 1 deletion src/DependencyInjection/HWIOAuthExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ final class HWIOAuthExtension extends Extension
* @throws OutOfBoundsException
* @throws ServiceNotFoundException
*/
public function load(array $configs, ContainerBuilder $container)
public function load(array $configs, ContainerBuilder $container): void
{
$loader = new PhpFileLoader($container, new FileLocator(__DIR__.'/../Resources/config/'));
$loader->load('controller.php');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public function createAuthenticator(
->addArgument(
$this->createOAuthAwareUserProvider($container, $firewallName, $config['oauth_user_provider'])
)
->addArgument($this->getResourceOwnerMapReference($firewallName))
->addArgument($this->createResourceOwnerMapReference($firewallName))
->addArgument($config['resource_owners'])
->addArgument(new Reference($this->createAuthenticationSuccessHandler($container, $firewallName, $config)))
->addArgument(new Reference($this->createAuthenticationFailureHandler($container, $firewallName, $config)))
Expand Down Expand Up @@ -124,14 +124,6 @@ public function getPosition(): string
return 'http';
}

/**
* Gets a reference to the resource owner map.
*/
protected function getResourceOwnerMapReference(string $id): Reference
{
return new Reference('hwi_oauth.resource_ownermap.'.$id);
}

/**
* {@inheritdoc}
*/
Expand All @@ -144,7 +136,7 @@ protected function createAuthProvider(ContainerBuilder $container, string $id, a
$container
->setDefinition($providerId, new ChildDefinition('hwi_oauth.authentication.provider.oauth'))
->addArgument($this->createOAuthAwareUserProvider($container, $id, $config['oauth_user_provider']))
->addArgument($this->getResourceOwnerMapReference($id))
->addArgument($this->createResourceOwnerMapReference($id))
->addArgument(new Reference('hwi_oauth.user_checker'))
->addArgument(new Reference('security.token_storage'))
;
Expand All @@ -168,9 +160,6 @@ protected function createEntryPoint($container, $id, $config, ?string $defaultEn
return $entryPointId;
}

/**
* {@inheritdoc}
*/
protected function createListener(ContainerBuilder $container, string $id, array $config, string $userProvider): string
{
// @phpstan-ignore-next-line Symfony <5.4 BC layer
Expand All @@ -180,7 +169,7 @@ protected function createListener(ContainerBuilder $container, string $id, array

$container
->getDefinition($listenerId)
->addMethodCall('setResourceOwnerMap', [$this->getResourceOwnerMapReference($id)])
->addMethodCall('setResourceOwnerMap', [$this->createResourceOwnerMapReference($id)])
->addMethodCall('setCheckPaths', [$checkPaths])
;

Expand All @@ -195,24 +184,31 @@ protected function getListenerId(): string
return 'hwi_oauth.authentication.listener.oauth';
}

/**
* Gets a reference to the resource owner map.
*/
private function createResourceOwnerMapReference(string $firewallName): Reference
{
return new Reference('hwi_oauth.resource_ownermap.'.$firewallName);
}

/**
* Creates a resource owner map for the given configuration.
*
* @param ContainerBuilder $container Container to build for
* @param string $id Firewall id
* @param array $config Configuration
*/
private function createResourceOwnerMap(ContainerBuilder $container, string $id, array $config): void
private function createResourceOwnerMap(ContainerBuilder $container, string $firewallName, array $config): void
{
$resourceOwnersMap = [];
foreach ($config['resource_owners'] as $name => $checkPath) {
$resourceOwnersMap[$name] = $checkPath;
}
$container->setParameter('hwi_oauth.resource_ownermap.configured.'.$id, $resourceOwnersMap);
$container->setParameter('hwi_oauth.resource_ownermap.configured.'.$firewallName, $resourceOwnersMap);

$container
->setDefinition($this->getResourceOwnerMapReference($id), new ChildDefinition('hwi_oauth.abstract_resource_ownermap'))
->replaceArgument('$resourceOwners', new Parameter('hwi_oauth.resource_ownermap.configured.'.$id))
->setDefinition(
$this->createResourceOwnerMapReference($firewallName),
new ChildDefinition('hwi_oauth.abstract_resource_ownermap')
)
->replaceArgument('$resourceOwners', new Parameter('hwi_oauth.resource_ownermap.configured.'.$firewallName))
->setPublic(true)
;
}
Expand Down
32 changes: 17 additions & 15 deletions src/DependencyInjection/Security/Factory/OAuthFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -78,14 +78,6 @@ public function createAuthenticator(
throw new \RuntimeException('Deprecated "OAuthFactory" cannot create new Symfony Authenticator!');
}

/**
* Gets a reference to the resource owner map.
*/
protected function getResourceOwnerMapReference(string $id): Reference
{
return new Reference('hwi_oauth.resource_ownermap.'.$id);
}

/**
* {@inheritdoc}
*/
Expand All @@ -101,7 +93,7 @@ protected function createAuthProvider(ContainerBuilder $container, $id, $config,
$container
->setDefinition($providerId, new ChildDefinition('hwi_oauth.authentication.provider.oauth'))
->addArgument($this->createOAuthAwareUserProvider($container, $id, $config['oauth_user_provider']))
->addArgument($this->getResourceOwnerMapReference($id))
->addArgument($this->createResourceOwnerMapReference($id))
->addArgument(new Reference('hwi_oauth.user_checker'))
->addArgument(new Reference('security.token_storage'))
;
Expand Down Expand Up @@ -166,7 +158,7 @@ protected function createListener($container, $id, $config, $userProvider): stri

$container
->getDefinition($listenerId)
->addMethodCall('setResourceOwnerMap', [$this->getResourceOwnerMapReference($id)])
->addMethodCall('setResourceOwnerMap', [$this->createResourceOwnerMapReference($id)])
->addMethodCall('setCheckPaths', [$checkPaths])
;

Expand All @@ -193,24 +185,34 @@ protected function getListenerId(): string
return 'hwi_oauth.authentication.listener.oauth';
}

/**
* Gets a reference to the resource owner map.
*/
private function createResourceOwnerMapReference(string $firewallName): Reference
{
return new Reference('hwi_oauth.resource_ownermap.'.$firewallName);
}

/**
* Creates a resource owner map for the given configuration.
*
* @param ContainerBuilder $container Container to build for
* @param string $id Firewall id
* @param array $config Configuration
*/
private function createResourceOwnerMap(ContainerBuilder $container, string $id, array $config): void
private function createResourceOwnerMap(ContainerBuilder $container, string $firewallName, array $config): void
{
$resourceOwnersMap = [];
foreach ($config['resource_owners'] as $name => $checkPath) {
$resourceOwnersMap[$name] = $checkPath;
}
$container->setParameter('hwi_oauth.resource_ownermap.configured.'.$id, $resourceOwnersMap);
$container->setParameter('hwi_oauth.resource_ownermap.configured.'.$firewallName, $resourceOwnersMap);

$container
->setDefinition($this->getResourceOwnerMapReference($id), new ChildDefinition('hwi_oauth.abstract_resource_ownermap'))
->replaceArgument('$resourceOwners', new Parameter('hwi_oauth.resource_ownermap.configured.'.$id))
->setDefinition(
$this->createResourceOwnerMapReference($firewallName),
new ChildDefinition('hwi_oauth.abstract_resource_ownermap')
)
->replaceArgument('$resourceOwners', new Parameter('hwi_oauth.resource_ownermap.configured.'.$firewallName))
->setPublic(true)
;
}
Expand Down
7 changes: 1 addition & 6 deletions src/OAuth/RequestDataStorage/SessionStorage.php
Original file line number Diff line number Diff line change
Expand Up @@ -70,13 +70,8 @@ public function save(ResourceOwnerInterface $resourceOwner, $value, $type = 'tok

/**
* Key to for fetching or saving a token.
*
* @param string $key
* @param string $type
*
* @return string
*/
private function generateKey(ResourceOwnerInterface $resourceOwner, $key, $type)
private function generateKey(ResourceOwnerInterface $resourceOwner, string $key, string $type): string
{
return sprintf('_hwi_oauth.%s.%s.%s.%s', $resourceOwner->getName(), $resourceOwner->getOption('client_id'), $type, $key);
}
Expand Down
3 changes: 0 additions & 3 deletions src/OAuth/State/State.php
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,6 @@ public function get(string $key): ?string
return $this->values[$key];
}

/**
* {@inheritdoc}
*/
public function has(string $key): bool
{
return \array_key_exists($key, $this->values);
Expand Down
41 changes: 7 additions & 34 deletions src/Security/Core/Authentication/Token/AbstractOAuthToken.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,40 +22,13 @@
*/
abstract class AbstractOAuthToken extends AbstractToken
{
/**
* @var string
*/
private $accessToken;

/**
* @var array
*/
private $rawToken;

/**
* @var string
*/
private $refreshToken;

/**
* @var int
*/
private $expiresIn;

/**
* @var int
*/
private $createdAt;

/**
* @var string
*/
private $tokenSecret;

/**
* @var string
*/
private $resourceOwnerName;
private string $accessToken;
private array $rawToken;
private ?int $expiresIn = null;
private ?int $createdAt = null;
private string $resourceOwnerName;
private ?string $tokenSecret = null;
private ?string $refreshToken = null;

/**
* @param string|array $accessToken The OAuth access token
Expand Down

0 comments on commit 3ae855b

Please sign in to comment.