Skip to content

IBX-10124: Add support for Argon2 password hashes #581

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.

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
<?php

/**
* @copyright Copyright (C) Ibexa AS. All rights reserved.
* @license For full copyright and license information view LICENSE file distributed with this source code.
*/
declare(strict_types=1);

namespace Ibexa\Bundle\Core\DependencyInjection\Configuration\Parser;

use Ibexa\Bundle\Core\DependencyInjection\Configuration\AbstractParser;
use Ibexa\Bundle\Core\DependencyInjection\Configuration\SiteAccessAware\ContextualizerInterface;
use Symfony\Component\Config\Definition\Builder\NodeBuilder;

/**
* @internal
*
* Configuration parser for password hash configuration.
*
* Example configuration:
* ```yaml
* ibexa:
* system:
* default: # configuration per siteaccess or siteaccess group
* password_hash:
* default_type: 7
* update_type_on_change: false
* update_type_on_login: false
* ```
*/
final class PasswordHash extends AbstractParser
{
/**
* Adds semantic configuration definition.
*
* @param \Symfony\Component\Config\Definition\Builder\NodeBuilder $nodeBuilder Node just under ibexa.system.<siteaccess>
*/
public function addSemanticConfig(NodeBuilder $nodeBuilder)

Check failure on line 38 in src/bundle/Core/DependencyInjection/Configuration/Parser/PasswordHash.php

View workflow job for this annotation

GitHub Actions / Unit tests & SQLite integration tests (8.3)

Method Ibexa\Bundle\Core\DependencyInjection\Configuration\Parser\PasswordHash::addSemanticConfig() has no return type specified.
{
$nodeBuilder
->arrayNode('password_hash')
->info('Password hash options')
->children()
->integerNode('default_type')
->info('Default password hash type, see the constants in Ibexa\Contracts\Core\Repository\Values\User\User.')
->example('7')
->end()
->booleanNode('update_type_on_change')
->info('Whether the password hash type should be changed when the password is changed if it differs from the default type.')
->example('false')
->end()
->booleanNode('update_type_on_login')
->info('Whether the password hash type should be changed during login if it differs from the default type.')
->example('false')
->end()
->end()
->end();
}

public function mapConfig(array &$scopeSettings, $currentScope, ContextualizerInterface $contextualizer)

Check failure on line 60 in src/bundle/Core/DependencyInjection/Configuration/Parser/PasswordHash.php

View workflow job for this annotation

GitHub Actions / Unit tests & SQLite integration tests (8.3)

Method Ibexa\Bundle\Core\DependencyInjection\Configuration\Parser\PasswordHash::mapConfig() has parameter $scopeSettings with no value type specified in iterable type array.

Check failure on line 60 in src/bundle/Core/DependencyInjection/Configuration/Parser/PasswordHash.php

View workflow job for this annotation

GitHub Actions / Unit tests & SQLite integration tests (8.3)

Method Ibexa\Bundle\Core\DependencyInjection\Configuration\Parser\PasswordHash::mapConfig() has no return type specified.
{
if (!isset($scopeSettings['password_hash'])) {
return;
}

$settings = $scopeSettings['password_hash'];
if (isset($settings['default_type'])) {
$contextualizer->setContextualParameter('password_hash.default_type', $currentScope, $settings['default_type']);
}
if (isset($settings['update_type_on_change'])) {
$contextualizer->setContextualParameter('password_hash.update_type_on_change', $currentScope, $settings['update_type_on_change']);
}
if (isset($settings['update_type_on_login'])) {
$contextualizer->setContextualParameter('password_hash.update_type_on_login', $currentScope, $settings['update_type_on_login']);
}
}
}
1 change: 1 addition & 0 deletions src/bundle/Core/IbexaCoreBundle.php
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ public function getContainerExtension(): ExtensionInterface
new ConfigParser\UrlChecker(),
new ConfigParser\TwigVariablesParser(),
new ConfigParser\UserContentTypeIdentifier(),
new ConfigParser\PasswordHash(),
],
[
new RepositoryConfigParser\Storage(),
Expand Down
5 changes: 5 additions & 0 deletions src/bundle/Core/Resources/config/default_settings.yml
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,11 @@ parameters:
ibexa.site_access.config.default.user_content_type_identifier: ['user']
ibexa.site_access.config.default.api_keys: {} # Google Maps APIs v3 key (https://developers.google.com/maps/documentation/javascript/get-api-key)

# Password hash
ibexa.site_access.config.default.password_hash.default_type: 7 # Default password hash type, see the constants in Ibexa\Contracts\Core\Repository\Values\User\User
ibexa.site_access.config.default.password_hash.update_type_on_change: false # Whether the password hash type should be changed when the password is changed if it differs from the default hash type
ibexa.site_access.config.default.password_hash.update_type_on_login: false # Whether the password hash type should be changed during login if it differs from the default hash type

# IO
ibexa.site_access.config.default.io.metadata_handler: "default"
ibexa.site_access.config.default.io.binarydata_handler: "default"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,6 @@ class PasswordInUnsupportedFormatException extends AuthenticationException
{
public function __construct(?Throwable $previous = null)
{
parent::__construct("User's password is in a format which is not supported any more.", 0, $previous);
parent::__construct("User's password is in a format which is not supported.", 0, $previous);
}
}
6 changes: 6 additions & 0 deletions src/contracts/Repository/Values/User/User.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,19 @@ abstract class User extends Content implements UserReference
public const array SUPPORTED_PASSWORD_HASHES = [
self::PASSWORD_HASH_BCRYPT,
self::PASSWORD_HASH_PHP_DEFAULT,
self::PASSWORD_HASH_ARGON2I,
self::PASSWORD_HASH_ARGON2ID,
self::PASSWORD_HASH_INVALID,
];

public const int PASSWORD_HASH_BCRYPT = 6;

public const int PASSWORD_HASH_PHP_DEFAULT = 7;

public const int PASSWORD_HASH_ARGON2I = 8;

public const int PASSWORD_HASH_ARGON2ID = 9;

public const int PASSWORD_HASH_INVALID = 256;

public const int DEFAULT_PASSWORD_HASH = self::PASSWORD_HASH_PHP_DEFAULT;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
use Ibexa\Contracts\Core\Repository\Exceptions\PasswordInUnsupportedFormatException;
use Ibexa\Contracts\Core\Repository\UserService;
use Ibexa\Core\MVC\Symfony\Security\UserInterface as IbexaUserInterface;
use Ibexa\Core\Repository\User\Exception\PasswordHashTypeNotCompiled;
use Ibexa\Core\Repository\User\Exception\UnsupportedPasswordHashType;
use Psr\Log\LoggerAwareTrait;
use Psr\Log\LoggerInterface;
Expand Down Expand Up @@ -77,7 +78,7 @@ public function validateRepositoryUser(CheckPassportEvent $event): void
$user->getAPIUser(),
$user->getPassword() ?? ''
);
} catch (UnsupportedPasswordHashType $exception) {
} catch (UnsupportedPasswordHashType|PasswordHashTypeNotCompiled $exception) {
$this->sleepUsingConstantTimer($startTime);

throw new PasswordInUnsupportedFormatException($exception);
Expand Down
22 changes: 22 additions & 0 deletions src/lib/Repository/User/Exception/PasswordHashTypeNotCompiled.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<?php

/**
* @copyright Copyright (C) Ibexa AS. All rights reserved.
* @license For full copyright and license information view LICENSE file distributed with this source code.
*/
declare(strict_types=1);

namespace Ibexa\Core\Repository\User\Exception;

use Ibexa\Core\Base\Exceptions\InvalidArgumentException;

final class PasswordHashTypeNotCompiled extends InvalidArgumentException
{
public function __construct(string $hashType)
{
parent::__construct(
'hashType',
"Password hash algorithm $hashType is not compiled into PHP"
);
}
}
20 changes: 19 additions & 1 deletion src/lib/Repository/User/PasswordHashService.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

use Ibexa\Contracts\Core\Repository\PasswordHashService as PasswordHashServiceInterface;
use Ibexa\Contracts\Core\Repository\Values\User\User;
use Ibexa\Core\Repository\User\Exception\PasswordHashTypeNotCompiled;
use Ibexa\Core\Repository\User\Exception\UnsupportedPasswordHashType;

/**
Expand Down Expand Up @@ -40,6 +41,7 @@ public function getDefaultHashType(): int
}

/**
* @throws \Ibexa\Core\Repository\User\Exception\PasswordHashTypeNotCompiled
* @throws \Ibexa\Core\Repository\User\Exception\UnsupportedPasswordHashType
*/
public function createPasswordHash(
Expand All @@ -59,6 +61,20 @@ public function createPasswordHash(
case User::PASSWORD_HASH_INVALID:
return '';

case User::PASSWORD_HASH_ARGON2I:
if (!defined('PASSWORD_ARGON2I')) {
throw new PasswordHashTypeNotCompiled('PASSWORD_ARGON2I');
}

return password_hash($password, PASSWORD_ARGON2I);

case User::PASSWORD_HASH_ARGON2ID:
if (!defined('PASSWORD_ARGON2ID')) {
throw new PasswordHashTypeNotCompiled('PASSWORD_ARGON2ID');
}

return password_hash($password, PASSWORD_ARGON2ID);

default:
throw new UnsupportedPasswordHashType($hashType);
}
Expand All @@ -74,9 +90,11 @@ public function isValidPassword(
if (
$hashType === User::PASSWORD_HASH_BCRYPT
|| $hashType === User::PASSWORD_HASH_PHP_DEFAULT
|| $hashType === User::PASSWORD_HASH_ARGON2I
|| $hashType === User::PASSWORD_HASH_ARGON2ID
|| $hashType === User::PASSWORD_HASH_INVALID
) {
// In case of bcrypt let PHP's password functionality do its magic
// Let PHP's password functionality do its magic
return password_verify($plainPassword, $passwordHash);
}

Expand Down
10 changes: 9 additions & 1 deletion src/lib/Repository/UserService.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,15 @@
use Ibexa\Core\Base\Exceptions\MissingUserFieldTypeException;
use Ibexa\Core\Base\Exceptions\UnauthorizedException;
use Ibexa\Core\FieldType\User\Value as UserValue;
use Ibexa\Core\Repository\User\Exception\PasswordHashTypeNotCompiled;
use Ibexa\Core\Repository\User\Exception\UnsupportedPasswordHashType;
use Ibexa\Core\Repository\User\PasswordValidatorInterface;
use Ibexa\Core\Repository\Values\User\User;
use Ibexa\Core\Repository\Values\User\UserCreateStruct;
use Ibexa\Core\Repository\Values\User\UserGroup;
use Ibexa\Core\Repository\Values\User\UserGroupCreateStruct;
use Psr\Log\LoggerInterface;
use Psr\Log\LogLevel;

/**
* This service provides methods for managing users and user groups.
Expand Down Expand Up @@ -787,7 +789,13 @@ public function updateUserPassword(
$passwordHashAlgorithm = (int) $loadedUser->hashAlgorithm;
try {
$passwordHash = $this->passwordHashService->createPasswordHash($newPassword, $passwordHashAlgorithm);
} catch (UnsupportedPasswordHashType $e) {
} catch (UnsupportedPasswordHashType|PasswordHashTypeNotCompiled $e) {
if (isset($this->logger)) {
$this->logger->log(LogLevel::WARNING, $e->getMessage(), [
'exception' => $e,
]);
}

$passwordHashAlgorithm = $this->passwordHashService->getDefaultHashType();
$passwordHash = $this->passwordHashService->createPasswordHash($newPassword, $passwordHashAlgorithm);
}
Expand Down
32 changes: 30 additions & 2 deletions tests/integration/Core/Repository/UserServiceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -1325,7 +1325,7 @@ public function testCreateUserWithWeakPasswordThrowsUserPasswordValidationExcept

try {
// This call will fail with a "UserPasswordValidationException" because the
// the password does not follow specified rules.
// password does not follow specified rules.
$this->createTestUserWithPassword('pass', $userContentType);
} catch (ContentFieldValidationException $e) {
// Exception is caught, as there is no other way to check exception properties.
Expand Down Expand Up @@ -2177,13 +2177,41 @@ public function testUpdateUserPasswordWithUnsupportedHashType(): void
$wrongHashType = 1;
$this->updateRawPasswordHash($user->getUserId(), $wrongHashType);
$newPassword = 'new_secret123';
// no need to invalidate cache since there was no load between create & raw database update
// no need to invalidate cache since there was no load between creation
// and raw database update
$user = $userService->updateUserPassword($user, $newPassword);

self::assertTrue($userService->checkUserCredentials($user, $newPassword));
self::assertNotEquals($oldPasswordHash, $user->passwordHash);
}

/**
* @throws \Doctrine\DBAL\Exception
* @throws \ErrorException
* @throws \Ibexa\Contracts\Core\Repository\Exceptions\ContentFieldValidationException
* @throws \Ibexa\Contracts\Core\Repository\Exceptions\UnauthorizedException
*/
public function testUpdateUserPasswordHashToArgon2Id(): void
{
$repository = $this->getRepository();
$userService = $repository->getUserService();

$user = $this->createUser('john.doe', 'John', 'Doe');
$oldPasswordHash = $user->passwordHash;

$argon2IdHashType = User::PASSWORD_HASH_ARGON2ID;
$this->updateRawPasswordHash($user->getUserId(), $argon2IdHashType);
$newPassword = 'new_secret123';
// no need to invalidate cache since there was no load between creation
// and raw database update
$user = $userService->updateUserPassword($user, $newPassword);
$passwordInfo = password_get_info($user->passwordHash);

self::assertTrue($userService->checkUserCredentials($user, $newPassword));
self::assertNotEquals($oldPasswordHash, $user->passwordHash);
self::assertEquals(PASSWORD_ARGON2ID, $passwordInfo['algo']);
}

/**
* Test for the loadUserGroupsOfUser() method.
*
Expand Down
2 changes: 2 additions & 0 deletions tests/lib/Repository/User/PasswordHashServiceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ public function testGetSupportedHashTypes(): void
[
User::PASSWORD_HASH_BCRYPT,
User::PASSWORD_HASH_PHP_DEFAULT,
User::PASSWORD_HASH_ARGON2I,
User::PASSWORD_HASH_ARGON2ID,
User::PASSWORD_HASH_INVALID,
],
$this->passwordHashService->getSupportedHashTypes()
Expand Down
Loading