bug #9902 [Security] fixed pre/post authentication checks (fabpot)
This PR was merged into the 2.3 branch.
Discussion
----------
[Security] fixed pre/post authentication checks
| Q | A
| ------------- | ---
| Bug fix? | yes
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | #8510, #9622
| License | MIT
| Doc PR | N/A
After further investigation of #8510, I found that all checks in the `checkPreAuth` actually belongs to `checkPostAuth` and the same goes for checks in `CheckPostAuth` (I checked the original source from Spring and indeed, that's how it is implemented there: see https://github.com/spring-projects/spring-security/blob/master/core/src/main/java/org/springframework/security/authentication/dao/AbstractUserDetailsAuthenticationProvider.java#L305
).
So, this PR fixes that issue. I think that we can do this change safely in 2.3 as the error message is the same for all causes by default (`$hideUserNotFoundExceptions` is `true` by default in `UserAuthenticationProvider`).
The only "real" change is whether the authentication is checked or not.
Commits
-------
ada82a2
[Security] fixed pre/post authentication checks
This commit is contained in:
commit
3704e08476
@ -32,22 +32,6 @@ class UserChecker implements UserCheckerInterface
|
||||
return;
|
||||
}
|
||||
|
||||
if (!$user->isCredentialsNonExpired()) {
|
||||
$ex = new CredentialsExpiredException('User credentials have expired.');
|
||||
$ex->setUser($user);
|
||||
throw $ex;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* {@inheritdoc}
|
||||
*/
|
||||
public function checkPostAuth(UserInterface $user)
|
||||
{
|
||||
if (!$user instanceof AdvancedUserInterface) {
|
||||
return;
|
||||
}
|
||||
|
||||
if (!$user->isAccountNonLocked()) {
|
||||
$ex = new LockedException('User account is locked.');
|
||||
$ex->setUser($user);
|
||||
@ -66,4 +50,20 @@ class UserChecker implements UserCheckerInterface
|
||||
throw $ex;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* {@inheritdoc}
|
||||
*/
|
||||
public function checkPostAuth(UserInterface $user)
|
||||
{
|
||||
if (!$user instanceof AdvancedUserInterface) {
|
||||
return;
|
||||
}
|
||||
|
||||
if (!$user->isCredentialsNonExpired()) {
|
||||
$ex = new CredentialsExpiredException('User credentials have expired.');
|
||||
$ex->setUser($user);
|
||||
throw $ex;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@ -15,36 +15,6 @@ use Symfony\Component\Security\Core\User\UserChecker;
|
||||
|
||||
class UserCheckerTest extends \PHPUnit_Framework_TestCase
|
||||
{
|
||||
public function testCheckPreAuthNotAdvancedUserInterface()
|
||||
{
|
||||
$checker = new UserChecker();
|
||||
|
||||
$this->assertNull($checker->checkPreAuth($this->getMock('Symfony\Component\Security\Core\User\UserInterface')));
|
||||
}
|
||||
|
||||
public function testCheckPreAuthPass()
|
||||
{
|
||||
$checker = new UserChecker();
|
||||
|
||||
$account = $this->getMock('Symfony\Component\Security\Core\User\AdvancedUserInterface');
|
||||
$account->expects($this->once())->method('isCredentialsNonExpired')->will($this->returnValue(true));
|
||||
|
||||
$this->assertNull($checker->checkPreAuth($account));
|
||||
}
|
||||
|
||||
/**
|
||||
* @expectedException \Symfony\Component\Security\Core\Exception\CredentialsExpiredException
|
||||
*/
|
||||
public function testCheckPreAuthCredentialsExpired()
|
||||
{
|
||||
$checker = new UserChecker();
|
||||
|
||||
$account = $this->getMock('Symfony\Component\Security\Core\User\AdvancedUserInterface');
|
||||
$account->expects($this->once())->method('isCredentialsNonExpired')->will($this->returnValue(false));
|
||||
|
||||
$checker->checkPreAuth($account);
|
||||
}
|
||||
|
||||
public function testCheckPostAuthNotAdvancedUserInterface()
|
||||
{
|
||||
$checker = new UserChecker();
|
||||
@ -57,30 +27,60 @@ class UserCheckerTest extends \PHPUnit_Framework_TestCase
|
||||
$checker = new UserChecker();
|
||||
|
||||
$account = $this->getMock('Symfony\Component\Security\Core\User\AdvancedUserInterface');
|
||||
$account->expects($this->once())->method('isAccountNonLocked')->will($this->returnValue(true));
|
||||
$account->expects($this->once())->method('isEnabled')->will($this->returnValue(true));
|
||||
$account->expects($this->once())->method('isAccountNonExpired')->will($this->returnValue(true));
|
||||
$account->expects($this->once())->method('isCredentialsNonExpired')->will($this->returnValue(true));
|
||||
|
||||
$this->assertNull($checker->checkPostAuth($account));
|
||||
}
|
||||
|
||||
/**
|
||||
* @expectedException \Symfony\Component\Security\Core\Exception\CredentialsExpiredException
|
||||
*/
|
||||
public function testCheckPostAuthCredentialsExpired()
|
||||
{
|
||||
$checker = new UserChecker();
|
||||
|
||||
$account = $this->getMock('Symfony\Component\Security\Core\User\AdvancedUserInterface');
|
||||
$account->expects($this->once())->method('isCredentialsNonExpired')->will($this->returnValue(false));
|
||||
|
||||
$checker->checkPostAuth($account);
|
||||
}
|
||||
|
||||
public function testCheckPreAuthNotAdvancedUserInterface()
|
||||
{
|
||||
$checker = new UserChecker();
|
||||
|
||||
$this->assertNull($checker->checkPreAuth($this->getMock('Symfony\Component\Security\Core\User\UserInterface')));
|
||||
}
|
||||
|
||||
public function testCheckPreAuthPass()
|
||||
{
|
||||
$checker = new UserChecker();
|
||||
|
||||
$account = $this->getMock('Symfony\Component\Security\Core\User\AdvancedUserInterface');
|
||||
$account->expects($this->once())->method('isAccountNonLocked')->will($this->returnValue(true));
|
||||
$account->expects($this->once())->method('isEnabled')->will($this->returnValue(true));
|
||||
$account->expects($this->once())->method('isAccountNonExpired')->will($this->returnValue(true));
|
||||
|
||||
$this->assertNull($checker->checkPreAuth($account));
|
||||
}
|
||||
|
||||
/**
|
||||
* @expectedException \Symfony\Component\Security\Core\Exception\LockedException
|
||||
*/
|
||||
public function testCheckPostAuthAccountLocked()
|
||||
public function testCheckPreAuthAccountLocked()
|
||||
{
|
||||
$checker = new UserChecker();
|
||||
|
||||
$account = $this->getMock('Symfony\Component\Security\Core\User\AdvancedUserInterface');
|
||||
$account->expects($this->once())->method('isAccountNonLocked')->will($this->returnValue(false));
|
||||
|
||||
$checker->checkPostAuth($account);
|
||||
$checker->checkPreAuth($account);
|
||||
}
|
||||
|
||||
/**
|
||||
* @expectedException \Symfony\Component\Security\Core\Exception\DisabledException
|
||||
*/
|
||||
public function testCheckPostAuthDisabled()
|
||||
public function testCheckPreAuthDisabled()
|
||||
{
|
||||
$checker = new UserChecker();
|
||||
|
||||
@ -88,13 +88,13 @@ class UserCheckerTest extends \PHPUnit_Framework_TestCase
|
||||
$account->expects($this->once())->method('isAccountNonLocked')->will($this->returnValue(true));
|
||||
$account->expects($this->once())->method('isEnabled')->will($this->returnValue(false));
|
||||
|
||||
$checker->checkPostAuth($account);
|
||||
$checker->checkPreAuth($account);
|
||||
}
|
||||
|
||||
/**
|
||||
* @expectedException \Symfony\Component\Security\Core\Exception\AccountExpiredException
|
||||
*/
|
||||
public function testCheckPostAuthAccountExpired()
|
||||
public function testCheckPreAuthAccountExpired()
|
||||
{
|
||||
$checker = new UserChecker();
|
||||
|
||||
@ -103,6 +103,6 @@ class UserCheckerTest extends \PHPUnit_Framework_TestCase
|
||||
$account->expects($this->once())->method('isEnabled')->will($this->returnValue(true));
|
||||
$account->expects($this->once())->method('isAccountNonExpired')->will($this->returnValue(false));
|
||||
|
||||
$checker->checkPostAuth($account);
|
||||
$checker->checkPreAuth($account);
|
||||
}
|
||||
}
|
||||
|
Reference in New Issue
Block a user