Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add compatibility with league/oauth2-server:^9 #186

Merged
merged 1 commit into from
Aug 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
6 changes: 3 additions & 3 deletions src/Command/CreateClientCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -135,11 +135,11 @@ private function buildClientFromInput(InputInterface $input): ClientInterface
$client->setActive(true);
$client->setAllowPlainTextPkce($input->getOption('allow-plain-text-pkce'));

/** @var list<string> $redirectUriStrings */
/** @var list<non-empty-string> $redirectUriStrings */
$redirectUriStrings = $input->getOption('redirect-uri');
/** @var list<string> $grantStrings */
/** @var list<non-empty-string> $grantStrings */
$grantStrings = $input->getOption('grant-type');
/** @var list<string> $scopeStrings */
/** @var list<non-empty-string> $scopeStrings */
$scopeStrings = $input->getOption('scope');

return $client
Expand Down
6 changes: 3 additions & 3 deletions src/Command/ListClientsCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -80,11 +80,11 @@ protected function execute(InputInterface $input, OutputInterface $output): int

private function getFindByCriteria(InputInterface $input): ClientFilter
{
/** @var list<string> $grantStrings */
/** @var list<non-empty-string> $grantStrings */
$grantStrings = $input->getOption('grant-type');
/** @var list<string> $redirectUriStrings */
/** @var list<non-empty-string> $redirectUriStrings */
$redirectUriStrings = $input->getOption('redirect-uri');
/** @var list<string> $scopeStrings */
/** @var list<non-empty-string> $scopeStrings */
$scopeStrings = $input->getOption('scope');

return ClientFilter::create()
Expand Down
22 changes: 21 additions & 1 deletion src/Converter/UserConverter.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -18,9 +31,16 @@ 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) {
$identifier = $this->anonymousUserIdentifier;
}
} else {
$identifier = $this->anonymousUserIdentifier;
}

$userEntity->setIdentifier($identifier);

return $userEntity;
}
}
3 changes: 2 additions & 1 deletion src/DBAL/Type/ImplodedArray.php
Original file line number Diff line number Diff line change
Expand Up @@ -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<non-empty-string> $values */
$values = explode(self::VALUE_DELIMITER, $value);

return $this->convertDatabaseValues($values);
Expand Down Expand Up @@ -87,7 +88,7 @@ private function assertValueCanBeImploded($value): void
}

/**
* @param list<string> $values
* @param list<non-empty-string> $values
*
* @return list<T>
*/
Expand Down
2 changes: 1 addition & 1 deletion src/DBAL/Type/Scope.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public function getName(): string
}

/**
* @param list<string> $values
* @param list<non-empty-string> $values
*
* @return list<ScopeModel>
*/
Expand Down
6 changes: 6 additions & 0 deletions src/DependencyInjection/Configuration.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -31,6 +32,11 @@ public function getConfigTreeBuilder(): TreeBuilder
->defaultValue('ROLE_OAUTH2_')
->cannotBeEmpty()
->end()
->scalarNode('anonymous_user_identifier')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please remind me under which flow do we expect anonymous users? Wording-wise the anonymous concept has been removed in Symfony but I feel like this one is different and specific to this bundle, what's the case I'm missing?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The AuthorizationController does not require authentication (although it is highly recommended), so when the AuthorizationRequestResolveEvent is dispatched, there is a valid use case where the AuthorizationRequestUserResolvingListener cannot set the user instance in the event:

public function onAuthorizationRequest(AuthorizationRequestResolveEvent $event): void
{
$user = $this->security->getUser();
if ($user instanceof UserInterface) {
$event->setUser($user);
}
}

So the $event->getUser() call can return null with a resolved authorization

$authRequest->setUser($this->userConverter->toLeague($event->getUser()));

This is the use case I think we should disallow.

->info('Set a default user identifier for anonymous users')
->defaultValue(UserConverter::DEFAULT_ANONYMOUS_USER_IDENTIFIER)
->cannotBeEmpty()
->end()
->end();

return $treeBuilder;
Expand Down
4 changes: 4 additions & 0 deletions src/DependencyInjection/LeagueOAuth2ServerExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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');

Expand Down
10 changes: 5 additions & 5 deletions src/Event/AuthorizationRequestResolveEvent.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -17,7 +17,7 @@ final class AuthorizationRequestResolveEvent extends Event
public const AUTHORIZATION_DENIED = false;

/**
* @var AuthorizationRequest
* @var AuthorizationRequestInterface
*/
private $authorizationRequest;

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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();
}
Expand Down
4 changes: 2 additions & 2 deletions src/Event/AuthorizationRequestResolveEventFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand All @@ -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()));

Expand Down
6 changes: 3 additions & 3 deletions src/Event/ScopeResolveEvent.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,14 @@ final class ScopeResolveEvent extends Event
private $client;

/**
* @var string|null
* @var string|int|null
*/
private $userIdentifier;

/**
* @param list<Scope> $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;
Expand Down Expand Up @@ -68,7 +68,7 @@ public function getClient(): AbstractClient
return $this->client;
}

public function getUserIdentifier(): ?string
public function getUserIdentifier(): string|int|null
{
return $this->userIdentifier;
}
Expand Down
4 changes: 2 additions & 2 deletions src/EventListener/AddClientDefaultScopesListener.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,12 @@
class AddClientDefaultScopesListener
{
/**
* @var list<string>
* @var list<non-empty-string>
*/
private $defaultScopes;

/**
* @param list<string> $defaultScopes
* @param list<non-empty-string> $defaultScopes
*/
public function __construct(array $defaultScopes)
{
Expand Down
3 changes: 3 additions & 0 deletions src/Model/AbstractClient.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
abstract class AbstractClient implements ClientInterface
{
private string $name;
/** @var non-empty-string */
protected string $identifier;
private ?string $secret;

Expand All @@ -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)
{
Expand Down
3 changes: 3 additions & 0 deletions src/Model/ClientInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@
*/
interface ClientInterface
{
/**
* @return non-empty-string
*/
public function getIdentifier(): string;

public function getSecret(): ?string;
Expand Down
20 changes: 6 additions & 14 deletions src/Repository/AccessTokenRepository.php
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,13 @@ public function __construct(
$this->scopeConverter = $scopeConverter;
}

public function getNewToken(ClientEntityInterface $clientEntity, array $scopes, $userIdentifier = null)
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);
Expand All @@ -69,10 +70,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);

Expand All @@ -85,10 +83,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);

Expand All @@ -105,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(),
Expand Down
11 changes: 4 additions & 7 deletions src/Repository/AuthCodeRepository.php
Original file line number Diff line number Diff line change
Expand Up @@ -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());

Expand All @@ -62,7 +59,7 @@ public function persistNewAuthCode(AuthCodeEntityInterface $authCodeEntity)
$this->authorizationCodeManager->save($authorizationCode);
}

public function revokeAuthCode($codeId): void
public function revokeAuthCode(string $codeId): void
{
$authorizationCode = $this->authorizationCodeManager->find($codeId);

Expand All @@ -75,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);

Expand All @@ -93,7 +90,7 @@ private function buildAuthorizationCode(AuthCodeEntityInterface $authCodeEntity)

$userIdentifier = $authCodeEntity->getUserIdentifier();
if (null !== $userIdentifier) {
$userIdentifier = (string) $userIdentifier;
$userIdentifier = $userIdentifier;
}

return new AuthorizationCode(
Expand Down
5 changes: 3 additions & 2 deletions src/Repository/ClientRepository.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -21,7 +22,7 @@ public function __construct(ClientManagerInterface $clientManager)
$this->clientManager = $clientManager;
}

public function getClientEntity($clientIdentifier)
public function getClientEntity(string $clientIdentifier): ?ClientEntityInterface
{
$client = $this->clientManager->find($clientIdentifier);

Expand All @@ -32,7 +33,7 @@ public function getClientEntity($clientIdentifier)
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);

Expand Down
Loading
Loading