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 8e50eb9

Browse filesBrowse files
fabpotfabpot
committed
feature #31177 #21571 Comparing roles to detected that users has changed (oleg-andreyev)
This PR was merged into the 4.4 branch. Discussion ---------- #21571 Comparing roles to detected that users has changed | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | yes | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | License | MIT | Fixed tickets | #21571 (comment) | Docs | symfony/symfony-docs#11457 **Case 1:** User A has roles `foo, bar and admin`, User A is signed-in into application and token is persisted, later another User B with role `admin`, decided to restrict role `admin` for User A, so User A won't lose it's privileges until session is expired or logout, because token is persisted with `roles` and `authenticated=true` and roles are not compared. Ref. to the previous attempt: #27121 Commits ------- 4f4c30d - updated AbstractToken to compare Roles - Updated isEqualTo method to match roles as default User implements EquatableInterface - added test case - bumped symfony/security-core to 4.4
2 parents cab412f + 4f4c30d commit 8e50eb9
Copy full SHA for 8e50eb9

File tree

12 files changed

+310
-3
lines changed
Filter options

12 files changed

+310
-3
lines changed

‎src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/SecuredPageBundle/Controller/AdminController.php

Copy file name to clipboard
+17Lines changed: 17 additions & 0 deletions
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+
}

‎src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/SecuredPageBundle/Resources/config/routing.yml

Copy file name to clipboard
+3Lines changed: 3 additions & 0 deletions
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 }

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

Copy file name to clipboard
+9Lines 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+
}

‎src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/SecuredPageBundle/Security/Core/User/ArrayUserProvider.php

Copy file name to clipboard
+57Lines changed: 57 additions & 0 deletions
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

Copy file name to clipboardExpand all lines: src/Symfony/Bundle/SecurityBundle/Tests/Functional/SecurityTest.php
+146Lines changed: 146 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 AbstractWebTestCase
1820
{
@@ -31,4 +33,148 @@ 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+
[
41+
new User('user1', 'test', ['ROLE_ADMIN']),
42+
new User('user1', 'test', ['ROLE_USER']),
43+
],
44+
[
45+
new UserWithoutEquatable('user1', 'test', ['ROLE_ADMIN']),
46+
new UserWithoutEquatable('user1', 'test', ['ROLE_USER']),
47+
],
48+
];
49+
}
50+
51+
/**
52+
* @dataProvider userWillBeMarkedAsChangedIfRolesHasChangedProvider
53+
*/
54+
public function testUserWillBeMarkedAsChangedIfRolesHasChanged(UserInterface $userWithAdminRole, UserInterface $userWithoutAdminRole)
55+
{
56+
$client = $this->createClient(['test_case' => 'AbstractTokenCompareRoles', 'root_config' => 'config.yml']);
57+
$client->disableReboot();
58+
59+
/** @var ArrayUserProvider $userProvider */
60+
$userProvider = static::$kernel->getContainer()->get('security.user.provider.array');
61+
$userProvider->addUser($userWithAdminRole);
62+
63+
$client->request('POST', '/login', [
64+
'_username' => 'user1',
65+
'_password' => 'test',
66+
]);
67+
68+
// user1 has ROLE_ADMIN and can visit secure page
69+
$client->request('GET', '/admin');
70+
$this->assertEquals(200, $client->getResponse()->getStatusCode());
71+
72+
// updating user provider with same user but revoked ROLE_ADMIN from user1
73+
$userProvider->setUser('user1', $userWithoutAdminRole);
74+
75+
// user1 has lost ROLE_ADMIN and MUST be redirected away from secure page
76+
$client->request('GET', '/admin');
77+
$this->assertEquals(302, $client->getResponse()->getStatusCode());
78+
}
79+
}
80+
81+
final class UserWithoutEquatable implements UserInterface
82+
{
83+
private $username;
84+
private $password;
85+
private $enabled;
86+
private $accountNonExpired;
87+
private $credentialsNonExpired;
88+
private $accountNonLocked;
89+
private $roles;
90+
91+
public function __construct(?string $username, ?string $password, array $roles = [], bool $enabled = true, bool $userNonExpired = true, bool $credentialsNonExpired = true, bool $userNonLocked = true)
92+
{
93+
if ('' === $username || null === $username) {
94+
throw new \InvalidArgumentException('The username cannot be empty.');
95+
}
96+
97+
$this->username = $username;
98+
$this->password = $password;
99+
$this->enabled = $enabled;
100+
$this->accountNonExpired = $userNonExpired;
101+
$this->credentialsNonExpired = $credentialsNonExpired;
102+
$this->accountNonLocked = $userNonLocked;
103+
$this->roles = $roles;
104+
}
105+
106+
public function __toString()
107+
{
108+
return $this->getUsername();
109+
}
110+
111+
/**
112+
* {@inheritdoc}
113+
*/
114+
public function getRoles()
115+
{
116+
return $this->roles;
117+
}
118+
119+
/**
120+
* {@inheritdoc}
121+
*/
122+
public function getPassword()
123+
{
124+
return $this->password;
125+
}
126+
127+
/**
128+
* {@inheritdoc}
129+
*/
130+
public function getSalt()
131+
{
132+
}
133+
134+
/**
135+
* {@inheritdoc}
136+
*/
137+
public function getUsername()
138+
{
139+
return $this->username;
140+
}
141+
142+
/**
143+
* {@inheritdoc}
144+
*/
145+
public function isAccountNonExpired()
146+
{
147+
return $this->accountNonExpired;
148+
}
149+
150+
/**
151+
* {@inheritdoc}
152+
*/
153+
public function isAccountNonLocked()
154+
{
155+
return $this->accountNonLocked;
156+
}
157+
158+
/**
159+
* {@inheritdoc}
160+
*/
161+
public function isCredentialsNonExpired()
162+
{
163+
return $this->credentialsNonExpired;
164+
}
165+
166+
/**
167+
* {@inheritdoc}
168+
*/
169+
public function isEnabled()
170+
{
171+
return $this->enabled;
172+
}
173+
174+
/**
175+
* {@inheritdoc}
176+
*/
177+
public function eraseCredentials()
178+
{
179+
}
34180
}

‎src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/AbstractTokenCompareRoles/bundles.php

Copy file name to clipboard
+20Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
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+
use Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\SecuredPageBundle\SecuredPageBundle;
15+
16+
return [
17+
new FrameworkBundle(),
18+
new SecurityBundle(),
19+
new SecuredPageBundle(),
20+
];

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

Copy file name to clipboard
+32Lines 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 }

‎src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/AbstractTokenCompareRoles/routing.yml

Copy file name to clipboard
+8Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
login:
2+
path: /login
3+
4+
logout:
5+
path: /logout
6+
7+
admin_bundle:
8+
resource: '@SecuredPageBundle/Resources/config/routing.yml'

‎src/Symfony/Bundle/SecurityBundle/composer.json

Copy file name to clipboardExpand all lines: src/Symfony/Bundle/SecurityBundle/composer.json
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
"symfony/config": "^4.2|^5.0",
2222
"symfony/dependency-injection": "^4.2|^5.0",
2323
"symfony/http-kernel": "^4.4",
24-
"symfony/security-core": "^4.3",
24+
"symfony/security-core": "^4.4",
2525
"symfony/security-csrf": "^4.2|^5.0",
2626
"symfony/security-guard": "^4.2|^5.0",
2727
"symfony/security-http": "^4.3"

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Core/Authentication/Token/AbstractToken.php
+8Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,7 @@ public function __serialize(): array
166166
* @return string
167167
*
168168
* @final since Symfony 4.3, use __serialize() instead
169+
*
169170
* @internal since Symfony 4.3, use __serialize() instead
170171
*/
171172
public function serialize()
@@ -316,6 +317,13 @@ private function hasUserChanged(UserInterface $user)
316317
return true;
317318
}
318319

320+
$userRoles = array_map('strval', (array) $user->getRoles());
321+
$rolesChanged = \count($userRoles) !== \count($this->getRoleNames()) || \count($userRoles) !== \count(array_intersect($userRoles, $this->getRoleNames()));
322+
323+
if ($rolesChanged) {
324+
return true;
325+
}
326+
319327
if ($this->user->getUsername() !== $user->getUsername()) {
320328
return true;
321329
}

‎src/Symfony/Component/Security/Core/Tests/User/UserTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Core/Tests/User/UserTest.php
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,8 +117,8 @@ public static function isEqualToData()
117117
{
118118
return [
119119
[true, new User('username', 'password'), new User('username', 'password')],
120-
[true, new User('username', 'password', ['ROLE']), new User('username', 'password')],
121-
[true, new User('username', 'password', ['ROLE']), new User('username', 'password', ['NO ROLE'])],
120+
[false, new User('username', 'password', ['ROLE']), new User('username', 'password')],
121+
[false, new User('username', 'password', ['ROLE']), new User('username', 'password', ['NO ROLE'])],
122122
[false, new User('diff', 'diff'), new User('username', 'password')],
123123
[false, new User('diff', 'diff', [], false), new User('username', 'password')],
124124
[false, new User('diff', 'diff', [], false, false), new User('username', 'password')],

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Core/User/User.php
+7Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,13 @@ public function isEqualTo(UserInterface $user): bool
143143
return false;
144144
}
145145

146+
$currentRoles = array_map('strval', (array) $this->getRoles());
147+
$newRoles = array_map('strval', (array) $user->getRoles());
148+
$rolesChanged = \count($currentRoles) !== \count($newRoles) || \count($currentRoles) !== \count(array_intersect($currentRoles, $newRoles));
149+
if ($rolesChanged) {
150+
return false;
151+
}
152+
146153
if ($this->getUsername() !== $user->getUsername()) {
147154
return false;
148155
}

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.