From 5a12ad5298260b7bc226395ca255df12dd185ef4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonio=20J=2E=20Garc=C3=ADa=20Lagar?= Date: Mon, 15 Apr 2024 09:22:30 +0200 Subject: [PATCH 01/10] Update league/oauth2-server requirement --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 84f36ea..bf3c157 100644 --- a/composer.json +++ b/composer.json @@ -20,7 +20,7 @@ "ext-openssl": "*", "doctrine/doctrine-bundle": "^2.8.0", "doctrine/orm": "^2.14|^3.0", - "league/oauth2-server": "^8.3", + "league/oauth2-server": "^9", "nyholm/psr7": "^1.4", "psr/http-factory": "^1.0", "symfony/event-dispatcher": "^5.4|^6.2|^7.0", From c92fa4bfcecfe134e7f9397ec29546d900734914 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonio=20J=2E=20Garc=C3=ADa=20Lagar?= Date: Mon, 15 Apr 2024 09:43:23 +0200 Subject: [PATCH 02/10] Fix method declaration compatibility --- src/Event/AuthorizationRequestResolveEventFactory.php | 4 ++-- src/Repository/AccessTokenRepository.php | 2 +- src/Repository/AuthCodeRepository.php | 5 +---- src/Repository/ClientRepository.php | 3 ++- src/Repository/RefreshTokenRepository.php | 2 +- src/Repository/ScopeRepository.php | 9 ++++----- tests/Fixtures/FakeGrant.php | 4 ++-- 7 files changed, 13 insertions(+), 16 deletions(-) diff --git a/src/Event/AuthorizationRequestResolveEventFactory.php b/src/Event/AuthorizationRequestResolveEventFactory.php index 07bc2a9..ee31342 100644 --- a/src/Event/AuthorizationRequestResolveEventFactory.php +++ b/src/Event/AuthorizationRequestResolveEventFactory.php @@ -6,7 +6,7 @@ use League\Bundle\OAuth2ServerBundle\Converter\ScopeConverterInterface; use League\Bundle\OAuth2ServerBundle\Manager\ClientManagerInterface; -use League\OAuth2\Server\RequestTypes\AuthorizationRequest; +use League\OAuth2\Server\RequestTypes\AuthorizationRequestInterface; class AuthorizationRequestResolveEventFactory { @@ -26,7 +26,7 @@ public function __construct(ScopeConverterInterface $scopeConverter, ClientManag $this->clientManager = $clientManager; } - public function fromAuthorizationRequest(AuthorizationRequest $authorizationRequest): AuthorizationRequestResolveEvent + public function fromAuthorizationRequest(AuthorizationRequestInterface $authorizationRequest): AuthorizationRequestResolveEvent { $scopes = $this->scopeConverter->toDomainArray(array_values($authorizationRequest->getScopes())); diff --git a/src/Repository/AccessTokenRepository.php b/src/Repository/AccessTokenRepository.php index ede9232..cc1d5d7 100644 --- a/src/Repository/AccessTokenRepository.php +++ b/src/Repository/AccessTokenRepository.php @@ -42,7 +42,7 @@ public function __construct( $this->scopeConverter = $scopeConverter; } - public function getNewToken(ClientEntityInterface $clientEntity, array $scopes, $userIdentifier = null) + public function getNewToken(ClientEntityInterface $clientEntity, array $scopes, mixed $userIdentifier = null): AccessTokenEntityInterface { /** @var int|string|null $userIdentifier */ $accessToken = new AccessTokenEntity(); diff --git a/src/Repository/AuthCodeRepository.php b/src/Repository/AuthCodeRepository.php index ce6e162..5b93b76 100644 --- a/src/Repository/AuthCodeRepository.php +++ b/src/Repository/AuthCodeRepository.php @@ -46,10 +46,7 @@ public function getNewAuthCode(): AuthCode return new AuthCode(); } - /** - * @return void - */ - public function persistNewAuthCode(AuthCodeEntityInterface $authCodeEntity) + public function persistNewAuthCode(AuthCodeEntityInterface $authCodeEntity): void { $authorizationCode = $this->authorizationCodeManager->find($authCodeEntity->getIdentifier()); diff --git a/src/Repository/ClientRepository.php b/src/Repository/ClientRepository.php index 8e5d56a..609fe92 100644 --- a/src/Repository/ClientRepository.php +++ b/src/Repository/ClientRepository.php @@ -7,6 +7,7 @@ use League\Bundle\OAuth2ServerBundle\Entity\Client as ClientEntity; use League\Bundle\OAuth2ServerBundle\Manager\ClientManagerInterface; use League\Bundle\OAuth2ServerBundle\Model\ClientInterface; +use League\OAuth2\Server\Entities\ClientEntityInterface; use League\OAuth2\Server\Repositories\ClientRepositoryInterface; final class ClientRepository implements ClientRepositoryInterface @@ -21,7 +22,7 @@ public function __construct(ClientManagerInterface $clientManager) $this->clientManager = $clientManager; } - public function getClientEntity($clientIdentifier) + public function getClientEntity($clientIdentifier): ?ClientEntityInterface { $client = $this->clientManager->find($clientIdentifier); diff --git a/src/Repository/RefreshTokenRepository.php b/src/Repository/RefreshTokenRepository.php index a81527d..d8ae288 100644 --- a/src/Repository/RefreshTokenRepository.php +++ b/src/Repository/RefreshTokenRepository.php @@ -32,7 +32,7 @@ public function __construct( $this->accessTokenManager = $accessTokenManager; } - public function getNewRefreshToken() + public function getNewRefreshToken(): RefreshTokenEntityInterface { return new RefreshTokenEntity(); } diff --git a/src/Repository/ScopeRepository.php b/src/Repository/ScopeRepository.php index e750b30..f3ab67d 100644 --- a/src/Repository/ScopeRepository.php +++ b/src/Repository/ScopeRepository.php @@ -52,7 +52,7 @@ public function __construct( $this->eventDispatcher = $eventDispatcher; } - public function getScopeEntityByIdentifier($identifier) + public function getScopeEntityByIdentifier($identifier): ?ScopeEntityInterface { $scope = $this->scopeManager->find($identifier); @@ -65,16 +65,15 @@ public function getScopeEntityByIdentifier($identifier) /** * @param ScopeEntityInterface[] $scopes - * @param string $grantType - * @param string|null $userIdentifier * * @return list */ public function finalizeScopes( array $scopes, - $grantType, + string $grantType, ClientEntityInterface $clientEntity, - $userIdentifier = null + string|int|null $userIdentifier = null, + ?string $authCodeId = null ): array { /** @var AbstractClient $client */ $client = $this->clientManager->find($clientEntity->getIdentifier()); diff --git a/tests/Fixtures/FakeGrant.php b/tests/Fixtures/FakeGrant.php index 1e2cf70..04216f4 100644 --- a/tests/Fixtures/FakeGrant.php +++ b/tests/Fixtures/FakeGrant.php @@ -12,12 +12,12 @@ final class FakeGrant extends AbstractGrant implements GrantTypeInterface { - public function getIdentifier() + public function getIdentifier(): string { return 'fake_grant'; } - public function respondToAccessTokenRequest(ServerRequestInterface $request, ResponseTypeInterface $responseType, \DateInterval $accessTokenTTL) + public function respondToAccessTokenRequest(ServerRequestInterface $request, ResponseTypeInterface $responseType, \DateInterval $accessTokenTTL): ResponseTypeInterface { return new Response(); } From 57f961f9fa8ccb6244bd55131940d781b915358a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonio=20J=2E=20Garc=C3=ADa=20Lagar?= Date: Mon, 22 Apr 2024 12:15:58 +0200 Subject: [PATCH 03/10] Fix types --- src/Event/AuthorizationRequestResolveEvent.php | 10 +++++----- src/Event/ScopeResolveEvent.php | 6 +++--- src/Repository/AccessTokenRepository.php | 10 ++-------- src/Repository/AuthCodeRepository.php | 4 ++-- src/Repository/ClientRepository.php | 4 ++-- src/Repository/NullAccessTokenRepository.php | 6 +++--- src/Repository/UserRepository.php | 6 +++--- 7 files changed, 20 insertions(+), 26 deletions(-) diff --git a/src/Event/AuthorizationRequestResolveEvent.php b/src/Event/AuthorizationRequestResolveEvent.php index cc1bb18..33d621a 100644 --- a/src/Event/AuthorizationRequestResolveEvent.php +++ b/src/Event/AuthorizationRequestResolveEvent.php @@ -6,7 +6,7 @@ use League\Bundle\OAuth2ServerBundle\Model\ClientInterface; use League\Bundle\OAuth2ServerBundle\ValueObject\Scope; -use League\OAuth2\Server\RequestTypes\AuthorizationRequest; +use League\OAuth2\Server\RequestTypes\AuthorizationRequestInterface; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\Security\Core\User\UserInterface; use Symfony\Contracts\EventDispatcher\Event; @@ -17,7 +17,7 @@ final class AuthorizationRequestResolveEvent extends Event public const AUTHORIZATION_DENIED = false; /** - * @var AuthorizationRequest + * @var AuthorizationRequestInterface */ private $authorizationRequest; @@ -49,7 +49,7 @@ final class AuthorizationRequestResolveEvent extends Event /** * @param Scope[] $scopes */ - public function __construct(AuthorizationRequest $authorizationRequest, array $scopes, ClientInterface $client) + public function __construct(AuthorizationRequestInterface $authorizationRequest, array $scopes, ClientInterface $client) { $this->authorizationRequest = $authorizationRequest; $this->scopes = $scopes; @@ -137,12 +137,12 @@ public function getState(): ?string return $this->authorizationRequest->getState(); } - public function getCodeChallenge(): string + public function getCodeChallenge(): ?string { return $this->authorizationRequest->getCodeChallenge(); } - public function getCodeChallengeMethod(): string + public function getCodeChallengeMethod(): ?string { return $this->authorizationRequest->getCodeChallengeMethod(); } diff --git a/src/Event/ScopeResolveEvent.php b/src/Event/ScopeResolveEvent.php index 9248d0f..129bbfb 100644 --- a/src/Event/ScopeResolveEvent.php +++ b/src/Event/ScopeResolveEvent.php @@ -27,14 +27,14 @@ final class ScopeResolveEvent extends Event private $client; /** - * @var string|null + * @var string|int|null */ private $userIdentifier; /** * @param list $scopes */ - public function __construct(array $scopes, Grant $grant, AbstractClient $client, ?string $userIdentifier) + public function __construct(array $scopes, Grant $grant, AbstractClient $client, string|int|null $userIdentifier) { $this->scopes = $scopes; $this->grant = $grant; @@ -68,7 +68,7 @@ public function getClient(): AbstractClient return $this->client; } - public function getUserIdentifier(): ?string + public function getUserIdentifier(): string|int|null { return $this->userIdentifier; } diff --git a/src/Repository/AccessTokenRepository.php b/src/Repository/AccessTokenRepository.php index cc1d5d7..5ea11b6 100644 --- a/src/Repository/AccessTokenRepository.php +++ b/src/Repository/AccessTokenRepository.php @@ -69,10 +69,7 @@ public function persistNewAccessToken(AccessTokenEntityInterface $accessTokenEnt $this->accessTokenManager->save($accessToken); } - /** - * @param string $tokenId - */ - public function revokeAccessToken($tokenId): void + public function revokeAccessToken(string $tokenId): void { $accessToken = $this->accessTokenManager->find($tokenId); @@ -85,10 +82,7 @@ public function revokeAccessToken($tokenId): void $this->accessTokenManager->save($accessToken); } - /** - * @param string $tokenId - */ - public function isAccessTokenRevoked($tokenId): bool + public function isAccessTokenRevoked(string $tokenId): bool { $accessToken = $this->accessTokenManager->find($tokenId); diff --git a/src/Repository/AuthCodeRepository.php b/src/Repository/AuthCodeRepository.php index 5b93b76..602d4fb 100644 --- a/src/Repository/AuthCodeRepository.php +++ b/src/Repository/AuthCodeRepository.php @@ -59,7 +59,7 @@ public function persistNewAuthCode(AuthCodeEntityInterface $authCodeEntity): voi $this->authorizationCodeManager->save($authorizationCode); } - public function revokeAuthCode($codeId): void + public function revokeAuthCode(string $codeId): void { $authorizationCode = $this->authorizationCodeManager->find($codeId); @@ -72,7 +72,7 @@ public function revokeAuthCode($codeId): void $this->authorizationCodeManager->save($authorizationCode); } - public function isAuthCodeRevoked($codeId): bool + public function isAuthCodeRevoked(string $codeId): bool { $authorizationCode = $this->authorizationCodeManager->find($codeId); diff --git a/src/Repository/ClientRepository.php b/src/Repository/ClientRepository.php index 609fe92..af84b73 100644 --- a/src/Repository/ClientRepository.php +++ b/src/Repository/ClientRepository.php @@ -22,7 +22,7 @@ public function __construct(ClientManagerInterface $clientManager) $this->clientManager = $clientManager; } - public function getClientEntity($clientIdentifier): ?ClientEntityInterface + public function getClientEntity(string $clientIdentifier): ?ClientEntityInterface { $client = $this->clientManager->find($clientIdentifier); @@ -33,7 +33,7 @@ public function getClientEntity($clientIdentifier): ?ClientEntityInterface return $this->buildClientEntity($client); } - public function validateClient($clientIdentifier, $clientSecret, $grantType): bool + public function validateClient(string $clientIdentifier, ?string $clientSecret, ?string $grantType): bool { $client = $this->clientManager->find($clientIdentifier); diff --git a/src/Repository/NullAccessTokenRepository.php b/src/Repository/NullAccessTokenRepository.php index d315c92..fe85c58 100644 --- a/src/Repository/NullAccessTokenRepository.php +++ b/src/Repository/NullAccessTokenRepository.php @@ -11,7 +11,7 @@ final class NullAccessTokenRepository implements AccessTokenRepositoryInterface { - public function getNewToken(ClientEntityInterface $clientEntity, array $scopes, $userIdentifier = null): AccessTokenEntityInterface + public function getNewToken(ClientEntityInterface $clientEntity, array $scopes, mixed $userIdentifier = null): AccessTokenEntityInterface { /** @var int|string|null $userIdentifier */ $accessToken = new AccessTokenEntity(); @@ -30,12 +30,12 @@ public function persistNewAccessToken(AccessTokenEntityInterface $accessTokenEnt // do nothing } - public function revokeAccessToken($tokenId): void + public function revokeAccessToken(string $tokenId): void { // do nothing } - public function isAccessTokenRevoked($tokenId): bool + public function isAccessTokenRevoked(string $tokenId): bool { return false; } diff --git a/src/Repository/UserRepository.php b/src/Repository/UserRepository.php index 7c1f604..5c841c7 100644 --- a/src/Repository/UserRepository.php +++ b/src/Repository/UserRepository.php @@ -43,9 +43,9 @@ public function __construct( } public function getUserEntityByUserCredentials( - $username, - $password, - $grantType, + string $username, + string $password, + string $grantType, ClientEntityInterface $clientEntity ): ?UserEntityInterface { /** @var AbstractClient $client */ From c2f5c0e3123198818073235660e78eea963004a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonio=20J=2E=20Garc=C3=ADa=20Lagar?= Date: Mon, 15 Apr 2024 09:45:03 +0200 Subject: [PATCH 04/10] Migrate to league/event:^3 --- src/Resources/config/services.php | 6 +++--- .../SymfonyLeagueEventListenerProvider.php | 18 ++++++++---------- 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/src/Resources/config/services.php b/src/Resources/config/services.php index 7de150b..2f44537 100644 --- a/src/Resources/config/services.php +++ b/src/Resources/config/services.php @@ -38,8 +38,8 @@ use League\Bundle\OAuth2ServerBundle\Security\Authenticator\OAuth2Authenticator; use League\Bundle\OAuth2ServerBundle\Security\EventListener\CheckScopeListener; use League\Bundle\OAuth2ServerBundle\Service\SymfonyLeagueEventListenerProvider; -use League\Event\Emitter; use League\OAuth2\Server\AuthorizationServer; +use League\OAuth2\Server\EventEmitting\EventEmitter; use League\OAuth2\Server\Grant\AuthCodeGrant; use League\OAuth2\Server\Grant\ClientCredentialsGrant; use League\OAuth2\Server\Grant\ImplicitGrant; @@ -129,8 +129,8 @@ ]) ->alias(SymfonyLeagueEventListenerProvider::class, 'league.oauth2_server.symfony_league_listener_provider') - ->set('league.oauth2_server.emitter', Emitter::class) - ->call('useListenerProvider', [service('league.oauth2_server.symfony_league_listener_provider')]) + ->set('league.oauth2_server.emitter', EventEmitter::class) + ->call('subscribeListenersFrom', [service('league.oauth2_server.symfony_league_listener_provider')]) ->set('league.oauth2_server.authorization_server.grant_configurator', GrantConfigurator::class) ->args([ diff --git a/src/Service/SymfonyLeagueEventListenerProvider.php b/src/Service/SymfonyLeagueEventListenerProvider.php index 640bf84..3062027 100644 --- a/src/Service/SymfonyLeagueEventListenerProvider.php +++ b/src/Service/SymfonyLeagueEventListenerProvider.php @@ -4,12 +4,12 @@ namespace League\Bundle\OAuth2ServerBundle\Service; -use League\Event\EventInterface; -use League\Event\ListenerAcceptorInterface; -use League\Event\ListenerProviderInterface; +use League\Event\ListenerRegistry; +use League\Event\ListenerSubscriber; +use League\OAuth2\Server\RequestEvent; use Symfony\Contracts\EventDispatcher\EventDispatcherInterface; -final class SymfonyLeagueEventListenerProvider implements ListenerProviderInterface +final class SymfonyLeagueEventListenerProvider implements ListenerSubscriber { /** * @var EventDispatcherInterface @@ -21,17 +21,15 @@ public function __construct(EventDispatcherInterface $eventDispatcher) $this->eventDispatcher = $eventDispatcher; } - public function provideListeners(ListenerAcceptorInterface $listenerAcceptor) + public function subscribeListeners(ListenerRegistry $acceptor): void { $listener = \Closure::fromCallable([$this, 'dispatchLeagueEventWithSymfonyEventDispatcher']); - $listenerAcceptor->addListener('*', $listener); - - return $this; + $acceptor->subscribeTo(RequestEvent::class, $listener); } - private function dispatchLeagueEventWithSymfonyEventDispatcher(EventInterface $event): void + private function dispatchLeagueEventWithSymfonyEventDispatcher(RequestEvent $event): void { - $this->eventDispatcher->dispatch($event, $event->getName()); + $this->eventDispatcher->dispatch($event, $event->eventName()); } } From d343b9a8ab671f8091cb44c2c8bfe0aa589179c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonio=20J=2E=20Garc=C3=ADa=20Lagar?= Date: Tue, 14 May 2024 14:48:26 +0200 Subject: [PATCH 05/10] Fix psalm --- src/Command/CreateClientCommand.php | 6 +++--- src/Command/ListClientsCommand.php | 6 +++--- src/Converter/UserConverter.php | 6 +++++- src/DBAL/Type/ImplodedArray.php | 3 ++- src/DBAL/Type/Scope.php | 2 +- src/EventListener/AddClientDefaultScopesListener.php | 4 ++-- src/Model/AbstractClient.php | 3 +++ src/Model/ClientInterface.php | 3 +++ src/Repository/AccessTokenRepository.php | 10 ++++------ src/Repository/AuthCodeRepository.php | 2 +- src/Repository/NullAccessTokenRepository.php | 7 ++++--- src/ValueObject/Scope.php | 6 +++++- 12 files changed, 36 insertions(+), 22 deletions(-) diff --git a/src/Command/CreateClientCommand.php b/src/Command/CreateClientCommand.php index 4a067b4..c8fed82 100644 --- a/src/Command/CreateClientCommand.php +++ b/src/Command/CreateClientCommand.php @@ -135,11 +135,11 @@ private function buildClientFromInput(InputInterface $input): ClientInterface $client->setActive(true); $client->setAllowPlainTextPkce($input->getOption('allow-plain-text-pkce')); - /** @var list $redirectUriStrings */ + /** @var list $redirectUriStrings */ $redirectUriStrings = $input->getOption('redirect-uri'); - /** @var list $grantStrings */ + /** @var list $grantStrings */ $grantStrings = $input->getOption('grant-type'); - /** @var list $scopeStrings */ + /** @var list $scopeStrings */ $scopeStrings = $input->getOption('scope'); return $client diff --git a/src/Command/ListClientsCommand.php b/src/Command/ListClientsCommand.php index 6e72047..b70833b 100644 --- a/src/Command/ListClientsCommand.php +++ b/src/Command/ListClientsCommand.php @@ -80,11 +80,11 @@ protected function execute(InputInterface $input, OutputInterface $output): int private function getFindByCriteria(InputInterface $input): ClientFilter { - /** @var list $grantStrings */ + /** @var list $grantStrings */ $grantStrings = $input->getOption('grant-type'); - /** @var list $redirectUriStrings */ + /** @var list $redirectUriStrings */ $redirectUriStrings = $input->getOption('redirect-uri'); - /** @var list $scopeStrings */ + /** @var list $scopeStrings */ $scopeStrings = $input->getOption('scope'); return ClientFilter::create() diff --git a/src/Converter/UserConverter.php b/src/Converter/UserConverter.php index 77c7ba0..2c95dce 100644 --- a/src/Converter/UserConverter.php +++ b/src/Converter/UserConverter.php @@ -18,7 +18,11 @@ public function toLeague(?UserInterface $user): UserEntityInterface { $userEntity = new User(); if ($user instanceof UserInterface) { - $userEntity->setIdentifier(method_exists($user, 'getUserIdentifier') ? $user->getUserIdentifier() : $user->getUsername()); + $identifier = method_exists($user, 'getUserIdentifier') ? $user->getUserIdentifier() : $user->getUsername(); + if ('' === $identifier) { + throw new \RuntimeException('Emtpy identifier not allowed'); + } + $userEntity->setIdentifier($identifier); } return $userEntity; diff --git a/src/DBAL/Type/ImplodedArray.php b/src/DBAL/Type/ImplodedArray.php index 89d7dda..8f39aaa 100644 --- a/src/DBAL/Type/ImplodedArray.php +++ b/src/DBAL/Type/ImplodedArray.php @@ -49,6 +49,7 @@ public function convertToPHPValue(mixed $value, AbstractPlatform $platform): arr \assert(\is_string($value), 'Expected $value of be either a string or null.'); + /** @var list $values */ $values = explode(self::VALUE_DELIMITER, $value); return $this->convertDatabaseValues($values); @@ -87,7 +88,7 @@ private function assertValueCanBeImploded($value): void } /** - * @param list $values + * @param list $values * * @return list */ diff --git a/src/DBAL/Type/Scope.php b/src/DBAL/Type/Scope.php index 3d4b23a..dbd3fb1 100644 --- a/src/DBAL/Type/Scope.php +++ b/src/DBAL/Type/Scope.php @@ -22,7 +22,7 @@ public function getName(): string } /** - * @param list $values + * @param list $values * * @return list */ diff --git a/src/EventListener/AddClientDefaultScopesListener.php b/src/EventListener/AddClientDefaultScopesListener.php index da7ccca..10949cf 100644 --- a/src/EventListener/AddClientDefaultScopesListener.php +++ b/src/EventListener/AddClientDefaultScopesListener.php @@ -15,12 +15,12 @@ class AddClientDefaultScopesListener { /** - * @var list + * @var list */ private $defaultScopes; /** - * @param list $defaultScopes + * @param list $defaultScopes */ public function __construct(array $defaultScopes) { diff --git a/src/Model/AbstractClient.php b/src/Model/AbstractClient.php index 5dca622..43b4a98 100644 --- a/src/Model/AbstractClient.php +++ b/src/Model/AbstractClient.php @@ -16,6 +16,7 @@ abstract class AbstractClient implements ClientInterface { private string $name; + /** @var non-empty-string */ protected string $identifier; private ?string $secret; @@ -33,6 +34,8 @@ abstract class AbstractClient implements ClientInterface /** * @psalm-mutation-free + * + * @param non-empty-string $identifier */ public function __construct(string $name, string $identifier, ?string $secret) { diff --git a/src/Model/ClientInterface.php b/src/Model/ClientInterface.php index c25d375..a4b318b 100644 --- a/src/Model/ClientInterface.php +++ b/src/Model/ClientInterface.php @@ -13,6 +13,9 @@ */ interface ClientInterface { + /** + * @return non-empty-string + */ public function getIdentifier(): string; public function getSecret(): ?string; diff --git a/src/Repository/AccessTokenRepository.php b/src/Repository/AccessTokenRepository.php index 5ea11b6..62f4a5a 100644 --- a/src/Repository/AccessTokenRepository.php +++ b/src/Repository/AccessTokenRepository.php @@ -42,12 +42,13 @@ public function __construct( $this->scopeConverter = $scopeConverter; } - public function getNewToken(ClientEntityInterface $clientEntity, array $scopes, mixed $userIdentifier = null): AccessTokenEntityInterface + public function getNewToken(ClientEntityInterface $clientEntity, array $scopes, ?string $userIdentifier = null): AccessTokenEntityInterface { - /** @var int|string|null $userIdentifier */ $accessToken = new AccessTokenEntity(); $accessToken->setClient($clientEntity); - $accessToken->setUserIdentifier($userIdentifier); + if (null !== $userIdentifier && '' !== $userIdentifier) { + $accessToken->setUserIdentifier($userIdentifier); + } foreach ($scopes as $scope) { $accessToken->addScope($scope); @@ -99,9 +100,6 @@ private function buildAccessTokenModel(AccessTokenEntityInterface $accessTokenEn $client = $this->clientManager->find($accessTokenEntity->getClient()->getIdentifier()); $userIdentifier = $accessTokenEntity->getUserIdentifier(); - if (null !== $userIdentifier) { - $userIdentifier = (string) $userIdentifier; - } return new AccessTokenModel( $accessTokenEntity->getIdentifier(), diff --git a/src/Repository/AuthCodeRepository.php b/src/Repository/AuthCodeRepository.php index 602d4fb..31d3e7e 100644 --- a/src/Repository/AuthCodeRepository.php +++ b/src/Repository/AuthCodeRepository.php @@ -90,7 +90,7 @@ private function buildAuthorizationCode(AuthCodeEntityInterface $authCodeEntity) $userIdentifier = $authCodeEntity->getUserIdentifier(); if (null !== $userIdentifier) { - $userIdentifier = (string) $userIdentifier; + $userIdentifier = $userIdentifier; } return new AuthorizationCode( diff --git a/src/Repository/NullAccessTokenRepository.php b/src/Repository/NullAccessTokenRepository.php index fe85c58..98e9911 100644 --- a/src/Repository/NullAccessTokenRepository.php +++ b/src/Repository/NullAccessTokenRepository.php @@ -11,12 +11,13 @@ final class NullAccessTokenRepository implements AccessTokenRepositoryInterface { - public function getNewToken(ClientEntityInterface $clientEntity, array $scopes, mixed $userIdentifier = null): AccessTokenEntityInterface + public function getNewToken(ClientEntityInterface $clientEntity, array $scopes, ?string $userIdentifier = null): AccessTokenEntityInterface { - /** @var int|string|null $userIdentifier */ $accessToken = new AccessTokenEntity(); $accessToken->setClient($clientEntity); - $accessToken->setUserIdentifier($userIdentifier); + if (null !== $userIdentifier && '' !== $userIdentifier) { + $accessToken->setUserIdentifier($userIdentifier); + } foreach ($scopes as $scope) { $accessToken->addScope($scope); diff --git a/src/ValueObject/Scope.php b/src/ValueObject/Scope.php index ae1a30e..d7e23d1 100644 --- a/src/ValueObject/Scope.php +++ b/src/ValueObject/Scope.php @@ -10,12 +10,14 @@ class Scope { /** - * @var string + * @var non-empty-string */ private $scope; /** * @psalm-mutation-free + * + * @param non-empty-string $scope */ public function __construct(string $scope) { @@ -24,6 +26,8 @@ public function __construct(string $scope) /** * @psalm-mutation-free + * + * @return non-empty-string */ public function __toString(): string { From 3c2c6d1ab3791b776a107bb2267fda4f35b10ca0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonio=20J=2E=20Garc=C3=ADa=20Lagar?= Date: Mon, 15 Apr 2024 09:45:18 +0200 Subject: [PATCH 06/10] Use `error_description` instead of `message` field See https://github.com/thephpleague/oauth2-server/pull/1375 --- .../Acceptance/AuthorizationEndpointTest.php | 8 +-- tests/Acceptance/TokenEndpointTest.php | 4 +- tests/Integration/AuthorizationServerTest.php | 62 +++++++++---------- 3 files changed, 37 insertions(+), 37 deletions(-) diff --git a/tests/Acceptance/AuthorizationEndpointTest.php b/tests/Acceptance/AuthorizationEndpointTest.php index a467a9f..f640e0a 100644 --- a/tests/Acceptance/AuthorizationEndpointTest.php +++ b/tests/Acceptance/AuthorizationEndpointTest.php @@ -158,7 +158,7 @@ public function testAuthCodeRequestWithPublicClientWithoutCodeChallengeWhenTheCh $jsonResponse = json_decode($response->getContent(), true); $this->assertSame('invalid_request', $jsonResponse['error']); - $this->assertSame('The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed.', $jsonResponse['message']); + $this->assertSame('The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed.', $jsonResponse['error_description']); $this->assertSame('Code challenge must be provided for public clients', $jsonResponse['hint']); } @@ -198,7 +198,7 @@ public function testAuthCodeRequestWithClientWhoIsNotAllowedToMakeARequestWithPl $jsonResponse = json_decode($response->getContent(), true); $this->assertSame('invalid_request', $jsonResponse['error']); - $this->assertSame('The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed.', $jsonResponse['message']); + $this->assertSame('The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed.', $jsonResponse['error_description']); $this->assertSame('Plain code challenge method is not allowed for this client', $jsonResponse['hint']); } @@ -425,7 +425,7 @@ public function testFailedCodeRequestRedirectWithFakedRedirectUri(): void $jsonResponse = json_decode($response->getContent(), true); $this->assertSame('invalid_client', $jsonResponse['error']); - $this->assertSame('Client authentication failed', $jsonResponse['message']); + $this->assertSame('Client authentication failed', $jsonResponse['error_description']); } public function testFailedAuthorizeRequest(): void @@ -443,7 +443,7 @@ public function testFailedAuthorizeRequest(): void $jsonResponse = json_decode($response->getContent(), true); $this->assertSame('unsupported_grant_type', $jsonResponse['error']); - $this->assertSame('The authorization grant type is not supported by the authorization server.', $jsonResponse['message']); + $this->assertSame('The authorization grant type is not supported by the authorization server.', $jsonResponse['error_description']); $this->assertSame('Check that all required parameters have been provided', $jsonResponse['hint']); } } diff --git a/tests/Acceptance/TokenEndpointTest.php b/tests/Acceptance/TokenEndpointTest.php index c24657a..5f58671 100644 --- a/tests/Acceptance/TokenEndpointTest.php +++ b/tests/Acceptance/TokenEndpointTest.php @@ -303,7 +303,7 @@ public function testFailedTokenRequest(): void $jsonResponse = json_decode($response->getContent(), true); $this->assertSame('unsupported_grant_type', $jsonResponse['error']); - $this->assertSame('The authorization grant type is not supported by the authorization server.', $jsonResponse['message']); + $this->assertSame('The authorization grant type is not supported by the authorization server.', $jsonResponse['error_description']); $this->assertSame('Check that all required parameters have been provided', $jsonResponse['hint']); } @@ -335,7 +335,7 @@ public function testFailedClientCredentialsTokenRequest(): void $jsonResponse = json_decode($response->getContent(), true); $this->assertSame('invalid_client', $jsonResponse['error']); - $this->assertSame('Client authentication failed', $jsonResponse['message']); + $this->assertSame('Client authentication failed', $jsonResponse['error_description']); $this->assertSame('bar', $response->headers->get('foo')); $this->assertTrue($wasClientAuthenticationEventDispatched); diff --git a/tests/Integration/AuthorizationServerTest.php b/tests/Integration/AuthorizationServerTest.php index afbe0ee..61aec75 100644 --- a/tests/Integration/AuthorizationServerTest.php +++ b/tests/Integration/AuthorizationServerTest.php @@ -65,7 +65,7 @@ public function testMissingAuthorizationCredentials(): void // Response assertions. $this->assertSame('invalid_request', $response['error']); - $this->assertSame('The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed.', $response['message']); + $this->assertSame('The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed.', $response['error_description']); $this->assertSame('Check the `client_id` parameter', $response['hint']); } @@ -79,7 +79,7 @@ public function testInvalidAuthorizationCredentials(): void // Response assertions. $this->assertSame('invalid_client', $response['error']); - $this->assertSame('Client authentication failed', $response['message']); + $this->assertSame('Client authentication failed', $response['error_description']); } public function testMissingClient(): void @@ -92,7 +92,7 @@ public function testMissingClient(): void // Response assertions. $this->assertSame('invalid_client', $response['error']); - $this->assertSame('Client authentication failed', $response['message']); + $this->assertSame('Client authentication failed', $response['error_description']); } public function testInactiveClient(): void @@ -105,7 +105,7 @@ public function testInactiveClient(): void // Response assertions. $this->assertSame('invalid_client', $response['error']); - $this->assertSame('Client authentication failed', $response['message']); + $this->assertSame('Client authentication failed', $response['error_description']); } public function testRestrictedGrantClient(): void @@ -118,7 +118,7 @@ public function testRestrictedGrantClient(): void // Response assertions. $this->assertSame('invalid_client', $response['error']); - $this->assertSame('Client authentication failed', $response['message']); + $this->assertSame('Client authentication failed', $response['error_description']); } public function testRestrictedScopeClient(): void @@ -132,7 +132,7 @@ public function testRestrictedScopeClient(): void // Response assertions. $this->assertSame('invalid_scope', $response['error']); - $this->assertSame('The requested scope is invalid, unknown, or malformed', $response['message']); + $this->assertSame('The requested scope is invalid, unknown, or malformed', $response['error_description']); $this->assertSame('Check the `fancy` scope', $response['hint']); } @@ -146,7 +146,7 @@ public function testInvalidGrantType(): void // Response assertions. $this->assertSame('unsupported_grant_type', $response['error']); - $this->assertSame('The authorization grant type is not supported by the authorization server.', $response['message']); + $this->assertSame('The authorization grant type is not supported by the authorization server.', $response['error_description']); $this->assertSame('Check that all required parameters have been provided', $response['hint']); } @@ -161,7 +161,7 @@ public function testInvalidScope(): void // Response assertions. $this->assertSame('invalid_scope', $response['error']); - $this->assertSame('The requested scope is invalid, unknown, or malformed', $response['message']); + $this->assertSame('The requested scope is invalid, unknown, or malformed', $response['error_description']); $this->assertSame('Check the `non_existing` scope', $response['hint']); } @@ -326,7 +326,7 @@ public function testMissingUsernameFieldPasswordGrant(): void // Response assertions. $this->assertSame('invalid_request', $response['error']); - $this->assertSame('The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed.', $response['message']); + $this->assertSame('The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed.', $response['error_description']); $this->assertSame('Check the `username` parameter', $response['hint']); } @@ -341,7 +341,7 @@ public function testMissingPasswordFieldPasswordGrant(): void // Response assertions. $this->assertSame('invalid_request', $response['error']); - $this->assertSame('The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed.', $response['message']); + $this->assertSame('The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed.', $response['error_description']); $this->assertSame('Check the `password` parameter', $response['hint']); } @@ -388,7 +388,7 @@ public function testDifferentClientRefreshGrant(): void // Response assertions. $this->assertSame('invalid_request', $response['error']); - $this->assertSame('The refresh token is invalid.', $response['message']); + $this->assertSame('The refresh token is invalid.', $response['error_description']); $this->assertSame('Token is not linked to client', $response['hint']); } @@ -406,7 +406,7 @@ public function testDifferentScopeRefreshGrant(): void // Response assertions. $this->assertSame('invalid_scope', $response['error']); - $this->assertSame('The requested scope is invalid, unknown, or malformed', $response['message']); + $this->assertSame('The requested scope is invalid, unknown, or malformed', $response['error_description']); $this->assertSame('Check the `rock` scope', $response['hint']); } @@ -423,7 +423,7 @@ public function testExpiredRefreshGrant(): void // Response assertions. $this->assertSame('invalid_request', $response['error']); - $this->assertSame('The refresh token is invalid.', $response['message']); + $this->assertSame('The refresh token is invalid.', $response['error_description']); $this->assertSame('Token has expired', $response['hint']); } @@ -440,7 +440,7 @@ public function testRevokedRefreshGrant(): void // Response assertions. $this->assertSame('invalid_request', $response['error']); - $this->assertSame('The refresh token is invalid.', $response['message']); + $this->assertSame('The refresh token is invalid.', $response['error_description']); $this->assertSame('Token has been revoked', $response['hint']); } @@ -454,7 +454,7 @@ public function testMissingPayloadRefreshGrant(): void // Response assertions. $this->assertSame('invalid_request', $response['error']); - $this->assertSame('The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed.', $response['message']); + $this->assertSame('The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed.', $response['error_description']); $this->assertSame('Check the `refresh_token` parameter', $response['hint']); } @@ -469,7 +469,7 @@ public function testInvalidPayloadRefreshGrant(): void // Response assertions. $this->assertSame('invalid_request', $response['error']); - $this->assertSame('The refresh token is invalid.', $response['message']); + $this->assertSame('The refresh token is invalid.', $response['error_description']); $this->assertSame('Cannot decrypt the refresh token', $response['hint']); } @@ -543,7 +543,7 @@ public function testCodeRequestWithInvalidScope(): void $this->assertStringStartsWith(FixtureFactory::FIXTURE_CLIENT_FIRST_REDIRECT_URI, $response->getHeaderLine('Location')); $queryData = $this->extractQueryDataFromUri($response->getHeaderLine('Location')); $this->assertSame('invalid_scope', $queryData['error']); - $this->assertSame('The requested scope is invalid, unknown, or malformed', $queryData['message']); + $this->assertSame('The requested scope is invalid, unknown, or malformed', $queryData['error_description']); $this->assertSame('Check the `non_existing` scope', $queryData['hint']); } @@ -561,7 +561,7 @@ public function testCodeRequestWithInvalidRedirectUri(): void $this->assertSame(401, $response->getStatusCode()); $responseData = json_decode((string) $response->getBody(), true); $this->assertSame('invalid_client', $responseData['error']); - $this->assertSame('Client authentication failed', $responseData['message']); + $this->assertSame('Client authentication failed', $responseData['error_description']); } public function testDeniedCodeRequest(): void @@ -579,7 +579,7 @@ public function testDeniedCodeRequest(): void $this->assertStringStartsWith(FixtureFactory::FIXTURE_CLIENT_FIRST_REDIRECT_URI, $response->getHeaderLine('Location')); $queryData = $this->extractQueryDataFromUri($response->getHeaderLine('Location')); $this->assertSame('access_denied', $queryData['error']); - $this->assertSame('The resource owner or authorization server denied the request.', $queryData['message']); + $this->assertSame('The resource owner or authorization server denied the request.', $queryData['error_description']); $this->assertSame('The user denied the request', $queryData['hint']); } @@ -596,7 +596,7 @@ public function testCodeRequestWithMissingClient(): void $this->assertSame(401, $response->getStatusCode()); $responseData = json_decode((string) $response->getBody(), true); $this->assertSame('invalid_client', $responseData['error']); - $this->assertSame('Client authentication failed', $responseData['message']); + $this->assertSame('Client authentication failed', $responseData['error_description']); } public function testCodeRequestWithInactiveClient(): void @@ -612,7 +612,7 @@ public function testCodeRequestWithInactiveClient(): void $this->assertSame(401, $response->getStatusCode()); $responseData = json_decode((string) $response->getBody(), true); $this->assertSame('invalid_client', $responseData['error']); - $this->assertSame('Client authentication failed', $responseData['message']); + $this->assertSame('Client authentication failed', $responseData['error_description']); } public function testCodeRequestWithRestrictedGrantClient(): void @@ -628,7 +628,7 @@ public function testCodeRequestWithRestrictedGrantClient(): void $this->assertSame(401, $response->getStatusCode()); $responseData = json_decode((string) $response->getBody(), true); $this->assertSame('invalid_client', $responseData['error']); - $this->assertSame('Client authentication failed', $responseData['message']); + $this->assertSame('Client authentication failed', $responseData['error_description']); } public function testSuccessfulAuthorizationWithCode(): void @@ -665,7 +665,7 @@ public function testFailedAuthorizationWithCodeForOtherClient(): void // Response assertions. $this->assertSame('invalid_request', $response['error']); - $this->assertSame('The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed.', $response['message']); + $this->assertSame('The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed.', $response['error_description']); $this->assertSame('Authorization code was not issued to this client', $response['hint']); } @@ -683,7 +683,7 @@ public function testFailedAuthorizationWithExpiredCode(): void // Response assertions. $this->assertSame('invalid_request', $response['error']); - $this->assertSame('The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed.', $response['message']); + $this->assertSame('The request is missing a required parameter, includes an invalid parameter value, includes a parameter more than once, or is otherwise malformed.', $response['error_description']); $this->assertSame('Authorization code has expired', $response['hint']); } @@ -701,7 +701,7 @@ public function testFailedAuthorizationWithInvalidRedirectUri(): void // Response assertions. $this->assertSame('invalid_client', $response['error']); - $this->assertSame('Client authentication failed', $response['message']); + $this->assertSame('Client authentication failed', $response['error_description']); } public function testSuccessfulImplicitRequest(): void @@ -786,7 +786,7 @@ public function testImplicitRequestWithInvalidScope(): void // Response assertions. $this->assertSame('invalid_scope', $responseData['error']); - $this->assertSame('The requested scope is invalid, unknown, or malformed', $responseData['message']); + $this->assertSame('The requested scope is invalid, unknown, or malformed', $responseData['error_description']); $this->assertSame('Check the `non_existing` scope', $responseData['hint']); } @@ -804,7 +804,7 @@ public function testImplicitRequestWithInvalidRedirectUri(): void // Response assertions. $this->assertSame('invalid_client', $responseData['error']); - $this->assertSame('Client authentication failed', $responseData['message']); + $this->assertSame('Client authentication failed', $responseData['error_description']); } public function testDeniedImplicitRequest(): void @@ -821,7 +821,7 @@ public function testDeniedImplicitRequest(): void // Response assertions. $this->assertSame('access_denied', $responseData['error']); - $this->assertSame('The resource owner or authorization server denied the request.', $responseData['message']); + $this->assertSame('The resource owner or authorization server denied the request.', $responseData['error_description']); $this->assertSame('The user denied the request', $responseData['hint']); } @@ -838,7 +838,7 @@ public function testImplicitRequestWithMissingClient(): void // Response assertions. $this->assertSame('invalid_client', $responseData['error']); - $this->assertSame('Client authentication failed', $responseData['message']); + $this->assertSame('Client authentication failed', $responseData['error_description']); } public function testImplicitRequestWithInactiveClient(): void @@ -854,7 +854,7 @@ public function testImplicitRequestWithInactiveClient(): void // Response assertions. $this->assertSame('invalid_client', $responseData['error']); - $this->assertSame('Client authentication failed', $responseData['message']); + $this->assertSame('Client authentication failed', $responseData['error_description']); } public function testImplicitRequestWithRestrictedGrantClient(): void @@ -870,6 +870,6 @@ public function testImplicitRequestWithRestrictedGrantClient(): void // Response assertions. $this->assertSame('invalid_client', $responseData['error']); - $this->assertSame('Client authentication failed', $responseData['message']); + $this->assertSame('Client authentication failed', $responseData['error_description']); } } From ee229be2f0af6d7673e84f3075c96c77f1aed0a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonio=20J=2E=20Garc=C3=ADa=20Lagar?= Date: Mon, 22 Apr 2024 10:24:02 +0200 Subject: [PATCH 07/10] Check for error `invalid_grant` instead of `invalid_request` * See https://github.com/thephpleague/oauth2-server/pull/1042 * See https://github.com/thephpleague/oauth2-server/pull/1082 --- tests/Integration/AuthorizationServerTest.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/Integration/AuthorizationServerTest.php b/tests/Integration/AuthorizationServerTest.php index 61aec75..1b148b2 100644 --- a/tests/Integration/AuthorizationServerTest.php +++ b/tests/Integration/AuthorizationServerTest.php @@ -387,7 +387,7 @@ public function testDifferentClientRefreshGrant(): void $response = $this->handleTokenRequest($request); // Response assertions. - $this->assertSame('invalid_request', $response['error']); + $this->assertSame('invalid_grant', $response['error']); $this->assertSame('The refresh token is invalid.', $response['error_description']); $this->assertSame('Token is not linked to client', $response['hint']); } @@ -422,7 +422,7 @@ public function testExpiredRefreshGrant(): void $response = $this->handleTokenRequest($request); // Response assertions. - $this->assertSame('invalid_request', $response['error']); + $this->assertSame('invalid_grant', $response['error']); $this->assertSame('The refresh token is invalid.', $response['error_description']); $this->assertSame('Token has expired', $response['hint']); } @@ -439,7 +439,7 @@ public function testRevokedRefreshGrant(): void $response = $this->handleTokenRequest($request); // Response assertions. - $this->assertSame('invalid_request', $response['error']); + $this->assertSame('invalid_grant', $response['error']); $this->assertSame('The refresh token is invalid.', $response['error_description']); $this->assertSame('Token has been revoked', $response['hint']); } @@ -468,7 +468,7 @@ public function testInvalidPayloadRefreshGrant(): void $response = $this->handleTokenRequest($request); // Response assertions. - $this->assertSame('invalid_request', $response['error']); + $this->assertSame('invalid_grant', $response['error']); $this->assertSame('The refresh token is invalid.', $response['error_description']); $this->assertSame('Cannot decrypt the refresh token', $response['hint']); } From 564cdb25ef97f22d50fe194eda470ac8ae9a79d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonio=20J=2E=20Garc=C3=ADa=20Lagar?= Date: Mon, 22 Apr 2024 10:32:23 +0200 Subject: [PATCH 08/10] Fix token to string conversion --- tests/TestHelper.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/TestHelper.php b/tests/TestHelper.php index 84156e3..b4babff 100644 --- a/tests/TestHelper.php +++ b/tests/TestHelper.php @@ -91,7 +91,7 @@ public static function generateJwtToken(AccessTokenModel $accessToken): string $accessTokenEntity->addScope($scopeEntity); } - return (string) $accessTokenEntity; + return $accessTokenEntity->toString(); } /** From 98651ddd90ae342a8cde32d2e71614c42740e0e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonio=20J=2E=20Garc=C3=ADa=20Lagar?= Date: Mon, 22 Apr 2024 10:32:09 +0200 Subject: [PATCH 09/10] Avoid setting null when an string is required --- tests/TestHelper.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/TestHelper.php b/tests/TestHelper.php index b4babff..8041cee 100644 --- a/tests/TestHelper.php +++ b/tests/TestHelper.php @@ -82,7 +82,7 @@ public static function generateJwtToken(AccessTokenModel $accessToken): string $accessTokenEntity->setIdentifier($accessToken->getIdentifier()); $accessTokenEntity->setExpiryDateTime($accessToken->getExpiry()); $accessTokenEntity->setClient($clientEntity); - $accessTokenEntity->setUserIdentifier($accessToken->getUserIdentifier()); + $accessTokenEntity->setUserIdentifier((string) $accessToken->getUserIdentifier()); foreach ($accessToken->getScopes() as $scope) { $scopeEntity = new ScopeEntity(); From d2ebd4fa3e8d9b9bb614f98fefd6cc48b4b41d3c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Antonio=20J=2E=20Garc=C3=ADa=20Lagar?= Date: Thu, 16 May 2024 12:02:08 +0200 Subject: [PATCH 10/10] Prevent empty identifier for user entity --- src/Converter/UserConverter.php | 20 +++++++++++++++++-- src/DependencyInjection/Configuration.php | 6 ++++++ .../LeagueOAuth2ServerExtension.php | 4 ++++ 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/src/Converter/UserConverter.php b/src/Converter/UserConverter.php index 2c95dce..757980a 100644 --- a/src/Converter/UserConverter.php +++ b/src/Converter/UserConverter.php @@ -10,6 +10,19 @@ final class UserConverter implements UserConverterInterface { + public const DEFAULT_ANONYMOUS_USER_IDENTIFIER = 'anonymous'; + + /** @var non-empty-string */ + private string $anonymousUserIdentifier; + + /** + * @param non-empty-string $anonymousUserIdentifier + */ + public function __construct(string $anonymousUserIdentifier = self::DEFAULT_ANONYMOUS_USER_IDENTIFIER) + { + $this->anonymousUserIdentifier = $anonymousUserIdentifier; + } + /** * @psalm-suppress DeprecatedMethod * @psalm-suppress UndefinedInterfaceMethod @@ -20,11 +33,14 @@ public function toLeague(?UserInterface $user): UserEntityInterface if ($user instanceof UserInterface) { $identifier = method_exists($user, 'getUserIdentifier') ? $user->getUserIdentifier() : $user->getUsername(); if ('' === $identifier) { - throw new \RuntimeException('Emtpy identifier not allowed'); + $identifier = $this->anonymousUserIdentifier; } - $userEntity->setIdentifier($identifier); + } else { + $identifier = $this->anonymousUserIdentifier; } + $userEntity->setIdentifier($identifier); + return $userEntity; } } diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index 3ab21af..ab6d4d4 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -5,6 +5,7 @@ namespace League\Bundle\OAuth2ServerBundle\DependencyInjection; use Defuse\Crypto\Key; +use League\Bundle\OAuth2ServerBundle\Converter\UserConverter; use League\Bundle\OAuth2ServerBundle\Model\AbstractClient; use League\Bundle\OAuth2ServerBundle\Model\Client; use Symfony\Component\Config\Definition\Builder\NodeDefinition; @@ -31,6 +32,11 @@ public function getConfigTreeBuilder(): TreeBuilder ->defaultValue('ROLE_OAUTH2_') ->cannotBeEmpty() ->end() + ->scalarNode('anonymous_user_identifier') + ->info('Set a default user identifier for anonymous users') + ->defaultValue(UserConverter::DEFAULT_ANONYMOUS_USER_IDENTIFIER) + ->cannotBeEmpty() + ->end() ->end(); return $treeBuilder; diff --git a/src/DependencyInjection/LeagueOAuth2ServerExtension.php b/src/DependencyInjection/LeagueOAuth2ServerExtension.php index 25e76df..0926a13 100644 --- a/src/DependencyInjection/LeagueOAuth2ServerExtension.php +++ b/src/DependencyInjection/LeagueOAuth2ServerExtension.php @@ -8,6 +8,7 @@ use League\Bundle\OAuth2ServerBundle\AuthorizationServer\GrantTypeInterface; use League\Bundle\OAuth2ServerBundle\Command\CreateClientCommand; use League\Bundle\OAuth2ServerBundle\Command\GenerateKeyPairCommand; +use League\Bundle\OAuth2ServerBundle\Converter\UserConverter; use League\Bundle\OAuth2ServerBundle\DBAL\Type\Grant as GrantType; use League\Bundle\OAuth2ServerBundle\DBAL\Type\RedirectUri as RedirectUriType; use League\Bundle\OAuth2ServerBundle\DBAL\Type\Scope as ScopeType; @@ -68,6 +69,9 @@ public function load(array $configs, ContainerBuilder $container) $container->findDefinition(OAuth2Authenticator::class) ->setArgument(3, $config['role_prefix']); + $container->findDefinition(UserConverter::class) + ->setArgument(0, $config['anonymous_user_identifier']); + $container->registerForAutoconfiguration(GrantTypeInterface::class) ->addTag('league.oauth2_server.authorization_server.grant');