Skip to content

Commit f43b8fc

Browse files
weaverryanweaverryan
committed
refactoring to use a user provider
1 parent a7c5f61 commit f43b8fc

File tree

12 files changed

+161
-350
lines changed

12 files changed

+161
-350
lines changed

‎src/Symfony/Bridge/Doctrine/LoginLink/EntityLoginLinkUserHandler.php‎

Lines changed: 0 additions & 42 deletions
This file was deleted.

‎src/Symfony/Bridge/Doctrine/Tests/LoginLink/EntityLoginLinkUserHandlerTest.php‎

Lines changed: 0 additions & 68 deletions
This file was deleted.

‎src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/LoginLinkFactory.php‎

Lines changed: 9 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -38,15 +38,11 @@ public function addConfiguration(NodeDefinition $node)
3838
->isRequired()
3939
->info('Route that will validate the login link - e.g. app_login_link_verify')
4040
->end()
41-
->scalarNode('user_email_property')
42-
->isRequired()
43-
->info('The property path on the User object where the email address is stored.')
44-
->end()
45-
->scalarNode('user_entity_class')
46-
->info('If your user class is a Doctrine entity, set to the full class name - e.g. App\\Entity\\User.')
47-
->end()
48-
->scalarNode('user_handler_service')
49-
->info(sprintf('If your User is not a Doctrine entity, a service that implements "%s".', LoginLinkUserHandlerInterface::class))
41+
->arrayNode('signature_properties')
42+
->prototype('scalar')->end()
43+
->requiresAtLeastOneElement()
44+
->info('An array of properties on your User that are used to sign the link. If any of these change, all existing links will become invalid')
45+
->example(['email', 'password'])
5046
->end()
5147
->integerNode('lifetime')
5248
->defaultValue(600)
@@ -65,18 +61,8 @@ public function addConfiguration(NodeDefinition $node)
6561
->scalarNode('failure_handler')
6662
->info(sprintf('A service id that implements %s', AuthenticationFailureHandlerInterface::class))
6763
->end()
68-
;
69-
70-
// get the parent array node builder ("firewalls") from inside the children builder
71-
$factoryRootNode = $builder->end();
72-
$factoryRootNode
73-
->validate()
74-
->ifTrue(function ($v) { return !isset($v['user_entity_class']) && !isset($v['user_handler_service']); })
75-
->thenInvalid('Either the "user_entity_class" or "user_handler_service" option must be configured.')
76-
->end()
77-
->validate()
78-
->ifTrue(function ($v) { return isset($v['user_entity_class']) && isset($v['user_handler_service']); })
79-
->thenInvalid('Either the "user_entity_class" or "user_handler_service" option must be configured, but not both.')
64+
->scalarNode('provider')
65+
->info('the user provider to load users from.')
8066
->end()
8167
;
8268

@@ -105,16 +91,6 @@ public function createAuthenticator(ContainerBuilder $container, string $firewal
10591
$loader->load('security_authenticator_login_link.php');
10692
}
10793

108-
if (isset($config['user_entity_class'])) {
109-
$userHandlerId = 'security.authenticator.entity_login_link_user_handler_'.$firewallName;
110-
$container
111-
->setDefinition($userHandlerId, new ChildDefinition('security.authenticator.entity_login_link_user_handler'))
112-
->replaceArgument(1, $config['user_entity_class'])
113-
;
114-
} else {
115-
$userHandlerId = $config['user_handler_service'];
116-
}
117-
11894
if (null !== $config['max_uses'] && !isset($config['used_link_cache'])) {
11995
$config['used_link_cache'] = 'cache.app';
12096
}
@@ -135,8 +111,8 @@ public function createAuthenticator(ContainerBuilder $container, string $firewal
135111
];
136112
$container
137113
->setDefinition($linkerId, new ChildDefinition('security.authenticator.abstract_login_link_handler'))
138-
->replaceArgument(1, new Reference($userHandlerId))
139-
->replaceArgument(3, $config['user_email_property'])
114+
->replaceArgument(1, new Reference($userProviderId))
115+
->replaceArgument(3, $config['signature_properties'])
140116
->replaceArgument(5, $linkerOptions)
141117
->replaceArgument(6, $expiredStorageId ? new Reference($expiredStorageId) : null)
142118
->addTag('security.authenticator.login_linker', ['firewall' => $firewallName])

‎src/Symfony/Bundle/SecurityBundle/Resources/config/security_authenticator_login_link.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,9 @@
3434
->abstract()
3535
->args([
3636
service('router'),
37-
abstract_arg('user handler'),
37+
abstract_arg('user provider'),
3838
service('property_accessor'),
39-
abstract_arg('email_property'),
39+
abstract_arg('signature properties'),
4040
'%kernel.secret%',
4141
abstract_arg('options'),
4242
abstract_arg('expired login link storage'),

‎src/Symfony/Bundle/SecurityBundle/Tests/Functional/LoginLinkAuthenticationTest.php‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,17 +36,16 @@ public function testLoginLinkSuccess()
3636

3737
/** @var LoginLinkHandlerInterface $loginLinkHandler */
3838
$loginLinkHandler = self::$container->get(LoginLinkHandlerInterface::class);
39-
$user = new User('major_tom@example.com', 'passwordz');
39+
$user = new User('weaverryan', 'foo');
4040
$loginLink = $loginLinkHandler->createLoginLink($user);
41-
$this->assertStringContainsString('id=10', $loginLink);
42-
$this->assertStringContainsString('email=', $loginLink);
41+
$this->assertStringContainsString('user=weaverryan', $loginLink);
4342
$this->assertStringContainsString('hash=', $loginLink);
4443
$this->assertStringContainsString('expires=', $loginLink);
4544
$client->request('GET', $loginLink->getUrl());
4645
$response = $client->getResponse();
4746

4847
$this->assertSame(200, $response->getStatusCode());
49-
$this->assertSame(['message' => 'Welcome major_tom@example.com!'], json_decode($response->getContent(), true));
48+
$this->assertSame(['message' => 'Welcome weaverryan!'], json_decode($response->getContent(), true));
5049

5150
$client->request('GET', $loginLink->getUrl());
5251
$response = $client->getResponse();

‎src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/LoginLink/config.yml‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,16 +7,16 @@ security:
77
providers:
88
in_memory:
99
memory:
10-
users: []
10+
users:
11+
weaverryan: { password: foo, roles: [ROLE_USER] }
1112

1213
firewalls:
1314
main:
1415
pattern: ^/
1516
login_link:
1617
check_route: login_link_check
17-
user_email_property: 'username'
18+
signature_properties: ['password']
1819
max_uses: 2
19-
user_handler_service: Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\LoginLink\TestLoginLinkUserHandler
2020
success_handler: Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\LoginLink\TestCustomLoginLinkSuccessHandler
2121

2222
services:

‎src/Symfony/Component/Security/Http/Authenticator/LoginLinkAuthenticator.php‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -52,14 +52,14 @@ public function supports(Request $request): ?bool
5252

5353
public function authenticate(Request $request): PassportInterface
5454
{
55-
$id = $request->get('id');
55+
$username = $request->get('user');
5656

57-
if (!$id) {
58-
throw new InvalidLoginLinkAuthenticationException('Missing user id from link.');
57+
if (!$username) {
58+
throw new InvalidLoginLinkAuthenticationException('Missing user from link.');
5959
}
6060

6161
return new SelfValidatingPassport(
62-
new UserBadge($id, function () use ($request) {
62+
new UserBadge($username, function () use ($request) {
6363
try {
6464
$user = $this->loginLinkHandler
6565
->consumeLoginLink($request);

‎src/Symfony/Component/Security/Http/LoginLink/LoginLinkDetails.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
/**
1515
* @author Ryan Weaver <ryan@symfonycasts.com>
1616
*/
17-
final class LoginLinkDetails
17+
class LoginLinkDetails
1818
{
1919
private $url;
2020
private $expiresAt;

‎src/Symfony/Component/Security/Http/LoginLink/LoginLinkHandler.php‎

Lines changed: 27 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,11 @@
1212
namespace Symfony\Component\Security\Http\LoginLink;
1313

1414
use Symfony\Component\HttpFoundation\Request;
15-
use Symfony\Component\HttpKernel\UriSigner;
1615
use Symfony\Component\PropertyAccess\PropertyAccessorInterface;
1716
use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
17+
use Symfony\Component\Security\Core\Exception\UsernameNotFoundException;
1818
use Symfony\Component\Security\Core\User\UserInterface;
19+
use Symfony\Component\Security\Core\User\UserProviderInterface;
1920
use Symfony\Component\Security\Http\LoginLink\Exception\ExpiredLoginLinkException;
2021
use Symfony\Component\Security\Http\LoginLink\Exception\InvalidLoginLinkException;
2122

@@ -25,20 +26,19 @@
2526
final class LoginLinkHandler implements LoginLinkHandlerInterface
2627
{
2728
private $urlGenerator;
28-
private $userHandler;
29+
private $userProvider;
2930
private $propertyAccessor;
30-
private $emailProperty;
31+
private $signatureProperties;
3132
private $secret;
3233
private $options;
3334
private $expiredStorage;
34-
private $uriSigner;
3535

36-
public function __construct(UrlGeneratorInterface $urlGenerator, LoginLinkUserHandlerInterface $userHandler, PropertyAccessorInterface $propertyAccessor, string $emailProperty, string $secret, array $options, ?ExpiredLoginLinkStorage $expiredStorage)
36+
public function __construct(UrlGeneratorInterface $urlGenerator, UserProviderInterface $userProvider, PropertyAccessorInterface $propertyAccessor, array $signatureProperties, string $secret, array $options, ?ExpiredLoginLinkStorage $expiredStorage)
3737
{
3838
$this->urlGenerator = $urlGenerator;
39-
$this->userHandler = $userHandler;
39+
$this->userProvider = $userProvider;
4040
$this->propertyAccessor = $propertyAccessor;
41-
$this->emailProperty = $emailProperty;
41+
$this->signatureProperties = $signatureProperties;
4242
$this->secret = $secret;
4343
$this->options = array_merge([
4444
'route_name' => null,
@@ -52,10 +52,11 @@ public function createLoginLink(UserInterface $user): LoginLinkDetails
5252
{
5353
$expiresAt = new \DateTimeImmutable(sprintf('+%d seconds', $this->options['lifetime']));
5454

55+
$expires = $expiresAt->format('U');
5556
$parameters = [
56-
'id' => $this->userHandler->getUserIdentifier($user),
57-
'email' => $this->extractEmailAndHash($user),
58-
'expires' => $expiresAt->format('U'),
57+
'user' => $user->getUsername(),
58+
'expires' => $expires,
59+
'hash' => $this->computeSignatureHash($user, $expires),
5960
];
6061

6162
$url = $this->urlGenerator->generate(
@@ -64,30 +65,25 @@ public function createLoginLink(UserInterface $user): LoginLinkDetails
6465
UrlGeneratorInterface::ABSOLUTE_URL
6566
);
6667

67-
$signedUrl = $this->getUriSigner()->sign($url);
68-
69-
return new LoginLinkDetails($signedUrl, $expiresAt);
68+
return new LoginLinkDetails($url, $expiresAt);
7069
}
7170

7271
public function consumeLoginLink(Request $request): UserInterface
7372
{
74-
if (!$this->getUriSigner()->checkRequest($request)) {
75-
throw new InvalidLoginLinkException('Invalid signature.');
76-
}
73+
$username = $request->get('user');
7774

78-
$id = $request->get('id');
79-
$user = $this->userHandler->loadUserByIdentifier($id);
80-
81-
if (!$user) {
82-
throw new InvalidLoginLinkException('User not found.');
75+
try {
76+
$user = $this->userProvider->loadUserByUsername($username);
77+
} catch (UsernameNotFoundException $exception) {
78+
throw new InvalidLoginLinkException('User not found.', 0, $exception);
8379
}
8480

85-
$hashedEmail = $request->get('email');
86-
if (false === hash_equals($hashedEmail, $this->extractEmailAndHash($user))) {
87-
throw new InvalidLoginLinkException('Login link is for the wrong email.');
81+
$hash = $request->get('hash');
82+
$expires = $request->get('expires');
83+
if (false === hash_equals($hash, $this->computeSignatureHash($user, $expires))) {
84+
throw new InvalidLoginLinkException('User has changed since link was sent.');
8885
}
8986

90-
$expires = $request->get('expires');
9187
if ($expires < time()) {
9288
throw new ExpiredLoginLinkException('Login link has expired.');
9389
}
@@ -104,31 +100,15 @@ public function consumeLoginLink(Request $request): UserInterface
104100
return $user;
105101
}
106102

107-
private function extractEmailAndHash(UserInterface $user): string
103+
private function computeSignatureHash(UserInterface $user, int $expires): string
108104
{
109-
$email = $this->propertyAccessor->getValue($user, $this->emailProperty);
105+
$signatureFields = [base64_encode($user->getUsername()), $expires];
110106

111-
if (!$email) {
112-
throw new \LogicException('The email cannot be blank.');
107+
foreach ($this->signatureProperties as $property) {
108+
$value = $this->propertyAccessor->getValue($user, $property);
109+
$signatureFields[] = base64_encode($value);
113110
}
114111

115-
return hash_hmac('sha256', $email, $this->secret);
116-
}
117-
118-
private function getUriSigner(): UriSigner
119-
{
120-
if (null === $this->uriSigner) {
121-
$this->uriSigner = new UriSigner($this->secret, 'hash');
122-
}
123-
124-
return $this->uriSigner;
125-
}
126-
127-
/**
128-
* @internal
129-
*/
130-
public function setUriSigner(UriSigner $uriSigner)
131-
{
132-
$this->uriSigner = $uriSigner;
112+
return base64_encode(hash_hmac('sha256', implode(':', $signatureFields), $this->secret));
133113
}
134114
}

0 commit comments

Comments
 (0)