Skip to content

Commit

Permalink
bug #24644 [Security] Fixed auth provider authenticate() cannot retur…
Browse files Browse the repository at this point in the history
…n void (glye)

This PR was merged into the 2.7 branch.

Discussion
----------

[Security] Fixed auth provider authenticate() cannot return void

| Q             | A
| ------------- | ---
| Branch?       | 2.7 and up
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no (arguably)
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets |
| License       | MIT
| Doc PR        |

The `AuthenticationManagerInterface` [requires](https://github.com/symfony/symfony/blob/2.7/src/Symfony/Component/Security/Core/Authentication/AuthenticationManagerInterface.php#L30) that `authenticate()` must return a TokenInterface, never null. Several authentication providers are violating this. Changed to throw exception instead.

See discussion in earlier PR #24585 which was changing the docblock rather than the implementations.

Commits
-------

6e18b56 [Security] Fixed auth provider authenticate() cannot return void
  • Loading branch information
fabpot committed Oct 20, 2017
2 parents 5e7a03b + 6e18b56 commit 2661da4
Show file tree
Hide file tree
Showing 8 changed files with 27 additions and 8 deletions.
Expand Up @@ -12,6 +12,7 @@
namespace Symfony\Component\Security\Core\Authentication\Provider;

use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
use Symfony\Component\Security\Core\Exception\AuthenticationException;
use Symfony\Component\Security\Core\Exception\BadCredentialsException;
use Symfony\Component\Security\Core\Authentication\Token\AnonymousToken;

Expand All @@ -38,7 +39,7 @@ public function __construct($key)
public function authenticate(TokenInterface $token)
{
if (!$this->supports($token)) {
return;
throw new AuthenticationException('The token is not supported by this authentication provider.');
}

if ($this->key !== $token->getKey()) {
Expand Down
Expand Up @@ -13,6 +13,7 @@

use Symfony\Component\Security\Core\User\UserProviderInterface;
use Symfony\Component\Security\Core\User\UserCheckerInterface;
use Symfony\Component\Security\Core\Exception\AuthenticationException;
use Symfony\Component\Security\Core\Exception\BadCredentialsException;
use Symfony\Component\Security\Core\Authentication\Token\PreAuthenticatedToken;
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
Expand Down Expand Up @@ -51,7 +52,7 @@ public function __construct(UserProviderInterface $userProvider, UserCheckerInte
public function authenticate(TokenInterface $token)
{
if (!$this->supports($token)) {
return;
throw new AuthenticationException('The token is not supported by this authentication provider.');
}

if (!$user = $token->getUser()) {
Expand Down
Expand Up @@ -14,6 +14,7 @@
use Symfony\Component\Security\Core\User\UserCheckerInterface;
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
use Symfony\Component\Security\Core\Authentication\Token\RememberMeToken;
use Symfony\Component\Security\Core\Exception\AuthenticationException;
use Symfony\Component\Security\Core\Exception\BadCredentialsException;

class RememberMeAuthenticationProvider implements AuthenticationProviderInterface
Expand All @@ -40,7 +41,7 @@ public function __construct(UserCheckerInterface $userChecker, $key, $providerKe
public function authenticate(TokenInterface $token)
{
if (!$this->supports($token)) {
return;
throw new AuthenticationException('The token is not supported by this authentication provider.');
}

if ($this->key !== $token->getKey()) {
Expand Down
Expand Up @@ -56,7 +56,7 @@ public function __construct(UserCheckerInterface $userChecker, $providerKey, $hi
public function authenticate(TokenInterface $token)
{
if (!$this->supports($token)) {
return;
throw new AuthenticationException('The token is not supported by this authentication provider.');
}

$username = $token->getUsername();
Expand Down
Expand Up @@ -24,11 +24,15 @@ public function testSupports()
$this->assertFalse($provider->supports($this->getMockBuilder('Symfony\Component\Security\Core\Authentication\Token\TokenInterface')->getMock()));
}

/**
* @expectedException \Symfony\Component\Security\Core\Exception\AuthenticationException
* @expectedExceptionMessage The token is not supported by this authentication provider.
*/
public function testAuthenticateWhenTokenIsNotSupported()
{
$provider = $this->getProvider('foo');

$this->assertNull($provider->authenticate($this->getMockBuilder('Symfony\Component\Security\Core\Authentication\Token\TokenInterface')->getMock()));
$provider->authenticate($this->getMockBuilder('Symfony\Component\Security\Core\Authentication\Token\TokenInterface')->getMock());
}

/**
Expand Down
Expand Up @@ -36,11 +36,15 @@ public function testSupports()
$this->assertFalse($provider->supports($token));
}

/**
* @expectedException \Symfony\Component\Security\Core\Exception\AuthenticationException
* @expectedExceptionMessage The token is not supported by this authentication provider.
*/
public function testAuthenticateWhenTokenIsNotSupported()
{
$provider = $this->getProvider();

$this->assertNull($provider->authenticate($this->getMockBuilder('Symfony\Component\Security\Core\Authentication\Token\TokenInterface')->getMock()));
$provider->authenticate($this->getMockBuilder('Symfony\Component\Security\Core\Authentication\Token\TokenInterface')->getMock());
}

/**
Expand Down
Expand Up @@ -26,12 +26,16 @@ public function testSupports()
$this->assertFalse($provider->supports($this->getMockBuilder('Symfony\Component\Security\Core\Authentication\Token\TokenInterface')->getMock()));
}

/**
* @expectedException \Symfony\Component\Security\Core\Exception\AuthenticationException
* @expectedExceptionMessage The token is not supported by this authentication provider.
*/
public function testAuthenticateWhenTokenIsNotSupported()
{
$provider = $this->getProvider();

$token = $this->getMockBuilder('Symfony\Component\Security\Core\Authentication\Token\TokenInterface')->getMock();
$this->assertNull($provider->authenticate($token));
$provider->authenticate($token);
}

/**
Expand Down
Expand Up @@ -29,11 +29,15 @@ public function testSupports()
$this->assertFalse($provider->supports($this->getMockBuilder('Symfony\Component\Security\Core\Authentication\Token\TokenInterface')->getMock()));
}

/**
* @expectedException \Symfony\Component\Security\Core\Exception\AuthenticationException
* @expectedExceptionMessage The token is not supported by this authentication provider.
*/
public function testAuthenticateWhenTokenIsNotSupported()
{
$provider = $this->getProvider();

$this->assertNull($provider->authenticate($this->getMockBuilder('Symfony\Component\Security\Core\Authentication\Token\TokenInterface')->getMock()));
$provider->authenticate($this->getMockBuilder('Symfony\Component\Security\Core\Authentication\Token\TokenInterface')->getMock());
}

/**
Expand Down

0 comments on commit 2661da4

Please sign in to comment.