From b93d732209ecd5eab35ed4e32e931760cb329122 Mon Sep 17 00:00:00 2001 From: Julian Finkler Date: Wed, 19 Sep 2018 17:00:54 +0200 Subject: [PATCH] Migrated SF AdvancedUserInterface to FOS UserInterface (#2815) * Migrated SF AdvancedUserInterface to FOS UserInterface AdvancedUserInterface is deprecated since Symfony 4.1 - https://github.com/symfony/symfony/pull/23508 Issue: - #2803 Deprecation with Symfony 4.1 - AdvancedUserInterface * Code style fixed and using `getMockBuilder` instead of `createMock` * Code style fixed and using attributes instead of `$this->expectException` * Change to restart travis * EquatableInterface added to `UserInterface` and implementation added to `User` * Tests after merge of master fixed * Tests after merge of master fixed * Update README.md * Added compatibility for apps that check against AdvancedUserInterface * Code style fixed to pass all travis tests * fos_user.user_checker Service marked as non-public --- Model/User.php | 25 +++++++ Model/UserInterface.php | 78 +++++++++++++++++++-- Resources/config/security.xml | 2 + Resources/doc/index.rst | 1 + Security/UserChecker.php | 63 +++++++++++++++++ Tests/Mailer/MailerTest.php | 3 +- Tests/Model/UserTest.php | 19 ++++++ Tests/Security/UserCheckerTest.php | 105 +++++++++++++++++++++++++++++ 8 files changed, 290 insertions(+), 6 deletions(-) create mode 100644 Security/UserChecker.php create mode 100644 Tests/Security/UserCheckerTest.php diff --git a/Model/User.php b/Model/User.php index fd7edb5c81..71d64eeaaa 100644 --- a/Model/User.php +++ b/Model/User.php @@ -13,6 +13,7 @@ use Doctrine\Common\Collections\ArrayCollection; use Doctrine\Common\Collections\Collection; +use Symfony\Component\Security\Core\User\UserInterface as BaseUserInterface; /** * Storage agnostic user object. @@ -554,4 +555,28 @@ public function removeGroup(GroupInterface $group) return $this; } + + /** + * {@inheritdoc} + */ + public function isEqualTo(BaseUserInterface $user) + { + if (!$user instanceof self) { + return false; + } + + if ($this->password !== $user->getPassword()) { + return false; + } + + if ($this->salt !== $user->getSalt()) { + return false; + } + + if ($this->username !== $user->getUsername()) { + return false; + } + + return true; + } } diff --git a/Model/UserInterface.php b/Model/UserInterface.php index 56d1007447..64bcfc7c73 100644 --- a/Model/UserInterface.php +++ b/Model/UserInterface.php @@ -11,13 +11,13 @@ namespace FOS\UserBundle\Model; -use Symfony\Component\Security\Core\User\AdvancedUserInterface; +use Symfony\Component\Security\Core\User\EquatableInterface; +use Symfony\Component\Security\Core\User\UserInterface as BaseUserInterface; /** - * @author Thibault Duplessis - * @author Johannes M. Schmitt + * @internal Only for back compatibility. Remove / merge when dropping support for Symfony 4 */ -interface UserInterface extends AdvancedUserInterface, \Serializable +interface FosUserInterface extends \Serializable { const ROLE_DEFAULT = 'ROLE_USER'; @@ -227,4 +227,74 @@ public function addRole($role); * @return static */ public function removeRole($role); + + /** + * Checks whether the user's account has expired. + * + * Internally, if this method returns false, the authentication system + * will throw an AccountExpiredException and prevent login. + * + * @return bool true if the user's account is non expired, false otherwise + * + * @see AccountExpiredException + */ + public function isAccountNonExpired(); + + /** + * Checks whether the user is locked. + * + * Internally, if this method returns false, the authentication system + * will throw a LockedException and prevent login. + * + * @return bool true if the user is not locked, false otherwise + * + * @see LockedException + */ + public function isAccountNonLocked(); + + /** + * Checks whether the user's credentials (password) has expired. + * + * Internally, if this method returns false, the authentication system + * will throw a CredentialsExpiredException and prevent login. + * + * @return bool true if the user's credentials are non expired, false otherwise + * + * @see CredentialsExpiredException + */ + public function isCredentialsNonExpired(); + + /** + * Checks whether the user is enabled. + * + * Internally, if this method returns false, the authentication system + * will throw a DisabledException and prevent login. + * + * @return bool true if the user is enabled, false otherwise + * + * @see DisabledException + */ + public function isEnabled(); +} + +// This is required to support apps that explicitly check if a user is an instance of AdvancedUserInterface +if (interface_exists('\Symfony\Component\Security\Core\User\AdvancedUserInterface')) { + /** + * @author Thibault Duplessis + * @author Johannes M. Schmitt + * + * @deprecated since Symfony 4.1. Remove in Nov 2023 (End of support for security fixes SF 4.4) + */ + interface UserInterface extends FosUserInterface, \Symfony\Component\Security\Core\User\AdvancedUserInterface + { + } +} else { + /** + * @author Thibault Duplessis + * @author Johannes M. Schmitt + * @author Julian Finkler + */ + interface UserInterface extends FosUserInterface, BaseUserInterface, EquatableInterface + { + } } diff --git a/Resources/config/security.xml b/Resources/config/security.xml index 4fb73ec4e0..1dbc3084b5 100644 --- a/Resources/config/security.xml +++ b/Resources/config/security.xml @@ -45,6 +45,8 @@ + + diff --git a/Resources/doc/index.rst b/Resources/doc/index.rst index 9998702e7d..286e786b51 100644 --- a/Resources/doc/index.rst +++ b/Resources/doc/index.rst @@ -274,6 +274,7 @@ in your application: firewalls: main: pattern: ^/ + user_checker: fos_user.user_checker form_login: provider: fos_userbundle csrf_token_generator: security.csrf.token_manager diff --git a/Security/UserChecker.php b/Security/UserChecker.php new file mode 100644 index 0000000000..7741c4887a --- /dev/null +++ b/Security/UserChecker.php @@ -0,0 +1,63 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace FOS\UserBundle\Security; + +use Symfony\Component\Security\Core\Exception\AccountExpiredException; +use Symfony\Component\Security\Core\Exception\CredentialsExpiredException; +use Symfony\Component\Security\Core\Exception\DisabledException; +use Symfony\Component\Security\Core\Exception\LockedException; +use Symfony\Component\Security\Core\User\UserChecker as BaseUserChecker; +use Symfony\Component\Security\Core\User\UserInterface as BaseUserInterface; + +/** + * UserChecker checks the user account flags. + * + * @author Julian Finkler (Devtronic) + */ +class UserChecker extends BaseUserChecker +{ + /** + * {@inheritdoc} + */ + public function checkPreAuth(BaseUserInterface $user) + { + if (!$user->isAccountNonLocked()) { + $ex = new LockedException('User account is locked.'); + $ex->setUser($user); + throw $ex; + } + + if (!$user->isEnabled()) { + $ex = new DisabledException('User account is disabled.'); + $ex->setUser($user); + throw $ex; + } + + if (!$user->isAccountNonExpired()) { + $ex = new AccountExpiredException('User account has expired.'); + $ex->setUser($user); + throw $ex; + } + } + + /** + * {@inheritdoc} + */ + public function checkPostAuth(BaseUserInterface $user) + { + if (!$user->isCredentialsNonExpired()) { + $ex = new CredentialsExpiredException('User credentials have expired.'); + $ex->setUser($user); + throw $ex; + } + } +} diff --git a/Tests/Mailer/MailerTest.php b/Tests/Mailer/MailerTest.php index fb03ead806..bb4e4b2d49 100644 --- a/Tests/Mailer/MailerTest.php +++ b/Tests/Mailer/MailerTest.php @@ -13,7 +13,6 @@ use FOS\UserBundle\Mailer\Mailer; use PHPUnit\Framework\TestCase; -use Swift_Events_EventDispatcher; use Swift_Mailer; use Swift_Transport_NullTransport; @@ -84,7 +83,7 @@ private function getMailer() return new Mailer( new Swift_Mailer( new Swift_Transport_NullTransport( - $this->getMockBuilder(Swift_Events_EventDispatcher::class)->getMock() + $this->getMockBuilder('\Swift_Events_EventDispatcher')->getMock() ) ), $this->getMockBuilder('Symfony\Component\Routing\Generator\UrlGeneratorInterface')->getMock(), diff --git a/Tests/Model/UserTest.php b/Tests/Model/UserTest.php index 2bf0094d61..e3249303c0 100644 --- a/Tests/Model/UserTest.php +++ b/Tests/Model/UserTest.php @@ -79,6 +79,25 @@ public function testFalseHasRole() $this->assertTrue($user->hasRole($newrole)); } + public function testIsEqualTo() + { + $user = $this->getUser(); + $this->assertTrue($user->isEqualTo($user)); + $this->assertFalse($user->isEqualTo($this->getMockBuilder('FOS\UserBundle\Model\UserInterface')->getMock())); + + $user2 = $this->getUser(); + $user2->setPassword('secret'); + $this->assertFalse($user->isEqualTo($user2)); + + $user3 = $this->getUser(); + $user3->setSalt('pepper'); + $this->assertFalse($user->isEqualTo($user3)); + + $user4 = $this->getUser(); + $user4->setUsername('f00b4r'); + $this->assertFalse($user->isEqualTo($user4)); + } + /** * @return User */ diff --git a/Tests/Security/UserCheckerTest.php b/Tests/Security/UserCheckerTest.php new file mode 100644 index 0000000000..b00761e6ab --- /dev/null +++ b/Tests/Security/UserCheckerTest.php @@ -0,0 +1,105 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace FOS\UserBundle\Tests\Security; + +use FOS\UserBundle\Security\UserChecker; +use PHPUnit\Framework\TestCase; + +class UserCheckerTest extends TestCase +{ + /** + * @expectedException \Symfony\Component\Security\Core\Exception\LockedException + * @expectedExceptionMessage User account is locked. + */ + public function testCheckPreAuthFailsLockedOut() + { + $userMock = $this->getUser(false, false, false, false); + $checker = new UserChecker(); + $checker->checkPreAuth($userMock); + } + + /** + * @expectedException \Symfony\Component\Security\Core\Exception\DisabledException + * @expectedExceptionMessage User account is disabled. + */ + public function testCheckPreAuthFailsIsEnabled() + { + $userMock = $this->getUser(true, false, false, false); + $checker = new UserChecker(); + $checker->checkPreAuth($userMock); + } + + /** + * @expectedException \Symfony\Component\Security\Core\Exception\AccountExpiredException + * @expectedExceptionMessage User account has expired. + */ + public function testCheckPreAuthFailsIsAccountNonExpired() + { + $userMock = $this->getUser(true, true, false, false); + $checker = new UserChecker(); + $checker->checkPreAuth($userMock); + } + + public function testCheckPreAuthSuccess() + { + $userMock = $this->getUser(true, true, true, false); + $checker = new UserChecker(); + + try { + $this->assertNull($checker->checkPreAuth($userMock)); + } catch (\Exception $ex) { + $this->fail(); + } + } + + /** + * @expectedException \Symfony\Component\Security\Core\Exception\CredentialsExpiredException + * @expectedExceptionMessage User credentials have expired. + */ + public function testCheckPostAuthFailsIsCredentialsNonExpired() + { + $userMock = $this->getUser(true, true, true, false); + $checker = new UserChecker(); + $checker->checkPostAuth($userMock); + } + + public function testCheckPostAuthSuccess() + { + $userMock = $this->getUser(true, true, true, true); + $checker = new UserChecker(); + + try { + $this->assertNull($checker->checkPostAuth($userMock)); + } catch (\Exception $ex) { + $this->fail(); + } + } + + private function getUser($isAccountNonLocked, $isEnabled, $isAccountNonExpired, $isCredentialsNonExpired) + { + $userMock = $this->getMockBuilder('FOS\UserBundle\Model\User')->getMock(); + $userMock + ->method('isAccountNonLocked') + ->willReturn($isAccountNonLocked); + $userMock + ->method('isEnabled') + ->willReturn($isEnabled); + $userMock + ->method('isAccountNonExpired') + ->willReturn($isAccountNonExpired); + $userMock + ->method('isCredentialsNonExpired') + ->willReturn($isCredentialsNonExpired); + + return $userMock; + } +}