Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Automatically upgrade new password hashes #1027

Merged
merged 2 commits into from
Nov 27, 2019
Merged

Automatically upgrade new password hashes #1027

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
729ae5d
Automatically upgrade new password hashes
Toflar Nov 27, 2019
0a0b575
Add the is_a() check and fix the unit tests
leofeyer Nov 27, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 13 additions & 8 deletions core-bundle/src/Security/User/ContaoUserProvider.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,19 @@ public function supportsClass($class): bool
return $this->userClass === $class;
}

/**
* @param User $user
*/
public function upgradePassword(UserInterface $user, string $newEncodedPassword): void
{
if (!is_a($user, $this->userClass)) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't happen because of the supports() call. But anyway :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right. I have just copied the code from the refreshUser() method, but maybe it is unnecessary there as well?

contao/core-bundle/src/Security/User/ContaoUserProvider.php

Lines 87 to 91 in 1e08698

public function refreshUser(UserInterface $user)
{
if (!is_a($user, $this->userClass)) {
throw new UnsupportedUserException(sprintf('Unsupported class "%s".', \get_class($user)));
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, defensive programming - guess that's fine :)

throw new UnsupportedUserException(sprintf('Unsupported class "%s".', \get_class($user)));
}

$user->password = $newEncodedPassword;
$user->save();
}

/**
* Validates the session lifetime and logs the user out if the session has expired.
*
Expand Down Expand Up @@ -153,12 +166,4 @@ private function triggerPostAuthenticateHook(User $user): void
$system->importStatic($callback[0])->{$callback[1]}($user);
}
}

public function upgradePassword(UserInterface $user, string $newEncodedPassword): void
{
/** @var User $user */
$user->password = $newEncodedPassword;

$user->save();
}
}
25 changes: 18 additions & 7 deletions core-bundle/tests/Security/User/ContaoUserProviderTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,24 +207,35 @@ public function testDoesNotHandleUnknownUserClasses(): void
$this->getProvider(null, 'LdapUser');
}

public function testStoresUpgradedPasswords(): void
public function testUpgradesPasswords(): void
{
/** @var BackendUser&MockObject $user */
$user = $this->mockClassWithProperties(BackendUser::class);
$user->expects($this->once())->method('save');
$user->username = 'foobar';
$user->password = 'superhash';

$userProvider = new ContaoUserProvider(
$this->mockContaoFramework(),
$this->createMock(SessionInterface::class),
BackendUser::class
);
$user
->expects($this->once())
->method('save')
;

$userProvider = $this->getProvider(null, BackendUser::class);
$userProvider->upgradePassword($user, 'newsuperhash');

$this->assertSame('newsuperhash', $user->password);
}

public function testFailsToUpgradePasswordsOfUnsupportedUsers(): void
{
$user = $this->createMock(UserInterface::class);
$provider = $this->getProvider();

$this->expectException(UnsupportedUserException::class);
$this->expectExceptionMessage(sprintf('Unsupported class "%s".', \get_class($user)));

$provider->upgradePassword($user, 'newsuperhash');
}

/**
* @group legacy
*
Expand Down