diff --git a/DependencyInjection/Configuration.php b/DependencyInjection/Configuration.php index 571fcafd..8aff98d7 100644 --- a/DependencyInjection/Configuration.php +++ b/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() diff --git a/DependencyInjection/FOSOAuthServerExtension.php b/DependencyInjection/FOSOAuthServerExtension.php index 256bff31..821b7156 100644 --- a/DependencyInjection/FOSOAuthServerExtension.php +++ b/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']); diff --git a/Resources/config/oauth.xml b/Resources/config/oauth.xml index 9ec29f6d..be6303c0 100644 --- a/Resources/config/oauth.xml +++ b/Resources/config/oauth.xml @@ -14,7 +14,11 @@ + + + + diff --git a/Storage/OAuthStorage.php b/Storage/OAuthStorage.php index a56dd25a..5263ed2b 100644 --- a/Storage/OAuthStorage.php +++ b/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, ]; diff --git a/Storage/PasswordChecker.php b/Storage/PasswordChecker.php new file mode 100644 index 00000000..2dd36ee8 --- /dev/null +++ b/Storage/PasswordChecker.php @@ -0,0 +1,43 @@ + + * + * 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()); + } +} diff --git a/Storage/PasswordCheckerInterface.php b/Storage/PasswordCheckerInterface.php new file mode 100644 index 00000000..c937242b --- /dev/null +++ b/Storage/PasswordCheckerInterface.php @@ -0,0 +1,29 @@ + + * + * 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; +} diff --git a/Tests/Storage/OAuthStorageTest.php b/Tests/Storage/OAuthStorageTest.php index f628327b..6201ee7d 100644 --- a/Tests/Storage/OAuthStorageTest.php +++ b/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,20 +365,6 @@ 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') @@ -386,10 +372,10 @@ public function testCheckUserCredentialsReturnsTrueOnValidCredentials() ->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,20 +390,6 @@ 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') @@ -425,10 +397,10 @@ public function testCheckUserCredentialsReturnsFalseOnInvalidCredentials() ->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')); diff --git a/Tests/Storage/PasswordCheckerTest.php b/Tests/Storage/PasswordCheckerTest.php new file mode 100644 index 00000000..b475870a --- /dev/null +++ b/Tests/Storage/PasswordCheckerTest.php @@ -0,0 +1,94 @@ + + * + * 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')); + } +}