- 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
This commit is contained in:
Oleg Andreyev 2019-04-19 01:01:25 +03:00 committed by Oleg Andreyev
parent cab412f0cb
commit 4f4c30d59e
12 changed files with 310 additions and 3 deletions

View File

@ -0,0 +1,17 @@
<?php
namespace Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\SecuredPageBundle\Controller;
use Symfony\Component\DependencyInjection\ContainerAwareInterface;
use Symfony\Component\DependencyInjection\ContainerAwareTrait;
use Symfony\Component\HttpFoundation\Response;
class AdminController implements ContainerAwareInterface
{
use ContainerAwareTrait;
public function indexAction()
{
return new Response('admin');
}
}

View File

@ -0,0 +1,3 @@
admin:
path: /admin
defaults: { _controller: \Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\SecuredPageBundle\Controller\AdminController::indexAction }

View File

@ -0,0 +1,9 @@
<?php
namespace Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\SecuredPageBundle;
use Symfony\Component\HttpKernel\Bundle\Bundle;
class SecuredPageBundle extends Bundle
{
}

View File

@ -0,0 +1,57 @@
<?php
namespace Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\SecuredPageBundle\Security\Core\User;
use Symfony\Component\Security\Core\Exception\UnsupportedUserException;
use Symfony\Component\Security\Core\Exception\UsernameNotFoundException;
use Symfony\Component\Security\Core\User\UserInterface;
use Symfony\Component\Security\Core\User\UserProviderInterface;
class ArrayUserProvider implements UserProviderInterface
{
/** @var UserInterface[] */
private $users = [];
public function addUser(UserInterface $user)
{
$this->users[$user->getUsername()] = $user;
}
public function setUser($username, UserInterface $user)
{
$this->users[$username] = $user;
}
public function getUser($username)
{
return $this->users[$username];
}
public function loadUserByUsername($username)
{
$user = $this->getUser($username);
if (null === $user) {
throw new UsernameNotFoundException(sprintf('User "%s" not found.', $username));
}
return $user;
}
public function refreshUser(UserInterface $user)
{
if (!$user instanceof UserInterface) {
throw new UnsupportedUserException(sprintf('Instances of "%s" are not supported.', \get_class($user)));
}
$storedUser = $this->getUser($user->getUsername());
$class = \get_class($storedUser);
return new $class($storedUser->getUsername(), $storedUser->getPassword(), $storedUser->getRoles(), $storedUser->isEnabled(), $storedUser->isAccountNonExpired(), $storedUser->isCredentialsNonExpired() && $storedUser->getPassword() === $user->getPassword(), $storedUser->isAccountNonLocked());
}
public function supportsClass($class)
{
return 'Symfony\Component\Security\Core\User\User' === $class;
}
}

View File

@ -11,8 +11,10 @@
namespace Symfony\Bundle\SecurityBundle\Tests\Functional;
use Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\SecuredPageBundle\Security\Core\User\ArrayUserProvider;
use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;
use Symfony\Component\Security\Core\User\User;
use Symfony\Component\Security\Core\User\UserInterface;
class SecurityTest extends AbstractWebTestCase
{
@ -31,4 +33,148 @@ class SecurityTest extends AbstractWebTestCase
$this->assertTrue($security->isGranted('ROLE_USER'));
$this->assertSame($token, $security->getToken());
}
public function userWillBeMarkedAsChangedIfRolesHasChangedProvider()
{
return [
[
new User('user1', 'test', ['ROLE_ADMIN']),
new User('user1', 'test', ['ROLE_USER']),
],
[
new UserWithoutEquatable('user1', 'test', ['ROLE_ADMIN']),
new UserWithoutEquatable('user1', 'test', ['ROLE_USER']),
],
];
}
/**
* @dataProvider userWillBeMarkedAsChangedIfRolesHasChangedProvider
*/
public function testUserWillBeMarkedAsChangedIfRolesHasChanged(UserInterface $userWithAdminRole, UserInterface $userWithoutAdminRole)
{
$client = $this->createClient(['test_case' => 'AbstractTokenCompareRoles', 'root_config' => 'config.yml']);
$client->disableReboot();
/** @var ArrayUserProvider $userProvider */
$userProvider = static::$kernel->getContainer()->get('security.user.provider.array');
$userProvider->addUser($userWithAdminRole);
$client->request('POST', '/login', [
'_username' => 'user1',
'_password' => 'test',
]);
// user1 has ROLE_ADMIN and can visit secure page
$client->request('GET', '/admin');
$this->assertEquals(200, $client->getResponse()->getStatusCode());
// updating user provider with same user but revoked ROLE_ADMIN from user1
$userProvider->setUser('user1', $userWithoutAdminRole);
// user1 has lost ROLE_ADMIN and MUST be redirected away from secure page
$client->request('GET', '/admin');
$this->assertEquals(302, $client->getResponse()->getStatusCode());
}
}
final class UserWithoutEquatable implements UserInterface
{
private $username;
private $password;
private $enabled;
private $accountNonExpired;
private $credentialsNonExpired;
private $accountNonLocked;
private $roles;
public function __construct(?string $username, ?string $password, array $roles = [], bool $enabled = true, bool $userNonExpired = true, bool $credentialsNonExpired = true, bool $userNonLocked = true)
{
if ('' === $username || null === $username) {
throw new \InvalidArgumentException('The username cannot be empty.');
}
$this->username = $username;
$this->password = $password;
$this->enabled = $enabled;
$this->accountNonExpired = $userNonExpired;
$this->credentialsNonExpired = $credentialsNonExpired;
$this->accountNonLocked = $userNonLocked;
$this->roles = $roles;
}
public function __toString()
{
return $this->getUsername();
}
/**
* {@inheritdoc}
*/
public function getRoles()
{
return $this->roles;
}
/**
* {@inheritdoc}
*/
public function getPassword()
{
return $this->password;
}
/**
* {@inheritdoc}
*/
public function getSalt()
{
}
/**
* {@inheritdoc}
*/
public function getUsername()
{
return $this->username;
}
/**
* {@inheritdoc}
*/
public function isAccountNonExpired()
{
return $this->accountNonExpired;
}
/**
* {@inheritdoc}
*/
public function isAccountNonLocked()
{
return $this->accountNonLocked;
}
/**
* {@inheritdoc}
*/
public function isCredentialsNonExpired()
{
return $this->credentialsNonExpired;
}
/**
* {@inheritdoc}
*/
public function isEnabled()
{
return $this->enabled;
}
/**
* {@inheritdoc}
*/
public function eraseCredentials()
{
}
}

View File

@ -0,0 +1,20 @@
<?php
/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/
use Symfony\Bundle\FrameworkBundle\FrameworkBundle;
use Symfony\Bundle\SecurityBundle\SecurityBundle;
use Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\SecuredPageBundle\SecuredPageBundle;
return [
new FrameworkBundle(),
new SecurityBundle(),
new SecuredPageBundle(),
];

View File

@ -0,0 +1,32 @@
imports:
- { resource: ./../config/framework.yml }
services:
_defaults: { public: true }
security.user.provider.array:
class: Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\SecuredPageBundle\Security\Core\User\ArrayUserProvider
security:
encoders:
\Symfony\Component\Security\Core\User\UserInterface: plaintext
providers:
array:
id: security.user.provider.array
firewalls:
default:
form_login:
check_path: login
remember_me: true
require_previous_session: false
logout: ~
anonymous: ~
stateless: false
access_control:
- { path: ^/admin$, roles: ROLE_ADMIN }
- { path: ^/login$, roles: IS_AUTHENTICATED_ANONYMOUSLY }
- { path: .*, roles: IS_AUTHENTICATED_FULLY }

View File

@ -0,0 +1,8 @@
login:
path: /login
logout:
path: /logout
admin_bundle:
resource: '@SecuredPageBundle/Resources/config/routing.yml'

View File

@ -21,7 +21,7 @@
"symfony/config": "^4.2|^5.0",
"symfony/dependency-injection": "^4.2|^5.0",
"symfony/http-kernel": "^4.4",
"symfony/security-core": "^4.3",
"symfony/security-core": "^4.4",
"symfony/security-csrf": "^4.2|^5.0",
"symfony/security-guard": "^4.2|^5.0",
"symfony/security-http": "^4.3"

View File

@ -166,6 +166,7 @@ abstract class AbstractToken implements TokenInterface
* @return string
*
* @final since Symfony 4.3, use __serialize() instead
*
* @internal since Symfony 4.3, use __serialize() instead
*/
public function serialize()
@ -316,6 +317,13 @@ abstract class AbstractToken implements TokenInterface
return true;
}
$userRoles = array_map('strval', (array) $user->getRoles());
$rolesChanged = \count($userRoles) !== \count($this->getRoleNames()) || \count($userRoles) !== \count(array_intersect($userRoles, $this->getRoleNames()));
if ($rolesChanged) {
return true;
}
if ($this->user->getUsername() !== $user->getUsername()) {
return true;
}

View File

@ -117,8 +117,8 @@ class UserTest extends TestCase
{
return [
[true, new User('username', 'password'), new User('username', 'password')],
[true, new User('username', 'password', ['ROLE']), new User('username', 'password')],
[true, new User('username', 'password', ['ROLE']), new User('username', 'password', ['NO ROLE'])],
[false, new User('username', 'password', ['ROLE']), new User('username', 'password')],
[false, new User('username', 'password', ['ROLE']), new User('username', 'password', ['NO ROLE'])],
[false, new User('diff', 'diff'), new User('username', 'password')],
[false, new User('diff', 'diff', [], false), new User('username', 'password')],
[false, new User('diff', 'diff', [], false, false), new User('username', 'password')],

View File

@ -143,6 +143,13 @@ final class User implements UserInterface, EquatableInterface, AdvancedUserInter
return false;
}
$currentRoles = array_map('strval', (array) $this->getRoles());
$newRoles = array_map('strval', (array) $user->getRoles());
$rolesChanged = \count($currentRoles) !== \count($newRoles) || \count($currentRoles) !== \count(array_intersect($currentRoles, $newRoles));
if ($rolesChanged) {
return false;
}
if ($this->getUsername() !== $user->getUsername()) {
return false;
}