From 407b9fc116d3a5baa1ce6d5bdaa7ff6c18773d29 Mon Sep 17 00:00:00 2001 From: Robin Chalas Date: Wed, 20 Dec 2023 19:49:00 +0100 Subject: [PATCH] Fix tests --- .github/workflows/unit-tests.yml | 26 ++++------- docs/index.md | 2 +- psalm.xml | 2 +- src/Command/CreateClientCommand.php | 9 ++-- src/Converter/UserConverter.php | 1 + src/DBAL/Type/ImplodedArray.php | 4 +- src/Entity/Scope.php | 2 +- .../AuthorizationRequestResolveEvent.php | 8 ++-- src/LeagueOAuth2ServerBundle.php | 4 ++ src/Manager/Null/AccessTokenManager.php | 3 +- src/Model/AbstractClient.php | 5 -- src/Model/ClientInterface.php | 18 ++++++++ src/Repository/ClientRepository.php | 6 +-- .../Authenticator/OAuth2Authenticator.php | 18 +++++++- .../EventListener/CheckScopeListener.php | 3 -- .../DoctrineCredentialsRevoker.php | 4 ++ tests/Acceptance/AbstractAcceptanceTest.php | 3 +- tests/Fixtures/User.php | 2 +- tests/TestKernel.php | 46 +++++++++++++------ 19 files changed, 105 insertions(+), 61 deletions(-) diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index e21d40a9..119b836d 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -14,25 +14,17 @@ jobs: strategy: fail-fast: false matrix: - #Stable supported versions - php: ['8.1', '8.2'] - symfony: ['5.4.*', '6.2.*'] - composer-flags: ['--prefer-stable'] - can-fail: [false] - exclude: - - php: '8.1' - symfony: '6.2.*' include: - # Lowest supported versions - - php: '8.1' - symfony: '5.4.*' - composer-flags: '--prefer-stable --prefer-lowest' + # Lowest Deps + - php: 8.1 + symfony: 5.4.* + composer-flags: '--prefer-stable' + can-fail: false + # Stable deps + - php: 8.2 + symfony: 6.3.* + composer-flags: '--prefer-stable' can-fail: false - # Development versions - - php: '8.3' - symfony: '6.3.x-dev' - composer-flags: '' - can-fail: true name: "PHP ${{ matrix.php }} - Symfony ${{ matrix.symfony }}${{ matrix.composer-flags != '' && format(' - Composer {0}', matrix.composer-flags) || '' }}" diff --git a/docs/index.md b/docs/index.md index 7ec5fd9b..6b293992 100644 --- a/docs/index.md +++ b/docs/index.md @@ -12,7 +12,7 @@ For implementation into Symfony projects, please see [bundle documentation](basi ## Requirements -* [PHP 7.2](http://php.net/releases/7_2_0.php) or greater +* [PHP 8.1](http://php.net/releases/8_1_0.php) or greater * [Symfony 5.4](https://symfony.com/roadmap/5.4) or greater ## Installation diff --git a/psalm.xml b/psalm.xml index 8424411a..d8554992 100644 --- a/psalm.xml +++ b/psalm.xml @@ -2,7 +2,7 @@ getArgument('name'); - - /** @var string $identifier */ - $identifier = $input->getArgument('identifier') ?? hash('md5', random_bytes(16)); - + $identifier = (string) $input->getArgument('identifier') ?: hash('md5', random_bytes(16)); $isPublic = $input->getOption('public'); if ($isPublic && null !== $input->getArgument('secret')) { throw new \InvalidArgumentException('The client cannot have a secret and be public.'); } - /** @var string $secret */ $secret = $isPublic ? null : $input->getArgument('secret') ?? hash('sha512', random_bytes(32)); /** @var AbstractClient $client */ diff --git a/src/Converter/UserConverter.php b/src/Converter/UserConverter.php index 24bab012..77c7ba05 100644 --- a/src/Converter/UserConverter.php +++ b/src/Converter/UserConverter.php @@ -12,6 +12,7 @@ final class UserConverter implements UserConverterInterface { /** * @psalm-suppress DeprecatedMethod + * @psalm-suppress UndefinedInterfaceMethod */ public function toLeague(?UserInterface $user): UserEntityInterface { diff --git a/src/DBAL/Type/ImplodedArray.php b/src/DBAL/Type/ImplodedArray.php index 5044dade..89d7ddae 100644 --- a/src/DBAL/Type/ImplodedArray.php +++ b/src/DBAL/Type/ImplodedArray.php @@ -20,7 +20,7 @@ abstract class ImplodedArray extends TextType /** * @psalm-suppress MixedArgumentTypeCoercion */ - public function convertToDatabaseValue($value, AbstractPlatform $platform): ?string + public function convertToDatabaseValue(mixed $value, AbstractPlatform $platform): ?string { if (!\is_array($value)) { throw new \LogicException('This type can only be used in combination with arrays.'); @@ -41,7 +41,7 @@ public function convertToDatabaseValue($value, AbstractPlatform $platform): ?str /** * @psalm-return list */ - public function convertToPHPValue($value, AbstractPlatform $platform): array + public function convertToPHPValue(mixed $value, AbstractPlatform $platform): array { if (null === $value) { return []; diff --git a/src/Entity/Scope.php b/src/Entity/Scope.php index 08a9c168..ad61965f 100644 --- a/src/Entity/Scope.php +++ b/src/Entity/Scope.php @@ -12,7 +12,7 @@ final class Scope implements ScopeEntityInterface use EntityTrait; #[\ReturnTypeWillChange] - public function jsonSerialize() + public function jsonSerialize(): mixed { return $this->getIdentifier(); } diff --git a/src/Event/AuthorizationRequestResolveEvent.php b/src/Event/AuthorizationRequestResolveEvent.php index dd431f29..cc1bb18f 100644 --- a/src/Event/AuthorizationRequestResolveEvent.php +++ b/src/Event/AuthorizationRequestResolveEvent.php @@ -4,7 +4,7 @@ namespace League\Bundle\OAuth2ServerBundle\Event; -use League\Bundle\OAuth2ServerBundle\Model\AbstractClient; +use League\Bundle\OAuth2ServerBundle\Model\ClientInterface; use League\Bundle\OAuth2ServerBundle\ValueObject\Scope; use League\OAuth2\Server\RequestTypes\AuthorizationRequest; use Symfony\Component\HttpFoundation\Response; @@ -27,7 +27,7 @@ final class AuthorizationRequestResolveEvent extends Event private $scopes; /** - * @var AbstractClient + * @var ClientInterface */ private $client; @@ -49,7 +49,7 @@ final class AuthorizationRequestResolveEvent extends Event /** * @param Scope[] $scopes */ - public function __construct(AuthorizationRequest $authorizationRequest, array $scopes, AbstractClient $client) + public function __construct(AuthorizationRequest $authorizationRequest, array $scopes, ClientInterface $client) { $this->authorizationRequest = $authorizationRequest; $this->scopes = $scopes; @@ -94,7 +94,7 @@ public function getGrantTypeId(): string /** * @psalm-mutation-free */ - public function getClient(): AbstractClient + public function getClient(): ClientInterface { return $this->client; } diff --git a/src/LeagueOAuth2ServerBundle.php b/src/LeagueOAuth2ServerBundle.php index 1f9b06a1..1e8baf08 100644 --- a/src/LeagueOAuth2ServerBundle.php +++ b/src/LeagueOAuth2ServerBundle.php @@ -33,6 +33,9 @@ public function getContainerExtension(): ExtensionInterface return new LeagueOAuth2ServerExtension(); } + /** + * @psalm-suppress UndefinedMethod + */ private function configureSecurityExtension(ContainerBuilder $container): void { /** @var SecurityExtension $extension */ @@ -61,6 +64,7 @@ private function configureDoctrineExtension(ContainerBuilder $container): void 'league.oauth2_server.persistence.doctrine.enabled' ) ); + $container->addCompilerPass(new EncryptionKeyPass()); } } diff --git a/src/Manager/Null/AccessTokenManager.php b/src/Manager/Null/AccessTokenManager.php index 1476e887..18db050c 100644 --- a/src/Manager/Null/AccessTokenManager.php +++ b/src/Manager/Null/AccessTokenManager.php @@ -6,6 +6,7 @@ use League\Bundle\OAuth2ServerBundle\Manager\AccessTokenManagerInterface; use League\Bundle\OAuth2ServerBundle\Model\AccessToken; +use League\Bundle\OAuth2ServerBundle\Model\AccessTokenInterface; final class AccessTokenManager implements AccessTokenManagerInterface { @@ -14,7 +15,7 @@ public function find(string $identifier): ?AccessToken return null; } - public function save(AccessToken $accessToken): void + public function save(AccessTokenInterface $accessToken): void { } diff --git a/src/Model/AbstractClient.php b/src/Model/AbstractClient.php index e98a9a0d..d653440f 100644 --- a/src/Model/AbstractClient.php +++ b/src/Model/AbstractClient.php @@ -114,11 +114,6 @@ public function setRedirectUris(RedirectUri ...$redirectUris): ClientInterface return $this; } - /** - * @return list - * - * @psalm-mutation-free - */ public function getGrants(): array { return $this->grants; diff --git a/src/Model/ClientInterface.php b/src/Model/ClientInterface.php index aa74ae95..c25d3753 100644 --- a/src/Model/ClientInterface.php +++ b/src/Model/ClientInterface.php @@ -8,20 +8,38 @@ use League\Bundle\OAuth2ServerBundle\ValueObject\RedirectUri; use League\Bundle\OAuth2ServerBundle\ValueObject\Scope; +/** + * @method string getName() + */ interface ClientInterface { public function getIdentifier(): string; public function getSecret(): ?string; + /** + * @return list + * + * @psalm-mutation-free + */ public function getRedirectUris(): array; public function setRedirectUris(RedirectUri ...$redirectUris): self; + /** + * @return list + * + * @psalm-mutation-free + */ public function getGrants(): array; public function setGrants(Grant ...$grants): self; + /** + * @return list + * + * @psalm-mutation-free + */ public function getScopes(): array; public function setScopes(Scope ...$scopes): self; diff --git a/src/Repository/ClientRepository.php b/src/Repository/ClientRepository.php index afb4867d..8e5d56a3 100644 --- a/src/Repository/ClientRepository.php +++ b/src/Repository/ClientRepository.php @@ -6,7 +6,7 @@ use League\Bundle\OAuth2ServerBundle\Entity\Client as ClientEntity; use League\Bundle\OAuth2ServerBundle\Manager\ClientManagerInterface; -use League\Bundle\OAuth2ServerBundle\Model\AbstractClient; +use League\Bundle\OAuth2ServerBundle\Model\ClientInterface; use League\OAuth2\Server\Repositories\ClientRepositoryInterface; final class ClientRepository implements ClientRepositoryInterface @@ -55,7 +55,7 @@ public function validateClient($clientIdentifier, $clientSecret, $grantType): bo return false; } - private function buildClientEntity(AbstractClient $client): ClientEntity + private function buildClientEntity(ClientInterface $client): ClientEntity { $clientEntity = new ClientEntity(); $clientEntity->setName($client->getName()); @@ -67,7 +67,7 @@ private function buildClientEntity(AbstractClient $client): ClientEntity return $clientEntity; } - private function isGrantSupported(AbstractClient $client, ?string $grant): bool + private function isGrantSupported(ClientInterface $client, ?string $grant): bool { if (null === $grant) { return true; diff --git a/src/Security/Authenticator/OAuth2Authenticator.php b/src/Security/Authenticator/OAuth2Authenticator.php index 68e4acfa..647f0d72 100644 --- a/src/Security/Authenticator/OAuth2Authenticator.php +++ b/src/Security/Authenticator/OAuth2Authenticator.php @@ -101,12 +101,16 @@ public function doAuthenticate(Request $request) /* : Passport */ /** @var string $oauthClientId */ $oauthClientId = $psr7Request->getAttribute('oauth_client_id', ''); + /** @psalm-suppress MixedInferredReturnType */ $userLoader = function (string $userIdentifier): UserInterface { if ('' === $userIdentifier) { return new NullUser(); } if (!method_exists($this->userProvider, 'loadUserByIdentifier')) { - /** @psalm-suppress DeprecatedMethod */ + /** + * @psalm-suppress DeprecatedMethod + * @psalm-suppress MixedReturnStatement + */ return $this->userProvider->loadUserByUsername($userIdentifier); } @@ -127,6 +131,9 @@ public function doAuthenticate(Request $request) /* : Passport */ * @return OAuth2Token * * @psalm-suppress DeprecatedInterface + * @psalm-suppress UndefinedClass + * @psalm-suppress MixedInferredReturnType + * @psalm-suppress RedundantCondition */ public function createAuthenticatedToken(PassportInterface $passport, string $firewallName): TokenInterface { @@ -135,6 +142,10 @@ public function createAuthenticatedToken(PassportInterface $passport, string $fi } $token = $this->createToken($passport, $firewallName); + /** + * @psalm-suppress TooManyArguments + * @psalm-suppress UndefinedMethod + */ $token->setAuthenticated(true); return $token; @@ -157,7 +168,10 @@ public function createToken(Passport $passport, string $firewallName): TokenInte $token = new OAuth2Token($passport->getUser(), $accessTokenId, $oauthClientId, $scopeBadge->getScopes(), $this->rolePrefix); if (method_exists(AuthenticatorInterface::class, 'createAuthenticatedToken') && !method_exists(AuthenticatorInterface::class, 'createToken')) { // symfony 5.4 only - /** @psalm-suppress TooManyArguments */ + /** + * @psalm-suppress TooManyArguments + * @psalm-suppress UndefinedMethod + */ $token->setAuthenticated(true, false); } diff --git a/src/Security/EventListener/CheckScopeListener.php b/src/Security/EventListener/CheckScopeListener.php index 9723f6c7..d0d285d5 100644 --- a/src/Security/EventListener/CheckScopeListener.php +++ b/src/Security/EventListener/CheckScopeListener.php @@ -9,7 +9,6 @@ use Symfony\Component\EventDispatcher\EventSubscriberInterface; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\RequestStack; -use Symfony\Component\Security\Http\Authenticator\Passport\Passport; use Symfony\Component\Security\Http\Event\CheckPassportEvent; /** @@ -29,8 +28,6 @@ public function __construct(RequestStack $requestStack) public function checkPassport(CheckPassportEvent $event): void { /** - * @var Passport $passport - * * @psalm-suppress DeprecatedInterface */ $passport = $event->getPassport(); diff --git a/src/Service/CredentialsRevoker/DoctrineCredentialsRevoker.php b/src/Service/CredentialsRevoker/DoctrineCredentialsRevoker.php index a3449bc9..429a2f61 100644 --- a/src/Service/CredentialsRevoker/DoctrineCredentialsRevoker.php +++ b/src/Service/CredentialsRevoker/DoctrineCredentialsRevoker.php @@ -33,9 +33,13 @@ public function __construct(EntityManagerInterface $entityManager, ClientManager /** * @psalm-suppress DeprecatedMethod + * @psalm-suppress UndefinedInterfaceMethod */ public function revokeCredentialsForUser(UserInterface $user): void { + /** + * @psalm-suppress MixedAssignment + */ $userIdentifier = method_exists($user, 'getUserIdentifier') ? $user->getUserIdentifier() : $user->getUsername(); $this->entityManager->createQueryBuilder() diff --git a/tests/Acceptance/AbstractAcceptanceTest.php b/tests/Acceptance/AbstractAcceptanceTest.php index 18624c28..23509d50 100644 --- a/tests/Acceptance/AbstractAcceptanceTest.php +++ b/tests/Acceptance/AbstractAcceptanceTest.php @@ -4,6 +4,7 @@ namespace League\Bundle\OAuth2ServerBundle\Tests\Acceptance; +use Doctrine\DBAL\Platforms\SqlitePlatform; use League\Bundle\OAuth2ServerBundle\Tests\TestHelper; use Symfony\Bundle\FrameworkBundle\Console\Application; use Symfony\Bundle\FrameworkBundle\KernelBrowser; @@ -30,7 +31,7 @@ protected function setUp(): void TestHelper::initializeDoctrineSchema($this->application); $connection = $this->client->getContainer()->get('database_connection'); - if ('sqlite' === $connection->getDatabasePlatform()->getName()) { + if ($connection->getDatabasePlatform() instanceof SqlitePlatform) { // https://www.sqlite.org/foreignkeys.html $connection->executeQuery('PRAGMA foreign_keys = ON'); } diff --git a/tests/Fixtures/User.php b/tests/Fixtures/User.php index 43702bf7..277db961 100644 --- a/tests/Fixtures/User.php +++ b/tests/Fixtures/User.php @@ -33,7 +33,7 @@ public function getUserIdentifier(): string return FixtureFactory::FIXTURE_USER; } - public function eraseCredentials() + public function eraseCredentials(): void { } } diff --git a/tests/TestKernel.php b/tests/TestKernel.php index 12abdec2..7423ce85 100644 --- a/tests/TestKernel.php +++ b/tests/TestKernel.php @@ -5,6 +5,7 @@ namespace League\Bundle\OAuth2ServerBundle\Tests; use Doctrine\DBAL\Platforms\SqlitePlatform; +use Doctrine\ORM\Configuration as OrmConfiguration; use League\Bundle\OAuth2ServerBundle\Manager\AccessTokenManagerInterface; use League\Bundle\OAuth2ServerBundle\Manager\AuthorizationCodeManagerInterface; use League\Bundle\OAuth2ServerBundle\Manager\ClientManagerInterface; @@ -13,14 +14,16 @@ use League\Bundle\OAuth2ServerBundle\Tests\Fixtures\FakeGrant; use League\Bundle\OAuth2ServerBundle\Tests\Fixtures\FixtureFactory; use League\Bundle\OAuth2ServerBundle\Tests\Fixtures\SecurityTestController; +use Symfony\Bundle\SecurityBundle\Security; use Symfony\Component\Config\Loader\LoaderInterface; use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\HttpKernel\Controller\ValueResolverInterface; use Symfony\Component\HttpKernel\Kernel; final class TestKernel extends Kernel implements CompilerPassInterface { - public function boot() + public function boot(): void { $this->initializeEnvironmentVariables(); @@ -47,29 +50,33 @@ public function getLogDir(): string return sprintf('%s/tests/.kernel/logs', $this->getProjectDir()); } - public function process(ContainerBuilder $container) + public function process(ContainerBuilder $container): void { $this->exposeManagerServices($container); } - public function registerContainerConfiguration(LoaderInterface $loader) + public function registerContainerConfiguration(LoaderInterface $loader): void { $loader->load(function (ContainerBuilder $container) { - $container->loadFromExtension('doctrine', [ + $doctrine = [ 'dbal' => [ - 'driver' => 'sqlite', + 'driver' => 'pdo_sqlite', 'charset' => 'utf8mb4', 'url' => 'sqlite:///:memory:', 'default_table_options' => [ 'charset' => 'utf8mb4', 'utf8mb4_unicode_ci' => 'utf8mb4_unicode_ci', ], - 'platform_service' => SqlitePlatform::class, ], - 'orm' => null, - ]); + ]; + + if (method_exists(OrmConfiguration::class, 'setLazyGhostObjectEnabled')) { + $doctrine['orm'] = ['enable_lazy_ghost_objects' => true]; + } - $container->loadFromExtension('framework', [ + $container->loadFromExtension('doctrine', $doctrine); + + $framework = [ 'secret' => 'nope', 'test' => null, 'router' => [ @@ -77,7 +84,15 @@ public function registerContainerConfiguration(LoaderInterface $loader) 'type' => 'php', 'utf8' => true, ], - ]); + 'http_method_override' => true, + 'php_errors' => ['log' => true], + ]; + + if (interface_exists(ValueResolverInterface::class)) { + $framework['handle_all_throwables'] = true; + } + + $container->loadFromExtension('framework', $framework); if (!$container->hasDefinition('kernel')) { $container->register('kernel', static::class) @@ -86,8 +101,7 @@ public function registerContainerConfiguration(LoaderInterface $loader) ->addTag('routing.route_loader'); } - $container->loadFromExtension('security', [ - 'enable_authenticator_manager' => true, + $security = [ 'firewalls' => [ 'test' => [ 'provider' => 'in_memory', @@ -116,7 +130,13 @@ public function registerContainerConfiguration(LoaderInterface $loader) ], ], ], - ]); + ]; + + if (!class_exists(Security::class)) { + $security['enable_authenticator_manager'] = true; + } + + $container->loadFromExtension('security', $security); $container->loadFromExtension('league_oauth2_server', [ 'authorization_server' => [