Skip to content

Commit 595b626

Browse files
Seldaeknicolas-grekas
authored andcommitted
[Security] Return default value instead of deferring to lower prio resolvers when using #[CurrentUser] and no user is found
1 parent aa3b7d5 commit 595b626

File tree

2 files changed

+12
-16
lines changed

2 files changed

+12
-16
lines changed

Controller/UserValueResolver.php

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -47,11 +47,6 @@ public function supports(Request $request, ArgumentMetadata $argument): bool
4747
return false;
4848
}
4949

50-
// if no user is present but a default value exists we delegate to DefaultValueResolver
51-
if ($argument->hasDefaultValue() && null === $this->tokenStorage->getToken()?->getUser()) {
52-
return false;
53-
}
54-
5550
return true;
5651
}
5752

@@ -62,20 +57,21 @@ public function resolve(Request $request, ArgumentMetadata $argument): array
6257
if (UserInterface::class !== $argument->getType() && !$argument->getAttributesOfType(CurrentUser::class, ArgumentMetadata::IS_INSTANCEOF)) {
6358
return [];
6459
}
65-
$user = $this->tokenStorage->getToken()?->getUser();
6660

67-
// if no user is present but a default value exists we delegate to DefaultValueResolver
68-
if ($argument->hasDefaultValue() && null === $user) {
69-
return [];
70-
}
61+
if (null === $user = $this->tokenStorage->getToken()?->getUser()) {
62+
// if no user is present but a default value exists we use it to prevent the EntityValueResolver or others
63+
// from attempting resolution of the User as the current logged in user was requested here
64+
if ($argument->hasDefaultValue()) {
65+
return [$argument->getDefaultValue()];
66+
}
7167

72-
if (null === $user) {
7368
if (!$argument->isNullable()) {
7469
throw new AccessDeniedException(sprintf('There is no logged-in user to pass to $%s, make the argument nullable if you want to allow anonymous access to the action.', $argument->getName()));
7570
}
7671

7772
return [null];
7873
}
74+
7975
if (null === $argument->getType() || $user instanceof ($argument->getType())) {
8076
return [$user];
8177
}

Tests/Controller/UserValueResolverTest.php

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@
2828
class UserValueResolverTest extends TestCase
2929
{
3030
/**
31-
* In Symfony 7, keep this test case but remove the call to supports()
31+
* In Symfony 7, keep this test case but remove the call to supports().
3232
*
3333
* @group legacy
3434
*/
@@ -43,18 +43,18 @@ public function testSupportsFailsWithNoType()
4343
}
4444

4545
/**
46-
* In Symfony 7, keep this test case but remove the call to supports()
46+
* In Symfony 7, keep this test case but remove the call to supports().
4747
*
4848
* @group legacy
4949
*/
5050
public function testSupportsFailsWhenDefaultValAndNoUser()
5151
{
5252
$tokenStorage = new TokenStorage();
5353
$resolver = new UserValueResolver($tokenStorage);
54-
$metadata = new ArgumentMetadata('foo', UserInterface::class, false, true, new InMemoryUser('username', 'password'));
54+
$metadata = new ArgumentMetadata('foo', UserInterface::class, false, true, $default = new InMemoryUser('username', 'password'));
5555

56-
$this->assertSame([], $resolver->resolve(Request::create('/'), $metadata));
57-
$this->assertFalse($resolver->supports(Request::create('/'), $metadata));
56+
$this->assertSame([$default], $resolver->resolve(Request::create('/'), $metadata));
57+
$this->assertTrue($resolver->supports(Request::create('/'), $metadata));
5858
}
5959

6060
public function testResolveSucceedsWithUserInterface()

0 commit comments

Comments
 (0)