Skip to content
This repository was archived by the owner on May 31, 2024. It is now read-only.

Commit 1821118

Browse files
committed
Splitting the getting of the user and checking credentials into two steps
This looks like a subjective change (one more method, but the method implementations are simpler), but it wasn't. The problem was that the UserChecker checkPreAuth should happen *after* we get the user, but *before* the credentials are checked, and that wasn't possible before this change. Now it is.
1 parent dd9cccb commit 1821118

File tree

3 files changed

+41
-12
lines changed

3 files changed

+41
-12
lines changed

Guard/GuardAuthenticatorInterface.php

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ interface GuardAuthenticatorInterface extends AuthenticationEntryPointInterface
2727
* as any type (e.g. an associate array). If you return null, authentication
2828
* will be skipped.
2929
*
30-
* Whatever value you return here will be passed to authenticate()
30+
* Whatever value you return here will be passed to getUser() and checkCredentials()
3131
*
3232
* For example, for a form login, you might:
3333
*
@@ -47,19 +47,33 @@ interface GuardAuthenticatorInterface extends AuthenticationEntryPointInterface
4747
public function getCredentials(Request $request);
4848

4949
/**
50-
* Return a UserInterface object based on the credentials OR throw
51-
* an AuthenticationException.
50+
* Return a UserInterface object based on the credentials
5251
*
5352
* The *credentials* are the return value from getCredentials()
5453
*
54+
* You may throw an AuthenticationException if you wish. If you return
55+
* null, then a UsernameNotFoundException is thrown for you.
56+
*
5557
* @param mixed $credentials
5658
* @param UserProviderInterface $userProvider
5759
*
5860
* @throws AuthenticationException
5961
*
60-
* @return UserInterface
62+
* @return UserInterface|null
63+
*/
64+
public function getUser($credentials, UserProviderInterface $userProvider);
65+
66+
/**
67+
* Throw an AuthenticationException if the credentials are invalid
68+
*
69+
* The *credentials* are the return value from getCredentials()
70+
*
71+
* @param mixed $credentials
72+
* @param UserInterface $user
73+
* @throws AuthenticationException
74+
* @return void
6175
*/
62-
public function authenticate($credentials, UserProviderInterface $userProvider);
76+
public function checkCredentials($credentials, UserInterface $user);
6377

6478
/**
6579
* Create an authenticated token for the given user.

Guard/Provider/GuardAuthenticationProvider.php

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
use Symfony\Component\Security\Core\Authentication\Provider\AuthenticationProviderInterface;
66
use Symfony\Component\Security\Core\Authentication\Token\AnonymousToken;
7+
use Symfony\Component\Security\Core\Exception\UsernameNotFoundException;
78
use Symfony\Component\Security\Guard\GuardAuthenticatorInterface;
89
use Symfony\Component\Security\Guard\Token\GuardTokenInterface;
910
use Symfony\Component\Security\Guard\Token\PreAuthenticationGuardToken;
@@ -95,18 +96,28 @@ public function authenticate(TokenInterface $token)
9596
private function authenticateViaGuard(GuardAuthenticatorInterface $guardAuthenticator, PreAuthenticationGuardToken $token)
9697
{
9798
// get the user from the GuardAuthenticator
98-
$user = $guardAuthenticator->authenticate($token->getCredentials(), $this->userProvider);
99+
$user = $guardAuthenticator->getUser($token->getCredentials(), $this->userProvider);
100+
101+
if (null === $user) {
102+
throw new UsernameNotFoundException(sprintf(
103+
'Null returned from %s::getUser()',
104+
get_class($guardAuthenticator)
105+
));
106+
}
99107

100108
if (!$user instanceof UserInterface) {
101109
throw new \UnexpectedValueException(sprintf(
102-
'The %s::authenticate method must return a UserInterface. You returned %s.',
110+
'The %s::getUser method must return a UserInterface. You returned %s.',
103111
get_class($guardAuthenticator),
104112
is_object($user) ? get_class($user) : gettype($user)
105113
));
106114
}
107115

108-
// check the AdvancedUserInterface methods!
116+
// check the preAuth UserChecker
109117
$this->userChecker->checkPreAuth($user);
118+
// check the credentials
119+
$guardAuthenticator->checkCredentials($token->getCredentials(), $user);
120+
// check the postAuth UserChecker
110121
$this->userChecker->checkPostAuth($user);
111122

112123
// turn the UserInterface into a TokenInterface

Guard/Tests/Provider/GuardAuthenticationProviderTest.php

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,21 +43,25 @@ public function testAuthenticate()
4343
'username' => '_weaverryan_test_user',
4444
'password' => 'guard_auth_ftw',
4545
);
46-
$this->preAuthenticationToken->expects($this->once())
46+
$this->preAuthenticationToken->expects($this->atLeastOnce())
4747
->method('getCredentials')
4848
->will($this->returnValue($enteredCredentials));
4949

5050
// authenticators A and C are never called
5151
$authenticatorA->expects($this->never())
52-
->method('authenticate');
52+
->method('getUser');
5353
$authenticatorC->expects($this->never())
54-
->method('authenticate');
54+
->method('getUser');
5555

5656
$mockedUser = $this->getMock('Symfony\Component\Security\Core\User\UserInterface');
5757
$authenticatorB->expects($this->once())
58-
->method('authenticate')
58+
->method('getUser')
5959
->with($enteredCredentials, $this->userProvider)
6060
->will($this->returnValue($mockedUser));
61+
// checkCredentials is called
62+
$authenticatorB->expects($this->once())
63+
->method('checkCredentials')
64+
->with($enteredCredentials, $mockedUser);
6165
$authedToken = $this->getMock('Symfony\Component\Security\Core\Authentication\Token\TokenInterface');
6266
$authenticatorB->expects($this->once())
6367
->method('createAuthenticatedToken')

0 commit comments

Comments
 (0)