diff --git a/.gitignore b/.gitignore index b1a9bef1..73fda3e5 100644 --- a/.gitignore +++ b/.gitignore @@ -1,8 +1,12 @@ +phpstan.neon phpunit.xml Tests/autoload.php +var/ vendor/ Propel/om/ Propel/map/ composer.lock .php_cs.cache .phpunit.result.cache + +.idea/ diff --git a/Command/CleanCommand.php b/Command/CleanCommand.php index fdc4ef9f..ea1c8465 100644 --- a/Command/CleanCommand.php +++ b/Command/CleanCommand.php @@ -23,26 +23,18 @@ class CleanCommand extends Command { protected static $defaultName = 'fos:oauth-server:clean'; - private $accessTokenManager; - private $refreshTokenManager; - private $authCodeManager; - public function __construct( - TokenManagerInterface $accessTokenManager, - TokenManagerInterface $refreshTokenManager, - AuthCodeManagerInterface $authCodeManager + private TokenManagerInterface $accessTokenManager, + private TokenManagerInterface $refreshTokenManager, + private AuthCodeManagerInterface $authCodeManager ) { parent::__construct(); - - $this->accessTokenManager = $accessTokenManager; - $this->refreshTokenManager = $refreshTokenManager; - $this->authCodeManager = $authCodeManager; } /** * {@inheritdoc} */ - protected function configure() + protected function configure(): void { parent::configure(); @@ -64,9 +56,15 @@ protected function execute(InputInterface $input, OutputInterface $output): int { foreach ([$this->accessTokenManager, $this->refreshTokenManager, $this->authCodeManager] as $service) { $result = $service->deleteExpired(); - $output->writeln(sprintf('Removed %d items from %s storage.', $result, get_class($service))); + $output->writeln( + sprintf( + 'Removed %d items from %s storage.', + $result, + get_class($service) + ) + ); } - return 0; + return Command::SUCCESS; } } diff --git a/Command/CreateClientCommand.php b/Command/CreateClientCommand.php index 865fb680..e6bc3ce1 100644 --- a/Command/CreateClientCommand.php +++ b/Command/CreateClientCommand.php @@ -24,19 +24,16 @@ class CreateClientCommand extends Command { protected static $defaultName = 'fos:oauth-server:create-client'; - private $clientManager; - - public function __construct(ClientManagerInterface $clientManager) - { + public function __construct( + private ClientManagerInterface $clientManager + ) { parent::__construct(); - - $this->clientManager = $clientManager; } /** * {@inheritdoc} */ - protected function configure() + protected function configure(): void { parent::configure(); @@ -92,6 +89,6 @@ protected function execute(InputInterface $input, OutputInterface $output): int $io->table($headers, $rows); - return 0; + return Command::SUCCESS; } } diff --git a/Controller/AuthorizeController.php b/Controller/AuthorizeController.php index 69c10101..eced78ad 100644 --- a/Controller/AuthorizeController.php +++ b/Controller/AuthorizeController.php @@ -18,6 +18,7 @@ use FOS\OAuthServerBundle\Form\Handler\AuthorizeFormHandler; use FOS\OAuthServerBundle\Model\ClientInterface; use FOS\OAuthServerBundle\Model\ClientManagerInterface; +use RuntimeException; use OAuth2\OAuth2; use OAuth2\OAuth2ServerException; use Symfony\Component\EventDispatcher\EventDispatcherInterface; @@ -31,7 +32,7 @@ use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface; use Symfony\Component\Security\Core\Exception\AccessDeniedException; use Symfony\Component\Security\Core\User\UserInterface; -use Twig\Environment as TwigEnvironment; +use Twig\Environment; /** * Controller handling basic authorization. @@ -40,61 +41,6 @@ */ class AuthorizeController { - /** - * @var ClientInterface - */ - private $client; - - /** - * @var SessionInterface - */ - private $session; - - /** - * @var Form - */ - private $authorizeForm; - - /** - * @var AuthorizeFormHandler - */ - private $authorizeFormHandler; - - /** - * @var OAuth2 - */ - private $oAuth2Server; - - /** - * @var RequestStack - */ - private $requestStack; - - /** - * @var TokenStorageInterface - */ - private $tokenStorage; - - /** - * @var TwigEnvironment - */ - private $twig; - - /** - * @var UrlGeneratorInterface - */ - private $router; - - /** - * @var ClientManagerInterface - */ - private $clientManager; - - /** - * @var EventDispatcherInterface - */ - private $eventDispatcher; - /** * This controller had been made as a service due to support symfony 4 where all* services are private by default. * Thus, this is considered a bad practice to fetch services directly from container. @@ -104,16 +50,16 @@ class AuthorizeController * @param SessionInterface $session */ public function __construct( - RequestStack $requestStack, - Form $authorizeForm, - AuthorizeFormHandler $authorizeFormHandler, - OAuth2 $oAuth2Server, - TokenStorageInterface $tokenStorage, - UrlGeneratorInterface $router, - ClientManagerInterface $clientManager, - EventDispatcherInterface $eventDispatcher, - TwigEnvironment $twig, - SessionInterface $session = null + private RequestStack $requestStack, + private Form $authorizeForm, + private AuthorizeFormHandler $authorizeFormHandler, + private OAuth2 $oAuth2Server, + private TokenStorageInterface $tokenStorage, + private UrlGeneratorInterface $router, + private ClientManagerInterface $clientManager, + private EventDispatcherInterface $eventDispatcher, + private TwigEnvironment $twig, + private ?SessionInterface $session = null ) { $this->requestStack = $requestStack; $this->session = $session; @@ -130,7 +76,7 @@ public function __construct( /** * Authorize. */ - public function authorizeAction(Request $request) + public function authorizeAction(Request $request): Response { $user = $this->tokenStorage->getToken()->getUser(); @@ -165,10 +111,7 @@ public function authorizeAction(Request $request) ]); } - /** - * @return Response - */ - protected function processSuccess(UserInterface $user, AuthorizeFormHandler $formHandler, Request $request) + protected function processSuccess(UserInterface $user, AuthorizeFormHandler $formHandler, Request $request): Response { if ($this->session && true === $this->session->get('_fos_oauth_server.ensure_logout')) { $this->tokenStorage->setToken(null); @@ -194,17 +137,14 @@ protected function processSuccess(UserInterface $user, AuthorizeFormHandler $for /** * Generate the redirection url when the authorize is completed. * - * @return string */ - protected function getRedirectionUrl(UserInterface $user) + protected function getRedirectionUrl(UserInterface $user): string { return $this->router->generate('fos_oauth_server_profile_show'); } - /** - * @return ClientInterface - */ - protected function getClient() + + protected function getClient(): ClientInterface { if (null !== $this->client) { return $this->client; @@ -238,11 +178,11 @@ protected function renderAuthorize(array $context): Response /** * @return Request|null */ - private function getCurrentRequest() + private function getCurrentRequest(): ?Request { $request = $this->requestStack->getCurrentRequest(); if (null === $request) { - throw new \RuntimeException('No current request.'); + throw new RuntimeException('No current request.'); } return $request; diff --git a/Controller/TokenController.php b/Controller/TokenController.php index d5840276..1b863aaa 100644 --- a/Controller/TokenController.php +++ b/Controller/TokenController.php @@ -20,20 +20,12 @@ class TokenController { - /** - * @var OAuth2 - */ - protected $server; - public function __construct(OAuth2 $server) + public function __construct(Private OAuth2 $server) { - $this->server = $server; } - /** - * @return Response - */ - public function tokenAction(Request $request) + public function tokenAction(Request $request): Response { try { return $this->server->grantAccessToken($request); diff --git a/DependencyInjection/Configuration.php b/DependencyInjection/Configuration.php index 76e75bd9..86d8aa14 100644 --- a/DependencyInjection/Configuration.php +++ b/DependencyInjection/Configuration.php @@ -27,7 +27,7 @@ class Configuration implements ConfigurationInterface /** * {@inheritdoc} */ - public function getConfigTreeBuilder() + public function getConfigTreeBuilder(): TreeBuilder { $treeBuilder = new TreeBuilder('fos_oauth_server'); $rootNode = $treeBuilder->getRootNode(); diff --git a/DependencyInjection/Security/Factory/OAuthFactory.php b/DependencyInjection/Security/Factory/OAuthFactory.php index c638bcc3..c7dba350 100644 --- a/DependencyInjection/Security/Factory/OAuthFactory.php +++ b/DependencyInjection/Security/Factory/OAuthFactory.php @@ -13,6 +13,7 @@ namespace FOS\OAuthServerBundle\DependencyInjection\Security\Factory; +use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\AuthenticatorFactoryInterface; use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\SecurityFactoryInterface; use Symfony\Component\Config\Definition\Builder\NodeDefinition; use Symfony\Component\DependencyInjection\ChildDefinition; @@ -24,8 +25,25 @@ * * @author Arnaud Le Blanc */ -class OAuthFactory implements SecurityFactoryInterface +class OAuthFactory implements AuthenticatorFactoryInterface, SecurityFactoryInterface { + + /** + * {@inheritdoc} + */ + public function createAuthenticator(ContainerBuilder $container, string $id, array $config, string $userProviderId) + { + $providerId = 'fos_oauth_server.security.authentication.authenticator.'.$id; + $container + ->setDefinition($providerId, new ChildDefinition('fos_oauth_server.security.authentication.authenticator')) + ->replaceArgument(0, new Reference('fos_oauth_server.server')) + ->replaceArgument(1, new Reference('security.user_checker.'.$id)) + ->replaceArgument(2, new Reference($userProviderId)) + ; + + return $providerId; + } + /** * {@inheritdoc} */ diff --git a/Resources/config/doctrine/AccessToken.mongodb.xml b/Resources/config/doctrine/AccessToken.mongodb.xml index 9b02f5aa..a21d9d12 100644 --- a/Resources/config/doctrine/AccessToken.mongodb.xml +++ b/Resources/config/doctrine/AccessToken.mongodb.xml @@ -5,8 +5,8 @@ http://doctrine-project.org/schemas/odm/doctrine-mongo-mapping.xsd"> - - - + + + diff --git a/Resources/config/doctrine/AuthCode.mongodb.xml b/Resources/config/doctrine/AuthCode.mongodb.xml index b9104f83..993d8adb 100644 --- a/Resources/config/doctrine/AuthCode.mongodb.xml +++ b/Resources/config/doctrine/AuthCode.mongodb.xml @@ -5,9 +5,9 @@ http://doctrine-project.org/schemas/odm/doctrine-mongo-mapping.xsd"> - - - - + + + + diff --git a/Resources/config/doctrine/Client.mongodb.xml b/Resources/config/doctrine/Client.mongodb.xml index eb8fd726..8064f9bb 100644 --- a/Resources/config/doctrine/Client.mongodb.xml +++ b/Resources/config/doctrine/Client.mongodb.xml @@ -5,9 +5,9 @@ http://doctrine-project.org/schemas/odm/doctrine-mongo-mapping.xsd"> - - - - + + + + diff --git a/Resources/config/doctrine/RefreshToken.mongodb.xml b/Resources/config/doctrine/RefreshToken.mongodb.xml index 31399c10..000aa346 100644 --- a/Resources/config/doctrine/RefreshToken.mongodb.xml +++ b/Resources/config/doctrine/RefreshToken.mongodb.xml @@ -5,8 +5,8 @@ http://doctrine-project.org/schemas/odm/doctrine-mongo-mapping.xsd"> - - - + + + diff --git a/Resources/config/security.xml b/Resources/config/security.xml index ff0aae12..d2961b6d 100644 --- a/Resources/config/security.xml +++ b/Resources/config/security.xml @@ -5,12 +5,19 @@ xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd"> + FOS\OAuthServerBundle\Security\Authentication\Authenticator\OAuthAuthenticator FOS\OAuthServerBundle\Security\Authentication\Provider\OAuthProvider FOS\OAuthServerBundle\Security\Firewall\OAuthListener FOS\OAuthServerBundle\Security\EntryPoint\OAuthEntryPoint + + + + + + diff --git a/Security/Authentication/Authenticator/OAuthAuthenticator.php b/Security/Authentication/Authenticator/OAuthAuthenticator.php new file mode 100644 index 00000000..7cd501b9 --- /dev/null +++ b/Security/Authentication/Authenticator/OAuthAuthenticator.php @@ -0,0 +1,163 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace FOS\OAuthServerBundle\Security\Authentication\Authenticator; + +use FOS\OAuthServerBundle\Security\Authentication\Passport\OAuthCredentials; +use FOS\OAuthServerBundle\Security\Authentication\Token\OAuthToken; +use OAuth2\OAuth2; +use OAuth2\OAuth2AuthenticateException; +use OAuth2\OAuth2ServerException; +use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpFoundation\Response; +use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; +use Symfony\Component\Security\Core\Exception\AccountStatusException; +use Symfony\Component\Security\Core\Exception\AuthenticationException; +use Symfony\Component\Security\Core\User\UserCheckerInterface; +use Symfony\Component\Security\Core\User\UserInterface; +use Symfony\Component\Security\Core\User\UserProviderInterface; +use Symfony\Component\Security\Http\Authenticator\AuthenticatorInterface; +use Symfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge; +use Symfony\Component\Security\Http\Authenticator\Passport\Passport; +use Symfony\Component\Security\Http\Authenticator\Passport\PassportInterface; +use Symfony\Component\Security\Http\Authenticator\Passport\UserPassportInterface; + +/** + * OAuthAuthenticator class. + * + * @author Israel J. Carberry + */ +class OAuthAuthenticator implements AuthenticatorInterface +{ + public function __construct( + protected OAuth2 $serverService, + protected UserCheckerInterface $userChecker, + protected UserProviderInterface $userProvider + ) { + } + + /** + * {@inheritdoc} + */ + public function authenticate(Request $request): UserPassportInterface + { + // remove the authorization header from the request on this check + $tokenString = $this->serverService->getBearerToken($request, true); + $accessToken = $scope = $user = $username = null; + + try { + $accessToken = $this->serverService->verifyAccessToken($tokenString); + $scope = $accessToken->getScope(); + $user = $accessToken->getUser(); + // allow for dependency on deprecated getUsername method + $username = $user instanceof UserInterface + ? (method_exists($user, 'getUserIdentifier') ? $user->getUserIdentifier() : $user->getUsername()) + : null + ; + } catch (OAuth2AuthenticateException $e) { + // do nothing - credentials will remain unresolved below + } + + // configure the passport badges, ensuring requisite string types + $userBadge = new UserBadge($username ?? ''); + $credentials = new OAuthCredentials($tokenString ?? '', $scope ?? ''); + + // check the user if not null + if ($user instanceof UserInterface) { + try { + $this->userChecker->checkPreAuth($user); + + // mark the credentials as resolved + $credentials->markResolved(); + } catch (AccountStatusException $e) { + // do nothing - credentials remain unresolved + } + } + + // passport will only be valid if all badges are resolved (user badge + // is always resolved, credentials badge if passing the above check) + return new Passport($userBadge, $credentials); + } + + /** + * {@inheritdoc} + */ + public function createAuthenticatedToken(PassportInterface $passport, string $firewallName): TokenInterface + { + try { + // expect the badges in the passport from authenticate method above + if (!$passport->hasBadge(OAuthCredentials::class) + || !$passport->hasBadge(UserBadge::class) + ) { + throw new OAuth2AuthenticateException((string) Response::HTTP_UNAUTHORIZED, OAuth2::TOKEN_TYPE_BEARER, $this->serverService->getVariable(OAuth2::CONFIG_WWW_REALM), 'access_denied', 'Unexpected credentials type.'); + } + + // get the passport badges + $credentials = $passport->getBadge(OAuthCredentials::class); + $user = $this->userProvider->loadUserByIdentifier( + $passport->getBadge(UserBadge::class)->getUserIdentifier() + ); + + // check the user + try { + $this->userChecker->checkPostAuth($user); + } catch (AccountStatusException $e) { + throw new OAuth2AuthenticateException((string) Response::HTTP_UNAUTHORIZED, OAuth2::TOKEN_TYPE_BEARER, $this->serverService->getVariable(OAuth2::CONFIG_WWW_REALM), 'access_denied', $e->getMessage()); + } + } catch (OAuth2ServerException $e) { + throw new AuthenticationException('OAuth2 authentication failed', 0, $e); + } + + $token = new OAuthToken($credentials->getRoles($user)); + $token->setAuthenticated(true); + $token->setToken($credentials->getTokenString()); + $token->setUser($user); + + return $token; + } + + /** + * {@inheritdoc} + */ + public function onAuthenticationSuccess(Request $request, TokenInterface $token, string $firewallName): ?Response + { + return null; + } + + /** + * {@inheritdoc} + */ + public function onAuthenticationFailure(Request $request, AuthenticationException $exception): ?Response + { + $responseException = new OAuth2AuthenticateException( + (string) Response::HTTP_UNAUTHORIZED, + OAuth2::TOKEN_TYPE_BEARER, + $this->serverService->getVariable(OAuth2::CONFIG_WWW_REALM), + 'access_denied', + $exception->getMessage() + ); + + return $responseException->getHttpResponse(); + } + + /** + * {@inheritdoc} + */ + public function supports(Request $request): ?bool + { + // do not remove the authorization header from the request on this check + $tokenString = $this->serverService->getBearerToken($request); + + return is_string($tokenString) && !empty($tokenString); + } +} diff --git a/Security/Authentication/Passport/OAuthCredentials.php b/Security/Authentication/Passport/OAuthCredentials.php new file mode 100644 index 00000000..76c7b13a --- /dev/null +++ b/Security/Authentication/Passport/OAuthCredentials.php @@ -0,0 +1,65 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace FOS\OAuthServerBundle\Security\Authentication\Passport; + +use Symfony\Component\Security\Core\User\UserInterface; +use Symfony\Component\Security\Http\Authenticator\Passport\Credentials\CredentialsInterface; + +/** + * Implements credentials checking for an OAuth token. + * + * @author Israel J. Carberry + * + * @final + */ +class OAuthCredentials implements CredentialsInterface +{ + private bool $resolved = false; + + public function __construct( + private ?string $tokenString, + private string $scope, + ) { + } + + public function getRoles(UserInterface $user): array + { + $roles = $user->getRoles(); + + if (empty($this->scope)) { + return $roles; + } + + foreach (explode(' ', $this->scope) as $role) { + $roles[] = 'ROLE_'.mb_strtoupper($role); + } + + return array_unique($roles, SORT_REGULAR); + } + + public function getTokenString(): ?string + { + return $this->tokenString; + } + + public function markResolved(): void + { + $this->resolved = true; + } + + public function isResolved(): bool + { + return $this->resolved; + } +} diff --git a/Storage/OAuthStorage.php b/Storage/OAuthStorage.php index 5ab6b7c9..5ead512b 100644 --- a/Storage/OAuthStorage.php +++ b/Storage/OAuthStorage.php @@ -152,7 +152,7 @@ public function checkUserCredentials(IOAuth2Client $client, $username, $password } try { - $user = $this->userProvider->loadUserByUsername($username); + $user = $this->userProvider->loadUserByIdentifier($username); } catch (AuthenticationException $e) { return false; } diff --git a/Tests/Controller/AuthorizeControllerTest.php b/Tests/Controller/AuthorizeControllerTest.php index 04d2319a..a0bb9c70 100644 --- a/Tests/Controller/AuthorizeControllerTest.php +++ b/Tests/Controller/AuthorizeControllerTest.php @@ -236,13 +236,13 @@ public function testAuthorizeActionWillThrowAccessDeniedException(): void ; $this->tokenStorage - ->expects($this->at(0)) + ->expects($this->once()) ->method('getToken') ->willReturn($token) ; $token - ->expects($this->at(0)) + ->expects($this->once()) ->method('getUser') ->willReturn(null) ; @@ -261,19 +261,19 @@ public function testAuthorizeActionWillRenderTemplate(): void ; $this->tokenStorage - ->expects($this->at(0)) + ->expects($this->once()) ->method('getToken') ->willReturn($token) ; $token - ->expects($this->at(0)) + ->expects($this->once()) ->method('getUser') ->willReturn($this->user) ; $this->session - ->expects($this->at(0)) + ->expects($this->once()) ->method('get') ->with('_fos_oauth_server.ensure_logout') ->willReturn(false) @@ -297,14 +297,14 @@ public function testAuthorizeActionWillRenderTemplate(): void ; $this->authorizeFormHandler - ->expects($this->at(0)) + ->expects($this->once()) ->method('process') ->with() ->willReturn(false) ; $this->form - ->expects($this->at(0)) + ->expects($this->once()) ->method('createView') ->willReturn($this->formView) ; @@ -333,19 +333,19 @@ public function testAuthorizeActionWillFinishClientAuthorization(): void ; $this->tokenStorage - ->expects($this->at(0)) + ->expects($this->once()) ->method('getToken') ->willReturn($token) ; $token - ->expects($this->at(0)) + ->expects($this->once()) ->method('getUser') ->willReturn($this->user) ; $this->session - ->expects($this->at(0)) + ->expects($this->once()) ->method('get') ->with('_fos_oauth_server.ensure_logout') ->willReturn(false) @@ -371,7 +371,7 @@ public function testAuthorizeActionWillFinishClientAuthorization(): void $randomScope = 'scope'.\random_bytes(10); $this->request - ->expects($this->at(0)) + ->expects($this->once()) ->method('get') ->with('scope', null) ->willReturn($randomScope) @@ -380,7 +380,7 @@ public function testAuthorizeActionWillFinishClientAuthorization(): void $response = new Response(); $this->oAuth2Server - ->expects($this->at(0)) + ->expects($this->once()) ->method('finishClientAuthorization') ->with( true, @@ -402,33 +402,33 @@ public function testAuthorizeActionWillEnsureLogout(): void ; $this->tokenStorage - ->expects($this->at(0)) + ->expects($this->once()) ->method('getToken') ->willReturn($token) ; $token - ->expects($this->at(0)) + ->expects($this->once()) ->method('getUser') ->willReturn($this->user) ; $this->session - ->expects($this->at(0)) + ->expects($this->once()) ->method('get') ->with('_fos_oauth_server.ensure_logout') ->willReturn(true) ; $this->session - ->expects($this->at(1)) + ->expects($this->once()) ->method('invalidate') ->with(600) ->willReturn(true) ; $this->session - ->expects($this->at(2)) + ->expects($this->once()) ->method('set') ->with('_fos_oauth_server.ensure_logout', true) ->willReturn(null) @@ -452,14 +452,14 @@ public function testAuthorizeActionWillEnsureLogout(): void ; $this->authorizeFormHandler - ->expects($this->at(0)) + ->expects($this->once()) ->method('process') ->with() ->willReturn(false) ; $this->form - ->expects($this->at(0)) + ->expects($this->once()) ->method('createView') ->willReturn($this->formView) ; @@ -510,19 +510,14 @@ public function testAuthorizeActionWillProcessAuthorizationForm(): void $propertyReflection->setAccessible(true); $propertyReflection->setValue($this->instance, $this->client); - $this->eventDispatcher - ->expects($this->at(0)) - ->method('dispatch') - ->with(new PreAuthorizationEvent($this->user, $this->client)) - ->willReturn($this->preAuthorizationEvent) - ; - $this->preAuthorizationEvent ->expects($this->once()) ->method('isAuthorizedClient') ->willReturn(false) ; + $postAuthorizationEvent = new PostAuthorizationEvent($this->user, $this->client, true); + $this->authorizeFormHandler ->expects($this->once()) ->method('process') @@ -536,9 +531,16 @@ public function testAuthorizeActionWillProcessAuthorizationForm(): void ; $this->eventDispatcher - ->expects($this->at(1)) + ->expects($this->exactly(2)) ->method('dispatch') - ->with(new PostAuthorizationEvent($this->user, $this->client, true)) + ->withConsecutive( + [$this->equalTo(new PreAuthorizationEvent($this->user, $this->client))], + [$this->equalTo($postAuthorizationEvent)] + ) + ->willReturn( + $this->preAuthorizationEvent, + $postAuthorizationEvent + ) ; $formName = 'formName'.\random_bytes(10); diff --git a/Tests/DependencyInjection/Compiler/GrantExtensionsCompilerPassTest.php b/Tests/DependencyInjection/Compiler/GrantExtensionsCompilerPassTest.php index 81ab02d2..760897f4 100644 --- a/Tests/DependencyInjection/Compiler/GrantExtensionsCompilerPassTest.php +++ b/Tests/DependencyInjection/Compiler/GrantExtensionsCompilerPassTest.php @@ -169,7 +169,7 @@ public function testProcessWillFailIfUriIsEmpty(): void $exceptionMessage = 'Service "%s" must define the "uri" attribute on "fos_oauth_server.grant_extension" tags.'; - $idx = 0; + $addMethodCallWiths = []; foreach ($data as $id => $tags) { foreach ($tags as $tag) { if (empty($tag['uri'])) { @@ -177,20 +177,22 @@ public function testProcessWillFailIfUriIsEmpty(): void break; } - $storageDefinition - ->expects($this->at(++$idx)) - ->method('addMethodCall') - ->with( - 'setGrantExtension', - [ - $tag['uri'], - new Reference($id), - ] - ) - ; + $addMethodCallWiths[] = [ + $this->equalTo('setGrantExtension'), + $this->equalTo([ + $tag['uri'], + new Reference($id), + ]), + ]; } } + $storageDefinition + ->expects($this->exactly(count($addMethodCallWiths))) + ->method('addMethodCall') + ->withConsecutive(...$addMethodCallWiths) + ; + $this->expectException(InvalidArgumentException::class); $this->expectExceptionMessage($exceptionMessage); @@ -279,23 +281,26 @@ public function testProcess(): void ->willReturn($data) ; - $idx = 0; + $addMethodCallWiths = []; foreach ($data as $id => $tags) { foreach ($tags as $tag) { - $storageDefinition - ->expects($this->at(++$idx)) - ->method('addMethodCall') - ->with( - 'setGrantExtension', - [ - $tag['uri'], - new Reference($id), - ] - ) - ; + $addMethodCallWiths[] = [ + $this->equalTo('setGrantExtension'), + $this->equalTo([ + $tag['uri'], + new Reference($id), + ]), + $this->equalTo(false), + ]; } } + $storageDefinition + ->expects($this->exactly(count($addMethodCallWiths))) + ->method('addMethodCall') + ->withConsecutive(...$addMethodCallWiths) + ; + $this->assertNull($this->instance->process($container)); } } diff --git a/Tests/DependencyInjection/Security/Factory/OAuthFactoryTest.php b/Tests/DependencyInjection/Security/Factory/OAuthFactoryTest.php index 301e1712..22daf5a8 100644 --- a/Tests/DependencyInjection/Security/Factory/OAuthFactoryTest.php +++ b/Tests/DependencyInjection/Security/Factory/OAuthFactoryTest.php @@ -113,6 +113,64 @@ public function testCreate(): void ], $this->instance->create($container, $id, $config, $userProvider, $defaultEntryPoint)); } + public function testCreateAuthenticator(): void + { + $container = $this->getMockBuilder(ContainerBuilder::class) + ->disableOriginalConstructor() + ->setMethods([ + 'setDefinition', + ]) + ->getMock() + ; + $id = '12'; + $config = []; + $userProviderId = 'mock.user.provider.service'; + + $definition = $this->getMockBuilder(Definition::class) + ->disableOriginalConstructor() + ->getMock() + ; + + $container + ->expects($this->once()) + ->method('setDefinition') + ->with( + 'fos_oauth_server.security.authentication.authenticator.'.$id, + new ChildDefinition('fos_oauth_server.security.authentication.authenticator') + ) + ->will($this->returnValue($definition)) + ; + + $definition + ->expects($this->exactly(3)) + ->method('replaceArgument') + ->withConsecutive( + [ + 0, + new Reference('fos_oauth_server.server'), + ], + [ + 1, + new Reference('security.user_checker.'.$id), + ], + [ + 2, + new Reference($userProviderId), + ] + ) + ->willReturnOnConsecutiveCalls( + $definition, + $definition, + $definition + ) + ; + + $this->assertSame( + 'fos_oauth_server.security.authentication.authenticator.'.$id, + $this->instance->createAuthenticator($container, $id, $config, $userProviderId) + ); + } + public function testAddConfigurationDoesNothing(): void { $nodeDefinition = $this->getMockBuilder(NodeDefinition::class) diff --git a/Tests/Document/AuthCodeManagerTest.php b/Tests/Document/AuthCodeManagerTest.php index 1f0033ca..4ccb5a52 100644 --- a/Tests/Document/AuthCodeManagerTest.php +++ b/Tests/Document/AuthCodeManagerTest.php @@ -200,7 +200,7 @@ public function testDeleteExpired(): void $collection, [ 'type' => Query::TYPE_REMOVE, - 'query' => null, + 'query' => [], ], [], false diff --git a/Tests/Document/TokenManagerTest.php b/Tests/Document/TokenManagerTest.php index f7f6a667..02fbb8f0 100644 --- a/Tests/Document/TokenManagerTest.php +++ b/Tests/Document/TokenManagerTest.php @@ -196,7 +196,7 @@ public function testDeleteExpired(): void $collection, [ 'type' => Query::TYPE_REMOVE, - 'query' => null, + 'query' => [], ], [], false diff --git a/Tests/FOSOAuthServerBundleTest.php b/Tests/FOSOAuthServerBundleTest.php index d437c0bb..53f75235 100644 --- a/Tests/FOSOAuthServerBundleTest.php +++ b/Tests/FOSOAuthServerBundleTest.php @@ -47,21 +47,21 @@ public function testConstruction(): void ; $containerBuilder - ->expects($this->at(0)) + ->expects($this->once()) ->method('getExtension') ->with('security') ->willReturn($securityExtension) ; $securityExtension - ->expects($this->at(0)) + ->expects($this->once()) ->method('addSecurityListenerFactory') ->with(new OAuthFactory()) ->willReturn(null) ; $containerBuilder - ->expects($this->at(1)) + ->expects($this->exactly(2)) ->method('addCompilerPass') ->withConsecutive( [new Compiler\GrantExtensionsCompilerPass()], diff --git a/Tests/Form/Handler/AuthorizeFormHandlerTest.php b/Tests/Form/Handler/AuthorizeFormHandlerTest.php index 3c261c5a..168f048d 100644 --- a/Tests/Form/Handler/AuthorizeFormHandlerTest.php +++ b/Tests/Form/Handler/AuthorizeFormHandlerTest.php @@ -188,7 +188,7 @@ public function testGetCurrentRequestWillReturnRequestServiceFromContainerIfNone $randomData = \random_bytes(10); $this->container - ->expects($this->at(0)) + ->expects($this->once()) ->method('get') ->with('request') ->willReturn($randomData) diff --git a/Tests/Functional/TestBundle/Entity/AccessToken.php b/Tests/Functional/TestBundle/Entity/AccessToken.php index 78ffb576..af12896c 100644 --- a/Tests/Functional/TestBundle/Entity/AccessToken.php +++ b/Tests/Functional/TestBundle/Entity/AccessToken.php @@ -15,6 +15,8 @@ use Doctrine\ORM\Mapping as ORM; use FOS\OAuthServerBundle\Entity\AccessToken as BaseAccessToken; +use FOS\OAuthServerBundle\Model\ClientInterface; +use Symfony\Component\Security\Core\User\UserInterface; /** * @ORM\Entity @@ -32,11 +34,14 @@ class AccessToken extends BaseAccessToken /** * @ORM\ManyToOne(targetEntity="Client") * @ORM\JoinColumn(nullable=false) + * @var ClientInterface */ protected $client; /** * @ORM\ManyToOne(targetEntity="User") + * + * @var UserInterface */ protected $user; } diff --git a/Tests/Functional/TestBundle/Entity/AuthCode.php b/Tests/Functional/TestBundle/Entity/AuthCode.php index 0d654696..db55c06e 100644 --- a/Tests/Functional/TestBundle/Entity/AuthCode.php +++ b/Tests/Functional/TestBundle/Entity/AuthCode.php @@ -15,6 +15,8 @@ use Doctrine\ORM\Mapping as ORM; use FOS\OAuthServerBundle\Entity\AuthCode as BaseAuthCode; +use FOS\OAuthServerBundle\Model\ClientInterface; +use Symfony\Component\Security\Core\User\UserInterface; /** * @ORM\Entity @@ -32,11 +34,15 @@ class AuthCode extends BaseAuthCode /** * @ORM\ManyToOne(targetEntity="Client") * @ORM\JoinColumn(nullable=false) + * + * @var ClientInterface */ protected $client; /** * @ORM\ManyToOne(targetEntity="User") + * + * @var UserInterface */ protected $user; } diff --git a/Tests/Functional/TestBundle/Entity/RefreshToken.php b/Tests/Functional/TestBundle/Entity/RefreshToken.php index f8aadfa9..1b55b2c3 100644 --- a/Tests/Functional/TestBundle/Entity/RefreshToken.php +++ b/Tests/Functional/TestBundle/Entity/RefreshToken.php @@ -15,6 +15,8 @@ use Doctrine\ORM\Mapping as ORM; use FOS\OAuthServerBundle\Entity\RefreshToken as BaseRefreshToken; +use FOS\OAuthServerBundle\Model\ClientInterface; +use Symfony\Component\Security\Core\User\UserInterface; /** * @ORM\Entity @@ -32,11 +34,15 @@ class RefreshToken extends BaseRefreshToken /** * @ORM\ManyToOne(targetEntity="Client") * @ORM\JoinColumn(nullable=false) + * + * @var ClientInterface */ protected $client; /** * @ORM\ManyToOne(targetEntity="User") + * + * @var UserInterface */ protected $user; } diff --git a/Tests/Functional/TestCase.php b/Tests/Functional/TestCase.php index 3061ee51..7bacfaed 100644 --- a/Tests/Functional/TestCase.php +++ b/Tests/Functional/TestCase.php @@ -19,10 +19,6 @@ abstract class TestCase extends WebTestCase { - /** - * @var KernelInterface|null - */ - protected static $kernel; protected function setUp(): void { @@ -30,15 +26,10 @@ protected function setUp(): void $fs->remove(sys_get_temp_dir().'/FOSOAuthServerBundle/'); } - protected function tearDown(): void - { - static::$kernel = null; - } - /** * @param array $options */ - protected static function createKernel(array $options = []) + protected static function createKernel(array $options = []): KernelInterface { $env = @$options['env'] ?: 'test'; diff --git a/Tests/Storage/OAuthStorageTest.php b/Tests/Storage/OAuthStorageTest.php index ca70e609..47e76b6d 100644 --- a/Tests/Storage/OAuthStorageTest.php +++ b/Tests/Storage/OAuthStorageTest.php @@ -78,6 +78,12 @@ public function setUp(): void ; $this->userProvider = $this->getMockBuilder(UserProviderInterface::class) ->disableOriginalConstructor() + >setMethods([ + 'loadUserByIdentifier', + 'loadUserByUsername', + 'refreshUser', + 'supportsClass', + ]) ->getMock() ; $this->encoderFactory = $this->getMockBuilder(EncoderFactoryInterface::class) @@ -363,7 +369,7 @@ public function testCheckUserCredentialsCatchesAuthenticationExceptions(): void $this->userProvider ->expects(self::once()) - ->method('loadUserByUsername') + ->method('loadUserByIdentifier') ->with('Joe') ->willThrowException(new AuthenticationException('No such user')) ; @@ -396,7 +402,7 @@ public function testCheckUserCredentialsReturnsTrueOnValidCredentials(): void ; $this->userProvider->expects($this->once()) - ->method('loadUserByUsername') + ->method('loadUserByIdentifier') ->with('Joe') ->will($this->returnValue($user)) ; @@ -435,7 +441,7 @@ public function testCheckUserCredentialsReturnsFalseOnInvalidCredentials(): void ; $this->userProvider->expects($this->once()) - ->method('loadUserByUsername') + ->method('loadUserByIdentifier') ->with('Joe') ->will($this->returnValue($user)) ; @@ -454,7 +460,7 @@ public function testCheckUserCredentialsReturnsFalseIfUserNotExist(): void $client = new Client(); $this->userProvider->expects($this->once()) - ->method('loadUserByUsername') + ->method('loadUserByIdentifier') ->with('Joe') ->willThrowException(new AuthenticationException('No such user')) ; diff --git a/composer.json b/composer.json index 7fe0c669..442c072f 100644 --- a/composer.json +++ b/composer.json @@ -1,8 +1,14 @@ { - "name": "friendsofsymfony/oauth-server-bundle", - "description": "Symfony2 OAuth Server Bundle", + "name": "socloz/oauth-server-bundle", + "description": "Symfony OAuth Server Bundle", "license": "MIT", "type": "symfony-bundle", + "repositories": [ + { + "type": "git", + "url": "git@github.com:SoCloz/oauth2-php.git" + } + ], "keywords": [ "oauth", "oauth2", @@ -20,31 +26,28 @@ ], "homepage": "http://friendsofsymfony.github.com", "require": { - "php": "^7.2 || ^8.0", - "friendsofsymfony/oauth2-php": "~1.1", - "symfony/dependency-injection": "^4.4 || ^5.1", - "symfony/framework-bundle": "^4.4 || ^5.1", - "symfony/security-bundle": "^4.4 || ^5.1", - "symfony/twig-bundle": "^4.4 || ^5.1" + "php": "^8.1", + "socloz/oauth2-php": "~1.4", + "symfony/dependency-injection": "~5.3|~6.0", + "symfony/framework-bundle": "~5.3|~6.0", + "symfony/security-bundle": "~5.3|~6.0", + "symfony/twig-bundle": "~5.3|~6.0" }, "require-dev": { - "doctrine/doctrine-bundle": "^2.0", - "doctrine/mongodb-odm": "^2.2", + "doctrine/doctrine-bundle": "~2.0", + "doctrine/mongodb-odm": "~2.0", "doctrine/orm": "~2.2", + "friendsofphp/php-cs-fixer": "^3.0", "phing/phing": "~2.4", - "php-mock/php-mock-phpunit": "^2.5", - "phpstan/phpstan": "^0.12", - "phpstan/phpstan-phpunit": "~0.9", - "phpunit/phpunit": "^8.5.23 || ^9.0", - "symfony/console": "^4.4 || ^5.1", - "symfony/form": "^4.4 || ^5.1", - "symfony/http-kernel": "^4.4 || ^5.1", - "symfony/phpunit-bridge": "^4.4 || ^5.1", - "symfony/security-core": "^4.4 || ^5.1", - "symfony/yaml": "^4.4 || ^5.1" - }, - "conflict": { - "twig/twig": "<1.40 || >=2.0,<2.9" + "php-mock/php-mock-phpunit": "~1.0|~2.0", + "phpstan/phpstan-phpunit": "^0.12.20", + "phpunit/phpunit": "~6.0|~8.0", + "symfony/class-loader": "^2.0", + "symfony/console": "^5.3", + "symfony/form": "^5.3", + "symfony/phpunit-bridge": "^5.3", + "symfony/templating": "^5.3", + "symfony/yaml": "^5.3" }, "suggest": { "doctrine/doctrine-bundle": "*",