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

Maintain | Improved code quality by adding hint & return types #1863

Merged
merged 1 commit into from
Dec 24, 2021
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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 @@ -63,7 +63,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 @@ -95,14 +95,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 @@ -115,7 +107,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 @@ -139,9 +131,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 @@ -151,7 +140,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 @@ -166,24 +155,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 @@ -77,14 +77,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 @@ -100,7 +92,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 @@ -163,7 +155,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 @@ -178,24 +170,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