Skip to content

Commit ff96226

Browse files
nicolas-grekasnicolas-grekas
committed
bug #27016 [Security][Guard] GuardAuthenticationProvider::authenticate cannot return null (biomedia-thomas)
This PR was merged into the 2.8 branch. Discussion ---------- [Security][Guard] GuardAuthenticationProvider::authenticate cannot return null | Q | A | ------------- | --- | Branch? | 2.8 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #26942 | License | MIT Authenticate method in GuardAuthenticationProvider returned null when the token does not originate from any of the guard authenticators. This check was not done in the supports method. According to the interface authenticate cannot return null. This patch copies theguard authenticator checks to the supports method. Commits ------- 9dff22c [Security] guardAuthenticationProvider::authenticate cannot return null according to interface specification
2 parents 447ce8e + 9dff22c commit ff96226

File tree

2 files changed

+67
-25
lines changed

2 files changed

+67
-25
lines changed

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

+32-25
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

+35
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 <weaverryan@gmail.com>
@@ -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)