From 2b90b511f17119294133486dae24dddbdbd6db3e Mon Sep 17 00:00:00 2001 From: Joseph Bielawski Date: Thu, 23 Dec 2021 11:53:58 +0100 Subject: [PATCH] Maintain | Improved code quality by adding hint & return types --- .../ResourceOwnerMapCompilerPass.php | 2 +- src/DependencyInjection/Configuration.php | 26 +++------ src/DependencyInjection/HWIOAuthExtension.php | 2 +- .../Factory/OAuthAuthenticatorFactory.php | 40 ++++++------- .../Security/Factory/OAuthFactory.php | 32 ++++++----- .../RequestDataStorage/SessionStorage.php | 7 +-- src/OAuth/State/State.php | 3 - .../Token/AbstractOAuthToken.php | 41 +++---------- src/Security/Core/User/EntityUserProvider.php | 30 ++++------ src/Security/Core/User/OAuthUserProvider.php | 19 ++----- .../Passport/SelfValidatedOAuthPassport.php | 2 +- src/Security/OAuthUtils.php | 57 ++++++------------- .../AbstractConnectControllerTest.php | 6 +- tests/Fixtures/CustomOAuthToken.php | 1 + tests/Security/OAuthUtilsTest.php | 35 +++++------- 15 files changed, 108 insertions(+), 195 deletions(-) diff --git a/src/DependencyInjection/CompilerPass/ResourceOwnerMapCompilerPass.php b/src/DependencyInjection/CompilerPass/ResourceOwnerMapCompilerPass.php index 82c25f929..de5e38c5a 100644 --- a/src/DependencyInjection/CompilerPass/ResourceOwnerMapCompilerPass.php +++ b/src/DependencyInjection/CompilerPass/ResourceOwnerMapCompilerPass.php @@ -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'); diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index e3185a8e9..d0b901b22 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -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> */ - private static $resourceOwners = [ + private static array $resourceOwners = [ 'oauth2' => [ 'amazon', 'apple', @@ -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'; } @@ -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'); diff --git a/src/DependencyInjection/HWIOAuthExtension.php b/src/DependencyInjection/HWIOAuthExtension.php index 656901266..50ef6837c 100644 --- a/src/DependencyInjection/HWIOAuthExtension.php +++ b/src/DependencyInjection/HWIOAuthExtension.php @@ -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'); diff --git a/src/DependencyInjection/Security/Factory/OAuthAuthenticatorFactory.php b/src/DependencyInjection/Security/Factory/OAuthAuthenticatorFactory.php index 487a618f1..d9a29ca10 100644 --- a/src/DependencyInjection/Security/Factory/OAuthAuthenticatorFactory.php +++ b/src/DependencyInjection/Security/Factory/OAuthAuthenticatorFactory.php @@ -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))) @@ -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} */ @@ -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')) ; @@ -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 @@ -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]) ; @@ -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) ; } diff --git a/src/DependencyInjection/Security/Factory/OAuthFactory.php b/src/DependencyInjection/Security/Factory/OAuthFactory.php index c63c00447..591223933 100644 --- a/src/DependencyInjection/Security/Factory/OAuthFactory.php +++ b/src/DependencyInjection/Security/Factory/OAuthFactory.php @@ -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} */ @@ -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')) ; @@ -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]) ; @@ -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) ; } diff --git a/src/OAuth/RequestDataStorage/SessionStorage.php b/src/OAuth/RequestDataStorage/SessionStorage.php index ca13eeb52..7c22de5ff 100644 --- a/src/OAuth/RequestDataStorage/SessionStorage.php +++ b/src/OAuth/RequestDataStorage/SessionStorage.php @@ -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); } diff --git a/src/OAuth/State/State.php b/src/OAuth/State/State.php index 5e8146a78..f4937d5f8 100644 --- a/src/OAuth/State/State.php +++ b/src/OAuth/State/State.php @@ -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); diff --git a/src/Security/Core/Authentication/Token/AbstractOAuthToken.php b/src/Security/Core/Authentication/Token/AbstractOAuthToken.php index c4997159b..9a333a51a 100644 --- a/src/Security/Core/Authentication/Token/AbstractOAuthToken.php +++ b/src/Security/Core/Authentication/Token/AbstractOAuthToken.php @@ -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 diff --git a/src/Security/Core/User/EntityUserProvider.php b/src/Security/Core/User/EntityUserProvider.php index e2b0ae8f2..d4be3f4ba 100644 --- a/src/Security/Core/User/EntityUserProvider.php +++ b/src/Security/Core/User/EntityUserProvider.php @@ -37,15 +37,13 @@ final class EntityUserProvider implements UserProviderInterface, OAuthAwareUserP /** * @var array */ - private $properties = [ + private array $properties = [ 'identifier' => 'id', ]; /** - * @param ManagerRegistry $registry manager registry - * @param string $class user entity class to load - * @param array $properties Mapping of resource owners to properties - * @param string|null $managerName Optional name of the entity manager to use + * @param string $class User entity class to load + * @param array $properties Mapping of resource owners to properties */ public function __construct(ManagerRegistry $registry, string $class, array $properties, ?string $managerName = null) { @@ -69,6 +67,8 @@ public function loadUserByIdentifier(string $identifier): UserInterface } /** + * Symfony <5.4 BC layer. + * * @param string $username * * @return UserInterface @@ -83,10 +83,7 @@ public function loadUserByUsername($username) return $user; } - /** - * @return UserInterface - */ - public function loadUserByOAuthUserResponse(UserResponseInterface $response) + public function loadUserByOAuthUserResponse(UserResponseInterface $response): ?UserInterface { $resourceOwnerName = $response->getResourceOwner()->getName(); @@ -102,10 +99,7 @@ public function loadUserByOAuthUserResponse(UserResponseInterface $response) return $user; } - /** - * @return UserInterface - */ - public function refreshUser(UserInterface $user) + public function refreshUser(UserInterface $user): ?UserInterface { $accessor = PropertyAccess::createPropertyAccessor(); $identifier = $this->properties['identifier']; @@ -125,12 +119,7 @@ public function refreshUser(UserInterface $user) return $user; } - /** - * @param string $class - * - * @return bool - */ - public function supportsClass($class) + public function supportsClass($class): bool { return $class === $this->class || is_subclass_of($class, $this->class); } @@ -144,6 +133,9 @@ private function findUser(array $criteria): ?UserInterface return $this->repository->findOneBy($criteria); } + /** + * @return UsernameNotFoundException|UserNotFoundException + */ private function createUserNotFoundException(string $username, string $message) { if (class_exists(UserNotFoundException::class)) { diff --git a/src/Security/Core/User/OAuthUserProvider.php b/src/Security/Core/User/OAuthUserProvider.php index 6c71ecc7b..e6ae11ca2 100644 --- a/src/Security/Core/User/OAuthUserProvider.php +++ b/src/Security/Core/User/OAuthUserProvider.php @@ -27,6 +27,8 @@ public function loadUserByIdentifier(string $identifier): UserInterface } /** + * Symfony <5.4 BC layer. + * * @param string $username * * @return UserInterface @@ -36,18 +38,12 @@ public function loadUserByUsername($username) return $this->loadUserByIdentifier($username); } - /** - * @return UserInterface - */ - public function loadUserByOAuthUserResponse(UserResponseInterface $response) + public function loadUserByOAuthUserResponse(UserResponseInterface $response): UserInterface { return $this->loadUserByIdentifier($response->getNickname()); } - /** - * @return UserInterface - */ - public function refreshUser(UserInterface $user) + public function refreshUser(UserInterface $user): UserInterface { if (!$this->supportsClass(\get_class($user))) { throw new UnsupportedUserException(sprintf('Unsupported user class "%s"', \get_class($user))); @@ -59,12 +55,7 @@ public function refreshUser(UserInterface $user) return $this->loadUserByUsername($username); } - /** - * @param string $class - * - * @return bool - */ - public function supportsClass($class) + public function supportsClass($class): bool { return 'HWI\\Bundle\\OAuthBundle\\Security\\Core\\User\\OAuthUser' === $class; } diff --git a/src/Security/Http/Authenticator/Passport/SelfValidatedOAuthPassport.php b/src/Security/Http/Authenticator/Passport/SelfValidatedOAuthPassport.php index c0dcdf2d4..3d60e4f77 100644 --- a/src/Security/Http/Authenticator/Passport/SelfValidatedOAuthPassport.php +++ b/src/Security/Http/Authenticator/Passport/SelfValidatedOAuthPassport.php @@ -19,7 +19,7 @@ /** * SelfValidatingPassport contained OAuthToken. */ -class SelfValidatedOAuthPassport extends SelfValidatingPassport +final class SelfValidatedOAuthPassport extends SelfValidatingPassport { private OAuthToken $token; diff --git a/src/Security/OAuthUtils.php b/src/Security/OAuthUtils.php index 956198d8c..c0fff46fd 100644 --- a/src/Security/OAuthUtils.php +++ b/src/Security/OAuthUtils.php @@ -30,9 +30,7 @@ final class OAuthUtils public const SIGNATURE_METHOD_PLAINTEXT = 'PLAINTEXT'; private bool $connect; - private string $grantRule; - private HttpUtils $httpUtils; /** @@ -54,7 +52,7 @@ public function __construct( $this->grantRule = $grantRule; } - public function addResourceOwnerMap(ResourceOwnerMapInterface $ownerMap) + public function addResourceOwnerMap(ResourceOwnerMapInterface $ownerMap): void { $this->ownerMaps[] = $ownerMap; } @@ -73,14 +71,7 @@ public function getResourceOwners(): array return array_keys($resourceOwners); } - /** - * @param string $name - * @param string $redirectUrl Optional - * @param array $extraParameters Optional - * - * @return string - */ - public function getAuthorizationUrl(Request $request, $name, $redirectUrl = null, array $extraParameters = []) + public function getAuthorizationUrl(Request $request, string $name, ?string $redirectUrl = null, array $extraParameters = []): string { $resourceOwner = $this->getResourceOwner($name); @@ -99,10 +90,7 @@ public function getAuthorizationUrl(Request $request, $name, $redirectUrl = null return $resourceOwner->getAuthorizationUrl($redirectUrl, $extraParameters); } - /** - * @return string - */ - public function getServiceAuthUrl(Request $request, ResourceOwnerInterface $resourceOwner) + public function getServiceAuthUrl(Request $request, ResourceOwnerInterface $resourceOwner): string { if ($resourceOwner->getOption('auth_with_one_url')) { return $this->httpUtils->generateUri($request, $this->getResourceOwnerCheckPath($resourceOwner->getName())); @@ -116,12 +104,7 @@ public function getServiceAuthUrl(Request $request, ResourceOwnerInterface $reso return $this->httpUtils->generateUri($request, 'hwi_oauth_connect_service'); } - /** - * @param string $name - * - * @return string - */ - public function getLoginUrl(Request $request, $name) + public function getLoginUrl(Request $request, string $name): string { // Just to check that this resource owner exists $this->getResourceOwner($name); @@ -148,12 +131,16 @@ public function getLoginUrl(Request $request, $name) * @param string $tokenSecret Optional token secret to use with signing * @param string $signatureMethod Optional signature method used to sign token * - * @return string - * * @throws \RuntimeException */ - public static function signRequest($method, $url, $parameters, $clientSecret, $tokenSecret = '', $signatureMethod = self::SIGNATURE_METHOD_HMAC) - { + public static function signRequest( + string $method, + string $url, + array $parameters, + string $clientSecret, + string $tokenSecret = '', + string $signatureMethod = self::SIGNATURE_METHOD_HMAC + ): string { // Validate required parameters foreach (['oauth_consumer_key', 'oauth_timestamp', 'oauth_nonce', 'oauth_version', 'oauth_signature_method'] as $parameter) { if (!isset($parameters[$parameter])) { @@ -224,7 +211,9 @@ public static function signRequest($method, $url, $parameters, $clientSecret, $t $signature = false; openssl_sign($baseString, $signature, $privateKey); - openssl_free_key($privateKey); + if (\PHP_VERSION_ID < 80000) { + openssl_free_key($privateKey); + } break; case self::SIGNATURE_METHOD_PLAINTEXT: @@ -238,14 +227,7 @@ public static function signRequest($method, $url, $parameters, $clientSecret, $t return base64_encode($signature); } - /** - * @param string $name - * - * @return ResourceOwnerInterface - * - * @throws \RuntimeException - */ - private function getResourceOwner($name) + private function getResourceOwner(string $name): ResourceOwnerInterface { foreach ($this->ownerMaps as $ownerMap) { $resourceOwner = $ownerMap->getResourceOwnerByName($name); @@ -257,12 +239,7 @@ private function getResourceOwner($name) throw new \RuntimeException(sprintf("No resource owner with name '%s'.", $name)); } - /** - * @param string $name - * - * @return string|null - */ - private function getResourceOwnerCheckPath($name) + private function getResourceOwnerCheckPath(string $name): ?string { foreach ($this->ownerMaps as $ownerMap) { if ($potentialResourceOwnerCheckPath = $ownerMap->getResourceOwnerCheckPath($name)) { diff --git a/tests/Controller/AbstractConnectControllerTest.php b/tests/Controller/AbstractConnectControllerTest.php index 2775e3d41..1b1a9c89c 100644 --- a/tests/Controller/AbstractConnectControllerTest.php +++ b/tests/Controller/AbstractConnectControllerTest.php @@ -135,8 +135,12 @@ protected function setUp(): void return 'facebook' === $owner ? $this->resourceOwner : null; }); + $httpUtils = $this->createMock(HttpUtils::class); + $httpUtils->method('generateUri') + ->willReturn('https://fake-url'); + $this->oAuthUtils = new OAuthUtils( - $this->createMock(HttpUtils::class), + $httpUtils, $this->createMock(AuthorizationCheckerInterface::class), true, 'IS_AUTHENTICATED_REMEMBERED' diff --git a/tests/Fixtures/CustomOAuthToken.php b/tests/Fixtures/CustomOAuthToken.php index 07882130e..a2c913fd9 100644 --- a/tests/Fixtures/CustomOAuthToken.php +++ b/tests/Fixtures/CustomOAuthToken.php @@ -27,6 +27,7 @@ public function __construct(array $accessToken = []) ] ); + $this->setResourceOwnerName('fake'); $this->setUser(new User()); } diff --git a/tests/Security/OAuthUtilsTest.php b/tests/Security/OAuthUtilsTest.php index adee135df..e86bee773 100644 --- a/tests/Security/OAuthUtilsTest.php +++ b/tests/Security/OAuthUtilsTest.php @@ -170,11 +170,8 @@ public function testGetServiceAuthUrlWithStateQueryParameters(): void /** * @dataProvider provideValidData - * - * @param string $signature - * @param string $url */ - public function testSignatureIsGeneratedCorrectly($signature, $url) + public function testSignatureIsGeneratedCorrectly(string $signature, string $url): void { // Parameters from http://oauth.net/core/1.0a/#anchor46 $parameters = [ @@ -194,10 +191,8 @@ public function testSignatureIsGeneratedCorrectly($signature, $url) /** * @dataProvider provideInvalidData - * - * @param array $parameters */ - public function testThrowsExceptionIfRequiredParameterIsMissing($parameters) + public function testThrowsExceptionIfRequiredParameterIsMissing(array $parameters): void { $this->expectException(\RuntimeException::class); @@ -244,23 +239,23 @@ public function testGetLoginUrlWithStateQueryParameters(): void ); } - public function provideValidData(): array + public function provideValidData(): iterable { - return [ - ['iflJZCKxEsZ58FFDyCysxfLbuKM=', 'http://photos.example.net/photos'], - ['tR3+Ty81lMeYAr/Fid0kMTYa/WM=', 'http://photos.example.net/photos?file=vacation.jpg&size=original'], - ]; + yield 'simple' => ['iflJZCKxEsZ58FFDyCysxfLbuKM=', 'http://photos.example.net/photos']; + yield 'with additional data' => ['tR3+Ty81lMeYAr/Fid0kMTYa/WM=', 'http://photos.example.net/photos?file=vacation.jpg&size=original']; } - public function provideInvalidData(): array + public function provideInvalidData(): iterable { - return [ - ['oauth_timestamp' => '', 'oauth_nonce' => '', 'oauth_version' => '', 'oauth_signature_method' => ''], - ['oauth_consumer_key' => '', 'oauth_nonce' => '', 'oauth_version' => '', 'oauth_signature_method' => ''], - ['oauth_consumer_key' => '', 'oauth_timestamp' => '', 'oauth_version' => '', 'oauth_signature_method' => ''], - ['oauth_consumer_key' => '', 'oauth_timestamp' => '', 'oauth_nonce' => '', 'oauth_signature_method' => ''], - ['oauth_consumer_key' => '', 'oauth_timestamp' => '', 'oauth_nonce' => '', 'oauth_version' => ''], - ]; + yield 'missing "oauth_consumer_key"' => [['oauth_timestamp' => '', 'oauth_nonce' => '', 'oauth_version' => '', 'oauth_signature_method' => '']]; + + yield 'missing "oauth_timestamp"' => [['oauth_consumer_key' => '', 'oauth_nonce' => '', 'oauth_version' => '', 'oauth_signature_method' => '']]; + + yield 'missing "oauth_nonce"' => [['oauth_consumer_key' => '', 'oauth_timestamp' => '', 'oauth_version' => '', 'oauth_signature_method' => '']]; + + yield 'missing "oauth_version"' => [['oauth_consumer_key' => '', 'oauth_timestamp' => '', 'oauth_nonce' => '', 'oauth_signature_method' => '']]; + + yield 'missing "oauth_signature_method"' => [['oauth_consumer_key' => '', 'oauth_timestamp' => '', 'oauth_nonce' => '', 'oauth_version' => '']]; } private function getRequest(string $url): Request