Skip to content

Commit

Permalink
Merge branch '7.5'
Browse files Browse the repository at this point in the history
  • Loading branch information
adamwojs committed Oct 7, 2019
2 parents 956d45b + 2e04c6b commit c54917f
Show file tree
Hide file tree
Showing 8 changed files with 131 additions and 13 deletions.
2 changes: 2 additions & 0 deletions data/update/mysql/dbupdate-7.5.4-to-7.5.5.sql
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

ALTER TABLE ezuser ADD COLUMN password_updated_at INT(11) NULL;

UPDATE ezuser SET password_updated_at = UNIX_TIMESTAMP();

--
-- EZP-30797: end.
--
2 changes: 2 additions & 0 deletions data/update/postgres/dbupdate-7.5.4-to-7.5.5.sql
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

ALTER TABLE ezuser ADD COLUMN password_updated_at integer;

UPDATE ezuser SET password_updated_at = cast(extract(epoch from CURRENT_TIMESTAMP) as integer);

--
-- EZP-30797: end.
--
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,10 @@ public function getValidatorSchema()
'type' => 'int',
'default' => null,
],
'requireNewPassword' => [
'type' => 'int',
'default' => null,
],
'minLength' => [
'type' => 'int',
'default' => 10,
Expand All @@ -128,6 +132,7 @@ public function getValidValidatorConfiguration()
'requireAtLeastOneLowerCaseCharacter' => true,
'requireAtLeastOneNumericCharacter' => true,
'requireAtLeastOneNonAlphanumericCharacter' => false,
'requireNewPassword' => false,
'minLength' => 10,
],
];
Expand Down
28 changes: 26 additions & 2 deletions eZ/Publish/API/Repository/Tests/UserServiceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -2898,7 +2898,7 @@ public function testValidatePasswordWithDefaultContext()
* @covers \eZ\Publish\API\Repository\UserService::validatePassword()
* @dataProvider dataProviderForValidatePassword
*/
public function testValidatePassword(string $password, array $expectedErrorr)
public function testValidatePassword(string $password, array $expectedErrors)
{
$userService = $this->getRepository()->getUserService();
$contentType = $this->createUserContentTypeWithStrongPassword();
Expand All @@ -2911,7 +2911,30 @@ public function testValidatePassword(string $password, array $expectedErrorr)
$actualErrors = $userService->validatePassword($password, $context);
/* END: Use Case */

$this->assertEquals($expectedErrorr, $actualErrors);
$this->assertEquals($expectedErrors, $actualErrors);
}

public function testValidatePasswordReturnsErrorWhenOldPasswordIsReused(): void
{
$password = 'P@blish123!';

$userService = $this->getRepository()->getUserService();
// Password expiration needs to be enabled
$contentType = $this->createUserContentTypeWithPasswordExpirationDate();

$user = $this->createTestUserWithPassword($password, $contentType);

$context = new PasswordValidationContext([
'contentType' => $contentType,
'user' => $user,
]);

$actualErrors = $userService->validatePassword($password, $context);

$this->assertEquals(
[new ValidationError('New password cannot be the same as old password', null, [], 'password')],
$actualErrors
);
}

/**
Expand Down Expand Up @@ -3059,6 +3082,7 @@ private function createUserContentTypeWithStrongPassword(): ContentType
'requireAtLeastOneLowerCaseCharacter' => 1,
'requireAtLeastOneNumericCharacter' => 1,
'requireAtLeastOneNonAlphanumericCharacter' => 1,
'requireNewPassword' => 1,
'minLength' => 8,
],
]);
Expand Down
4 changes: 4 additions & 0 deletions eZ/Publish/Core/FieldType/Tests/UserTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,10 @@ protected function getValidatorConfigurationSchemaExpectation()
'type' => 'int',
'default' => null,
],
'requireNewPassword' => [
'type' => 'int',
'default' => null,
],
'minLength' => [
'type' => 'int',
'default' => 10,
Expand Down
35 changes: 33 additions & 2 deletions eZ/Publish/Core/FieldType/User/Type.php
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,10 @@ class Type extends FieldType
'type' => 'int',
'default' => null,
],
'requireNewPassword' => [
'type' => 'int',
'default' => null,
],
'minLength' => [
'type' => 'int',
'default' => 10,
Expand Down Expand Up @@ -350,12 +354,24 @@ public function validate(FieldDefinition $fieldDefinition, SPIValue $fieldValue)
}

if (!empty($fieldValue->plainPassword)) {
$passwordValidationError[] = $this->passwordValidator->validatePassword(
$passwordValidationErrors = $this->passwordValidator->validatePassword(
$fieldValue->plainPassword,
$fieldDefinition
);

$errors = array_merge($errors, ...$passwordValidationError);
$errors = array_merge($errors, $passwordValidationErrors);

if (!empty($fieldValue->passwordHash) && $this->isNewPasswordRequired($fieldDefinition)) {
$isPasswordReused = $this->passwordHashService->isValidPassword(
$fieldValue->plainPassword,
$fieldValue->passwordHash,
$fieldValue->passwordHashType
);

if ($isPasswordReused) {
$errors[] = new ValidationError('New password cannot be the same as old password', null, [], 'password');
}
}
}

return $errors;
Expand Down Expand Up @@ -468,4 +484,19 @@ private function validatePasswordTTLWarningSetting(string $name, $value, $fieldS

return null;
}

private function isNewPasswordRequired(FieldDefinition $fieldDefinition): bool
{
$isExplicitRequired = $fieldDefinition->validatorConfiguration['PasswordValueValidator']['requireNewPassword'] ?? false;
if ($isExplicitRequired) {
return true;
}

return $this->isPasswordTTLEnabled($fieldDefinition);
}

private function isPasswordTTLEnabled(FieldDefinition $fieldDefinition): bool
{
return ($fieldDefinition->fieldSettings[self::PASSWORD_TTL_SETTING] ?? null) > 0;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ class UserConverter implements Converter
private const REQUIRE_AT_LEAST_ONE_LOWER_CASE_CHAR = 2;
private const REQUIRE_AT_LEAST_ONE_NUMERIC_CHAR = 4;
private const REQUIRE_AT_LEAST_ONE_NON_ALPHANUMERIC_CHAR = 8;
private const REQUIRE_NEW_PASSWORD = 16;

/**
* {@inheritdoc}
Expand Down Expand Up @@ -56,6 +57,7 @@ public function toStorageFieldDefinition(FieldDefinition $fieldDef, StorageField
'requireAtLeastOneLowerCaseCharacter' => self::REQUIRE_AT_LEAST_ONE_LOWER_CASE_CHAR,
'requireAtLeastOneNumericCharacter' => self::REQUIRE_AT_LEAST_ONE_NUMERIC_CHAR,
'requireAtLeastOneNonAlphanumericCharacter' => self::REQUIRE_AT_LEAST_ONE_NON_ALPHANUMERIC_CHAR,
'requireNewPassword' => self::REQUIRE_NEW_PASSWORD,
];

$storageDef->dataInt1 = 0;
Expand Down Expand Up @@ -88,6 +90,7 @@ public function toFieldDefinition(StorageFieldDefinition $storageDef, FieldDefin
self::REQUIRE_AT_LEAST_ONE_LOWER_CASE_CHAR => 'requireAtLeastOneLowerCaseCharacter',
self::REQUIRE_AT_LEAST_ONE_NUMERIC_CHAR => 'requireAtLeastOneNumericCharacter',
self::REQUIRE_AT_LEAST_ONE_NON_ALPHANUMERIC_CHAR => 'requireAtLeastOneNonAlphanumericCharacter',
self::REQUIRE_NEW_PASSWORD => 'requireNewPassword',
];

foreach ($rules as $flag => $rule) {
Expand Down
65 changes: 56 additions & 9 deletions eZ/Publish/Core/Repository/UserService.php
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
use eZ\Publish\Core\Base\Exceptions\UnauthorizedException;
use eZ\Publish\Core\FieldType\User\Value as UserValue;
use eZ\Publish\Core\FieldType\User\Type as UserType;
use eZ\Publish\Core\FieldType\ValidationError;
use eZ\Publish\Core\Repository\Values\User\User;
use eZ\Publish\Core\Repository\Values\User\UserGroup;
use eZ\Publish\Core\Repository\Values\User\UserGroupCreateStruct;
Expand Down Expand Up @@ -521,7 +522,7 @@ public function loadUserByCredentials($login, $password, array $prioritizedLangu
}

$spiUser = $this->userHandler->loadByLogin($login);
if (!$this->verifyPassword($login, $password, $spiUser)) {
if (!$this->comparePasswordHashForSPIUser($login, $password, $spiUser)) {
throw new NotFoundException('user', $login);
}

Expand Down Expand Up @@ -1100,6 +1101,7 @@ public function newUserCreateStruct($login, $email, $password, $mainLanguageCode
$this->settings['userClassID']
);
}

$fieldDefIdentifier = '';
foreach ($contentType->fieldDefinitions as $fieldDefinition) {
if ($fieldDefinition->fieldTypeIdentifier === 'ezuser') {
Expand Down Expand Up @@ -1184,6 +1186,8 @@ public function newUserGroupUpdateStruct()
*/
public function validatePassword(string $password, PasswordValidationContext $context = null): array
{
$errors = [];

if ($context === null) {
$contentType = $this->repository->getContentTypeService()->loadContentType(
$this->settings['userClassID']
Expand All @@ -1208,11 +1212,22 @@ public function validatePassword(string $password, PasswordValidationContext $co
}

$configuration = $userFieldDefinition->getValidatorConfiguration();
if (!isset($configuration['PasswordValueValidator'])) {
return [];
if (isset($configuration['PasswordValueValidator'])) {
$errors = (new UserPasswordValidator($configuration['PasswordValueValidator']))->validate($password);
}

if ($context->user !== null) {
$isPasswordTTLEnabled = $this->getPasswordInfo($context->user)->hasExpirationDate();
$isNewPasswordRequired = $configuration['PasswordValueValidator']['requireNewPassword'] ?? false;

if (($isPasswordTTLEnabled || $isNewPasswordRequired) &&
$this->comparePasswordHashForAPIUser($context->user->login, $password, $context->user)
) {
$errors[] = new ValidationError('New password cannot be the same as old password', null, [], 'password');
}
}

return (new UserPasswordValidator($configuration['PasswordValueValidator']))->validate($password);
return $errors;
}

/**
Expand Down Expand Up @@ -1320,18 +1335,50 @@ private function getUserFieldDefinition(ContentType $contentType): ?FieldDefinit
}

/**
* Verifies if the provided login and password are valid.
* Verifies if the provided login and password are valid for eZ\Publish\SPI\Persistence\User.
*
* @param string $login User login
* @param string $password User password
* @param \eZ\Publish\SPI\Persistence\User $spiUser Loaded user handler
*
* @return bool return true if the login and password are sucessfully
* validate and false, if not.
* @return bool return true if the login and password are sucessfully validated and false, if not.
*/
protected function verifyPassword($login, $password, $spiUser)
protected function comparePasswordHashForSPIUser(string $login, string $password, SPIUser $spiUser): bool
{
return $this->passwordHashService->isValidPassword($password, $spiUser->passwordHash, $spiUser->hashAlgorithm);
return $this->comparePasswordHashes($login, $password, $spiUser->passwordHash, $spiUser->hashAlgorithm);
}

/**
* Verifies if the provided login and password are valid for eZ\Publish\API\Repository\Values\User\User.
*
* @param string $login User login
* @param string $password User password
* @param \eZ\Publish\API\Repository\Values\User\User $apiUser Loaded user
*
* @return bool return true if the login and password are sucessfully validated and false, if not.
*/
protected function comparePasswordHashForAPIUser(string $login, string $password, APIUser $apiUser): bool
{
return $this->comparePasswordHashes($login, $password, $apiUser->passwordHash, $apiUser->hashAlgorithm);
}

/**
* Verifies if the provided login and password are valid against given password hash and hash type.
*
* @param string $login User login
* @param string $plainPassword User password
* @param string $passwordHash User password hash
* @param int $hashAlgorithm Hash type
*
* @return bool return true if the login and password are sucessfully validated and false, if not.
*/
private function comparePasswordHashes(
string $login,
string $plainPassword,
string $passwordHash,
int $hashAlgorithm
): bool {
return $this->passwordHashService->isValidPassword($plainPassword, $passwordHash, $hashAlgorithm);
}

/**
Expand Down

0 comments on commit c54917f

Please sign in to comment.