8000 feature #16395 checkCredentials() force it to be an affirmative yes! … · symfony/symfony@64917c7 · GitHub
[go: up one dir, main page]
Skip to content

Commit 64917c7

Browse files
fabpotfabpot
committed
feature #16395 checkCredentials() force it to be an affirmative yes! (weaverryan)
This PR was squashed before being merged into the 2.8 branch (closes #16395). Discussion ---------- checkCredentials() force it to be an affirmative yes! | Q | A | ------------- | --- | Bug fix? | no | New feature? | no | BC breaks? | no (because 2.8 isn't released) | Deprecations? | no | Tests pass? | yes | Fixed tickets | n/a | License | MIT | Doc PR | n/a This changes `GuardAuthenticatorInterface::checkCredentials()`: you now *must* return true in order for authentication to pass. Before: You could do nothing (i.e. return null) and authentication would pass. You threw an AuthenticationException to cause a failure. New: You *must* return `true` for authentication to pass. If you do nothing, we will throw a `BadCredentialsException` on your behalf. You can still throw your own exception. This was a suggestion at symfony_live to make things more secure. I think it makes sense. Commits ------- 14acadd checkCredentials() force it to be an affirmative yes!
2 parents a15c9eb + 14acadd commit 64917c7

File tree

3 files changed

+45
-3
lines changed

3 files changed

+45
-3
lines changed

‎src/Symfony/Component/Security/Guard/GuardAuthenticatorInterface.php‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,11 @@ public function getCredentials(Request $request);
7373
public function getUser($credentials, UserProviderInterface $userProvider);
7474

7575
/**
76-
* Throw an AuthenticationException if the credentials are invalid.
76+
* Returns true if the credentials are valid.
77+
*
78+
* If any value other than true is returned, authentication will
79+
* fail. You may also throw an AuthenticationException if you wish
80+
* to cause authentication to fail.
7781
*
7882
* The *credentials* are the return value from getCredentials()
7983
*

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

Lines changed: 4 additions & 1 deletion
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\BadCredentialsException;
1617
use Symfony\Component\Security\Core\Exception\UsernameNotFoundException;
1718
use Symfony\Component\Security\Guard\GuardAuthenticatorInterface;
1819
use Symfony\Component\Security\Guard\Token\GuardTokenInterface;
@@ -122,7 +123,9 @@ private function authenticateViaGuard(GuardAuthenticatorInterface $guardAuthenti
122123
}
123124

124125
$this->userChecker->checkPreAuth($user);
125-
$guardAuthenticator->checkCredentials($token->getCredentials(), $user);
126+
if (true !== $guardAuthenticator->checkCredentials($token->getCredentials(), $user)) {
127+
throw new BadCredentialsException(sprintf('Authentication failed because %s::checkCredentials() did not return true.', get_class($guardAuthenticator)));
128+
}
126129
$this->userChecker->checkPostAuth($user);
127130

128131
// turn the UserInterface into a TokenInterface

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

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,9 @@ public function testAuthenticate()
6060
// checkCredentials is called
6161
$authenticatorB->expects($this->once())
6262
->method('checkCredentials')
63-
->with($enteredCredentials, $mockedUser);
63+
->with($enteredCredentials, $mockedUser)
64+
// authentication works!
65+
->will($this->returnValue(true));
6466
$authedToken = $this->getMock('Symfony\Component\Security\Core\Authentication\Token\TokenInterface');
6567
$authenticatorB->expects($this->once())
6668
->method('createAuthenticatedToken')
@@ -80,6 +82,39 @@ public function testAuthenticate()
8082
$this->assertSame($authedToken, $actualAuthedToken);
8183
}
8284

85+
/**
86+
* @expectedException \Symfony\Component\Security\Core\Exception\BadCredentialsException
87+
*/
88+
public function testCheckCredentialsReturningNonTrueFailsAuthentication()
89+
{
90+
$providerKey = 'my_uncool_firewall';
91+
92+
$authenticator = $this->getMock('Symfony\Component\Security\Guard\GuardAuthenticatorInterface');
93+
94+
// make sure the authenticator is used
95+
$this->preAuthenticationToken->expects($this->any())
96+
->method('getGuardProviderKey')
97+
// the 0 index, to match the only authenticator
98+
->will($this->returnValue('my_uncool_firewall_0'));
99+
100+
$this->preAuthenticationToken->expects($this->atLeastOnce())
101+
->method('getCredentials')
102+
->will($this->returnValue('non-null-value'));
103+
104+
$mockedUser = $this->getMock('Symfony\Component\Security\Core\User\UserInterface');
105+
$authenticator->expects($this->once())
106+
->method('getUser')
107+
->will($this->returnValue($mockedUser));
108+
// checkCredentials is called
109+
$authenticator->expects($this->once())
110+
->method('checkCredentials')
111+
// authentication fails :(
112+
->will($this->returnValue(null));
113+
114< 5BE3 code class="diff-text syntax-highlighted-line addition">+
$provider = new GuardAuthenticationProvider(array($authenticator), $this->userProvider, $providerKey, $this->userChecker);
115+
$provider->authenticate($this->preAuthenticationToken);
116+
}
117+
83118
/**
84119
* @expectedException \Symfony\Component\Security\Core\Exception\AuthenticationExpiredException
85120
*/

0 commit comments

Comments
 (0)
0