Skip to content

Commit

Permalink
bug #13627 [Security] InMemoryUserProvider now concerns whether user'…
Browse files Browse the repository at this point in the history
…s password is changed when refreshing (issei-m)

This PR was merged into the 2.3 branch.

Discussion
----------

[Security] InMemoryUserProvider now concerns whether user's password is changed when refreshing

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

When a user has changed own password, I want to logout any sessions which is authenticated by its user except changer itself.

[DaoAuthenticationManager::checkAuthentication()](https://github.com/symfony/symfony/blob/2.3/src/Symfony/Component/Security/Core/Authentication/Provider/DaoAuthenticationProvider.php#L59) method seems to concern about it.

But, this situation actually never happens because both users that will be passed to this method are always identical in re-authentication.
It's because the token refreshes own user via [ContextListener](https://github.com/symfony/symfony/blob/2.3/src/Symfony/Component/Security/Http/Firewall/ContextListener.php#L90) before re-authentication.

Commits
-------

729902a [Security] InMemoryUserProvider now concerns whether user's password is changed when refreshing
  • Loading branch information
fabpot committed Oct 5, 2015
2 parents 2455b69 + 729902a commit d3b8176
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 17 deletions.
37 changes: 26 additions & 11 deletions src/Symfony/Component/Security/Core/User/InMemoryUserProvider.php
Expand Up @@ -67,17 +67,9 @@ public function createUser(UserInterface $user)
*/
public function loadUserByUsername($username)
{
if (!isset($this->users[strtolower($username)])) {
$ex = new UsernameNotFoundException(sprintf('Username "%s" does not exist.', $username));
$ex->setUsername($username);

throw $ex;
}
$user = $this->getUser($username);

$user = $this->users[strtolower($username)];

return new User($user->getUsername(), $user->getPassword(), $user->getRoles(), $user->isEnabled(), $user->isAccountNonExpired(),
$user->isCredentialsNonExpired(), $user->isAccountNonLocked());
return new User($user->getUsername(), $user->getPassword(), $user->getRoles(), $user->isEnabled(), $user->isAccountNonExpired(), $user->isCredentialsNonExpired(), $user->isAccountNonLocked());
}

/**
Expand All @@ -89,7 +81,9 @@ public function refreshUser(UserInterface $user)
throw new UnsupportedUserException(sprintf('Instances of "%s" are not supported.', get_class($user)));
}

return $this->loadUserByUsername($user->getUsername());
$storedUser = $this->getUser($user->getUsername());

return new User($storedUser->getUsername(), $storedUser->getPassword(), $storedUser->getRoles(), $storedUser->isEnabled(), $storedUser->isAccountNonExpired(), $storedUser->isCredentialsNonExpired() && $storedUser->getPassword() === $user->getPassword(), $storedUser->isAccountNonLocked());
}

/**
Expand All @@ -99,4 +93,25 @@ public function supportsClass($class)
{
return $class === 'Symfony\Component\Security\Core\User\User';
}

/**
* Returns the user by given username.
*
* @param string $username The username.
*
* @return User
*
* @throws UsernameNotFoundException If user whose given username does not exist.
*/
private function getUser($username)
{
if (!isset($this->users[strtolower($username)])) {
$ex = new UsernameNotFoundException(sprintf('Username "%s" does not exist.', $username));
$ex->setUsername($username);

throw $ex;
}

return $this->users[strtolower($username)];
}
}
Expand Up @@ -18,18 +18,39 @@ class InMemoryUserProviderTest extends \PHPUnit_Framework_TestCase
{
public function testConstructor()
{
$provider = new InMemoryUserProvider(array(
$provider = $this->createProvider();

$user = $provider->loadUserByUsername('fabien');
$this->assertEquals('foo', $user->getPassword());
$this->assertEquals(array('ROLE_USER'), $user->getRoles());
$this->assertFalse($user->isEnabled());
}

public function testRefresh()
{
$user = new User('fabien', 'bar');

$provider = $this->createProvider();

$refreshedUser = $provider->refreshUser($user);
$this->assertEquals('foo', $refreshedUser->getPassword());
$this->assertEquals(array('ROLE_USER'), $refreshedUser->getRoles());
$this->assertFalse($refreshedUser->isEnabled());
$this->assertFalse($refreshedUser->isCredentialsNonExpired());
}

/**
* @return InMemoryUserProvider
*/
protected function createProvider()
{
return new InMemoryUserProvider(array(
'fabien' => array(
'password' => 'foo',
'enabled' => false,
'roles' => array('ROLE_USER'),
),
));

$user = $provider->loadUserByUsername('fabien');
$this->assertEquals('foo', $user->getPassword());
$this->assertEquals(array('ROLE_USER'), $user->getRoles());
$this->assertFalse($user->isEnabled());
}

public function testCreateUser()
Expand Down

0 comments on commit d3b8176

Please sign in to comment.