8000 feature #45761 Throw access denied if CurrentUser cannot be resolved … · symfony/symfony@b0b5ff7 · GitHub
[go: up one dir, main page]
Skip to content

Commit b0b5ff7

Browse files
fabpotfabpot
committed
feature #45761 Throw access denied if CurrentUser cannot be resolved instead of a 500 (Seldaek)
This PR was squashed before being merged into the 6.1 branch. Discussion ---------- Throw access denied if CurrentUser cannot be resolved instead of a 500 | Q | A | ------------- | --- | Branch? | 6.1 | Bug fix? | no | New feature? | yes | Deprecations? | no | Tickets | Fix #45257 | License | MIT | Doc PR | symfony/symfony-docs#... <!-- required for new features --> When using `#[CurrentUser] User $user` in my controller I do expect to get that, and if the user is not logged in clearly I am expecting a logged in user here so throwing an AccessDeniedException for me would be super convenient. Right now it simply stops resolving that param, and we end up with a 500 for example: > [2022-03-16T06:33:37.867185+00:00] request.CRITICAL: Uncaught PHP Exception Symfony\Component\DependencyInjection\Exception\RuntimeException: "Cannot autowire argument $loggedUser of "App\Controller\UserController::fooAction()": it references class "App\Entity\User" but no such service exists." Yes it's a failure of my firewall config if you will, but on the other hand I don't see the point in having to list every URL that needs a user in the firewall, which is a very un-DRY and error prone process, if it can be done for me in this way. I would personally consider this a bugfix and submit the PR against 5.4, but I thought I'd start the discussion with a PR for 6.1 :) Commits ------- fcafa58 Throw access denied if CurrentUser cannot be resolved instead of a 500
2 parents 8e8207b + fcafa58 commit b0b5ff7

File tree

2 files changed

+96
-21
lines changed

2 files changed

+96
-21
lines changed

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

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
use Symfony\Component\HttpKernel\Controller\ArgumentValueResolverInterface;
1616
use Symfony\Component\HttpKernel\ControllerMetadata\ArgumentMetadata;
1717
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
18-
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
18+
use Symfony\Component\Security\Core\Exception\AccessDeniedException;
1919
use Symfony\Component\Security\Core\User\UserInterface;
2020
use Symfony\Component\Security\Http\Attribute\CurrentUser;
2121

@@ -41,13 +41,27 @@ public function supports(Request $request, ArgumentMetadata $argument): bool
4141
return false;
4242
}
4343

44-
$token = $this->tokenStorage->getToken();
44+
// if no user is present but a default value exists we delegate to DefaultValueResolver
45+
if ($argument->hasDefaultValue() && null === $this->tokenStorage->getToken()?->getUser()) {
46+
return false;
47+
}
4548

46-
return $token instanceof TokenInterface;
49+
return true;
4750
}
4851

4952
public function resolve(Request $request, ArgumentMetadata $argument): iterable
5053
{
51-
yield $this->tokenStorage->getToken()->getUser();
54+
$user = $this->tokenStorage->getToken()?->getUser();
55+
56+
if (null === $user) {
57+
if (!$argument->isNullable()) {
58+
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()));
59+
}
60+
yield null;
61+
} elseif (null === $argument->getType() || $user instanceof ($argument->getType())) {
62+
yield $user;
63+
} else {
64+
throw new AccessDeniedException(sprintf('The logged-in user is an instance of "%s" and an user of type "%s" is expected.', $user::class, $argument->getType()));
65+
}
5266
}
5367
}

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

Lines changed: 78 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -16,61 +16,77 @@
1616
use Symfony\Component\HttpKernel\Controller\ArgumentResolver;
1717
use Symfony\Component\HttpKernel\Controller\ArgumentResolver\DefaultValueResolver;
1818
use Symfony\Component\HttpKernel\ControllerMetadata\ArgumentMetadata;
19+
use Symfony\Component\Security\Core\Authentication\Token\NullToken;
1920
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage;
2021
use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;
22+
use Symfony\Component\Security\Core\Exception\AccessDeniedException;
2123
use Symfony\Component\Security\Core\User\InMemoryUser;
2224
use Symfony\Component\Security\Core\User\UserInterface;
2325
use Symfony\Component\Security\Http\Attribute\CurrentUser;
2426
use Symfony\Component\Security\Http\Controller\UserValueResolver;
2527

2628
class UserValueResolverTest extends TestCase
2729
{
28-
public function testResolveNoToken()
30+
public function testSupportsFailsWithNoType()
2931
{
3032
$tokenStorage = new TokenStorage();
3133
$resolver = new UserValueResolver($tokenStorage);
32-
$metadata = new ArgumentMetadata('foo', UserInterface::class, false, false, null);
34+
$metadata = new ArgumentMetadata('foo', null, false, false, null);
3335

3436
$this->assertFalse($resolver->supports(Request::create('/'), $metadata));
3537
}
3638

37-
public function testResolveNoUser()
39+
public function testSupportsFailsWhenDefaultValAndNoUser()
3840
{
39-
$mock = $this->createMock(UserInterface::class);
40-
$token = new UsernamePasswordToken(new InMemoryUser('username', 'password'), 'provider');
4141
$tokenStorage = new TokenStorage();
42-
$tokenStorage->setToken($token);
43-
4442
$resolver = new UserValueResolver($tokenStorage);
45-
$metadata = new ArgumentMetadata('foo', \get_class($mock), false, false, null);
43+
$metadata = new ArgumentMetadata('foo', UserInterface::class, false, true, new InMemoryUser('username', 'password'));
4644

4745
$this->assertFalse($resolver->supports(Request::create('/'), $metadata));
4846
}
4947

50-
public function testResolveWrongType()
48+
public function testResolveSucceedsWithUserInterface()
5149
{
50+
$user = new InMemoryUser('username', 'password');
51+
$token = new UsernamePasswordToken($user, 'provider');
5252
$tokenStorage = new TokenStorage();
53+
$tokenStorage->setToken($token);
54+
5355
$resolver = new UserValueResolver($tokenStorage);
54-
$metadata = new ArgumentMetadata('foo', null, false, false, null);
56+
$metadata = new ArgumentMetadata('foo', UserInterface::class, false, false, null);
5557

56-
$this->assertFalse($resolver->supports(Request::create('/'), $metadata));
58+
$this->assertTrue($resolver->supports(Request::create('/'), $metadata));
59+
$this->assertSame([$user], iterator_to_array($resolver->resolve(Request::create('/'), $metadata)));
5760
}
5861

59-
public function testResolve()
62+
public function testResolveSucceedsWithSubclassType()
6063
{
6164
$user = new InMemoryUser('username', 'password');
6265
$token = new UsernamePasswordToken($user, 'provider');
6366
$tokenStorage = new TokenStorage();
6467
$tokenStorage->setToken($token);
6568

6669
$resolver = new UserValueResolver($tokenStorage);
67-
$metadata = new ArgumentMetadata('foo', UserInterface::class, false, false, null);
70+
$metadata = new ArgumentMetadata('foo', InMemoryUser::class, false, false, null, false, [new CurrentUser()]);
6871

6972
$this->assertTrue($resolver->supports(Request::create('/'), $metadata));
7073
$this->assertSame([$user], iterator_to_array($resolver->resolve(Request::create('/'), $metadata)));
7174
}
7275

73-
public function testResolveWithAttribute()
76+
public function testResolveSucceedsWithNullableParamAndNoUser()
77+
{
78+
$token = new NullToken();
79+
$tokenStorage = new TokenStorage();
80+
$tokenStorage->setToken($token);
81+
82+
$resolver = new UserValueResolver($tokenStorage);
83+
$metadata = new ArgumentMetadata('foo', InMemoryUser::class, false, false, null, true, [new CurrentUser()]);
84+
85+
$this->assertTrue($resolver->supports(Request::create('/'), $metadata));
86+
$this->assertSame([null], iterator_to_array($resolver->resolve(Request::create('/'), $metadata)));
87+
}
88+
89+
public function testResolveSucceedsWithNullableAttribute()
7490
{
7591
$user = new InMemoryUser('username', 'password');
7692
$token = new UsernamePasswordToken($user, 'provider');
@@ -85,14 +101,59 @@ public function testResolveWithAttribute()
85101
$this->assertSame([$user], iterator_to_array($resolver->resolve(Request::create('/'), $metadata)));
86102
}
87103

88-
public function testResolveWithAttributeAndNoUser()
104+
public function testResolveSucceedsWithTypedAttribute()
89105
{
106+
$user = new InMemoryUser('username', 'password');
107+
$token = new UsernamePasswordToken($user, 'provider');
90108
$tokenStorage = new TokenStorage();
109+
$tokenStorage->setToken($token);
91110

92111
$resolver = new UserValueResolver($tokenStorage);
93-
$metadata = new ArgumentMetadata('foo', null, false, false, null, false, [new CurrentUser()]);
112+
$metadata = $this->createMock(ArgumentMetadata::class);
113+
$metadata = new ArgumentMetadata('foo', InMemoryUser::class, false, false, null, false, [new CurrentUser()]);
94114

95-
$this->assertFalse($resolver->supports(Request::create('/'), $metadata));
115+
$this->assertTrue($resolver->supports(Request::create('/'), $metadata));
116+
$this->assertSame([$user], iterator_to_array($resolver->resolve(Request::create('/'), $metadata)));
117+
}
118+
119+
public function testResolveThrowsAccessDeniedWithWrongUserClass()
120+
{
121+
$user = $this->createMock(UserInterface::class);
122+
$token = new UsernamePasswordToken($user, 'provider');
123+
$tokenStorage = new TokenStorage();
124+
$tokenStorage->setToken($token);
125+
126+
$resolver = new UserValueResolver($tokenStorage);
127+
$metadata = new ArgumentMetadata('foo', InMemoryUser::class, false, false, null, false, [new CurrentUser()]);
128+
129+
$this->assertTrue($resolver->supports(Request::create('/'), $metadata));
130+
$this->expectException(AccessDeniedException::class);
131+
$this->expectExceptionMessageMatches('/^The logged-in user is an instance of "Mock_UserInterface[^"]+" and an user of type "Symfony\\\\Component\\\\Security\\\\Core\\\\User\\\\InMemoryUser" is expected.$/');
132+
iterator_to_array($resolver->resolve(Request::create('/'), $metadata));
133+
}
134+
135+
public function testResolveThrowsAccessDeniedWithAttributeAndNoUser()
136+
{
137+
$tokenStorage = new TokenStorage();
138+
139+
$resolver = new UserValueResolver($tokenStorage);
140+
$metadata = new ArgumentMetadata('foo', UserInterface::class, false, false, null, false, [new CurrentUser()]);
141+
142+
$this->assertTrue($resolver->supports(Request::create('/'), $metadata));
143+
$this->expectException(AccessDeniedException::class);
144+
$this->expectExceptionMessage('There is no logged-in user to pass to $foo, make the argument nullable if you want to allow anonymous access to the action.');
145+
iterator_to_array($resolver->resolve(Request::create('/'), $metadata));
146+
}
147+
148+
public function testResolveThrowsAcessDeniedWithNoToken()
149+
{
150+
$tokenStorage = new TokenStorage();
151+
$resolver = new UserValueResolver($tokenStorage);
152+
$metadata = new ArgumentMetadata('foo', UserInterface::class, false, false, null);
153+
154+
$this->assertTrue($resolver->supports(Request::create('/'), $metadata));
155+
$this->expectException(AccessDeniedException::class);
156+
iterator_to_array($resolver->resolve(Request::create('/'), $metadata));
96157
}
97158

98159
public function testIntegration()

0 commit comments

Comments
 (0)
0