Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit ec73043

Browse filesBrowse files
nicolas-grekasnicolas-grekas
committed
bug #49034 [Security] Return default value instead of deferring to lower prio resolvers when using #[CurrentUser] and no user is found (Seldaek)
This PR was merged into the 6.2 branch. Discussion ---------- [Security] Return default value instead of deferring to lower prio resolvers when using #[CurrentUser] and no user is found | Q | A | ------------- | --- | Branch? | 6.2 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | - The issue here is that if you have an action with `fooAction(string $email, #[CurrentUser] User $user = null)` i.e. you want the current user if logged in but otherwise null, if there is no user logged in the EntityValueResolver gets called after UserValueResolver, and if you also have another param which would allow it to try to resolve a User it will do so as it is a mapped entity. But if it gives you a random user here instead of the logged user this is BAD :) Commits ------- abc5203 [Security] Return default value instead of deferring to lower prio resolvers when using #[CurrentUser] and no user is found
2 parents 6bd4770 + abc5203 commit ec73043
Copy full SHA for ec73043

File tree

2 files changed

+12
-16
lines changed
Filter options

2 files changed

+12
-16
lines changed

‎src/Symfony/Component/Security/Http/Controller/UserValueResolver.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Http/Controller/UserValueResolver.php
+7-11Lines 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
}

‎src/Symfony/Component/Security/Http/Tests/Controller/UserValueResolverTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Http/Tests/Controller/UserValueResolverTest.php
+5-5Lines 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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.