D5301.1775285335.diff
No OneTemporary
Actions

Authored By

Unknown

Size

39 KB

Referenced Files

None

Subscribers

None

D5301.1775285335.diff
View Options

	diff --git a/src/app/Http/Controllers/API/V4/PolicyController.php b/src/app/Http/Controllers/API/V4/PolicyController.php
	--- a/src/app/Http/Controllers/API/V4/PolicyController.php
	+++ b/src/app/Http/Controllers/API/V4/PolicyController.php
	@@ -5,10 +5,10 @@
	use App\Http\Controllers\Controller;
	use App\Policy\Greylist;
	use App\Policy\Mailfilter;
	+use App\Policy\Password;
	use App\Policy\RateLimit;
	use App\Policy\SmtpAccess;
	use App\Policy\SPF;
	-use App\Rules\Password;
	use App\User;
	use Illuminate\Http\JsonResponse;
	use Illuminate\Http\Request;
	@@ -27,11 +27,8 @@

	$user = !empty($userId) ? User::find($userId) : null;

	- // Get the policy
	- $policy = new Password($user ? $user->walletOwner() : null, $user);
	-
	// Check the password
	- $status = $policy->check($request->input('password'));
	+ $status = Password::checkPolicy($request->input('password'), $user, $user ? $user->walletOwner() : null);

	$passed = array_filter(
	$status,
	@@ -83,8 +80,7 @@
	$policy_config = [];

	// Get the password policies
	- $policy = new Password($owner);
	- $password_policy = $policy->rules(true);
	+ $password_policy = Password::rules($owner, true);
	$policy_config['max_password_age'] = $config['max_password_age'];

	// Get the mail delivery policies
	diff --git a/src/app/Observers/UserObserver.php b/src/app/Observers/UserObserver.php
	--- a/src/app/Observers/UserObserver.php
	+++ b/src/app/Observers/UserObserver.php
	@@ -11,8 +11,8 @@
	use App\Jobs\User\CreateJob;
	use App\Jobs\User\DeleteJob;
	use App\Jobs\User\UpdateJob;
	+use App\Policy\Password;
	use App\Policy\RateLimit\Whitelist;
	-use App\Rules\Password;
	use App\Tenant;
	use App\Transaction;
	use App\User;
	@@ -319,29 +319,6 @@
	'password_update' => now()->format('Y-m-d H:i:s'),
	]);

	- // Note: All this is kinda heavy and complicated because we don't want to store
	- // more old passwords than we need. However, except the complication/performance,
	- // there's one issue with it. E.g. the policy changes from 2 to 4, and we already
	- // removed the old passwords that were excessive before, but not now.
	-
	- // Get the account password policy
	- $policy = new Password($user->walletOwner());
	- $rules = $policy->rules();
	-
	- // Password history disabled?
	- if (empty($rules['last']) \|\| $rules['last']['param'] < 2) {
	- return;
	- }
	-
	- // Store the old password
	- $user->passwords()->create(['password' => $password]);
	-
	- // Remove passwords that we don't need anymore
	- $limit = $rules['last']['param'] - 1;
	- $ids = $user->passwords()->latest()->limit($limit)->pluck('id')->all();
	-
	- if (count($ids) >= $limit) {
	- $user->passwords()->where('id', '<', $ids[count($ids) - 1])->delete();
	- }
	+ Password::saveHash($user, $password);
	}
	}
	diff --git a/src/app/Rules/Password.php b/src/app/Policy/Password.php
	copy from src/app/Rules/Password.php
	copy to src/app/Policy/Password.php
	--- a/src/app/Rules/Password.php
	+++ b/src/app/Policy/Password.php
	@@ -1,68 +1,65 @@
	<?php

	-namespace App\Rules;
	+namespace App\Policy;

	use App\User;
	-use Illuminate\Contracts\Validation\Rule;
	use Illuminate\Support\Facades\Hash;

	-class Password implements Rule
	+class Password
	{
	- /** @var ?string The validation error message */
	- private $message;
	-
	- /** @var ?User The account owner which to take the policy from */
	- private $owner;
	-
	- /** @var ?User The user to whom the checked password belongs */
	- private $user;
	-
	- /**
	- * Class constructor.
	- *
	- * @param ?User $owner The account owner (to take the policy from)
	- * @param ?User $user The user the password is for (Null for a new user)
	- */
	- public function __construct(?User $owner = null, ?User $user = null)
	- {
	- $this->owner = $owner;
	- $this->user = $user;
	- }
	-
	/**
	- * Determine if the validation rule passes.
	+ * Check if the password input is the same as the existing password
	*
	- * @param string $attribute Attribute name
	- * @param mixed $password Password string
	+ * @param string $input Password input (clear text)
	+ * @param string $password Existing password (hash)
	*/
	- public function passes($attribute, $password): bool
	+ public static function checkHash(string $input, string $password): bool
	{
	- foreach ($this->check($password) as $rule) {
	- if (empty($rule['status'])) {
	- $this->message = \trans('validation.password-policy-error');
	- return false;
	+ if (preg_match('/^\{([A-Z0-9_-]+)\}/', $password, $matches)) {
	+ switch ($matches[1]) {
	+ case 'SSHA':
	+ $salt = substr(base64_decode(substr($password, 6)), 20);
	+ $hash = '{SSHA}' . base64_encode(sha1($input . $salt, true) . $salt);
	+ break;
	+ case 'SSHA512':
	+ $salt = substr(base64_decode(substr($password, 9)), 64);
	+ $hash = '{SSHA512}' . base64_encode(pack('H*', hash('sha512', $input . $salt)) . $salt);
	+ break;
	+ case 'PBKDF2_SHA256':
	+ // Algorithm based on https://github.com/thesubtlety/389-ds-password-check/blob/master/389ds-pwdcheck.py
	+ $decoded = base64_decode(substr($password, 15));
	+ $param = unpack('Niterations/a64salt', $decoded);
	+ $hash = hash_pbkdf2('sha256', $input, $param['salt'], $param['iterations'], 256, true);
	+ $hash = '{' . $matches[1] . '}' . base64_encode(substr($decoded, 0, 68) . $hash);
	+ break;
	+ case 'PBKDF2-SHA512':
	+ [, $algo] = explode('-', $matches[1]);
	+ [$iterations, $salt] = explode('$', substr($password, 15));
	+ $hash = hash_pbkdf2($algo, $input, base64_decode($salt), (int) $iterations, 0, true);
	+ $hash = '{' . $matches[1] . '}' . $iterations . '$' . $salt . '$' . base64_encode($hash);
	+ break;
	+ default:
	+ \Log::warning("Unsupported password hashing algorithm {$matches[1]}");
	}
	+
	+ return isset($hash) && $hash === $password;
	}

	- return true;
	- }
	-
	- /**
	- * Get the validation error message.
	- */
	- public function message(): ?string
	- {
	- return $this->message;
	+ return Hash::check($input, $password);
	}

	/**
	* Check a password against the policy rules
	*
	* @param string $password The password
	+ * @param ?User $user The user
	+ * @param ?User $owner The account owner (to get the policy rules from)
	+ *
	+ * @return array Password policy rules with validation status
	*/
	- public function check($password): array
	+ public static function checkPolicy($password, ?User $user = null, ?User $owner = null): array
	{
	- $rules = $this->rules();
	+ $rules = self::rules($owner);

	foreach ($rules as $name => $rule) {
	switch ($name) {
	@@ -94,7 +91,11 @@
	case 'last':
	// TODO: For performance reasons we might consider checking the history
	// only when the password passed all other checks
	- $status = $this->checkPasswordHistory($password, (int) $rule['param']);
	+ if (!$user) {
	+ $status = true;
	+ } else {
	+ $status = self::checkPasswordHistory($user, $password, (int) $rule['param']);
	+ }
	break;
	default:
	// Ignore unknown rule name
	@@ -107,21 +108,38 @@
	return $rules;
	}

	+ /**
	+ * Parse configured policy string
	+ *
	+ * @param ?string $policy Policy specification
	+ *
	+ * @return array Policy specification as an array indexed by the policy rule type
	+ */
	+ public static function parsePolicy(?string $policy): array
	+ {
	+ $policy = explode(',', strtolower((string) $policy));
	+ $policy = array_map('trim', $policy);
	+ $policy = array_unique(array_filter($policy));
	+
	+ return self::mapWithKeys($policy);
	+ }
	+
	/**
	* Get the list of rules for a password
	*
	- * @param bool $all List all supported rules, instead of the enabled ones
	+ * @param ?User $owner The account owner (to get password policy from)
	+ * @param bool $all List all supported rules, instead of the enabled ones
	*
	* @return array List of rule definitions
	*/
	- public function rules(bool $all = false): array
	+ public static function rules(?User $owner = null, bool $all = false): array
	{
	// All supported password policy rules (with default params)
	$supported = 'min:6,max:255,lower,upper,digit,special,last:3';

	// Get the password policy from the $owner settings
	- if ($this->owner) {
	- $conf = $this->owner->getSetting('password_policy');
	+ if ($owner) {
	+ $conf = $owner->getSetting('password_policy');
	}

	// Fallback to the configured policy
	@@ -155,48 +173,62 @@
	}

	/**
	- * Parse configured policy string
	- *
	- * @param ?string $policy Policy specification
	- *
	- * @return array Policy specification as an array indexed by the policy rule type
	+ * Save password into user password history
	*/
	- public static function parsePolicy(?string $policy): array
	+ public static function saveHash(User $user, string $password): void
	{
	- $policy = explode(',', strtolower((string) $policy));
	- $policy = array_map('trim', $policy);
	- $policy = array_unique(array_filter($policy));
	+ // Note: All this is kinda heavy and complicated because we don't want to store
	+ // more old passwords than we need. However, except the complication/performance,
	+ // there's one issue with it. E.g. the policy changes from 2 to 4, and we already
	+ // removed the old passwords that were excessive before, but not now.

	- return self::mapWithKeys($policy);
	+ $rules = self::rules($user->walletOwner());
	+
	+ // Password history disabled?
	+ if (empty($rules['last']) \|\| $rules['last']['param'] < 2) {
	+ return;
	+ }
	+
	+ // Store the old password
	+ $user->passwords()->create(['password' => $password]);
	+
	+ // Remove passwords that we don't need anymore
	+ $limit = $rules['last']['param'] - 1;
	+ $ids = $user->passwords()->latest()->limit($limit)->pluck('id')->all();
	+
	+ if (count($ids) >= $limit) {
	+ $user->passwords()->where('id', '<', $ids[count($ids) - 1])->delete();
	+ }
	}

	/**
	- * Check password agains <count> of old passwords in user history
	+ * Check password agains <count> of old passwords in user's history
	*
	+ * @param User $user The user
	* @param string $password The password to check
	* @param int $count Number of old passwords to check (including current one)
	*
	* @return bool True if password is unique, False otherwise
	*/
	- protected function checkPasswordHistory($password, int $count): bool
	+ private static function checkPasswordHistory(User $user, string $password, int $count): bool
	{
	$status = $password !== '';

	// Check if password is not the same as last X passwords
	- if ($status && $this->user && $count > 0) {
	+ if ($status && $count > 0) {
	// Current password
	- if ($this->user->password) {
	+ if ($user->password) {
	$count--;
	- if (Hash::check($password, $this->user->password)) {
	+ if (Hash::check($password, $user->password)) {
	return false;
	}
	}

	// Passwords from the history
	if ($count > 0) {
	- $this->user->passwords()->latest()->limit($count)->get()
	+ $user->passwords()->latest()->limit($count)->get()
	->each(static function ($oldPassword) use (&$status, $password) {
	- if (Hash::check($password, $oldPassword->password)) {
	+ if (self::checkHash($password, $oldPassword->password)) {
	$status = false;
	return false; // stop iteration
	}
	diff --git a/src/app/Rules/Password.php b/src/app/Rules/Password.php
	--- a/src/app/Rules/Password.php
	+++ b/src/app/Rules/Password.php
	@@ -2,9 +2,9 @@

	namespace App\Rules;

	+use App\Policy\Password as PasswordPolicy;
	use App\User;
	use Illuminate\Contracts\Validation\Rule;
	-use Illuminate\Support\Facades\Hash;

	class Password implements Rule
	{
	@@ -37,7 +37,7 @@
	*/
	public function passes($attribute, $password): bool
	{
	- foreach ($this->check($password) as $rule) {
	+ foreach (PasswordPolicy::checkPolicy($password, $this->user, $this->owner) as $rule) {
	if (empty($rule['status'])) {
	$this->message = \trans('validation.password-policy-error');
	return false;
	@@ -54,179 +54,4 @@
	{
	return $this->message;
	}
	-
	- /**
	- * Check a password against the policy rules
	- *
	- * @param string $password The password
	- */
	- public function check($password): array
	- {
	- $rules = $this->rules();
	-
	- foreach ($rules as $name => $rule) {
	- switch ($name) {
	- case 'min':
	- // Check the min length
	- $status = strlen($password) >= (int) $rule['param'];
	- break;
	- case 'max':
	- // Check the max length
	- $length = strlen($password);
	- $status = $length && $length <= (int) $rule['param'];
	- break;
	- case 'lower':
	- // Check if password contains a lower-case character
	- $status = preg_match('/[a-z]/', $password) > 0;
	- break;
	- case 'upper':
	- // Check if password contains a upper-case character
	- $status = preg_match('/[A-Z]/', $password) > 0;
	- break;
	- case 'digit':
	- // Check if password contains a digit
	- $status = preg_match('/[0-9]/', $password) > 0;
	- break;
	- case 'special':
	- // Check if password contains a special character
	- $status = preg_match('/[-~!@#$%^&*_+=`(){}[]\|:;"\'`<>,.?\/\]/', $password) > 0;
	- break;
	- case 'last':
	- // TODO: For performance reasons we might consider checking the history
	- // only when the password passed all other checks
	- $status = $this->checkPasswordHistory($password, (int) $rule['param']);
	- break;
	- default:
	- // Ignore unknown rule name
	- $status = true;
	- }
	-
	- $rules[$name]['status'] = $status;
	- }
	-
	- return $rules;
	- }
	-
	- /**
	- * Get the list of rules for a password
	- *
	- * @param bool $all List all supported rules, instead of the enabled ones
	- *
	- * @return array List of rule definitions
	- */
	- public function rules(bool $all = false): array
	- {
	- // All supported password policy rules (with default params)
	- $supported = 'min:6,max:255,lower,upper,digit,special,last:3';
	-
	- // Get the password policy from the $owner settings
	- if ($this->owner) {
	- $conf = $this->owner->getSetting('password_policy');
	- }
	-
	- // Fallback to the configured policy
	- if (empty($conf)) {
	- $conf = \config('app.password_policy');
	- }
	-
	- $supported = self::parsePolicy($supported);
	- $conf = self::parsePolicy($conf);
	- $rules = $all ? $supported : $conf;
	-
	- foreach ($rules as $idx => $rule) {
	- $param = $rule;
	-
	- if ($all && array_key_exists($idx, $conf)) {
	- $param = $conf[$idx];
	- $enabled = true;
	- } else {
	- $enabled = !$all;
	- }
	-
	- $rules[$idx] = [
	- 'label' => $idx,
	- 'name' => \trans("app.password-rule-{$idx}", ['param' => $param]),
	- 'param' => $param,
	- 'enabled' => $enabled,
	- ];
	- }
	-
	- return $rules;
	- }
	-
	- /**
	- * Parse configured policy string
	- *
	- * @param ?string $policy Policy specification
	- *
	- * @return array Policy specification as an array indexed by the policy rule type
	- */
	- public static function parsePolicy(?string $policy): array
	- {
	- $policy = explode(',', strtolower((string) $policy));
	- $policy = array_map('trim', $policy);
	- $policy = array_unique(array_filter($policy));
	-
	- return self::mapWithKeys($policy);
	- }
	-
	- /**
	- * Check password agains <count> of old passwords in user history
	- *
	- * @param string $password The password to check
	- * @param int $count Number of old passwords to check (including current one)
	- *
	- * @return bool True if password is unique, False otherwise
	- */
	- protected function checkPasswordHistory($password, int $count): bool
	- {
	- $status = $password !== '';
	-
	- // Check if password is not the same as last X passwords
	- if ($status && $this->user && $count > 0) {
	- // Current password
	- if ($this->user->password) {
	- $count--;
	- if (Hash::check($password, $this->user->password)) {
	- return false;
	- }
	- }
	-
	- // Passwords from the history
	- if ($count > 0) {
	- $this->user->passwords()->latest()->limit($count)->get()
	- ->each(static function ($oldPassword) use (&$status, $password) {
	- if (Hash::check($password, $oldPassword->password)) {
	- $status = false;
	- return false; // stop iteration
	- }
	- });
	- }
	- }
	-
	- return $status;
	- }
	-
	- /**
	- * Convert an array with password policy rules into one indexed by the rule name
	- *
	- * @param array $rules The rules list
	- */
	- private static function mapWithKeys(array $rules): array
	- {
	- $result = [];
	-
	- foreach ($rules as $rule) {
	- $key = $rule;
	- $value = null;
	-
	- if (strpos($key, ':')) {
	- [$key, $value] = explode(':', $key, 2);
	- }
	-
	- $result[$key] = $value;
	- }
	-
	- return $result;
	- }
	}
	diff --git a/src/app/Traits/UserConfigTrait.php b/src/app/Traits/UserConfigTrait.php
	--- a/src/app/Traits/UserConfigTrait.php
	+++ b/src/app/Traits/UserConfigTrait.php
	@@ -2,7 +2,7 @@

	namespace App\Traits;

	-use App\Rules\Password;
	+use App\Policy\Password;
	use App\Utils;

	trait UserConfigTrait
	diff --git a/src/app/User.php b/src/app/User.php
	--- a/src/app/User.php
	+++ b/src/app/User.php
	@@ -4,6 +4,7 @@

	use App\Auth\SecondFactor;
	use App\Auth\Utils as AuthUtils;
	+use App\Policy\Password;
	use App\Traits\AliasesTrait;
	use App\Traits\BelongsToTenantTrait;
	use App\Traits\EmailPropertyTrait;
	@@ -760,42 +761,13 @@
	*/
	public function validatePassword(string $password): bool
	{
	- $authenticated = false;
	-
	if (!empty($this->password)) {
	$authenticated = Hash::check($password, $this->password);
	} elseif (!empty($this->password_ldap)) {
	- if (preg_match('/^\{([A-Z0-9_-]+)\}/', $this->password_ldap, $matches)) {
	- switch ($matches[1]) {
	- case 'SSHA':
	- $salt = substr(base64_decode(substr($this->password_ldap, 6)), 20);
	- $hash = '{SSHA}' . base64_encode(sha1($password . $salt, true) . $salt);
	- break;
	- case 'SSHA512':
	- $salt = substr(base64_decode(substr($this->password_ldap, 9)), 64);
	- $hash = '{SSHA512}' . base64_encode(pack('H*', hash('sha512', $password . $salt)) . $salt);
	- break;
	- case 'PBKDF2_SHA256':
	- // Algorithm based on https://github.com/thesubtlety/389-ds-password-check/blob/master/389ds-pwdcheck.py
	- $decoded = base64_decode(substr($this->password_ldap, 15));
	- $param = unpack('Niterations/a64salt', $decoded);
	- $hash = hash_pbkdf2('sha256', $password, $param['salt'], $param['iterations'], 256, true);
	- $hash = '{' . $matches[1] . '}' . base64_encode(substr($decoded, 0, 68) . $hash);
	- break;
	- case 'PBKDF2-SHA512':
	- [, $algo] = explode('-', $matches[1]);
	- [$iterations, $salt] = explode('$', substr($this->password_ldap, 15));
	- $hash = hash_pbkdf2($algo, $password, base64_decode($salt), (int) $iterations, 0, true);
	- $hash = '{' . $matches[1] . '}' . $iterations . '$' . $salt . '$' . base64_encode($hash);
	- break;
	- default:
	- \Log::warning("Unsupported password hashing algorithm {$matches[1]}");
	- }
	- }
	-
	- $authenticated = isset($hash) && $hash === $this->password_ldap;
	+ $authenticated = Password::checkHash($password, $this->password_ldap);
	} else {
	\Log::error("Missing password for {$this->email}");
	+ $authenticated = false;
	}

	if ($authenticated) {
	diff --git a/src/tests/Feature/UserTest.php b/src/tests/Feature/UserTest.php
	--- a/src/tests/Feature/UserTest.php
	+++ b/src/tests/Feature/UserTest.php
	@@ -1730,32 +1730,7 @@
	$this->assertTrue($user->password_ldap == $ldap); // @phpstan-ignore-line
	$this->assertTrue(strlen($user->password) == strlen($hash)); // @phpstan-ignore-line

	- // Test PBKDF2-SHA512 algorithm (389-ds)
	- $pass = '{PBKDF2-SHA512}10000$hbMHmUXNh3UoHNlDu+NJBOW+hVAhP3C0$ax9vNLL1rr85ppODFTykPC46igHh92v'
	- . 'ULWpZaR/CQqyD4IGG/IyPYbbC2v7BxSPIDUXc0e9AGX9IuwinIj5a/w==';
	- $user->setRawAttributes(array_merge($attrs, ['password_ldap' => $pass, 'password' => null]));
	-
	- $this->assertTrue($user->validatePassword('12345678_kolab'));
	- $this->assertFalse($user->validatePassword('badpass'));
	-
	- // Test SSHA algorithm
	- $pass = '{SSHA}kor2Qo2qEsP1XojQe3esWFB8IvYKqwUH';
	- $user->setRawAttributes(array_merge($attrs, ['password_ldap' => $pass, 'password' => null]));
	-
	- $this->assertTrue($user->validatePassword('test123'));
	- $this->assertFalse($user->validatePassword('badpass'));
	-
	- // Test PBKDF2_SHA256 algorithm (389-ds)
	- $pass = '{PBKDF2_SHA256}AAAIABzpVq0s1Ta7cqubx+19QOzsI7n7KRLu0SovLVivxUVCn0+ghlj3+9+tf3jqurd'
	- . 'QhpQ/OWYmxMlAJCAeIU3jN0DDW7ODk9FpLFzhO2055J+vY5M7EXAGrvhUlkiyeH/zx/RBp2pVQq/2vtI+qmO'
	- . 'GOGUXdZ0hK00yNXpZ7K7WTwsnEXeWs4DGkGkxwyPgsGTyEwwdYK4YpCFdjJi/dXI6+kKf72j+B+epuzPtuvd'
	- . 'Mj5xGnqe9jS+BN9Huzkof4vRPX3bYecywPaeNcdXPUY3iSj8hxFqiWbBDZ0mYy9aYAy6QgMitcdEGadPcR+d'
	- . 'HXWNGK1qSLrFJJrB3cQtYhl+OgtHlwI0H4XTGBdp4MbegM3VgpUKuBNyIypwZ5oB/PRHA188bmsMjDmyN2kE'
	- . 'nHSb1CK9MXcuS4bCQzNtutmQCxBCo';
	- $user->setRawAttributes(array_merge($attrs, ['password_ldap' => $pass, 'password' => null]));
	-
	- $this->assertTrue($user->validatePassword('Simple321Simple321'));
	- $this->assertFalse($user->validatePassword('badpass'));
	+ // Note: We test other password algorithms in the Password policy tests
	}

	/**
	diff --git a/src/tests/Unit/Rules/PasswordTest.php b/src/tests/Unit/Policy/PasswordTest.php
	copy from src/tests/Unit/Rules/PasswordTest.php
	copy to src/tests/Unit/Policy/PasswordTest.php
	--- a/src/tests/Unit/Rules/PasswordTest.php
	+++ b/src/tests/Unit/Policy/PasswordTest.php
	@@ -1,70 +1,63 @@
	<?php

	-namespace Tests\Unit\Rules;
	+namespace Tests\Unit\Policy;

	-use App\Rules\Password;
	-use App\User;
	+use App\Policy\Password;
	use App\Utils;
	use Illuminate\Support\Facades\Hash;
	-use Illuminate\Support\Facades\Validator;
	use Tests\TestCase;

	class PasswordTest extends TestCase
	{
	/**
	- * Test password validation
	+ * Test checkHash() method
	*/
	- public function testValidator(): void
	+ public function testCheckHash(): void
	{
	- $error = "Specified password does not comply with the policy.";
	-
	- \config(['app.password_policy' => 'min:5']);
	- $this->assertSame($error, $this->validate('abcd'));
	- $this->assertNull($this->validate('abcde'));
	-
	- \config(['app.password_policy' => 'min:5,max:10']);
	- $this->assertSame($error, $this->validate('12345678901'));
	- $this->assertNull($this->validate('1234567890'));
	-
	- \config(['app.password_policy' => 'min:5,lower']);
	- $this->assertSame($error, $this->validate('12345'));
	- $this->assertSame($error, $this->validate('AAAAA'));
	- $this->assertNull($this->validate('12345a'));
	-
	- \config(['app.password_policy' => 'upper']);
	- $this->assertSame($error, $this->validate('5'));
	- $this->assertSame($error, $this->validate('a'));
	- $this->assertNull($this->validate('A'));
	-
	- \config(['app.password_policy' => 'digit']);
	- $this->assertSame($error, $this->validate('a'));
	- $this->assertSame($error, $this->validate('A'));
	- $this->assertNull($this->validate('5'));
	-
	- \config(['app.password_policy' => 'special']);
	- $this->assertSame($error, $this->validate('a'));
	- $this->assertSame($error, $this->validate('5'));
	- $this->assertNull($this->validate('*'));
	- $this->assertNull($this->validate('-'));
	-
	- // Test with an account policy
	- $user = $this->getTestUser('john@kolab.org');
	- $user->setSetting('password_policy', 'min:10,upper');
	+ // Test default algorithm w/o prefix
	+ $hash = Hash::make('test123');
	+
	+ $this->assertTrue(Password::checkHash('test123', $hash));
	+ $this->assertFalse(Password::checkHash('badpass', $hash));
	+
	+ // Test SSHA algorithm
	+ $hash = '{SSHA}kor2Qo2qEsP1XojQe3esWFB8IvYKqwUH';
	+
	+ $this->assertTrue(Password::checkHash('test123', $hash));
	+ $this->assertFalse(Password::checkHash('badpass', $hash));
	+
	+ // Test SSHA algorithm
	+ $hash = '{SSHA512}' . base64_encode(pack('H*', hash('sha512', 'test123')));
	+
	+ $this->assertTrue(Password::checkHash('test123', $hash));
	+ $this->assertFalse(Password::checkHash('badpass', $hash));

	- $this->assertSame($error, $this->validate('aaa', $user));
	- $this->assertSame($error, $this->validate('1234567890', $user));
	- $this->assertNull($this->validate('1234567890A', $user));
	+ // Test PBKDF2-SHA512 algorithm (389-ds)
	+ $hash = '{PBKDF2-SHA512}10000$hbMHmUXNh3UoHNlDu+NJBOW+hVAhP3C0$ax9vNLL1rr85ppODFTykPC46igHh92v'
	+ . 'ULWpZaR/CQqyD4IGG/IyPYbbC2v7BxSPIDUXc0e9AGX9IuwinIj5a/w==';
	+
	+ $this->assertTrue(Password::checkHash('12345678_kolab', $hash));
	+ $this->assertFalse(Password::checkHash('badpass', $hash));
	+
	+ // Test PBKDF2_SHA256 algorithm (389-ds)
	+ $hash = '{PBKDF2_SHA256}AAAIABzpVq0s1Ta7cqubx+19QOzsI7n7KRLu0SovLVivxUVCn0+ghlj3+9+tf3jqurd'
	+ . 'QhpQ/OWYmxMlAJCAeIU3jN0DDW7ODk9FpLFzhO2055J+vY5M7EXAGrvhUlkiyeH/zx/RBp2pVQq/2vtI+qmO'
	+ . 'GOGUXdZ0hK00yNXpZ7K7WTwsnEXeWs4DGkGkxwyPgsGTyEwwdYK4YpCFdjJi/dXI6+kKf72j+B+epuzPtuvd'
	+ . 'Mj5xGnqe9jS+BN9Huzkof4vRPX3bYecywPaeNcdXPUY3iSj8hxFqiWbBDZ0mYy9aYAy6QgMitcdEGadPcR+d'
	+ . 'HXWNGK1qSLrFJJrB3cQtYhl+OgtHlwI0H4XTGBdp4MbegM3VgpUKuBNyIypwZ5oB/PRHA188bmsMjDmyN2kE'
	+ . 'nHSb1CK9MXcuS4bCQzNtutmQCxBCo';
	+
	+ $this->assertTrue(Password::checkHash('Simple321Simple321', $hash));
	+ $this->assertFalse(Password::checkHash('badpass', $hash));
	}

	/**
	- * Test check() method
	+ * Test checkPolicy() method
	*/
	- public function testCheck(): void
	+ public function testCheckPolicy(): void
	{
	- $pass = new Password();
	-
	\config(['app.password_policy' => 'min:5,max:10,upper,lower,digit']);
	- $result = $pass->check('abcd');
	+ $result = Password::checkPolicy('abcd');

	$this->assertCount(5, $result);
	$this->assertSame('min', $result['min']['label']);
	@@ -100,13 +93,10 @@
	// Test password history check
	$user = $this->getTestUser('john@kolab.org');
	$user->passwords()->delete();
	- $user_pass = Utils::generatePassphrase(); // should be the same plain password as John already has
	-
	- $pass = new Password(null, $user);

	\config(['app.password_policy' => 'min:5,last:1']);

	- $result = $pass->check('abcd');
	+ $result = Password::checkPolicy('abcd', $user);

	$this->assertCount(2, $result);
	$this->assertSame('min', $result['min']['label']);
	@@ -121,7 +111,8 @@
	$this->assertTrue($result['last']['enabled']);
	$this->assertTrue($result['last']['status']);

	- $result = $pass->check($user_pass);
	+ $user_pass = Utils::generatePassphrase(); // should be the same plain password as John already has
	+ $result = Password::checkPolicy($user_pass, $user);

	$this->assertCount(2, $result);
	$this->assertSame('last', $result['last']['label']);
	@@ -133,14 +124,20 @@
	$user->passwords()->create(['password' => Hash::make('1234567891')]);
	$user->passwords()->create(['password' => Hash::make('1234567890')]);

	- $result = $pass->check('1234567890');
	-
	+ $result = Password::checkPolicy('1234567890', $user);
	$this->assertTrue($result['last']['status']);

	\config(['app.password_policy' => 'min:5,last:3']);

	- $result = $pass->check('1234567890');
	+ $result = Password::checkPolicy('1234567890', $user);
	+ $this->assertFalse($result['last']['status']);
	+
	+ // Test LDAP password in history
	+ \config(['app.password_policy' => 'min:5,last:5']);
	+ $hash = '{SSHA512}' . base64_encode(pack('H*', hash('sha512', 'test123aaa')));
	+ $user->passwords()->create(['password' => $hash]);

	+ $result = Password::checkPolicy('test123aaa', $user);
	$this->assertFalse($result['last']['status']);
	}

	@@ -152,11 +149,9 @@
	$user = $this->getTestUser('john@kolab.org');
	$user->setSetting('password_policy', 'min:10,upper');

	- $pass = new Password($user);
	-
	\config(['app.password_policy' => 'min:5,max:10,digit']);

	- $result = $pass->rules();
	+ $result = Password::rules($user);

	$this->assertCount(2, $result);
	$this->assertSame('min', $result['min']['label']);
	@@ -170,7 +165,7 @@
	$this->assertTrue($result['upper']['enabled']);

	// Expect to see all supported policy rules
	- $result = $pass->rules(true);
	+ $result = Password::rules($user, true);

	$this->assertCount(7, $result);
	$this->assertSame('min', $result['min']['label']);
	@@ -208,29 +203,4 @@
	$this->assertSame('3', $result['last']['param']);
	$this->assertFalse($result['last']['enabled']);
	}
	-
	- /**
	- * Validates the password using Laravel Validator API
	- *
	- * @param string $password The password to validate
	- * @param ?User $owner The account owner
	- *
	- * @return ?string Validation error message on error, NULL otherwise
	- */
	- private function validate($password, $owner = null): ?string
	- {
	- // Instead of doing direct tests, we use validator to make sure
	- // it works with the framework api
	-
	- $v = Validator::make(
	- ['password' => $password],
	- ['password' => new Password($owner)]
	- );
	-
	- if ($v->fails()) {
	- return $v->errors()->toArray()['password'][0];
	- }
	-
	- return null;
	- }
	}
	diff --git a/src/tests/Unit/Rules/PasswordTest.php b/src/tests/Unit/Rules/PasswordTest.php
	--- a/src/tests/Unit/Rules/PasswordTest.php
	+++ b/src/tests/Unit/Rules/PasswordTest.php
	@@ -4,8 +4,6 @@

	use App\Rules\Password;
	use App\User;
	-use App\Utils;
	-use Illuminate\Support\Facades\Hash;
	use Illuminate\Support\Facades\Validator;
	use Tests\TestCase;

	@@ -56,159 +54,6 @@
	$this->assertNull($this->validate('1234567890A', $user));
	}

	- /**
	- * Test check() method
	- */
	- public function testCheck(): void
	- {
	- $pass = new Password();
	-
	- \config(['app.password_policy' => 'min:5,max:10,upper,lower,digit']);
	- $result = $pass->check('abcd');
	-
	- $this->assertCount(5, $result);
	- $this->assertSame('min', $result['min']['label']);
	- $this->assertSame('Minimum password length: 5 characters', $result['min']['name']);
	- $this->assertSame('5', $result['min']['param']);
	- $this->assertTrue($result['min']['enabled']);
	- $this->assertFalse($result['min']['status']);
	-
	- $this->assertSame('max', $result['max']['label']);
	- $this->assertSame('Maximum password length: 10 characters', $result['max']['name']);
	- $this->assertSame('10', $result['max']['param']);
	- $this->assertTrue($result['max']['enabled']);
	- $this->assertTrue($result['max']['status']);
	-
	- $this->assertSame('upper', $result['upper']['label']);
	- $this->assertSame('Password contains an upper-case character', $result['upper']['name']);
	- $this->assertNull($result['upper']['param']);
	- $this->assertTrue($result['upper']['enabled']);
	- $this->assertFalse($result['upper']['status']);
	-
	- $this->assertSame('lower', $result['lower']['label']);
	- $this->assertSame('Password contains a lower-case character', $result['lower']['name']);
	- $this->assertNull($result['lower']['param']);
	- $this->assertTrue($result['lower']['enabled']);
	- $this->assertTrue($result['lower']['status']);
	-
	- $this->assertSame('digit', $result['digit']['label']);
	- $this->assertSame('Password contains a digit', $result['digit']['name']);
	- $this->assertNull($result['digit']['param']);
	- $this->assertTrue($result['digit']['enabled']);
	- $this->assertFalse($result['digit']['status']);
	-
	- // Test password history check
	- $user = $this->getTestUser('john@kolab.org');
	- $user->passwords()->delete();
	- $user_pass = Utils::generatePassphrase(); // should be the same plain password as John already has
	-
	- $pass = new Password(null, $user);
	-
	- \config(['app.password_policy' => 'min:5,last:1']);
	-
	- $result = $pass->check('abcd');
	-
	- $this->assertCount(2, $result);
	- $this->assertSame('min', $result['min']['label']);
	- $this->assertSame('Minimum password length: 5 characters', $result['min']['name']);
	- $this->assertSame('5', $result['min']['param']);
	- $this->assertTrue($result['min']['enabled']);
	- $this->assertFalse($result['min']['status']);
	-
	- $this->assertSame('last', $result['last']['label']);
	- $this->assertSame('Password cannot be the same as the last 1 passwords', $result['last']['name']);
	- $this->assertSame('1', $result['last']['param']);
	- $this->assertTrue($result['last']['enabled']);
	- $this->assertTrue($result['last']['status']);
	-
	- $result = $pass->check($user_pass);
	-
	- $this->assertCount(2, $result);
	- $this->assertSame('last', $result['last']['label']);
	- $this->assertSame('Password cannot be the same as the last 1 passwords', $result['last']['name']);
	- $this->assertSame('1', $result['last']['param']);
	- $this->assertTrue($result['last']['enabled']);
	- $this->assertFalse($result['last']['status']);
	-
	- $user->passwords()->create(['password' => Hash::make('1234567891')]);
	- $user->passwords()->create(['password' => Hash::make('1234567890')]);
	-
	- $result = $pass->check('1234567890');
	-
	- $this->assertTrue($result['last']['status']);
	-
	- \config(['app.password_policy' => 'min:5,last:3']);
	-
	- $result = $pass->check('1234567890');
	-
	- $this->assertFalse($result['last']['status']);
	- }
	-
	- /**
	- * Test rules() method
	- */
	- public function testRules(): void
	- {
	- $user = $this->getTestUser('john@kolab.org');
	- $user->setSetting('password_policy', 'min:10,upper');
	-
	- $pass = new Password($user);
	-
	- \config(['app.password_policy' => 'min:5,max:10,digit']);
	-
	- $result = $pass->rules();
	-
	- $this->assertCount(2, $result);
	- $this->assertSame('min', $result['min']['label']);
	- $this->assertSame('Minimum password length: 10 characters', $result['min']['name']);
	- $this->assertSame('10', $result['min']['param']);
	- $this->assertTrue($result['min']['enabled']);
	-
	- $this->assertSame('upper', $result['upper']['label']);
	- $this->assertSame('Password contains an upper-case character', $result['upper']['name']);
	- $this->assertNull($result['upper']['param']);
	- $this->assertTrue($result['upper']['enabled']);
	-
	- // Expect to see all supported policy rules
	- $result = $pass->rules(true);
	-
	- $this->assertCount(7, $result);
	- $this->assertSame('min', $result['min']['label']);
	- $this->assertSame('Minimum password length: 10 characters', $result['min']['name']);
	- $this->assertSame('10', $result['min']['param']);
	- $this->assertTrue($result['min']['enabled']);
	-
	- $this->assertSame('max', $result['max']['label']);
	- $this->assertSame('Maximum password length: 255 characters', $result['max']['name']);
	- $this->assertSame('255', $result['max']['param']);
	- $this->assertFalse($result['max']['enabled']);
	-
	- $this->assertSame('upper', $result['upper']['label']);
	- $this->assertSame('Password contains an upper-case character', $result['upper']['name']);
	- $this->assertNull($result['upper']['param']);
	- $this->assertTrue($result['upper']['enabled']);
	-
	- $this->assertSame('lower', $result['lower']['label']);
	- $this->assertSame('Password contains a lower-case character', $result['lower']['name']);
	- $this->assertNull($result['lower']['param']);
	- $this->assertFalse($result['lower']['enabled']);
	-
	- $this->assertSame('digit', $result['digit']['label']);
	- $this->assertSame('Password contains a digit', $result['digit']['name']);
	- $this->assertNull($result['digit']['param']);
	- $this->assertFalse($result['digit']['enabled']);
	-
	- $this->assertSame('special', $result['special']['label']);
	- $this->assertSame('Password contains a special character', $result['special']['name']);
	- $this->assertNull($result['digit']['param']);
	- $this->assertFalse($result['digit']['enabled']);
	-
	- $this->assertSame('last', $result['last']['label']);
	- $this->assertSame('Password cannot be the same as the last 3 passwords', $result['last']['name']);
	- $this->assertSame('3', $result['last']['param']);
	- $this->assertFalse($result['last']['enabled']);
	- }
	-
	/**
	* Validates the password using Laravel Validator API
	*

File Metadata

Mime Type: text/plain
Expires: Sat, Apr 4, 6:48 AM (6 h, 3 m ago)
Storage Engine: blob
Storage Format: Raw Data
Storage Handle: 18828430
Default Alt Text: D5301.1775285335.diff (39 KB)

D5301.1775285335.diffNo OneTemporaryActions

D5301.1775285335.diffView Options

File Metadata

Event Timeline

D5301.1775285335.diff
No OneTemporary
Actions

D5301.1775285335.diff
View Options