Skip to content

Commit 9dff22c

Browse files
biomedia-thomasnicolas-grekas
authored andcommitted
[Security] guardAuthenticationProvider::authenticate cannot return null according to interface specification
1 parent 4057067 commit 9dff22c

File tree

2 files changed

+67
-25
lines changed

2 files changed

+67
-25
lines changed

src/Symfony/Component/Security/Guard/Provider/GuardAuthenticationProvider.php

Lines changed: 32 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
use Symfony\Component\Security\Core\Authentication\Provider\AuthenticationProviderInterface;
1515
use Symfony\Component\Security\Core\Authentication\Token\AnonymousToken;
16+
use Symfony\Component\Security\Core\Exception\AuthenticationException;
1617
use Symfony\Component\Security\Core\Exception\BadCredentialsException;
1718
use Symfony\Component\Security\Core\Exception\UsernameNotFoundException;
1819
use Symfony\Component\Security\Guard\GuardAuthenticatorInterface;
@@ -63,7 +64,7 @@ public function __construct(array $guardAuthenticators, UserProviderInterface $u
6364
*/
6465
public function authenticate(TokenInterface $token)
6566
{
66-
if (!$this->supports($token)) {
67+
if (!$token instanceof GuardTokenInterface) {
6768
throw new \InvalidArgumentException('GuardAuthenticationProvider only supports GuardTokenInterface.');
6869
}
6970

@@ -87,19 +88,13 @@ public function authenticate(TokenInterface $token)
8788
throw new AuthenticationExpiredException();
8889
}
8990

90-
// find the *one* GuardAuthenticator that this token originated from
91-
foreach ($this->guardAuthenticators as $key => $guardAuthenticator) {
92-
// get a key that's unique to *this* guard authenticator
93-
// this MUST be the same as GuardAuthenticationListener
94-
$uniqueGuardKey = $this->providerKey.'_'.$key;
91+
$guardAuthenticator = $this->findOriginatingAuthenticator($token);
9592

96-
if ($uniqueGuardKey == $token->getGuardProviderKey()) {
97-
return $this->authenticateViaGuard($guardAuthenticator, $token);
98-
}
93+
if (null === $guardAuthenticator) {
94+
throw new AuthenticationException(sprintf('Token with provider key "%s" did not originate from any of the guard authenticators of provider "%s".', $token->getGuardProviderKey(), $this->providerKey));
9995
}
10096

101-
// no matching authenticator found - but there will be multiple GuardAuthenticationProvider
102-
// instances that will be checked if you have multiple firewalls.
97+
return $this->authenticateViaGuard($guardAuthenticator, $token);
10398
}
10499

105100
private function authenticateViaGuard(GuardAuthenticatorInterface $guardAuthenticator, PreAuthenticationGuardToken $token)
@@ -108,18 +103,11 @@ private function authenticateViaGuard(GuardAuthenticatorInterface $guardAuthenti
108103
$user = $guardAuthenticator->getUser($token->getCredentials(), $this->userProvider);
109104

110105
if (null === $user) {
111-
throw new UsernameNotFoundException(sprintf(
112-
'Null returned from %s::getUser()',
113-
get_class($guardAuthenticator)
114-
));
106+
throw new UsernameNotFoundException(sprintf('Null returned from %s::getUser()', get_class($guardAuthenticator)));
115107
}
116108

117109
if (!$user instanceof UserInterface) {
118-
throw new \UnexpectedValueException(sprintf(
119-
'The %s::getUser() method must return a UserInterface. You returned %s.',
120-
get_class($guardAuthenticator),
121-
is_object($user) ? get_class($user) : gettype($user)
122-
));
110+
throw new \UnexpectedValueException(sprintf('The %s::getUser() method must return a UserInterface. You returned %s.', get_class($guardAuthenticator), is_object($user) ? get_class($user) : gettype($user)));
123111
}
124112

125113
$this->userChecker->checkPreAuth($user);
@@ -131,18 +119,37 @@ private function authenticateViaGuard(GuardAuthenticatorInterface $guardAuthenti
131119
// turn the UserInterface into a TokenInterface
132120
$authenticatedToken = $guardAuthenticator->createAuthenticatedToken($user, $this->providerKey);
133121
if (!$authenticatedToken instanceof TokenInterface) {
134-
throw new \UnexpectedValueException(sprintf(
135-
'The %s::createAuthenticatedToken() method must return a TokenInterface. You returned %s.',
136-
get_class($guardAuthenticator),
137-
is_object($authenticatedToken) ? get_class($authenticatedToken) : gettype($authenticatedToken)
138-
));
122+
throw new \UnexpectedValueException(sprintf('The %s::createAuthenticatedToken() method must return a TokenInterface. You returned %s.', get_class($guardAuthenticator), is_object($authenticatedToken) ? get_class($authenticatedToken) : gettype($authenticatedToken)));
139123
}
140124

141125
return $authenticatedToken;
142126
}
143127

128+
private function findOriginatingAuthenticator(PreAuthenticationGuardToken $token)
129+
{
130+
// find the *one* GuardAuthenticator that this token originated from
131+
foreach ($this->guardAuthenticators as $key => $guardAuthenticator) {
132+
// get a key that's unique to *this* guard authenticator
133+
// this MUST be the same as GuardAuthenticationListener
134+
$uniqueGuardKey = $this->providerKey.'_'.$key;
135+
136+
if ($uniqueGuardKey === $token->getGuardProviderKey()) {
137+
return $guardAuthenticator;
138+
}
139+
}
140+
141+
// no matching authenticator found - but there will be multiple GuardAuthenticationProvider
142+
// instances that will be checked if you have multiple firewalls.
143+
144+
return null;
145+
}
146+
144147
public function supports(TokenInterface $token)
145148
{
149+
if ($token instanceof PreAuthenticationGuardToken) {
150+
return null !== $this->findOriginatingAuthenticator($token);
151+
}
152+
146153
return $token instanceof GuardTokenInterface;
147154
}
148155
}

src/Symfony/Component/Security/Guard/Tests/Provider/GuardAuthenticationProviderTest.php

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
use PHPUnit\Framework\TestCase;
1515
use Symfony\Component\Security\Guard\Provider\GuardAuthenticationProvider;
1616
use Symfony\Component\Security\Guard\Token\PostAuthenticationGuardToken;
17+
use Symfony\Component\Security\Guard\Token\PreAuthenticationGuardToken;
1718

1819
/**
1920
* @author Ryan Weaver <[email protected]>
@@ -133,6 +134,40 @@ public function testGuardWithNoLongerAuthenticatedTriggersLogout()
133134
$actualToken = $provider->authenticate($token);
134135
}
135136

137+
public function testSupportsChecksGuardAuthenticatorsTokenOrigin()
138+
{
139+
$authenticatorA = $this->getMockBuilder('Symfony\Component\Security\Guard\GuardAuthenticatorInterface')->getMock();
140+
$authenticatorB = $this->getMockBuilder('Symfony\Component\Security\Guard\GuardAuthenticatorInterface')->getMock();
141+
$authenticators = array($authenticatorA, $authenticatorB);
142+
143+
$mockedUser = $this->getMockBuilder('Symfony\Component\Security\Core\User\UserInterface')->getMock();
144+
$provider = new GuardAuthenticationProvider($authenticators, $this->userProvider, 'first_firewall', $this->userChecker);
145+
146+
$token = new PreAuthenticationGuardToken($mockedUser, 'first_firewall_1');
147+
$supports = $provider->supports($token);
148+
$this->assertTrue($supports);
149+
150+
$token = new PreAuthenticationGuardToken($mockedUser, 'second_firewall_0');
151+
$supports = $provider->supports($token);
152+
$this->assertFalse($supports);
153+
}
154+
155+
/**
156+
* @expectedException \Symfony\Component\Security\Core\Exception\AuthenticationException
157+
* @expectedExceptionMessageRegExp /second_firewall_0/
158+
*/
159+
public function testAuthenticateFailsOnNonOriginatingToken()
160+
{
161+
$authenticatorA = $this->getMockBuilder('Symfony\Component\Security\Guard\GuardAuthenticatorInterface')->getMock();
162+
$authenticators = array($authenticatorA);
163+
164+
$mockedUser = $this->getMockBuilder('Symfony\Component\Security\Core\User\UserInterface')->getMock();
165+
$provider = new GuardAuthenticationProvider($authenticators, $this->userProvider, 'first_firewall', $this->userChecker);
166+
167+
$token = new PreAuthenticationGuardToken($mockedUser, 'second_firewall_0');
168+
$provider->authenticate($token);
169+
}
170+
136171
protected function setUp()
137172
{
138173
$this->userProvider = $this->getMockBuilder('Symfony\Component\Security\Core\User\UserProviderInterface')->getMock();

0 commit comments

Comments
 (0)