8000 #21571 added logic to compare Roles · symfony/symfony@71ba983 · GitHub
[go: up one dir, main page]

Skip to content

Commit 71ba983

Browse files
committed
#21571 added logic to compare Roles
- updated AbstractToken to compare Roles - Updated isEqualTo method to match roles as default User implements EquatableInterface - added test case
1 parent dc7f6f7 commit 71ba983

File tree

10 files changed

+308
-0
lines changed

10 files changed

+308
-0
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
<?php
2+
3+
namespace Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\SecuredPageBundle\Controller;
4+
5+
use Symfony\Component\DependencyInjection\ContainerAwareInterface;
6+
use Symfony\Component\DependencyInjection\ContainerAwareTrait;
7+
use Symfony\Component\HttpFoundation\Response;
8+
9+
class AdminController implements ContainerAwareInterface
10+
{
11+
use ContainerAwareTrait;
12+
13+
public function indexAction()
14+
{
15+
return new Response('admin');
16+
}
17+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
admin:
2+
path: /admin
3+
defaults: { _controller: \Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\SecuredPageBundle\Controller\AdminController::indexAction }
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
<?php
2+
3+
namespace Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\SecuredPageBundle;
4+
5+
use Symfony\Component\HttpKernel\Bundle\Bundle;
6+
7+
class SecuredPageBundle extends Bundle
8+
{
9+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
<?php
2+
3+
namespace Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\SecuredPageBundle\Security\Core\User;
4+
5+
use Symfony\Component\Security\Core\Exception\UnsupportedUserException;
6+
use Symfony\Component\Security\Core\Exception\UsernameNotFoundException;
7+
use Symfony\Component\Security\Core\User\UserInterface;
8+
use Symfony\Component\Security\Core\User\UserProviderInterface;
9+
10+
class ArrayUserProvider implements UserProviderInterface
11+
{
12+
/** @var UserInterface[] */
13+
private $users = [];
14+
15+
public function addUser(UserInterface $user)
16+
{
17+
$this->users[$user->getUsername()] = $user;
18+
}
19+
20+
public function setUser($username, UserInterface $user)
21+
{
22+
$this->users[$username] = $user;
23+
}
24+
25+
public function getUser($username)
26+
{
27+
return $this->users[$username];
28+
}
29+
30+
public function loadUserByUsername($username)
31+
{
32+
$user = $this->getUser($username);
33+
34+
if (null === $user) {
35+
throw new UsernameNotFoundException(sprintf('User "%s" not found.', $username));
36+
}
37+
38+
return $user;
39+
}
40+
41+
public function refreshUser(UserInterface $user)
42+
{
43+
if (!$user instanceof UserInterface) {
44+
throw new UnsupportedUserException(sprintf('Instances of "%s" are not supported.', \get_class($user)));
45+
}
46+
47+
$storedUser = $this->getUser($user->getUsername());
48+
$class = \get_class($storedUser);
49+
50+
return new $class($storedUser->getUsername(), $storedUser->getPassword(), $storedUser->getRoles(), $storedUser->isEnabled(), $storedUser->isAccountNonExpired(), $storedUser->isCredentialsNonExpired() && $storedUser->getPassword() === $user->getPassword(), $storedUser->isAccountNonLocked());
51+
}
52+
53+
public function supportsClass($class)
54+
{
55+
return 'Symfony\Component\Security\Core\User\User' === $class;
56+
}
57+
}

src/Symfony/Bundle/SecurityBundle/Tests/Functional/SecurityTest.php

Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,10 @@
1111

1212
namespace Symfony\Bundle\SecurityBundle\Tests\Functional;
1313

14+
use Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\SecuredPageBundle\Security\Core\User\ArrayUserProvider;
1415
use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;
1516
use Symfony\Component\Security\Core\User\User;
17+
use Symfony\Component\Security\Core\User\UserInterface;
1618

1719
class SecurityTest extends WebTestCase
1820
{
@@ -31,4 +33,142 @@ public function testServiceIsFunctional()
3133
$this->assertTrue($security->isGranted('ROLE_USER'));
3234
$this->assertSame($token, $security->getToken());
3335
}
36+
37+
public function userWillBeMarkedAsChangedIfRolesHasChangedProvider()
38+
{
39+
return [
40+
[User::class],
41+
[UserWithoutEquatable::class],
42+
];
43+
}
44+
45+
/**
46+
* @dataProvider userWillBeMarkedAsChangedIfRolesHasChangedProvider
47+
*/
48+
public function testUserWillBeMarkedAsChangedIfRolesHasChanged($userClass)
49+
{
50+
$client = $this->createClient(['test_case' => 'AbstractTokenCompareRoles', 'root_config' => 'config.yml']);
51+
$client->disableReboot();
52+
53+
/** @var ArrayUserProvider $userProvider */
54+
$userProvider = static::$kernel->getContainer()->get('security.user.provider.array');
55+
$userProvider->addUser(new $userClass('user1', 'test', ['ROLE_ADMIN']));
56+
57+
$client->request('POST', '/login', [
58+
'_username' => 'user1',
59+
'_password' => 'test',
60+
]);
61+
62+
// user1 has ROLE_ADMIN and can visit secure page
63+
$client->request('GET', '/admin');
64+
$this->assertEquals(200, $client->getResponse()->getStatusCode());
65+
66+
// revoking ROLE_ADMIN from user1
67+
$userProvider->setUser('user1', new $userClass('user1', 'test', ['ROLE_USER']));
68+
69+
// user1 has lost ROLE_ADMIN and MUST be redirected away from secure page
70+
$client->request('GET', '/admin');
71+
$this->assertEquals(302, $client->getResponse()->getStatusCode());
72+
}
73+
}
74+
75+
final class UserWithoutEquatable implements UserInterface
76+
{
77+
private $username;
78+
private $password;
79+
private $enabled;
80+
private $accountNonExpired;
81+
private $credentialsNonExpired;
82+
private $accountNonLocked;
83+
private $roles;
84+
85+
public function __construct(?string $username, ?string $password, array $roles = [], bool $enabled = true, bool $userNonExpired = true, bool $credentialsNonExpired = true, bool $userNonLocked = true)
86+
{
87+
if ('' === $username || null === $username) {
88+
throw new \InvalidArgumentException('The username cannot be empty.');
89+
}
90+
91+
$this->username = $username;
92+
$this->password = $password;
93+
$this->enabled = $enabled;
94+
$this->accountNonExpired = $userNonExpired;
95+
$this->credentialsNonExpired = $credentialsNonExpired;
96+
$this->accountNonLocked = $userNonLocked;
97+
$this->roles = $roles;
98+
}
99+
100+
public function __toString()
101+
{
102+
return $this->getUsername();
103+
}
104+
105+
/**
106+
* {@inheritdoc}
107+
*/
108+
public function getRoles()
109+
{
110+
return $this->roles;
111+
}
112+
113+
/**
114+
* {@inheritdoc}
115+
*/
116+
public function getPassword()
117+
{
118+
return $this->password;
119+
}
120+
121+
/**
122+
* {@inheritdoc}
123+
*/
124+
public function getSalt()
125+
{
126+
}
127+
128+
/**
129+
* {@inheritdoc}
130+
*/
131+
public function getUsername()
132+
{
133+
return $this->username;
134+
}
135+
136+
/**
137+
* {@inheritdoc}
138+
*/
139+
public function isAccountNonExpired()
140+
{
141+
return $this->accountNonExpired;
142+
}
143+
144+
/**
145+
* {@inheritdoc}
146+
*/
147+
public function isAccountNonLocked()
148+
{
149+
return $this->accountNonLocked;
150+
}
151+
152+
/**
153+
* {@inheritdoc}
154+
*/
155+
public function isCredentialsNonExpired()
156+
{
157+
return $this->credentialsNonExpired;
158+
}
159+
160+
/**
161+
* {@inheritdoc}
162+
*/
163+
public function isEnabled()
164+
{
165+
return $this->enabled;
166+
}
167+
168+
/**
169+
* {@inheritdoc}
170+
*/
171+
public function eraseCredentials()
172+
{
173+
}
34174
}
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
use Symfony\Bundle\FrameworkBundle\FrameworkBundle;
13+
use Symfony\Bundle\SecurityBundle\SecurityBundle;
14+
15+
return [
16+
new FrameworkBundle(),
17+
new SecurityBundle(),
18+
new \Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\SecuredPageBundle\SecuredPageBundle(),
19+
];
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
imports:
2+
- { resource: ./../config/framework.yml }
3+
4+
services:
5+
_defaults: { public: true }
6+
7+
security.user.provider.array:
8+
class: Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\SecuredPageBundle\Security\Core\User\ArrayUserProvider
9+
10+
security:
11+
12+
encoders:
13+
\Symfony\Component\Security\Core\User\UserInterface: plaintext
14+
15+
providers:
16+
array:
17+
id: security.user.provider.array
18+
19+
firewalls:
20+
default:
21+
form_login:
22+
check_path: login
23+
remember_me: true
24+
require_previous_session: false
25+
logout: ~
26+
anonymous: ~
27+
stateless: false
28+
29+
access_control:
30+
- { path: ^/admin$, roles: ROLE_ADMIN }
31+
- { path: ^/login$, roles: IS_AUTHENTICATED_ANONYMOUSLY }
32+
- { path: .*, roles: IS_AUTHENTICATED_FULLY }
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
login:
2+
path: /login
3+
4+
logout:
5+
path: /logout
6+
7+
admin_bundle:
8+
resource: '@SecuredPageBundle/Resources/config/routing.yml'
9+

src/Symfony/Component/Security/Core/Authentication/Token/AbstractToken.php

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -317,6 +317,17 @@ private function hasUserChanged(UserInterface $user)
317317
return true;
318318
}
319319

320+
$rolesChanged = 1 === \count(
321+
array_diff(
322+
(array) \array_map('strval', $this->user->getRoles()),
323+
(array) \array_map('strval', $user->getRoles())
324+
)
325+
);
326+
327+
if ($rolesChanged) {
328+
return true;
329+
}
330+
320331
if ($this->user->getUsername() !== $user->getUsername()) {
321332
return true;
322333
}

src/Symfony/Component/Security/Core/User/User.php

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,17 @@ public function isEqualTo(UserInterface $user)
135135
return false;
136136
}
137137

138+
$rolesChanged = 1 === \count(
139+
array_diff(
140+
(array) \array_map('strval', $this->getRoles()),
141+
(array) \array_map('strval', $user->getRoles())
142+
)
143+
);
144+
145+
if ($rolesChanged) {
146+
return false;
147+
}
148+
138149
if ($this->getUsername() !== $user->getUsername()) {
139150
return false;
140151
}

0 commit comments

Comments
 (0)
0