Skip to content

Commit

Permalink
bug #11058 [Security] bug #10242 Missing checkPreAuth from RememberMe…
Browse files Browse the repository at this point in the history
…AuthenticationProvider (glutamatt)

This PR was submitted for the 2.4 branch but it was merged into the 2.3 branch instead (closes #11058).

Discussion
----------

[Security] bug #10242 Missing checkPreAuth from RememberMeAuthenticationProvider

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #10242
| License       | MIT

[Security] fixed missing call to UserChecker::checkPreAuth

edit : after the discution with @hellomedia , i replaced postcheck with precheck
glutamatt@e0730e0#commitcomment-6580764

Commits
-------

a38d1cd bug #10242 Missing checkPreAuth from RememberMeAuthenticationProvider
  • Loading branch information
fabpot committed Sep 24, 2014
2 parents bc8ee6f + a38d1cd commit a05a95c
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 13 deletions.
Expand Up @@ -50,7 +50,7 @@ public function authenticate(TokenInterface $token)
}

$user = $token->getUser();
$this->userChecker->checkPostAuth($user);
$this->userChecker->checkPreAuth($user);

$authenticatedToken = new RememberMeToken($user, $this->providerKey, $this->key);
$authenticatedToken->setAttributes($token->getAttributes());
Expand Down
Expand Up @@ -12,7 +12,7 @@
namespace Symfony\Component\Security\Tests\Core\Authentication\Provider;

use Symfony\Component\Security\Core\Authentication\Provider\RememberMeAuthenticationProvider;
use Symfony\Component\Security\Core\Exception\AccountExpiredException;
use Symfony\Component\Security\Core\Exception\DisabledException;
use Symfony\Component\Security\Core\Role\Role;

class RememberMeAuthenticationProviderTest extends \PHPUnit_Framework_TestCase
Expand Down Expand Up @@ -45,15 +45,14 @@ public function testAuthenticateWhenKeysDoNotMatch()
}

/**
* @expectedException \Symfony\Component\Security\Core\Exception\AccountExpiredException
* @expectedException \Symfony\Component\Security\Core\Exception\DisabledException
*/
public function testAuthenticateWhenPostChecksFails()
public function testAuthenticateWhenPreChecksFails()
{
$userChecker = $this->getMock('Symfony\Component\Security\Core\User\UserCheckerInterface');
$userChecker->expects($this->once())
->method('checkPostAuth')
->will($this->throwException(new AccountExpiredException()))
;
->method('checkPreAuth')
->will($this->throwException(new DisabledException()));

$provider = $this->getProvider($userChecker);

Expand All @@ -65,8 +64,7 @@ public function testAuthenticate()
$user = $this->getMock('Symfony\Component\Security\Core\User\UserInterface');
$user->expects($this->exactly(2))
->method('getRoles')
->will($this->returnValue(array('ROLE_FOO')))
;
->will($this->returnValue(array('ROLE_FOO')));

$provider = $this->getProvider();

Expand All @@ -86,16 +84,14 @@ protected function getSupportedToken($user = null, $key = 'test')
$user
->expects($this->any())
->method('getRoles')
->will($this->returnValue(array()))
;
->will($this->returnValue(array()));
}

$token = $this->getMock('Symfony\Component\Security\Core\Authentication\Token\RememberMeToken', array('getProviderKey'), array($user, 'foo', $key));
$token
->expects($this->once())
->method('getProviderKey')
->will($this->returnValue('foo'))
;
->will($this->returnValue('foo'));

return $token;
}
Expand Down

3 comments on commit a05a95c

@dschenk
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fabpot: You merged the fix into the 2.3 branch but the problem is present in 2.4 as well. Will 2.3 be merged into 2.4 or what are your plans / thoughts here?

@fabpot
Copy link
Member Author

@fabpot fabpot commented on a05a95c Sep 25, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dschenk Yes, older branches are merged to newer ones on a regular basis. Note that 2.4 is not maintained anymore though. So, this patch is going to be in the next version of 2.4, but that's going to be the last one.

@dschenk
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fabpot: I understand, thanks for the clarification.

Please sign in to comment.