feature #196 Require a logged in user to resolve an authorization req…

…uest (ajgarlag) This PR was squashed before being merged into the 0.9-dev branch. Discussion ---------- Require a logged in user to resolve an authorization request Introduces BC breaks Fix #195 Commits ------- aeaa2d3 Require a logged in user to resolve an authorization request
thephpleague · Aug 26, 2024 · 44272ff · 44272ff
2 parents 978087a + aeaa2d3
commit 44272ff
Show file tree

Hide file tree

Showing 12 changed files with 122 additions and 146 deletions.
diff --git a/src/Converter/UserConverter.php b/src/Converter/UserConverter.php
@@ -10,34 +10,16 @@
 
 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 ArgumentTypeCoercion
      * @psalm-suppress DeprecatedMethod
      * @psalm-suppress UndefinedInterfaceMethod
      */
-    public function toLeague(?UserInterface $user): UserEntityInterface
+    public function toLeague(UserInterface $user): UserEntityInterface
     {
         $userEntity = new User();
-        if ($user instanceof UserInterface) {
-            $identifier = method_exists($user, 'getUserIdentifier') ? $user->getUserIdentifier() : $user->getUsername();
-            if ('' === $identifier) {
-                $identifier = $this->anonymousUserIdentifier;
-            }
-        } else {
-            $identifier = $this->anonymousUserIdentifier;
-        }
+
+        $identifier = method_exists($user, 'getUserIdentifier') ? $user->getUserIdentifier() : $user->getUsername();
 
         $userEntity->setIdentifier($identifier);
 

diff --git a/src/Converter/UserConverterInterface.php b/src/Converter/UserConverterInterface.php
@@ -9,5 +9,5 @@
 
 interface UserConverterInterface
 {
-    public function toLeague(?UserInterface $user): UserEntityInterface;
+    public function toLeague(UserInterface $user): UserEntityInterface;
 }
diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php
@@ -5,7 +5,6 @@
 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;
@@ -32,11 +31,6 @@ 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
@@ -8,7 +8,6 @@
 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;
@@ -69,9 +68,6 @@ 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');
 

diff --git a/src/Event/AuthorizationRequestResolveEvent.php b/src/Event/AuthorizationRequestResolveEvent.php
@@ -42,18 +42,19 @@ final class AuthorizationRequestResolveEvent extends Event
     private $response;
 
     /**
-     * @var UserInterface|null
+     * @var UserInterface
      */
     private $user;
 
     /**
      * @param Scope[] $scopes
      */
-    public function __construct(AuthorizationRequestInterface $authorizationRequest, array $scopes, ClientInterface $client)
+    public function __construct(AuthorizationRequestInterface $authorizationRequest, array $scopes, ClientInterface $client, UserInterface $user)
     {
         $this->authorizationRequest = $authorizationRequest;
         $this->scopes = $scopes;
         $this->client = $client;
+        $this->user = $user;
     }
 
     public function getAuthorizationResolution(): bool
@@ -102,18 +103,11 @@ public function getClient(): ClientInterface
     /**
      * @psalm-mutation-free
      */
-    public function getUser(): ?UserInterface
+    public function getUser(): UserInterface
     {
         return $this->user;
     }
 
-    public function setUser(?UserInterface $user): self
-    {
-        $this->user = $user;
-
-        return $this;
-    }
-
     /**
      * @return Scope[]
      */

diff --git a/src/Event/AuthorizationRequestResolveEventFactory.php b/src/Event/AuthorizationRequestResolveEventFactory.php
@@ -6,36 +6,31 @@
 
 use League\Bundle\OAuth2ServerBundle\Converter\ScopeConverterInterface;
 use League\Bundle\OAuth2ServerBundle\Manager\ClientManagerInterface;
-use League\OAuth2\Server\RequestTypes\AuthorizationRequestInterface;
+use Symfony\Bundle\SecurityBundle\Security;
+use Symfony\Component\Security\Core\Security as LegacySecurity;
 
-class AuthorizationRequestResolveEventFactory
-{
-    /**
-     * @var ScopeConverterInterface
-     */
-    private $scopeConverter;
-
-    /**
-     * @var ClientManagerInterface
-     */
-    private $clientManager;
-
-    public function __construct(ScopeConverterInterface $scopeConverter, ClientManagerInterface $clientManager)
+if (class_exists(Security::class)) {
+    final class AuthorizationRequestResolveEventFactory
     {
-        $this->scopeConverter = $scopeConverter;
-        $this->clientManager = $clientManager;
-    }
+        use AuthorizationRequestResolveEventFactoryTrait;
 
-    public function fromAuthorizationRequest(AuthorizationRequestInterface $authorizationRequest): AuthorizationRequestResolveEvent
+        public function __construct(ScopeConverterInterface $scopeConverter, ClientManagerInterface $clientManager, Security $security)
+        {
+            $this->scopeConverter = $scopeConverter;
+            $this->clientManager = $clientManager;
+            $this->security = $security;
+        }
+    }
+} else {
+    final class AuthorizationRequestResolveEventFactory
     {
-        $scopes = $this->scopeConverter->toDomainArray(array_values($authorizationRequest->getScopes()));
-
-        $client = $this->clientManager->find($authorizationRequest->getClient()->getIdentifier());
+        use AuthorizationRequestResolveEventFactoryTrait;
 
-        if (null === $client) {
-            throw new \RuntimeException(\sprintf('No client found for the given identifier \'%s\'.', $authorizationRequest->getClient()->getIdentifier()));
+        public function __construct(ScopeConverterInterface $scopeConverter, ClientManagerInterface $clientManager, LegacySecurity $security)
+        {
+            $this->scopeConverter = $scopeConverter;
+            $this->clientManager = $clientManager;
+            $this->security = $security;
         }
-
-        return new AuthorizationRequestResolveEvent($authorizationRequest, $scopes, $client);
     }
 }
diff --git a/src/Event/AuthorizationRequestResolveEventFactoryTrait.php b/src/Event/AuthorizationRequestResolveEventFactoryTrait.php
@@ -0,0 +1,50 @@
+<?php
+
+declare(strict_types=1);
+
+namespace League\Bundle\OAuth2ServerBundle\Event;
+
+use League\Bundle\OAuth2ServerBundle\Converter\ScopeConverterInterface;
+use League\Bundle\OAuth2ServerBundle\Manager\ClientManagerInterface;
+use League\OAuth2\Server\RequestTypes\AuthorizationRequestInterface;
+use Symfony\Bundle\SecurityBundle\Security;
+use Symfony\Component\Security\Core\Security as LegacySecurity;
+
+/**
+ * @internal
+ */
+trait AuthorizationRequestResolveEventFactoryTrait
+{
+    /**
+     * @var ScopeConverterInterface
+     */
+    private $scopeConverter;
+
+    /**
+     * @var ClientManagerInterface
+     */
+    private $clientManager;
+
+    /**
+     * @var Security|LegacySecurity
+     */
+    private $security;
+
+    public function fromAuthorizationRequest(AuthorizationRequestInterface $authorizationRequest): AuthorizationRequestResolveEvent
+    {
+        $scopes = $this->scopeConverter->toDomainArray(array_values($authorizationRequest->getScopes()));
+
+        $client = $this->clientManager->find($authorizationRequest->getClient()->getIdentifier());
+
+        if (null === $client) {
+            throw new \RuntimeException(\sprintf('No client found for the given identifier \'%s\'.', $authorizationRequest->getClient()->getIdentifier()));
+        }
+
+        $user = $this->security->getUser();
+        if (null === $user) {
+            throw new \RuntimeException('A logged in user is required to resolve the authorization request.');
+        }
+
+        return new AuthorizationRequestResolveEvent($authorizationRequest, $scopes, $client, $user);
+    }
+}
diff --git a/src/EventListener/AuthorizationRequestUserResolvingListener.php b/src/EventListener/AuthorizationRequestUserResolvingListener.php
diff --git a/src/EventListener/AuthorizationRequestUserResolvingListenerTrait.php b/src/EventListener/AuthorizationRequestUserResolvingListenerTrait.php
diff --git a/src/Resources/config/services.php b/src/Resources/config/services.php
@@ -22,7 +22,6 @@
 use League\Bundle\OAuth2ServerBundle\Converter\UserConverterInterface;
 use League\Bundle\OAuth2ServerBundle\Event\AuthorizationRequestResolveEventFactory;
 use League\Bundle\OAuth2ServerBundle\EventListener\AddClientDefaultScopesListener;
-use League\Bundle\OAuth2ServerBundle\EventListener\AuthorizationRequestUserResolvingListener;
 use League\Bundle\OAuth2ServerBundle\Manager\AccessTokenManagerInterface;
 use League\Bundle\OAuth2ServerBundle\Manager\AuthorizationCodeManagerInterface;
 use League\Bundle\OAuth2ServerBundle\Manager\ClientManagerInterface;
@@ -55,9 +54,11 @@
 use Nyholm\Psr7\Factory\Psr17Factory;
 use Symfony\Bridge\PsrHttpMessage\Factory\HttpFoundationFactory;
 use Symfony\Bridge\PsrHttpMessage\Factory\PsrHttpFactory;
+use Symfony\Bundle\SecurityBundle\Security;
 use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
 use Symfony\Component\HttpFoundation\RequestStack;
+use Symfony\Component\Security\Core\Security as LegacySecurity;
 
 return static function (ContainerConfigurator $container): void {
     $container->services()
@@ -206,18 +207,6 @@
             ->tag('controller.service_arguments')
         ->alias(AuthorizationController::class, 'league.oauth2_server.controller.authorization')
 
-        // Authorization listeners
-        ->set('league.oauth2_server.listener.authorization_request_user_resolving', AuthorizationRequestUserResolvingListener::class)
-            ->args([
-                service('security.helper'),
-            ])
-            ->tag('kernel.event_listener', [
-                'event' => OAuth2Events::AUTHORIZATION_REQUEST_RESOLVE,
-                'method' => 'onAuthorizationRequest',
-                'priority' => 1024,
-            ])
-        ->alias(AuthorizationRequestUserResolvingListener::class, 'league.oauth2_server.listener.authorization_request_user_resolving')
-
         // Token controller
         ->set('league.oauth2_server.controller.token', TokenController::class)
             ->args([
@@ -292,6 +281,7 @@
             ->args([
                 service(ScopeConverterInterface::class),
                 service(ClientManagerInterface::class),
+                service(class_exists(Security::class) ? Security::class : LegacySecurity::class),
             ])
         ->alias(AuthorizationRequestResolveEventFactory::class, 'league.oauth2_server.factory.authorization_request_resolve_event')