Delegate user credential checking to PasswordChecker

DependencyInjection/Configuration.php

@@ -121,6 +121,7 @@ private function addServiceSection(ArrayNodeDefinition $node)
                     ->addDefaultsIfNotSet()
                         ->children()
                             ->scalarNode('storage')->defaultValue('fos_oauth_server.storage.default')->cannotBeEmpty()->end()
+                            ->scalarNode('password_checker')->defaultValue('fos_oauth_server.password_checker.default')->cannotBeEmpty()->end()
                             ->scalarNode('user_provider')->defaultNull()->end()
                             ->scalarNode('client_manager')->defaultValue('fos_oauth_server.client_manager.default')->end()
                             ->scalarNode('access_token_manager')->defaultValue('fos_oauth_server.access_token_manager.default')->end()

DependencyInjection/FOSOAuthServerExtension.php

@@ -46,6 +46,7 @@ public function load(array $configs, ContainerBuilder $container)
         }
 
         $container->setAlias('fos_oauth_server.storage', $config['service']['storage']);
+        $container->setAlias('fos_oauth_server.password_checker', $config['service']['password_checker']);
         $container->setAlias('fos_oauth_server.client_manager', $config['service']['client_manager']);
         $container->setAlias('fos_oauth_server.access_token_manager', $config['service']['access_token_manager']);
         $container->setAlias('fos_oauth_server.refresh_token_manager', $config['service']['refresh_token_manager']);

Resources/config/oauth.xml

@@ -14,7 +14,11 @@
             <argument type="service" id="fos_oauth_server.access_token_manager" />
             <argument type="service" id="fos_oauth_server.refresh_token_manager" />
             <argument type="service" id="fos_oauth_server.auth_code_manager" />
+            <argument type="service" id="fos_oauth_server.password_checker" />
             <argument type="service" id="fos_oauth_server.user_provider" on-invalid="null" />
+        </service>
+
+        <service id="fos_oauth_server.password_checker.default" class="FOS\OAuthServerBundle\Storage\PasswordChecker" public="false">
             <argument type="service" id="security.encoder_factory" />
         </service>
 

Storage/OAuthStorage.php

@@ -28,7 +28,6 @@
 use OAuth2\OAuth2;
 use OAuth2\OAuth2ServerException;
 use Symfony\Component\HttpFoundation\Response;
-use Symfony\Component\Security\Core\Encoder\EncoderFactoryInterface;
 use Symfony\Component\Security\Core\Exception\AuthenticationException;
 use Symfony\Component\Security\Core\User\UserProviderInterface;
 
@@ -60,9 +59,9 @@ class OAuthStorage implements IOAuth2RefreshTokens, IOAuth2GrantUser, IOAuth2Gra
     protected $userProvider;
 
     /**
-     * @var EncoderFactoryInterface
+     * @var PasswordCheckerInterface
      */
-    protected $encoderFactory;
+    protected $passwordChecker;
 
     /**
      * @var array [uri] => GrantExtensionInterface
@@ -74,19 +73,19 @@ class OAuthStorage implements IOAuth2RefreshTokens, IOAuth2GrantUser, IOAuth2Gra
      * @param AccessTokenManagerInterface  $accessTokenManager
      * @param RefreshTokenManagerInterface $refreshTokenManager
      * @param AuthCodeManagerInterface     $authCodeManager
+     * @param PasswordCheckerInterface     $passwordChecker
      * @param null|UserProviderInterface   $userProvider
-     * @param null|EncoderFactoryInterface $encoderFactory
      */
     public function __construct(ClientManagerInterface $clientManager, AccessTokenManagerInterface $accessTokenManager,
         RefreshTokenManagerInterface $refreshTokenManager, AuthCodeManagerInterface $authCodeManager,
-        UserProviderInterface $userProvider = null, EncoderFactoryInterface $encoderFactory = null)
+        PasswordCheckerInterface $passwordChecker, UserProviderInterface $userProvider = null)
     {
         $this->clientManager = $clientManager;
         $this->accessTokenManager = $accessTokenManager;
         $this->refreshTokenManager = $refreshTokenManager;
         $this->authCodeManager = $authCodeManager;
+        $this->passwordChecker = $passwordChecker;
         $this->userProvider = $userProvider;
-        $this->encoderFactory = $encoderFactory;
 
         $this->grantExtensions = [];
     }
@@ -165,8 +164,7 @@ public function checkUserCredentials(IOAuth2Client $client, $username, $password
             return false;
         }
 
-        $encoder = $this->encoderFactory->getEncoder($user);
-        if ($encoder->isPasswordValid($user->getPassword(), $password, $user->getSalt())) {
+        if ($this->passwordChecker->validate($user, $password)) {
             return [
                 'data' => $user,
             ];

Storage/PasswordChecker.php

@@ -0,0 +1,43 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the FOSOAuthServerBundle package.
+ *
+ * (c) FriendsOfSymfony <http://friendsofsymfony.github.com/>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace FOS\OAuthServerBundle\Storage;
+
+use Symfony\Component\Security\Core\Encoder\EncoderFactoryInterface;
+use Symfony\Component\Security\Core\User\UserInterface;
+
+class PasswordChecker implements PasswordCheckerInterface
+{
+    /**
+     * @var EncoderFactoryInterface
+     */
+    protected $encoderFactory;
+
+    /**
+     * @param EncoderFactoryInterface $encoderFactory
+     */
+    public function __construct(EncoderFactoryInterface $encoderFactory)
+    {
+        $this->encoderFactory = $encoderFactory;
+    }
+
+    /**
+     * {@inheritDoc}
+     */
+    public function validate(UserInterface $user, $password): bool
+    {
+        $encoder = $this->encoderFactory->getEncoder($user);
+
+        return $encoder->isPasswordValid($user->getPassword(), $password, $user->getSalt());
+    }
+}

Storage/PasswordCheckerInterface.php

@@ -0,0 +1,29 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the FOSOAuthServerBundle package.
+ *
+ * (c) FriendsOfSymfony <http://friendsofsymfony.github.com/>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace FOS\OAuthServerBundle\Storage;
+
+use Symfony\Component\Security\Core\User\UserInterface;
+
+interface PasswordCheckerInterface
+{
+    /**
+     * Validate the user's password matches.
+     *
+     * @param UserInterface $user
+     * @param string        $password
+     *
+     * @return bool
+     */
+    public function validate(UserInterface $user, $password): bool;
+}

Tests/Storage/OAuthStorageTest.php

@@ -22,7 +22,7 @@
 use FOS\OAuthServerBundle\Model\RefreshToken;
 use FOS\OAuthServerBundle\Model\RefreshTokenManagerInterface;
 use FOS\OAuthServerBundle\Storage\OAuthStorage;
-use Symfony\Component\Security\Core\Encoder\EncoderFactoryInterface;
+use FOS\OAuthServerBundle\Storage\PasswordCheckerInterface;
 use Symfony\Component\Security\Core\Exception\AuthenticationException;
 use Symfony\Component\Security\Core\User\UserInterface;
 use Symfony\Component\Security\Core\User\UserProviderInterface;
@@ -39,7 +39,7 @@ class OAuthStorageTest extends \PHPUnit\Framework\TestCase
 
     protected $userProvider;
 
-    protected $encoderFactory;
+    protected $passwordChecker;
 
     protected $storage;
 
@@ -65,12 +65,12 @@ public function setUp()
             ->disableOriginalConstructor()
             ->getMock()
         ;
-        $this->encoderFactory = $this->getMockBuilder(EncoderFactoryInterface::class)
+        $this->passwordChecker = $this->getMockBuilder(PasswordCheckerInterface::class)
             ->disableOriginalConstructor()
             ->getMock()
         ;
 
-        $this->storage = new OAuthStorage($this->clientManager, $this->accessTokenManager, $this->refreshTokenManager, $this->authCodeManager, $this->userProvider, $this->encoderFactory);
+        $this->storage = new OAuthStorage($this->clientManager, $this->accessTokenManager, $this->refreshTokenManager, $this->authCodeManager, $this->passwordChecker, $this->userProvider);
     }
 
     public function testGetClientReturnsClientWithGivenId()
@@ -365,31 +365,17 @@ public function testCheckUserCredentialsReturnsTrueOnValidCredentials()
             ->disableOriginalConstructor()
             ->getMock()
         ;
-        $user->expects($this->once())
-            ->method('getPassword')->with()->will($this->returnValue('foo'));
-        $user->expects($this->once())
-            ->method('getSalt')->with()->will($this->returnValue('bar'));
-
-        $encoder = $this->getMockBuilder('Symfony\Component\Security\Core\Encoder\PasswordEncoderInterface')
-            ->disableOriginalConstructor()
-            ->getMock()
-        ;
-        $encoder->expects($this->once())
-            ->method('isPasswordValid')
-            ->with('foo', 'baz', 'bar')
-            ->will($this->returnValue(true))
-        ;
 
         $this->userProvider->expects($this->once())
             ->method('loadUserByUsername')
             ->with('Joe')
             ->will($this->returnValue($user))
         ;
 
-        $this->encoderFactory->expects($this->once())
-            ->method('getEncoder')
-            ->with($user)
-            ->will($this->returnValue($encoder))
+        $this->passwordChecker->expects($this->once())
+            ->method('validate')
+            ->with($user, 'baz')
+            ->will($this->returnValue(true))
         ;
 
         $this->assertSame([
@@ -404,31 +390,17 @@ public function testCheckUserCredentialsReturnsFalseOnInvalidCredentials()
             ->disableOriginalConstructor()
             ->getMock()
         ;
-        $user->expects($this->once())
-            ->method('getPassword')->with()->will($this->returnValue('foo'));
-        $user->expects($this->once())
-            ->method('getSalt')->with()->will($this->returnValue('bar'));
-
-        $encoder = $this->getMockBuilder('Symfony\Component\Security\Core\Encoder\PasswordEncoderInterface')
-            ->disableOriginalConstructor()
-            ->getMock()
-        ;
-        $encoder->expects($this->once())
-            ->method('isPasswordValid')
-            ->with('foo', 'baz', 'bar')
-            ->will($this->returnValue(false))
-        ;
 
         $this->userProvider->expects($this->once())
             ->method('loadUserByUsername')
             ->with('Joe')
             ->will($this->returnValue($user))
         ;
 
-        $this->encoderFactory->expects($this->once())
-            ->method('getEncoder')
-            ->with($user)
-            ->will($this->returnValue($encoder))
+        $this->passwordChecker->expects($this->once())
+            ->method('validate')
+            ->with($user, 'baz')
+            ->will($this->returnValue(false))
         ;
 
         $this->assertFalse($this->storage->checkUserCredentials($client, 'Joe', 'baz'));

Tests/Storage/PasswordCheckerTest.php

@@ -0,0 +1,94 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the FOSOAuthServerBundle package.
+ *
+ * (c) FriendsOfSymfony <http://friendsofsymfony.github.com/>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace FOS\OAuthServerBundle\Tests\Storage;
+
+use FOS\OAuthServerBundle\Storage\PasswordChecker;
+use Symfony\Component\Security\Core\Encoder\EncoderFactoryInterface;
+
+class PasswordCheckerTest extends \PHPUnit\Framework\TestCase
+{
+    protected $encoderFactory;
+
+    protected $passwordChecker;
+
+    public function setUp()
+    {
+        $this->encoderFactory = $this->getMockBuilder(EncoderFactoryInterface::class)
+            ->disableOriginalConstructor()
+            ->getMock()
+        ;
+
+        $this->passwordChecker = new PasswordChecker($this->encoderFactory);
+    }
+
+    public function testValidateReturnsTrueOnValidCredentials()
+    {
+        $user = $this->getMockBuilder('Symfony\Component\Security\Core\User\UserInterface')
+            ->disableOriginalConstructor()
+            ->getMock()
+        ;
+        $user->expects($this->once())
+            ->method('getPassword')->with()->will($this->returnValue('foo'));
+        $user->expects($this->once())
+            ->method('getSalt')->with()->will($this->returnValue('bar'));
+
+        $encoder = $this->getMockBuilder('Symfony\Component\Security\Core\Encoder\PasswordEncoderInterface')
+            ->disableOriginalConstructor()
+            ->getMock()
+        ;
+        $encoder->expects($this->once())
+            ->method('isPasswordValid')
+            ->with('foo', 'baz', 'bar')
+            ->will($this->returnValue(true))
+        ;
+
+        $this->encoderFactory->expects($this->once())
+            ->method('getEncoder')
+            ->with($user)
+            ->will($this->returnValue($encoder))
+        ;
+
+        $this->assertTrue($this->passwordChecker->validate($user, 'baz'));
+    }
+
+    public function testValidateReturnsFalseOnInvalidCredentials()
+    {
+        $user = $this->getMockBuilder('Symfony\Component\Security\Core\User\UserInterface')
+            ->disableOriginalConstructor()
+            ->getMock()
+        ;
+        $user->expects($this->once())
+            ->method('getPassword')->with()->will($this->returnValue('foo'));
+        $user->expects($this->once())
+            ->method('getSalt')->with()->will($this->returnValue('bar'));
+
+        $encoder = $this->getMockBuilder('Symfony\Component\Security\Core\Encoder\PasswordEncoderInterface')
+            ->disableOriginalConstructor()
+            ->getMock()
+        ;
+        $encoder->expects($this->once())
+            ->method('isPasswordValid')
+            ->with('foo', 'baz', 'bar')
+            ->will($this->returnValue(false))
+        ;
+
+        $this->encoderFactory->expects($this->once())
+            ->method('getEncoder')
+            ->with($user)
+            ->will($this->returnValue($encoder))
+        ;
+
+        $this->assertFalse($this->passwordChecker->validate($user, 'baz'));
+    }
+}