From b5b4d5005a0e558c78b66af5abbbd811e60fd994 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julio=20Cesar=20Laura=20Avenda=C3=B1o?= Date: Wed, 22 Nov 2023 14:08:24 +0000 Subject: [PATCH 1/3] FOUR-11415 Password Policy Configuration --- .../Http/Controllers/Auth/LoginController.php | 3 +++ ProcessMaker/Models/User.php | 27 ++++++++++++++----- config/password-policies.php | 11 ++++++++ resources/views/admin/users/edit.blade.php | 6 ----- .../views/auth/passwords/change.blade.php | 13 +-------- resources/views/profile/edit.blade.php | 6 ----- .../views/shared/users/sidebar.blade.php | 6 ++--- 7 files changed, 39 insertions(+), 33 deletions(-) create mode 100644 config/password-policies.php diff --git a/ProcessMaker/Http/Controllers/Auth/LoginController.php b/ProcessMaker/Http/Controllers/Auth/LoginController.php index d4012a6877..45e8f64f58 100644 --- a/ProcessMaker/Http/Controllers/Auth/LoginController.php +++ b/ProcessMaker/Http/Controllers/Auth/LoginController.php @@ -38,6 +38,8 @@ class LoginController extends Controller */ protected $redirectTo = '/'; + protected $maxAttempts; + /** * Create a new controller instance. * @@ -46,6 +48,7 @@ class LoginController extends Controller public function __construct() { $this->middleware('guest')->except(['logout', 'beforeLogout', 'keepAlive']); + $this->maxAttempts = (int) config('password-policies.login_attempts'); } /** diff --git a/ProcessMaker/Models/User.php b/ProcessMaker/Models/User.php index 8e2cd4e8ef..541e91ba8c 100644 --- a/ProcessMaker/Models/User.php +++ b/ProcessMaker/Models/User.php @@ -8,12 +8,12 @@ use Illuminate\Foundation\Auth\User as Authenticatable; use Illuminate\Notifications\Notifiable; use Illuminate\Validation\Rule; +use Illuminate\Validation\Rules\Password; use Laravel\Passport\HasApiTokens; use ProcessMaker\Models\EmptyModel; use ProcessMaker\Notifications\ResetPassword as ResetPasswordNotification; use ProcessMaker\Query\Traits\PMQL; use ProcessMaker\Rules\StringHasAtLeastOneUpperCaseCharacter; -use ProcessMaker\Rules\StringHasNumberOrSpecialCharacter; use ProcessMaker\Traits\Exportable; use ProcessMaker\Traits\HasAuthorization; use ProcessMaker\Traits\HideSystemResources; @@ -183,13 +183,28 @@ public static function rules(self $existing = null) */ public static function passwordRules(self $existing = null) { - return array_filter([ + // Mandatory policies + $passwordPolicies = [ 'required', $existing ? 'sometimes' : '', - 'min:8', - new StringHasNumberOrSpecialCharacter(), - new StringHasAtLeastOneUpperCaseCharacter(), - ]); + ]; + // Configurable policies + $passwordRules = Password::min(config('password-policies.minimum_length')); + if (config('password-policies.maximum_length')) { + $passwordPolicies[] = 'max:' . config('password-policies.maximum_length'); + } + if (config('password-policies.numbers')) { + $passwordRules->numbers(); + } + if (config('password-policies.uppercase')) { + $passwordPolicies[] = new StringHasAtLeastOneUpperCaseCharacter(); + } + if (config('password-policies.special')) { + $passwordRules->symbols(); + } + $passwordPolicies[] = $passwordRules; + + return $passwordPolicies; } /** diff --git a/config/password-policies.php b/config/password-policies.php new file mode 100644 index 0000000000..1bb8d9cefa --- /dev/null +++ b/config/password-policies.php @@ -0,0 +1,11 @@ + env('PASSWORD_POLICY_MINIMUM_LENGTH', 8), + 'maximum_length' => env('PASSWORD_POLICY_MAXIMUM_LENGTH', null), + 'numbers' => env('PASSWORD_POLICY_NUMBERS', true), + 'uppercase' => env('PASSWORD_POLICY_UPPERCASE', true), + 'special' => env('PASSWORD_POLICY_SPECIAL', true), + //'expiration_days' => env('PASSWORD_POLICY_EXPIRATION_DAYS', 0), // 0 never expires + 'login_attempts' => env('PASSWORD_POLICY_LOGIN_ATTEMPTS', 5), +]; diff --git a/resources/views/admin/users/edit.blade.php b/resources/views/admin/users/edit.blade.php index f00ae382df..c34292b5cb 100644 --- a/resources/views/admin/users/edit.blade.php +++ b/resources/views/admin/users/edit.blade.php @@ -396,12 +396,6 @@ delete this.formData.password; return true } - if (this.formData.password.trim().length > 0 && this.formData.password.trim().length < 8) { - this.errors.password = ['Password must be at least 8 characters'] - this.password = '' - this.submitted = false - return false - } if (this.formData.password !== this.formData.confPassword) { this.errors.password = ['Passwords must match'] this.password = '' diff --git a/resources/views/auth/passwords/change.blade.php b/resources/views/auth/passwords/change.blade.php index 0627e07982..5b5ddca593 100644 --- a/resources/views/auth/passwords/change.blade.php +++ b/resources/views/auth/passwords/change.blade.php @@ -45,7 +45,7 @@ 'v-bind:class' => '{\'form-control\':true, \'is-invalid\':errors.password}']) !!} - @{{ errors.password[0] }} + @{{ error }}
{!!Form::label('confpassword', __('Confirm Password'))!!}* @@ -94,11 +94,6 @@ }); }, validatePassword() { - if (this.$refs.passwordStrength.strength.score < 2) { - this.errors.password = ['Password is too weak'] - return false - } - if (!this.formData.password && !this.formData.confpassword) { return false; } @@ -107,12 +102,6 @@ return false } - if (this.formData.password.trim().length > 0 && this.formData.password.trim().length < 8) { - this.errors.password = ['Password must be at least 8 characters'] - this.formData.password = '' - return false - } - if (this.formData.password !== this.formData.confpassword) { this.errors.password = ['Passwords must match'] return false diff --git a/resources/views/profile/edit.blade.php b/resources/views/profile/edit.blade.php index 56b227325b..994ce50737 100644 --- a/resources/views/profile/edit.blade.php +++ b/resources/views/profile/edit.blade.php @@ -151,12 +151,6 @@ delete this.formData.password; return true } - if (this.formData.password.trim().length > 0 && this.formData.password.trim().length < 8) { - this.errors.password = ['Password must be at least 8 characters'] - this.password = '' - this.submitted = false - return false - } if (this.formData.password !== this.formData.confPassword) { this.errors.password = ['Passwords must match'] this.password = '' diff --git a/resources/views/shared/users/sidebar.blade.php b/resources/views/shared/users/sidebar.blade.php index cb97917c33..513b4d10c1 100644 --- a/resources/views/shared/users/sidebar.blade.php +++ b/resources/views/shared/users/sidebar.blade.php @@ -22,8 +22,8 @@
-
- +
+
{{__('Login Information')}}
@@ -54,7 +54,7 @@ {!! Form::label('confPassword', __('Confirm Password')) !!} {!! Form::password('confPassword', ['id' => 'confPassword', 'rows' => 4, 'class'=> 'form-control', 'v-model' => 'formData.confPassword', 'autocomplete' => 'new-password', 'v-bind:class' => '{\'form-control\':true, \'is-invalid\':errors.password}']) !!} - +
@cannot('edit-user-and-password')
From 6a660a6d8d9700c81822cff185afff3f40300487 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julio=20Cesar=20Laura=20Avenda=C3=B1o?= Date: Fri, 24 Nov 2023 14:11:03 +0000 Subject: [PATCH 2/3] FOUR-11415 Password Policy Configuration - Fix observations made by Dave --- resources/views/shared/users/sidebar.blade.php | 3 ++- tests/Feature/Api/UsersTest.php | 14 +++++++------- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/resources/views/shared/users/sidebar.blade.php b/resources/views/shared/users/sidebar.blade.php index 513b4d10c1..361ce4d42a 100644 --- a/resources/views/shared/users/sidebar.blade.php +++ b/resources/views/shared/users/sidebar.blade.php @@ -54,7 +54,8 @@ {!! Form::label('confPassword', __('Confirm Password')) !!} {!! Form::password('confPassword', ['id' => 'confPassword', 'rows' => 4, 'class'=> 'form-control', 'v-model' => 'formData.confPassword', 'autocomplete' => 'new-password', 'v-bind:class' => '{\'form-control\':true, \'is-invalid\':errors.password}']) !!} - +
@cannot('edit-user-and-password')
diff --git a/tests/Feature/Api/UsersTest.php b/tests/Feature/Api/UsersTest.php index 686f6ac374..45fe563df8 100644 --- a/tests/Feature/Api/UsersTest.php +++ b/tests/Feature/Api/UsersTest.php @@ -61,7 +61,7 @@ public function getUpdatedData() 'timezone' => $faker->timezone(), 'status' => $faker->randomElement(['ACTIVE', 'INACTIVE']), 'birthdate' => $faker->dateTimeThisCentury()->format('Y-m-d'), - 'password' => $faker->sentence(10), + 'password' => $faker->sentence(8) . 'A_' . '1', ]; } @@ -100,7 +100,7 @@ public function testCreateUser() 'lastname' => 'name', 'email' => $faker->email(), 'status' => $faker->randomElement(['ACTIVE', 'INACTIVE']), - 'password' => $faker->sentence(10), + 'password' => $faker->password(8) . 'A_' . '1', ]); // Validate the header status code @@ -602,9 +602,9 @@ public function testCreateWithoutPassword() $response = $this->apiCall('POST', self::API_TEST_URL, $payload); $response->assertStatus(422); $json = $response->json(); - $this->assertEquals('The Password field must be at least 8 characters.', $json['errors']['password'][0]); + $this->assertTrue(in_array('The Password field must be at least 8 characters.', $json['errors']['password'])); - $payload['password'] = 'Abc12345'; + $payload['password'] = 'Abc12345_'; $response = $this->apiCall('POST', self::API_TEST_URL, $payload); $response->assertStatus(201); $json = $response->json(); @@ -616,9 +616,9 @@ public function testCreateWithoutPassword() $response = $this->apiCall('PUT', route('api.users.update', $userId), $payload); $response->assertStatus(422); $json = $response->json(); - $this->assertEquals('The Password field must be at least 8 characters.', $json['errors']['password'][0]); + $this->assertTrue(in_array('The Password field must be at least 8 characters.', $json['errors']['password'])); - $payload['password'] = 'Abc12345'; + $payload['password'] = 'Abc12345_'; $response = $this->apiCall('PUT', route('api.users.update', $userId), $payload); $response->assertStatus(204); @@ -673,7 +673,7 @@ public function testCreateUserValidateUsername() 'lastname' => $faker->lastName(), 'email' => $faker->email(), 'status' => $faker->randomElement(['ACTIVE', 'INACTIVE']), - 'password' => $faker->sentence(10), + 'password' => $faker->sentence(8) . 'A_' . '1', ]); // Validate the header status code $response->assertStatus(201); From 91a198ce4ed307e43ed8fe76ffa4a14f575e31c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julio=20Cesar=20Laura=20Avenda=C3=B1o?= Date: Fri, 24 Nov 2023 15:11:50 +0000 Subject: [PATCH 3/3] FOUR-11415 Password Policy Configuration - Fix unit test ChangePassword --- tests/Feature/Api/ChangePasswordTest.php | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/Feature/Api/ChangePasswordTest.php b/tests/Feature/Api/ChangePasswordTest.php index b30b08644a..80aa9e3ffe 100644 --- a/tests/Feature/Api/ChangePasswordTest.php +++ b/tests/Feature/Api/ChangePasswordTest.php @@ -45,8 +45,9 @@ public function testUserPasswordChangeWithInvalidPassword() 'confpassword' => 'ProcessMaker', ]); - $response->assertStatus(422) - ->assertSeeText('The password must contain either a number or a special character'); + $response->assertStatus(422); + $json = $response->json(); + $this->assertTrue(in_array('The Password field must contain at least one symbol.', $json['errors']['password'])); // Validate updated user password changed $updatedUser = User::where('id', $user->id)->first(); @@ -72,8 +73,8 @@ public function testUserChangePasswordMustSetFlagToFalse() // Post data with new password $response = $this->apiCall('PUT', self::API_TEST_URL, [ - 'password' => 'ProcessMaker1', - 'confpassword' => 'ProcessMaker1', + 'password' => 'ProcessMaker1_', + 'confpassword' => 'ProcessMaker1_', ]); // Validate the header status code