diff --git a/src/.env.example b/src/.env.example --- a/src/.env.example +++ b/src/.env.example @@ -164,6 +164,8 @@ PASSPORT_PRIVATE_KEY= PASSPORT_PUBLIC_KEY= +PASSWORD_POLICY= + COMPANY_NAME= COMPANY_ADDRESS= COMPANY_DETAILS= diff --git a/src/app/Http/Controllers/API/PasswordPolicyController.php b/src/app/Http/Controllers/API/PasswordPolicyController.php new file mode 100644 --- /dev/null +++ b/src/app/Http/Controllers/API/PasswordPolicyController.php @@ -0,0 +1,66 @@ +guard()->user()->walletOwner(); + + // Get the policy + $policy = new Password($owner); + $rules = $policy->rules(true); + + return response()->json([ + 'list' => array_values($rules), + 'count' => count($rules), + ]); + } + + /** + * Validate the password regarding the defined policies. + * + * @param \Illuminate\Http\Request $request + * + * @return \Illuminate\Http\JsonResponse + */ + public function check(Request $request) + { + $userId = $request->input('user'); + + $user = !empty($userId) ? \App\User::find($userId) : null; + + // Get the policy + $policy = new Password($user ? $user->walletOwner() : null); + + // Check the password + $status = $policy->check($request->input('password')); + + $passed = array_filter( + $status, + function ($rule) { + return !empty($rule['status']); + } + ); + + return response()->json([ + 'status' => count($passed) == count($status) ? 'success' : 'error', + 'list' => array_values($status), + 'count' => count($status), + ]); + } +} diff --git a/src/app/Http/Controllers/API/PasswordResetController.php b/src/app/Http/Controllers/API/PasswordResetController.php --- a/src/app/Http/Controllers/API/PasswordResetController.php +++ b/src/app/Http/Controllers/API/PasswordResetController.php @@ -4,6 +4,7 @@ use App\Http\Controllers\Controller; use App\Jobs\PasswordResetEmail; +use App\Rules\Password; use App\User; use App\VerificationCode; use Illuminate\Http\Request; @@ -99,8 +100,11 @@ // with single SQL query (->delete()) instead of two (::destroy()) $this->code = $code; - // Return user name and email/phone from the codes database on success - return response()->json(['status' => 'success']); + return response()->json([ + 'status' => 'success', + // we need user's ID for e.g. password policy checks + 'userId' => $code->user_id, + ]); } /** @@ -112,25 +116,23 @@ */ public function reset(Request $request) { - // Validate the request args + $v = $this->verify($request); + if ($v->status() !== 200) { + return $v; + } + + $user = $this->code->user; + + // Validate the password $v = Validator::make( $request->all(), - [ - 'password' => 'required|min:4|confirmed', - ] + ['password' => ['required', 'confirmed', new Password($user->walletOwner())]] ); if ($v->fails()) { return response()->json(['status' => 'error', 'errors' => $v->errors()], 422); } - $v = $this->verify($request); - if ($v->status() !== 200) { - return $v; - } - - $user = $this->code->user; - // Change the user password $user->setPasswordAttribute($request->password); $user->save(); diff --git a/src/app/Http/Controllers/API/SignupController.php b/src/app/Http/Controllers/API/SignupController.php --- a/src/app/Http/Controllers/API/SignupController.php +++ b/src/app/Http/Controllers/API/SignupController.php @@ -9,6 +9,7 @@ use App\Domain; use App\Plan; use App\Rules\SignupExternalEmail; +use App\Rules\Password; use App\Rules\UserEmailDomain; use App\Rules\UserEmailLocal; use App\SignupCode; @@ -207,7 +208,7 @@ $request->all(), [ 'login' => 'required|min:2', - 'password' => 'required|min:4|confirmed', + 'password' => ['required', 'confirmed', new Password()], 'domain' => 'required', 'voucher' => 'max:32', ] diff --git a/src/app/Http/Controllers/API/V4/UsersController.php b/src/app/Http/Controllers/API/V4/UsersController.php --- a/src/app/Http/Controllers/API/V4/UsersController.php +++ b/src/app/Http/Controllers/API/V4/UsersController.php @@ -5,6 +5,7 @@ use App\Http\Controllers\RelationController; use App\Domain; use App\Group; +use App\Rules\Password; use App\Rules\UserEmailDomain; use App\Rules\UserEmailLocal; use App\Sku; @@ -188,6 +189,7 @@ 'enableFolders' => $isController && $hasCustomDomain && in_array('beta-shared-folders', $skus), // TODO: Make 'enableResources' working for wallet controllers that aren't account owners 'enableResources' => $isController && $hasCustomDomain && in_array('beta-resources', $skus), + 'enableSettings' => $isController, 'enableUsers' => $isController, 'enableWallets' => $isController, ]; @@ -311,10 +313,6 @@ $user->setAliases($request->aliases); } - // TODO: Make sure that UserUpdate job is created in case of entitlements update - // and no password change. So, for example quota change is applied to LDAP - // TODO: Review use of $user->save() in the above context - DB::commit(); $response = [ @@ -471,6 +469,8 @@ 'aliases' => 'array|nullable', ]; + $controller = ($user ?: $this->guard()->user())->walletOwner(); + // Handle generated password reset code if ($code = $request->input('passwordLinkCode')) { // Accept - input @@ -491,7 +491,7 @@ if (empty($user) || !empty($request->password) || !empty($request->password_confirmation)) { if (empty($ignorePassword)) { - $rules['password'] = 'required|min:6|max:255|confirmed'; + $rules['password'] = ['required', 'confirmed', new Password($controller)]; } } @@ -504,8 +504,6 @@ $errors = $v->errors()->toArray(); } - $controller = $user ? $user->wallet()->owner : $this->guard()->user(); - // For new user validate email address if (empty($user)) { $email = $request->email; diff --git a/src/app/Rules/Password.php b/src/app/Rules/Password.php new file mode 100644 --- /dev/null +++ b/src/app/Rules/Password.php @@ -0,0 +1,199 @@ +owner = $owner; + } + + /** + * Determine if the validation rule passes. + * + * @param string $attribute Attribute name + * @param mixed $password Password string + * + * @return bool + */ + public function passes($attribute, $password): bool + { + foreach ($this->check($password) as $rule) { + if (empty($rule['status'])) { + $this->message = \trans('validation.password-policy-error'); + return false; + } + } + + return true; + } + + /** + * Get the validation error message. + * + * @return string + */ + public function message(): ?string + { + 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 + $pass = strlen($password) >= intval($rule['param']); + break; + + case 'max': + // Check the max length + $length = strlen($password); + $pass = $length && $length <= intval($rule['param']); + break; + + case 'lower': + // Check if password contains a lower-case character + $pass = preg_match('/[a-z]/', $password) > 0; + break; + + case 'upper': + // Check if password contains a upper-case character + $pass = preg_match('/[A-Z]/', $password) > 0; + break; + + case 'digit': + // Check if password contains a digit + $pass = preg_match('/[0-9]/', $password) > 0; + break; + + case 'special': + // Check if password contains a special character + $pass = preg_match('/[-~!@#$%^&*_+=`(){}[]|:;"\'`<>,.?\/\\]/', $password) > 0; + break; + + default: + // Ignore unknown rule name + $pass = true; + } + + $rules[$name]['status'] = $pass; + } + + 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'; + + // 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'); + } + + // Default policy, if not set + if (empty($conf)) { + $conf = 'min:6,max:255'; + } + + $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); + } + + /** + * Convert an array with password policy rules into one indexed by the rule name + * + * @param array $rules The rules list + * + * @return array + */ + private static function mapWithKeys(array $rules): array + { + $result = []; + + foreach ($rules as $rule) { + $key = $rule; + $value = null; + + if (strpos($key, ':')) { + list($key, $value) = explode(':', $key, 2); + } + + $result[$key] = $value; + } + + return $result; + } +} diff --git a/src/app/Traits/EntitleableTrait.php b/src/app/Traits/EntitleableTrait.php --- a/src/app/Traits/EntitleableTrait.php +++ b/src/app/Traits/EntitleableTrait.php @@ -255,4 +255,24 @@ return null; } + + /** + * Return the owner of the wallet (account) this entitleable is assigned to + * + * @return ?\App\User Account owner + */ + public function walletOwner(): ?\App\User + { + $wallet = $this->wallet(); + + if ($wallet) { + if ($this instanceof \App\User && $wallet->user_id == $this->id) { + return $this; + } + + return $wallet->owner; + } + + return null; + } } 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 @@ -16,6 +16,7 @@ // TODO: Should we store the default value somewhere in config? $config['greylist_enabled'] = $this->getSetting('greylist_enabled') !== 'false'; + $config['password_policy'] = $this->getSetting('password_policy'); return $config; } @@ -34,6 +35,20 @@ foreach ($config as $key => $value) { if ($key == 'greylist_enabled') { $this->setSetting('greylist_enabled', $value ? 'true' : 'false'); + } elseif ($key == 'password_policy') { + if (!is_string($value) || (strlen($value) && !preg_match('/^[a-z0-9:,]+$/', $value))) { + $errors[$key] = \trans('validation.invalid-password-policy'); + continue; + } + + foreach (explode(',', $value) as $rule) { + if ($error = $this->validatePasswordPolicyRule($rule)) { + $errors[$key] = $error; + continue 2; + } + } + + $this->setSetting('password_policy', $value); } else { $errors[$key] = \trans('validation.invalid-config-parameter'); } @@ -41,4 +56,42 @@ return $errors; } + + /** + * Validates password policy rule. + * + * @param string $rule Policy rule + * + * @return ?string An error message on error, Null otherwise + */ + protected function validatePasswordPolicyRule(string $rule): ?string + { + $regexp = [ + 'min:[0-9]+', 'max:[0-9]+', 'upper', 'lower', 'digit', 'special', + ]; + + if (empty($rule) || !preg_match('/^(' . implode('|', $regexp) . ')$/', $rule)) { + return \trans('validation.invalid-password-policy'); + } + + $systemPolicy = \App\Rules\Password::parsePolicy(\config('app.password_policy')); + + // Min/Max values cannot exceed the system defaults, i.e. if system policy + // is min:5, user's policy cannot be set to a smaller number. + if (!empty($systemPolicy['min'])) { + $value = trim(substr($rule, 4)); + if ($value < $systemPolicy['min']) { + return \trans('validation.password-policy-min-len-error', ['min' => $systemPolicy['min']]); + } + } + + if (!empty($systemPolicy['max'])) { + $value = trim(substr($rule, 4)); + if ($value > $systemPolicy['max']) { + return \trans('validation.password-policy-max-len-error', ['max' => $systemPolicy['max']]); + } + } + + return null; + } } diff --git a/src/config/app.php b/src/config/app.php --- a/src/config/app.php +++ b/src/config/app.php @@ -285,6 +285,8 @@ 'rate' => (float) env('VAT_RATE'), ], + 'password_policy' => env('PASSWORD_POLICY'), + 'payment' => [ 'methods_oneoff' => env('PAYMENT_METHODS_ONEOFF', "creditcard,paypal,banktransfer"), 'methods_recurring' => env('PAYMENT_METHODS_RECURRING', "creditcard"), diff --git a/src/database/migrations/2022_01_13_100000_verification_code_active_column.php b/src/database/migrations/2022_01_13_100000_verification_code_active_column.php --- a/src/database/migrations/2022_01_13_100000_verification_code_active_column.php +++ b/src/database/migrations/2022_01_13_100000_verification_code_active_column.php @@ -29,6 +29,10 @@ */ public function down() { + if (!Schema::hasTable('verification_codes')) { + return; + } + Schema::table( 'verification_codes', function (Blueprint $table) { diff --git a/src/resources/js/app.js b/src/resources/js/app.js --- a/src/resources/js/app.js +++ b/src/resources/js/app.js @@ -388,7 +388,7 @@ loadLangAsync().then(() => app.$mount('#app')) // Add a axios request interceptor -window.axios.interceptors.request.use( +axios.interceptors.request.use( config => { // This is the only way I found to change configuration options // on a running application. We need this for browser testing. @@ -403,7 +403,7 @@ ) // Add a axios response interceptor for general/validation error handler -window.axios.interceptors.response.use( +axios.interceptors.response.use( response => { if (response.config.onFinish) { response.config.onFinish() @@ -413,7 +413,7 @@ }, error => { // Do not display the error in a toast message, pass the error as-is - if (error.config.ignoreErrors) { + if (axios.isCancel(error) || error.config.ignoreErrors) { return Promise.reject(error) } diff --git a/src/resources/js/fontawesome.js b/src/resources/js/fontawesome.js --- a/src/resources/js/fontawesome.js +++ b/src/resources/js/fontawesome.js @@ -26,6 +26,7 @@ faPlus, faSearch, faSignInAlt, + faSlidersH, faSyncAlt, faTrashAlt, faUser, @@ -61,6 +62,7 @@ faPlus, faSearch, faSignInAlt, + faSlidersH, faSquare, faSyncAlt, faTrashAlt, diff --git a/src/resources/js/user/routes.js b/src/resources/js/user/routes.js --- a/src/resources/js/user/routes.js +++ b/src/resources/js/user/routes.js @@ -16,6 +16,7 @@ const MeetComponent = () => import(/* webpackChunkName: "../user/pages" */ '../../vue/Rooms') const ResourceInfoComponent = () => import(/* webpackChunkName: "../user/pages" */ '../../vue/Resource/Info') const ResourceListComponent = () => import(/* webpackChunkName: "../user/pages" */ '../../vue/Resource/List') +const SettingsComponent = () => import(/* webpackChunkName: "../user/pages" */ '../../vue/Settings') const SharedFolderInfoComponent = () => import(/* webpackChunkName: "../user/pages" */ '../../vue/SharedFolder/Info') const SharedFolderListComponent = () => import(/* webpackChunkName: "../user/pages" */ '../../vue/SharedFolder/List') const UserInfoComponent = () => import(/* webpackChunkName: "../user/pages" */ '../../vue/User/Info') @@ -108,6 +109,12 @@ meta: { requiresAuth: true } }, { + path: '/settings', + name: 'settings', + component: SettingsComponent, + meta: { requiresAuth: true, perm: 'settings' } + }, + { path: '/shared-folder/:folder', name: 'shared-folder', component: SharedFolderInfoComponent, diff --git a/src/resources/lang/en/app.php b/src/resources/lang/en/app.php --- a/src/resources/lang/en/app.php +++ b/src/resources/lang/en/app.php @@ -110,6 +110,12 @@ 'wallet-update-success' => 'User wallet updated successfully.', 'password-reset-code-delete-success' => 'Password reset code deleted successfully.', + 'password-rule-min' => 'Minimum password length: :param characters', + 'password-rule-max' => 'Maximum password length: :param characters', + 'password-rule-lower' => 'Password contains a lower-case character', + 'password-rule-upper' => 'Password contains an upper-case character', + 'password-rule-digit' => 'Password contains a digit', + 'password-rule-special' => 'Password contains a special character', 'wallet-notice-date' => 'With your current subscriptions your account balance will last until about :date (:days).', 'wallet-notice-nocredit' => 'You are out of credit, top up your balance now.', diff --git a/src/resources/lang/en/ui.php b/src/resources/lang/en/ui.php --- a/src/resources/lang/en/ui.php +++ b/src/resources/lang/en/ui.php @@ -46,6 +46,7 @@ 'invitations' => "Invitations", 'profile' => "Your profile", 'resources' => "Resources", + 'settings' => "Settings", 'shared-folders' => "Shared folders", 'users' => "User accounts", 'wallet' => "Wallet", @@ -423,6 +424,7 @@ 'pass-input' => "Enter password", 'pass-link' => "Set via link", 'pass-link-label' => "Link:", + 'passwordpolicy' => "Password Policy", 'price' => "Price", 'profile-title' => "Your profile", 'profile-delete' => "Delete account", diff --git a/src/resources/lang/en/validation.php b/src/resources/lang/en/validation.php --- a/src/resources/lang/en/validation.php +++ b/src/resources/lang/en/validation.php @@ -145,6 +145,10 @@ 'invalid-config-parameter' => 'The requested configuration parameter is not supported.', 'nameexists' => 'The specified name is not available.', 'nameinvalid' => 'The specified name is invalid.', + 'password-policy-error' => 'Specified password does not comply with the policy.', + 'invalid-password-policy' => 'Specified password policy is invalid.', + 'password-policy-min-len-error' => 'Minimum password length cannot be less than :min.', + 'password-policy-max-len-error' => 'Maximum password length cannot be more than :max.', /* |-------------------------------------------------------------------------- diff --git a/src/resources/themes/app.scss b/src/resources/themes/app.scss --- a/src/resources/themes/app.scss +++ b/src/resources/themes/app.scss @@ -301,6 +301,7 @@ // Some icons are too big, scale them down &.link-domains, &.link-resources, + &.link-settings, &.link-wallet, &.link-invitations { svg { diff --git a/src/resources/themes/forms.scss b/src/resources/themes/forms.scss --- a/src/resources/themes/forms.scss +++ b/src/resources/themes/forms.scss @@ -35,6 +35,17 @@ } } +.password-input { + ul { + svg { + width: 0.75em !important; + } + span { + padding: 0 0.05em; + } + } +} + .range-input { display: flex; diff --git a/src/resources/vue/Dashboard.vue b/src/resources/vue/Dashboard.vue --- a/src/resources/vue/Dashboard.vue +++ b/src/resources/vue/Dashboard.vue @@ -32,6 +32,9 @@ {{ $t('dashboard.chat') }} {{ $t('dashboard.beta') }} + + {{ $t('dashboard.settings') }} + {{ $t('dashboard.webmail') }} diff --git a/src/resources/vue/PasswordReset.vue b/src/resources/vue/PasswordReset.vue --- a/src/resources/vue/PasswordReset.vue +++ b/src/resources/vue/PasswordReset.vue @@ -41,15 +41,8 @@

-
- - -
-
- - -
-
+ +
@@ -68,15 +61,20 @@ diff --git a/src/resources/vue/Signup.vue b/src/resources/vue/Signup.vue --- a/src/resources/vue/Signup.vue +++ b/src/resources/vue/Signup.vue @@ -83,15 +83,8 @@
+
- - -
-
- - -
-
@@ -106,7 +99,12 @@ diff --git a/src/routes/api.php b/src/routes/api.php --- a/src/routes/api.php +++ b/src/routes/api.php @@ -42,6 +42,8 @@ 'prefix' => $prefix . 'api/auth' ], function ($router) { + Route::post('password-policy/check', 'API\PasswordPolicyController@check'); + Route::post('password-reset/init', 'API\PasswordResetController@init'); Route::post('password-reset/verify', 'API\PasswordResetController@verify'); Route::post('password-reset', 'API\PasswordResetController@reset'); @@ -100,6 +102,7 @@ Route::get('wallets/{id}/receipts', 'API\V4\WalletsController@receipts'); Route::get('wallets/{id}/receipts/{receipt}', 'API\V4\WalletsController@receiptDownload'); + Route::get('password-policy', 'API\PasswordPolicyController@index'); Route::post('password-reset/code', 'API\PasswordResetController@codeCreate'); Route::delete('password-reset/code/{id}', 'API\PasswordResetController@codeDelete'); diff --git a/src/tests/Browser/Pages/Settings.php b/src/tests/Browser/Pages/Settings.php new file mode 100644 --- /dev/null +++ b/src/tests/Browser/Pages/Settings.php @@ -0,0 +1,44 @@ +waitFor('@form') + ->waitUntilMissing('.app-loader'); + } + + /** + * Get the element shortcuts for the page. + * + * @return array + */ + public function elements(): array + { + return [ + '@app' => '#app', + '@form' => '#settings form', + ]; + } +} diff --git a/src/tests/Browser/PasswordResetTest.php b/src/tests/Browser/PasswordResetTest.php --- a/src/tests/Browser/PasswordResetTest.php +++ b/src/tests/Browser/PasswordResetTest.php @@ -36,7 +36,7 @@ /** * Test the link from logon to password-reset page */ - public function testPasswordResetLinkOnLogon(): void + public function testLinkOnLogon(): void { $this->browse(function (Browser $browser) { $browser->visit(new Home()); @@ -52,7 +52,7 @@ /** * Test 1st step of password-reset */ - public function testPasswordResetStep1(): void + public function testStep1(): void { $user = $this->getTestUser('passwordresettestdusk@' . \config('app.domain')); $user->setSetting('external_email', 'external@domain.tld'); @@ -105,9 +105,9 @@ /** * Test 2nd Step of the password reset process * - * @depends testPasswordResetStep1 + * @depends testStep1 */ - public function testPasswordResetStep2(): void + public function testStep2(): void { $user = $this->getTestUser('passwordresettestdusk@' . \config('app.domain')); $user->setSetting('external_email', 'external@domain.tld'); @@ -180,41 +180,43 @@ /** * Test 3rd Step of the password reset process * - * @depends testPasswordResetStep2 + * @depends testStep2 */ - public function testPasswordResetStep3(): void + public function testStep3(): void { $user = $this->getTestUser('passwordresettestdusk@' . \config('app.domain')); $user->setSetting('external_email', 'external@domain.tld'); + $user->setSetting('password_policy', 'upper,digit'); $this->browse(function (Browser $browser) { - $browser->assertVisible('@step3'); + $browser->assertVisible('@step3') + ->clearToasts(); // Here we expect 2 text inputs, Back and Continue buttons - $browser->with('@step3', function ($step) { - $step->assertVisible('#reset_password'); - $step->assertVisible('#reset_confirm'); - $step->assertVisible('[type=button]'); - $step->assertVisible('[type=submit]'); - $step->assertFocused('#reset_password'); + $browser->with('@step3', function (Browser $step) { + $step->assertVisible('#reset_password') + ->assertVisible('#reset_password_confirmation') + ->assertVisible('[type=button]') + ->assertVisible('[type=submit]') + ->assertFocused('#reset_password'); }); // Test Back button - $browser->click('@step3 [type=button]'); - $browser->waitFor('@step2'); - $browser->assertFocused('@step2 #reset_short_code'); - $browser->assertMissing('@step3'); - $browser->assertMissing('@step1'); + $browser->click('@step3 [type=button]') + ->waitFor('@step2') + ->assertFocused('@step2 #reset_short_code') + ->assertMissing('@step3') + ->assertMissing('@step1'); // TODO: Test form reset when going back // Because the verification code is removed in tearDown() // we'll start from the beginning (Step 1) - $browser->click('@step2 [type=button]'); - $browser->waitFor('@step1'); - $browser->assertFocused('@step1 #reset_email'); - $browser->assertMissing('@step3'); - $browser->assertMissing('@step2'); + $browser->click('@step2 [type=button]') + ->waitFor('@step1') + ->assertFocused('@step1 #reset_email') + ->assertMissing('@step3') + ->assertMissing('@step2'); // Submit valid data $browser->with('@step1', function ($step) { @@ -222,8 +224,8 @@ $step->click('[type=submit]'); }); - $browser->waitFor('@step2'); - $browser->waitUntilMissing('@step2 #reset_code[value=""]'); + $browser->waitFor('@step2') + ->waitUntilMissing('@step2 #reset_code[value=""]'); // Submit valid code again $browser->with('@step2', function ($step) { @@ -241,18 +243,27 @@ // Submit invalid data $browser->with('@step3', function ($step) use ($browser) { - $step->assertFocused('#reset_password'); - - $step->type('#reset_password', '12345678'); - $step->type('#reset_confirm', '123456789'); + $step->assertFocused('#reset_password') + ->whenAvailable('#reset_password_policy', function (Browser $browser) { + $browser->assertElementsCount('li', 2) + ->assertMissing('li:first-child svg.text-success') + ->assertSeeIn('li:first-child small', "Password contains an upper-case character") + ->assertMissing('li:last-child svg.text-success') + ->assertSeeIn('li:last-child small', "Password contains a digit"); + }) + ->type('#reset_password', 'A2345678') + ->type('#reset_password_confirmation', '123456789') + ->with('#reset_password_policy', function (Browser $browser) { + $browser->waitFor('li:first-child svg.text-success') + ->waitFor('li:last-child svg.text-success'); + }); $step->click('[type=submit]'); $browser->waitFor('.toast-error'); $step->waitFor('#reset_password.is-invalid') - ->assertVisible('#reset_password.is-invalid') - ->assertVisible('#reset_password + .invalid-feedback') + ->assertVisible('#reset_password_input .invalid-feedback') ->assertFocused('#reset_password'); $browser->click('.toast-error'); // remove the toast @@ -260,9 +271,8 @@ // Submit valid data $browser->with('@step3', function ($step) { - $step->type('#reset_confirm', '12345678'); - - $step->click('[type=submit]'); + $step->type('#reset_password_confirmation', 'A2345678') + ->click('[type=submit]'); }); $browser->waitUntilMissing('@step3'); @@ -273,4 +283,12 @@ // FIXME: Is it enough to be sure user is logged in? }); } + + /** + * Test password reset process for a user with 2FA enabled. + */ + public function testResetWith2FA(): void + { + $this->markTestIncomplete(); + } } diff --git a/src/tests/Browser/SettingsTest.php b/src/tests/Browser/SettingsTest.php new file mode 100644 --- /dev/null +++ b/src/tests/Browser/SettingsTest.php @@ -0,0 +1,100 @@ +browse(function (Browser $browser) { + $browser->visit('/settings')->on(new Home()); + }); + } + + /** + * Test settings "box" on Dashboard + */ + public function testDashboard(): void + { + $this->browse(function (Browser $browser) { + // Test a user that is not an account owner + $browser->visit(new Home()) + ->submitLogon('jack@kolab.org', 'simple123', true) + ->on(new Dashboard()) + ->assertMissing('@links .link-settings .name') + ->visit('/settings') + ->assertErrorPage(403) + ->within(new Menu(), function (Browser $browser) { + $browser->clickMenuItem('logout'); + }); + + // Test the account owner + $browser->waitForLocation('/login') + ->on(new Home()) + ->submitLogon('john@kolab.org', 'simple123', true) + ->on(new Dashboard()) + ->assertSeeIn('@links .link-settings .name', 'Settings'); + }); + } + + /** + * Test Settings page + * + * @depends testDashboard + */ + public function testSettings(): void + { + $john = $this->getTestUser('john@kolab.org'); + $john->setSetting('password_policy', 'min:5,max:100,lower'); + + $this->browse(function (Browser $browser) { + $browser->click('@links .link-settings') + ->on(new Settings()) + ->assertSeeIn('#settings .card-title', 'Settings') + // Password policy + ->assertSeeIn('@form .row:nth-child(1) > label', 'Password Policy') + ->with('@form #password_policy', function (Browser $browser) { + $browser->assertElementsCount('li', 6) + ->assertSeeIn('li:nth-child(1) label', 'Minimum password length') + ->assertChecked('li:nth-child(1) input[type=checkbox]') + ->assertValue('li:nth-child(1) input[type=text]', '5') + ->assertSeeIn('li:nth-child(2) label', 'Maximum password length') + ->assertChecked('li:nth-child(2) input[type=checkbox]') + ->assertValue('li:nth-child(2) input[type=text]', '100') + ->assertSeeIn('li:nth-child(3) label', 'Password contains a lower-case character') + ->assertChecked('li:nth-child(3) input[type=checkbox]') + ->assertMissing('li:nth-child(3) input[type=text]') + ->assertSeeIn('li:nth-child(4) label', 'Password contains an upper-case character') + ->assertNotChecked('li:nth-child(4) input[type=checkbox]') + ->assertMissing('li:nth-child(4) input[type=text]') + ->assertSeeIn('li:nth-child(5) label', 'Password contains a digit') + ->assertNotChecked('li:nth-child(5) input[type=checkbox]') + ->assertMissing('li:nth-child(5) input[type=text]') + ->assertSeeIn('li:nth-child(6) label', 'Password contains a special character') + ->assertNotChecked('li:nth-child(6) input[type=checkbox]') + ->assertMissing('li:nth-child(6) input[type=text]') + // Change the policy + ->type('li:nth-child(1) input[type=text]', '11') + ->type('li:nth-child(2) input[type=text]', '120') + ->click('li:nth-child(3) input[type=checkbox]') + ->click('li:nth-child(4) input[type=checkbox]'); + }) + ->click('button[type=submit]') + ->assertToast(Toast::TYPE_SUCCESS, 'User settings updated successfully.'); + }); + + $this->assertSame('min:11,max:120,upper', $john->getSetting('password_policy')); + } +} diff --git a/src/tests/Browser/SignupTest.php b/src/tests/Browser/SignupTest.php --- a/src/tests/Browser/SignupTest.php +++ b/src/tests/Browser/SignupTest.php @@ -305,7 +305,7 @@ ->assertMissing('#signup_first_name') ->assertVisible('#signup_login') ->assertVisible('#signup_password') - ->assertVisible('#signup_confirm') + ->assertVisible('#signup_password_confirmation') ->assertVisible('select#signup_domain') ->assertElementsCount('select#signup_domain option', $domains_count, false) ->assertText('select#signup_domain option:nth-child(1)', $domains[0]) @@ -319,7 +319,14 @@ ->assertSelected('select#signup_domain', \config('app.domain')) ->assertValue('#signup_login', '') ->assertValue('#signup_password', '') - ->assertValue('#signup_confirm', ''); + ->assertValue('#signup_password_confirmation', '') + ->with('#signup_password_policy', function (Browser $browser) { + $browser->assertElementsCount('li', 2) + ->assertMissing('li:first-child svg.text-success') + ->assertSeeIn('li:first-child small', "Minimum password length: 6 characters") + ->assertMissing('li:last-child svg.text-success') + ->assertSeeIn('li:last-child small', "Maximum password length: 255 characters"); + }); // TODO: Test domain selector }); @@ -351,12 +358,16 @@ $step->assertFocused('#signup_login') ->type('#signup_login', '*') ->type('#signup_password', '12345678') - ->type('#signup_confirm', '123456789') + ->type('#signup_password_confirmation', '123456789') + ->with('#signup_password_policy', function (Browser $browser) { + $browser->waitFor('li:first-child svg.text-success') + ->waitFor('li:last-child svg.text-success'); + }) ->click('[type=submit]') ->waitFor('#signup_login.is-invalid') ->assertVisible('#signup_domain + .invalid-feedback') ->assertVisible('#signup_password.is-invalid') - ->assertVisible('#signup_password + .invalid-feedback') + ->assertVisible('#signup_password_input .invalid-feedback') ->assertFocused('#signup_login') ->assertToast(Toast::TYPE_ERROR, 'Form validation error'); }); @@ -366,7 +377,7 @@ $step->type('#signup_login', 'SignupTestDusk') ->click('[type=submit]') ->waitFor('#signup_password.is-invalid') - ->assertVisible('#signup_password + .invalid-feedback') + ->assertVisible('#signup_password_input .invalid-feedback') ->assertMissing('#signup_login.is-invalid') ->assertMissing('#signup_domain + .invalid-feedback') ->assertFocused('#signup_password') @@ -375,7 +386,7 @@ // Submit valid data $browser->with('@step3', function ($step) { - $step->type('#signup_confirm', '12345678'); + $step->type('#signup_password_confirmation', '12345678'); $step->click('[type=submit]'); }); @@ -433,7 +444,7 @@ $browser->whenAvailable('@step3', function ($step) { $step->assertVisible('#signup_login') ->assertVisible('#signup_password') - ->assertVisible('#signup_confirm') + ->assertVisible('#signup_password_confirmation') ->assertVisible('input#signup_domain') ->assertVisible('[type=button]') ->assertVisible('[type=submit]') @@ -441,7 +452,7 @@ ->assertValue('input#signup_domain', '') ->assertValue('#signup_login', '') ->assertValue('#signup_password', '') - ->assertValue('#signup_confirm', ''); + ->assertValue('#signup_password_confirmation', ''); }); // Submit invalid login and password data @@ -450,12 +461,12 @@ ->type('#signup_login', '*') ->type('#signup_domain', 'test.com') ->type('#signup_password', '12345678') - ->type('#signup_confirm', '123456789') + ->type('#signup_password_confirmation', '123456789') ->click('[type=submit]') ->waitFor('#signup_login.is-invalid') ->assertVisible('#signup_domain + .invalid-feedback') ->assertVisible('#signup_password.is-invalid') - ->assertVisible('#signup_password + .invalid-feedback') + ->assertVisible('#signup_password_input .invalid-feedback') ->assertFocused('#signup_login') ->assertToast(Toast::TYPE_ERROR, 'Form validation error'); }); @@ -465,12 +476,12 @@ $step->type('#signup_login', 'admin') ->type('#signup_domain', 'aaa') ->type('#signup_password', '12345678') - ->type('#signup_confirm', '12345678') + ->type('#signup_password_confirmation', '12345678') ->click('[type=submit]') ->waitUntilMissing('#signup_login.is-invalid') ->waitFor('#signup_domain.is-invalid + .invalid-feedback') ->assertMissing('#signup_password.is-invalid') - ->assertMissing('#signup_password + .invalid-feedback') + ->assertMissing('#signup_password_input .invalid-feedback') ->assertFocused('#signup_domain') ->assertToast(Toast::TYPE_ERROR, 'Form validation error'); }); @@ -533,7 +544,7 @@ ->type('#signup_voucher', 'TESTXX') ->type('#signup_login', 'signuptestdusk') ->type('#signup_password', '123456789') - ->type('#signup_confirm', '123456789') + ->type('#signup_password_confirmation', '123456789') ->click('[type=submit]') ->waitFor('#signup_voucher.is-invalid') ->assertVisible('#signup_voucher + .invalid-feedback') @@ -585,7 +596,7 @@ ->assertVisible('#signup_first_name') ->assertVisible('#signup_login') ->assertVisible('#signup_password') - ->assertVisible('#signup_confirm') + ->assertVisible('#signup_password_confirmation') ->assertVisible('select#signup_domain') ->assertElementsCount('select#signup_domain option', $domains_count, false) ->assertVisible('[type=submit]') @@ -597,22 +608,22 @@ ->assertValue('#signup_last_name', '') ->assertValue('#signup_login', '') ->assertValue('#signup_password', '') - ->assertValue('#signup_confirm', ''); + ->assertValue('#signup_password_confirmation', ''); // Submit invalid data $step->type('#signup_login', '*') ->type('#signup_password', '12345678') - ->type('#signup_confirm', '123456789') + ->type('#signup_password_confirmation', '123456789') ->click('[type=submit]') ->waitFor('#signup_login.is-invalid') ->assertVisible('#signup_domain + .invalid-feedback') ->assertVisible('#signup_password.is-invalid') - ->assertVisible('#signup_password + .invalid-feedback') + ->assertVisible('#signup_password_input .invalid-feedback') ->assertFocused('#signup_login') ->assertToast(Toast::TYPE_ERROR, 'Form validation error'); // Submit valid data - $step->type('#signup_confirm', '12345678') + $step->type('#signup_password_confirmation', '12345678') ->type('#signup_login', 'signuptestdusk') ->type('#signup_first_name', 'First') ->type('#signup_last_name', 'Last') diff --git a/src/tests/Browser/UserProfileTest.php b/src/tests/Browser/UserProfileTest.php --- a/src/tests/Browser/UserProfileTest.php +++ b/src/tests/Browser/UserProfileTest.php @@ -62,6 +62,9 @@ */ public function testProfile(): void { + $user = $this->getTestUser('john@kolab.org'); + $user->setSetting('password_policy', 'min:10,upper,digit'); + $this->browse(function (Browser $browser) { $browser->visit(new Home()) ->submitLogon('john@kolab.org', 'simple123', true) @@ -95,8 +98,26 @@ ->assertValue('div.row:nth-child(9) input#password_confirmation', '') ->assertAttribute('#password', 'placeholder', 'Password') ->assertAttribute('#password_confirmation', 'placeholder', 'Confirm Password') + ->whenAvailable('#password_policy', function (Browser $browser) { + $browser->assertElementsCount('li', 3) + ->assertMissing('li:nth-child(1) svg.text-success') + ->assertSeeIn('li:nth-child(1) small', "Minimum password length: 10 characters") + ->assertMissing('li:nth-child(2) svg.text-success') + ->assertSeeIn('li:nth-child(2) small', "Password contains an upper-case character") + ->assertMissing('li:nth-child(3) svg.text-success') + ->assertSeeIn('li:nth-child(3) small', "Password contains a digit"); + }) ->assertSeeIn('button[type=submit]', 'Submit'); + // Test password policy checking + $browser->type('#password', '1A') + ->whenAvailable('#password_policy', function (Browser $browser) { + $browser->waitFor('li:nth-child(2) svg.text-success') + ->waitFor('li:nth-child(3) svg.text-success') + ->assertMissing('li:nth-child(1) svg.text-success'); + }) + ->vueClear('#password'); + // Test form error handling $browser->type('#phone', 'aaaaaa') ->type('#external_email', 'bbbbb') @@ -132,6 +153,9 @@ */ public function testProfileNonController(): void { + $user = $this->getTestUser('john@kolab.org'); + $user->setSetting('password_policy', 'min:10,upper,digit'); + // Test acting as non-controller $this->browse(function (Browser $browser) { $browser->visit('/logout') @@ -145,6 +169,16 @@ ->whenAvailable('@form', function (Browser $browser) { // TODO: decide on what fields the non-controller user should be able // to see/change + }) + // Check that the account policy is used + ->whenAvailable('#password_policy', function (Browser $browser) { + $browser->assertElementsCount('li', 3) + ->assertMissing('li:nth-child(1) svg.text-success') + ->assertSeeIn('li:nth-child(1) small', "Minimum password length: 10 characters") + ->assertMissing('li:nth-child(2) svg.text-success') + ->assertSeeIn('li:nth-child(2) small', "Password contains an upper-case character") + ->assertMissing('li:nth-child(3) svg.text-success') + ->assertSeeIn('li:nth-child(3) small', "Password contains a digit"); }); // Test that /profile/delete page is not accessible diff --git a/src/tests/Browser/UsersTest.php b/src/tests/Browser/UsersTest.php --- a/src/tests/Browser/UsersTest.php +++ b/src/tests/Browser/UsersTest.php @@ -97,6 +97,7 @@ $jack = $this->getTestUser('jack@kolab.org'); $john->verificationcodes()->delete(); $jack->verificationcodes()->delete(); + $john->setSetting('password_policy', 'min:10,upper,digit'); // Test that the page requires authentication $browser->visit('/user/' . $john->id) @@ -144,8 +145,17 @@ ->assertSeeIn('#user-info .card-title', 'User account') ->with('@general', function (Browser $browser) { // Test error handling (password) - $browser->type('#password', 'aaaaaa') + $browser->type('#password', 'aaaaaA') ->vueClear('#password_confirmation') + ->whenAvailable('#password_policy', function (Browser $browser) { + $browser->assertElementsCount('li', 3) + ->assertMissing('li:nth-child(1) svg.text-success') + ->assertSeeIn('li:nth-child(1) small', "Minimum password length: 10 characters") + ->waitFor('li:nth-child(2) svg.text-success') + ->assertSeeIn('li:nth-child(2) small', "Password contains an upper-case character") + ->assertMissing('li:nth-child(3) svg.text-success') + ->assertSeeIn('li:nth-child(3) small', "Password contains a digit"); + }) ->click('button[type=submit]') ->waitFor('#password_confirmation + .invalid-feedback') ->assertSeeIn( @@ -163,6 +173,7 @@ ->with(new ListInput('#aliases'), function (Browser $browser) { $browser->addListEntry('invalid address'); }) + ->scrollTo('button[type=submit]')->pause(500) ->click('button[type=submit]') ->assertToast(Toast::TYPE_ERROR, 'Form validation error'); @@ -389,6 +400,9 @@ */ public function testNewUser(): void { + $john = $this->getTestUser('john@kolab.org'); + $john->setSetting('password_policy', null); + $this->browse(function (Browser $browser) { $browser->visit(new UserList()) ->assertSeeIn('button.create-user', 'Create user') @@ -819,6 +833,7 @@ 'Access to calendaring resources' ) // Shared folders SKU + ->scrollTo('tbody tr:nth-child(10)')->pause(500) ->assertSeeIn('tbody tr:nth-child(10) td.name', 'Shared folders') ->assertSeeIn('tr:nth-child(10) td.price', '0,00 CHF/month') ->assertNotChecked('tbody tr:nth-child(10) td.selection input') @@ -858,7 +873,7 @@ ->on(new UserInfo()) ->waitFor('#sku-input-beta') ->click('#sku-input-beta') - ->scrollTo('@general button[type=submit]') + ->scrollTo('@general button[type=submit]')->pause(500) ->click('@general button[type=submit]') ->assertToast(Toast::TYPE_SUCCESS, 'User data updated successfully.'); diff --git a/src/tests/Feature/Controller/PasswordPolicyTest.php b/src/tests/Feature/Controller/PasswordPolicyTest.php new file mode 100644 --- /dev/null +++ b/src/tests/Feature/Controller/PasswordPolicyTest.php @@ -0,0 +1,133 @@ +getTestUser('jack@kolab.org'); + $john = $this->getTestUser('john@kolab.org'); + $john->setSetting('password_policy', 'min:8,max:100,upper,digit'); + + // Empty password + $post = ['user' => $john->id]; + $response = $this->post('/api/auth/password-policy/check', $post); + $response->assertStatus(200); + + $json = $response->json(); + + $this->assertCount(3, $json); + $this->assertSame('error', $json['status']); + $this->assertSame(4, $json['count']); + $this->assertFalse($json['list'][0]['status']); + $this->assertSame('min', $json['list'][0]['label']); + $this->assertFalse($json['list'][1]['status']); + $this->assertSame('max', $json['list'][1]['label']); + $this->assertFalse($json['list'][2]['status']); + $this->assertSame('upper', $json['list'][2]['label']); + $this->assertFalse($json['list'][3]['status']); + $this->assertSame('digit', $json['list'][3]['label']); + + // Test acting as Jack, password non-compliant + $post = ['password' => '9999999', 'user' => $jack->id]; + $response = $this->post('/api/auth/password-policy/check', $post); + $response->assertStatus(200); + + $json = $response->json(); + + $this->assertCount(3, $json); + $this->assertSame('error', $json['status']); + $this->assertSame(4, $json['count']); + $this->assertFalse($json['list'][0]['status']); // min + $this->assertTrue($json['list'][1]['status']); // max + $this->assertFalse($json['list'][2]['status']); // upper + $this->assertTrue($json['list'][3]['status']); // digit + + // Test with no user context, expect use of the default policy + $post = ['password' => '9']; + $response = $this->post('/api/auth/password-policy/check', $post); + $response->assertStatus(200); + + $json = $response->json(); + + $this->assertCount(3, $json); + $this->assertSame('error', $json['status']); + $this->assertSame(2, $json['count']); + $this->assertFalse($json['list'][0]['status']); + $this->assertSame('min', $json['list'][0]['label']); + $this->assertTrue($json['list'][1]['status']); + $this->assertSame('max', $json['list'][1]['label']); + } + + /** + * Test password-policy listing + */ + public function testIndex(): void + { + // Unauth access not allowed + $response = $this->get('/api/v4/password-policy'); + $response->assertStatus(401); + + $jack = $this->getTestUser('jack@kolab.org'); + $john = $this->getTestUser('john@kolab.org'); + $john->setSetting('password_policy', 'min:8,max:255,special'); + + // Get available policy rules + $response = $this->actingAs($john)->get('/api/v4/password-policy'); + $json = $response->json(); + + $response->assertStatus(200); + + $this->assertCount(2, $json); + $this->assertSame(6, $json['count']); + $this->assertCount(6, $json['list']); + $this->assertSame('Minimum password length: 8 characters', $json['list'][0]['name']); + $this->assertSame('min', $json['list'][0]['label']); + $this->assertSame('8', $json['list'][0]['param']); + $this->assertSame(true, $json['list'][0]['enabled']); + $this->assertSame('Maximum password length: 255 characters', $json['list'][1]['name']); + $this->assertSame('max', $json['list'][1]['label']); + $this->assertSame('255', $json['list'][1]['param']); + $this->assertSame(true, $json['list'][1]['enabled']); + $this->assertSame('lower', $json['list'][2]['label']); + $this->assertSame(false, $json['list'][2]['enabled']); + $this->assertSame('upper', $json['list'][3]['label']); + $this->assertSame(false, $json['list'][3]['enabled']); + $this->assertSame('digit', $json['list'][4]['label']); + $this->assertSame(false, $json['list'][4]['enabled']); + $this->assertSame('special', $json['list'][5]['label']); + $this->assertSame(true, $json['list'][5]['enabled']); + + // Test acting as Jack + $response = $this->actingAs($jack)->get('/api/v4/password-policy'); + $json = $response->json(); + + $response->assertStatus(200); + + $this->assertCount(2, $json); + $this->assertSame(6, $json['count']); + $this->assertCount(6, $json['list']); + $this->assertSame('Minimum password length: 8 characters', $json['list'][0]['name']); + $this->assertSame('min', $json['list'][0]['label']); + $this->assertSame('8', $json['list'][0]['param']); + $this->assertSame(true, $json['list'][0]['enabled']); + $this->assertSame('Maximum password length: 255 characters', $json['list'][1]['name']); + $this->assertSame('max', $json['list'][1]['label']); + $this->assertSame('255', $json['list'][1]['param']); + $this->assertSame(true, $json['list'][1]['enabled']); + $this->assertSame('lower', $json['list'][2]['label']); + $this->assertSame(false, $json['list'][2]['enabled']); + $this->assertSame('upper', $json['list'][3]['label']); + $this->assertSame(false, $json['list'][3]['enabled']); + $this->assertSame('digit', $json['list'][4]['label']); + $this->assertSame(false, $json['list'][4]['enabled']); + $this->assertSame('special', $json['list'][5]['label']); + $this->assertSame(true, $json['list'][5]['enabled']); + } +} diff --git a/src/tests/Feature/Controller/PasswordResetTest.php b/src/tests/Feature/Controller/PasswordResetTest.php --- a/src/tests/Feature/Controller/PasswordResetTest.php +++ b/src/tests/Feature/Controller/PasswordResetTest.php @@ -207,8 +207,9 @@ $json = $response->json(); $response->assertStatus(200); - $this->assertCount(1, $json); + $this->assertCount(2, $json); $this->assertSame('success', $json['status']); + $this->assertSame($user->id, $json['userId']); } /** @@ -222,12 +223,14 @@ $data = []; $response = $this->post('/api/auth/password-reset', $data); + $response->assertStatus(422); + $json = $response->json(); - $response->assertStatus(422); $this->assertSame('error', $json['status']); - $this->assertCount(1, $json['errors']); - $this->assertArrayHasKey('password', $json['errors']); + $this->assertCount(2, $json['errors']); + $this->assertArrayHasKey('code', $json['errors']); + $this->assertArrayHasKey('short_code', $json['errors']); $user = $this->getTestUser('passwordresettest@' . \config('app.domain')); $code = new VerificationCode(['mode' => 'password-reset']); @@ -236,12 +239,14 @@ // Data with existing code but missing password $data = [ 'code' => $code->code, + 'short_code' => $code->short_code, ]; $response = $this->post('/api/auth/password-reset', $data); + $response->assertStatus(422); + $json = $response->json(); - $response->assertStatus(422); $this->assertSame('error', $json['status']); $this->assertCount(1, $json['errors']); $this->assertArrayHasKey('password', $json['errors']); @@ -262,6 +267,22 @@ $this->assertCount(1, $json['errors']); $this->assertArrayHasKey('password', $json['errors']); + // Data with existing code but password too short + $data = [ + 'code' => $code->code, + 'short_code' => $code->short_code, + 'password' => 'pas', + 'password_confirmation' => 'pas', + ]; + + $response = $this->post('/api/auth/password-reset', $data); + $json = $response->json(); + + $response->assertStatus(422); + $this->assertSame('error', $json['status']); + $this->assertCount(1, $json['errors']); + $this->assertArrayHasKey('password', $json['errors']); + // Data with invalid short code $data = [ 'code' => $code->code, @@ -294,8 +315,8 @@ Queue::assertNothingPushed(); $data = [ - 'password' => 'test', - 'password_confirmation' => 'test', + 'password' => 'testtest', + 'password_confirmation' => 'testtest', 'code' => $code->code, 'short_code' => $code->short_code, ]; diff --git a/src/tests/Feature/Controller/SignupTest.php b/src/tests/Feature/Controller/SignupTest.php --- a/src/tests/Feature/Controller/SignupTest.php +++ b/src/tests/Feature/Controller/SignupTest.php @@ -453,7 +453,7 @@ $domain = $this->getPublicDomain(); - // Login too short + // Login too short, password too short $data = [ 'login' => '1', 'domain' => $domain, @@ -466,15 +466,16 @@ $response->assertStatus(422); $this->assertSame('error', $json['status']); - $this->assertCount(1, $json['errors']); + $this->assertCount(2, $json['errors']); $this->assertArrayHasKey('login', $json['errors']); + $this->assertArrayHasKey('password', $json['errors']); // Missing codes $data = [ 'login' => 'login-valid', 'domain' => $domain, - 'password' => 'test', - 'password_confirmation' => 'test', + 'password' => 'testtest', + 'password_confirmation' => 'testtest', ]; $response = $this->post('/api/auth/signup', $data); @@ -490,8 +491,8 @@ $data = [ 'login' => 'TestLogin', 'domain' => $domain, - 'password' => 'test', - 'password_confirmation' => 'test', + 'password' => 'testtest', + 'password_confirmation' => 'testtest', 'code' => $result['code'], 'short_code' => 'XXXX', ]; @@ -510,8 +511,8 @@ $data = [ 'login' => 'TestLogin', 'domain' => $domain, - 'password' => 'test', - 'password_confirmation' => 'test', + 'password' => 'testtest', + 'password_confirmation' => 'testtest', 'code' => $result['code'], 'short_code' => $code->short_code, 'voucher' => 'XXX', @@ -529,8 +530,8 @@ $data = [ 'login' => 'żżżżżż', 'domain' => $domain, - 'password' => 'test', - 'password_confirmation' => 'test', + 'password' => 'testtest', + 'password_confirmation' => 'testtest', 'code' => $result['code'], 'short_code' => $code->short_code, ]; @@ -559,8 +560,8 @@ $data = [ 'login' => 'SignupLogin', 'domain' => $domain, - 'password' => 'test', - 'password_confirmation' => 'test', + 'password' => 'testtest', + 'password_confirmation' => 'testtest', 'code' => $code->code, 'short_code' => $code->short_code, 'voucher' => 'TEST', @@ -672,8 +673,8 @@ $data = [ 'login' => $login, 'domain' => $domain, - 'password' => 'test', - 'password_confirmation' => 'test', + 'password' => 'testtest', + 'password_confirmation' => 'testtest', 'code' => $code->code, 'short_code' => $code->short_code, ]; @@ -745,8 +746,8 @@ 'last_name' => 'User', 'login' => 'test-inv', 'domain' => 'kolabnow.com', - 'password' => 'test', - 'password_confirmation' => 'test', + 'password' => 'testtest', + 'password_confirmation' => 'testtest', ]; // Test invalid invitation identifier diff --git a/src/tests/Feature/Controller/UsersTest.php b/src/tests/Feature/Controller/UsersTest.php --- a/src/tests/Feature/Controller/UsersTest.php +++ b/src/tests/Feature/Controller/UsersTest.php @@ -521,6 +521,7 @@ $john = $this->getTestUser('john@kolab.org'); $john->setSetting('greylist_enabled', null); + $john->setSetting('password_policy', null); // Test unknown user id $post = ['greylist_enabled' => 1]; @@ -555,7 +556,7 @@ $this->assertNull($john->fresh()->getSetting('greylist_enabled')); // Test some valid data - $post = ['greylist_enabled' => 1]; + $post = ['greylist_enabled' => 1, 'password_policy' => 'min:10,max:255,upper,lower,digit,special']; $response = $this->actingAs($john)->post("/api/v4/users/{$john->id}/config", $post); $response->assertStatus(200); @@ -566,10 +567,12 @@ $this->assertSame('User settings updated successfully.', $json['message']); $this->assertSame('true', $john->fresh()->getSetting('greylist_enabled')); + $this->assertSame('min:10,max:255,upper,lower,digit,special', $john->fresh()->getSetting('password_policy')); - // Test some valid data - $post = ['greylist_enabled' => 0]; - $response = $this->actingAs($john)->post("/api/v4/users/{$john->id}/config", $post); + // Test some valid data, acting as another account controller + $ned = $this->getTestUser('ned@kolab.org'); + $post = ['greylist_enabled' => 0, 'password_policy' => 'min:10,max:255,upper']; + $response = $this->actingAs($ned)->post("/api/v4/users/{$john->id}/config", $post); $response->assertStatus(200); $json = $response->json(); @@ -579,6 +582,7 @@ $this->assertSame('User settings updated successfully.', $json['message']); $this->assertSame('false', $john->fresh()->getSetting('greylist_enabled')); + $this->assertSame('min:10,max:255,upper', $john->fresh()->getSetting('password_policy')); } /** @@ -590,6 +594,7 @@ $jack = $this->getTestUser('jack@kolab.org'); $john = $this->getTestUser('john@kolab.org'); + $john->setSetting('password_policy', 'min:8,max:100,digit'); $deleted_priv = $this->getTestUser('deleted@kolab.org'); $deleted_priv->delete(); @@ -629,8 +634,8 @@ // Test existing user email $post = [ - 'password' => 'simple', - 'password_confirmation' => 'simple', + 'password' => 'simple123', + 'password_confirmation' => 'simple123', 'first_name' => 'John2', 'last_name' => 'Doe2', 'email' => 'jack.daniels@kolab.org', @@ -649,8 +654,8 @@ $package_domain = \App\Package::withEnvTenantContext()->where('title', 'domain-hosting')->first(); $post = [ - 'password' => 'simple', - 'password_confirmation' => 'simple', + 'password' => 'simple123', + 'password_confirmation' => 'simple123', 'first_name' => 'John2', 'last_name' => 'Doe2', 'email' => 'john2.doe2@kolab.org', @@ -679,8 +684,33 @@ $this->assertSame("Invalid package selected.", $json['errors']['package']); $this->assertCount(2, $json); - // Test full and valid data + // Test password policy checking $post['package'] = $package_kolab->id; + $post['password'] = 'password'; + $response = $this->actingAs($john)->post("/api/v4/users", $post); + $json = $response->json(); + + $response->assertStatus(422); + + $this->assertSame('error', $json['status']); + $this->assertSame("The password confirmation does not match.", $json['errors']['password'][0]); + $this->assertSame("Specified password does not comply with the policy.", $json['errors']['password'][1]); + $this->assertCount(2, $json); + + // Test password confirmation + $post['password_confirmation'] = 'password'; + $response = $this->actingAs($john)->post("/api/v4/users", $post); + $json = $response->json(); + + $response->assertStatus(422); + + $this->assertSame('error', $json['status']); + $this->assertSame("Specified password does not comply with the policy.", $json['errors']['password'][0]); + $this->assertCount(2, $json); + + // Test full and valid data + $post['password'] = 'password123'; + $post['password_confirmation'] = 'password123'; $response = $this->actingAs($john)->post("/api/v4/users", $post); $json = $response->json(); @@ -772,6 +802,7 @@ public function testUpdate(): void { $userA = $this->getTestUser('UsersControllerTest1@userscontroller.com'); + $userA->setSetting('password_policy', 'min:8,digit'); $jack = $this->getTestUser('jack@kolab.org'); $john = $this->getTestUser('john@kolab.org'); $ned = $this->getTestUser('ned@kolab.org'); @@ -800,7 +831,7 @@ $this->assertCount(3, $json); // Test some invalid data - $post = ['password' => '12345678', 'currency' => 'invalid']; + $post = ['password' => '1234567', 'currency' => 'invalid']; $response = $this->actingAs($userA)->put("/api/v4/users/{$userA->id}", $post); $response->assertStatus(422); @@ -808,13 +839,14 @@ $this->assertSame('error', $json['status']); $this->assertCount(2, $json); - $this->assertSame('The password confirmation does not match.', $json['errors']['password'][0]); - $this->assertSame('The currency must be 3 characters.', $json['errors']['currency'][0]); + $this->assertSame("The password confirmation does not match.", $json['errors']['password'][0]); + $this->assertSame("Specified password does not comply with the policy.", $json['errors']['password'][1]); + $this->assertSame("The currency must be 3 characters.", $json['errors']['currency'][0]); // Test full profile update including password $post = [ - 'password' => 'simple', - 'password_confirmation' => 'simple', + 'password' => 'simple123', + 'password_confirmation' => 'simple123', 'first_name' => 'John2', 'last_name' => 'Doe2', 'organization' => 'TestOrg', @@ -1175,6 +1207,7 @@ $this->assertTrue($result['statusInfo']['enableDomains']); $this->assertTrue($result['statusInfo']['enableWallets']); $this->assertTrue($result['statusInfo']['enableUsers']); + $this->assertTrue($result['statusInfo']['enableSettings']); // Ned is John's wallet controller $ned = $this->getTestUser('ned@kolab.org'); @@ -1196,6 +1229,7 @@ $this->assertTrue($result['statusInfo']['enableDomains']); $this->assertTrue($result['statusInfo']['enableWallets']); $this->assertTrue($result['statusInfo']['enableUsers']); + $this->assertTrue($result['statusInfo']['enableSettings']); // Test discount in a response $discount = Discount::where('code', 'TEST')->first(); @@ -1224,6 +1258,7 @@ $this->assertFalse($result['statusInfo']['enableDomains']); $this->assertFalse($result['statusInfo']['enableWallets']); $this->assertFalse($result['statusInfo']['enableUsers']); + $this->assertFalse($result['statusInfo']['enableSettings']); } /** diff --git a/src/tests/Feature/DomainTest.php b/src/tests/Feature/DomainTest.php --- a/src/tests/Feature/DomainTest.php +++ b/src/tests/Feature/DomainTest.php @@ -348,4 +348,24 @@ } ); } + + /** + * Tests for Domain::walletOwner() (from EntitleableTrait) + */ + public function testWalletOwner(): void + { + $domain = $this->getTestDomain('kolab.org'); + $john = $this->getTestUser('john@kolab.org'); + + $this->assertSame($john->id, $domain->walletOwner()->id); + + // A domain without an owner + $domain = $this->getTestDomain('gmail.com', [ + 'status' => Domain::STATUS_NEW | Domain::STATUS_SUSPENDED + | Domain::STATUS_LDAP_READY | Domain::STATUS_CONFIRMED, + 'type' => Domain::TYPE_PUBLIC, + ]); + + $this->assertSame(null, $domain->walletOwner()); + } } 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 @@ -383,18 +383,58 @@ { $john = $this->getTestUser('john@kolab.org'); $john->setSetting('greylist_enabled', null); + $john->setSetting('password_policy', null); - $this->assertSame(['greylist_enabled' => true], $john->getConfig()); + // Greylist_enabled + $this->assertSame(true, $john->getConfig()['greylist_enabled']); $result = $john->setConfig(['greylist_enabled' => false, 'unknown' => false]); - $this->assertSame(['greylist_enabled' => false], $john->getConfig()); + $this->assertSame(['unknown' => "The requested configuration parameter is not supported."], $result); + $this->assertSame(false, $john->getConfig()['greylist_enabled']); $this->assertSame('false', $john->getSetting('greylist_enabled')); $result = $john->setConfig(['greylist_enabled' => true]); - $this->assertSame(['greylist_enabled' => true], $john->getConfig()); + $this->assertSame([], $result); + $this->assertSame(true, $john->getConfig()['greylist_enabled']); $this->assertSame('true', $john->getSetting('greylist_enabled')); + + // Password_policy + $result = $john->setConfig(['password_policy' => true]); + + $this->assertSame(['password_policy' => "Specified password policy is invalid."], $result); + $this->assertSame(null, $john->getConfig()['password_policy']); + $this->assertSame(null, $john->getSetting('password_policy')); + + $result = $john->setConfig(['password_policy' => 'min:-1']); + + $this->assertSame(['password_policy' => "Specified password policy is invalid."], $result); + + $result = $john->setConfig(['password_policy' => 'min:-1']); + + $this->assertSame(['password_policy' => "Specified password policy is invalid."], $result); + + $result = $john->setConfig(['password_policy' => 'min:10,unknown']); + + $this->assertSame(['password_policy' => "Specified password policy is invalid."], $result); + + \config(['app.password_policy' => 'min:5,max:100']); + $result = $john->setConfig(['password_policy' => 'min:4,max:255']); + + $this->assertSame(['password_policy' => "Minimum password length cannot be less than 5."], $result); + + \config(['app.password_policy' => 'min:5,max:100']); + $result = $john->setConfig(['password_policy' => 'min:10,max:255']); + + $this->assertSame(['password_policy' => "Maximum password length cannot be more than 100."], $result); + + \config(['app.password_policy' => 'min:5,max:255']); + $result = $john->setConfig(['password_policy' => 'min:10,max:255']); + + $this->assertSame([], $result); + $this->assertSame('min:10,max:255', $john->getConfig()['password_policy']); + $this->assertSame('min:10,max:255', $john->getSetting('password_policy')); } /** @@ -1177,10 +1217,37 @@ } /** + * Tests for User::walletOwner() (from EntitleableTrait) + */ + public function testWalletOwner(): void + { + $jack = $this->getTestUser('jack@kolab.org'); + $john = $this->getTestUser('john@kolab.org'); + $ned = $this->getTestUser('ned@kolab.org'); + + $this->assertSame($john->id, $john->walletOwner()->id); + $this->assertSame($john->id, $jack->walletOwner()->id); + $this->assertSame($john->id, $ned->walletOwner()->id); + + // User with no entitlements + $user = $this->getTestUser('UserAccountA@UserAccount.com'); + $this->assertSame($user->id, $user->walletOwner()->id); + } + + /** * Tests for User::wallets() */ public function testWallets(): void { - $this->markTestIncomplete(); + $john = $this->getTestUser('john@kolab.org'); + $ned = $this->getTestUser('ned@kolab.org'); + + $this->assertSame(1, $john->wallets()->count()); + $this->assertCount(1, $john->wallets); + $this->assertInstanceOf(\App\Wallet::class, $john->wallets->first()); + + $this->assertSame(1, $ned->wallets()->count()); + $this->assertCount(1, $ned->wallets); + $this->assertInstanceOf(\App\Wallet::class, $ned->wallets->first()); } } diff --git a/src/tests/Unit/Rules/PasswordTest.php b/src/tests/Unit/Rules/PasswordTest.php new file mode 100644 --- /dev/null +++ b/src/tests/Unit/Rules/PasswordTest.php @@ -0,0 +1,182 @@ + 'min:5']); + $this->assertSame($error, $this->validate('abcd')); + $this->assertSame(null, $this->validate('abcde')); + + \config(['app.password_policy' => 'min:5,max:10']); + $this->assertSame($error, $this->validate('12345678901')); + $this->assertSame(null, $this->validate('1234567890')); + + \config(['app.password_policy' => 'min:5,lower']); + $this->assertSame($error, $this->validate('12345')); + $this->assertSame($error, $this->validate('AAAAA')); + $this->assertSame(null, $this->validate('12345a')); + + \config(['app.password_policy' => 'upper']); + $this->assertSame($error, $this->validate('5')); + $this->assertSame($error, $this->validate('a')); + $this->assertSame(null, $this->validate('A')); + + \config(['app.password_policy' => 'digit']); + $this->assertSame($error, $this->validate('a')); + $this->assertSame($error, $this->validate('A')); + $this->assertSame(null, $this->validate('5')); + + \config(['app.password_policy' => 'special']); + $this->assertSame($error, $this->validate('a')); + $this->assertSame($error, $this->validate('5')); + $this->assertSame(null, $this->validate('*')); + $this->assertSame(null, $this->validate('-')); + + // Test with an account policy + $user = $this->getTestUser('john@kolab.org'); + $user->setSetting('password_policy', 'min:10,upper'); + + $this->assertSame($error, $this->validate('aaa', $user)); + $this->assertSame($error, $this->validate('1234567890', $user)); + $this->assertSame(null, $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->assertSame(true, $result['min']['enabled']); + $this->assertSame(false, $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->assertSame(true, $result['max']['enabled']); + $this->assertSame(true, $result['max']['status']); + + $this->assertSame('upper', $result['upper']['label']); + $this->assertSame('Password contains an upper-case character', $result['upper']['name']); + $this->assertSame(null, $result['upper']['param']); + $this->assertSame(true, $result['upper']['enabled']); + $this->assertSame(false, $result['upper']['status']); + + $this->assertSame('lower', $result['lower']['label']); + $this->assertSame('Password contains a lower-case character', $result['lower']['name']); + $this->assertSame(null, $result['lower']['param']); + $this->assertSame(true, $result['lower']['enabled']); + $this->assertSame(true, $result['lower']['status']); + + $this->assertSame('digit', $result['digit']['label']); + $this->assertSame('Password contains a digit', $result['digit']['name']); + $this->assertSame(null, $result['digit']['param']); + $this->assertSame(true, $result['digit']['enabled']); + $this->assertSame(false, $result['digit']['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->assertSame(true, $result['min']['enabled']); + + $this->assertSame('upper', $result['upper']['label']); + $this->assertSame('Password contains an upper-case character', $result['upper']['name']); + $this->assertSame(null, $result['upper']['param']); + $this->assertSame(true, $result['upper']['enabled']); + + // Expect to see all supported policy rules + $result = $pass->rules(true); + + $this->assertCount(6, $result); + $this->assertSame('min', $result['min']['label']); + $this->assertSame('Minimum password length: 10 characters', $result['min']['name']); + $this->assertSame('10', $result['min']['param']); + $this->assertSame(true, $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->assertSame(false, $result['max']['enabled']); + + $this->assertSame('upper', $result['upper']['label']); + $this->assertSame('Password contains an upper-case character', $result['upper']['name']); + $this->assertSame(null, $result['upper']['param']); + $this->assertSame(true, $result['upper']['enabled']); + + $this->assertSame('lower', $result['lower']['label']); + $this->assertSame('Password contains a lower-case character', $result['lower']['name']); + $this->assertSame(null, $result['lower']['param']); + $this->assertSame(false, $result['lower']['enabled']); + + $this->assertSame('digit', $result['digit']['label']); + $this->assertSame('Password contains a digit', $result['digit']['name']); + $this->assertSame(null, $result['digit']['param']); + $this->assertSame(false, $result['digit']['enabled']); + + $this->assertSame('special', $result['special']['label']); + $this->assertSame('Password contains a special character', $result['special']['name']); + $this->assertSame(null, $result['digit']['param']); + $this->assertSame(false, $result['digit']['enabled']); + } + + /** + * Validates the password using Laravel Validator API + * + * @param string $password The password to validate + * @param ?\App\User $user The account owner + * + * @return ?string Validation error message on error, NULL otherwise + */ + private function validate($password, $user = 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($user)] + ); + + if ($v->fails()) { + return $v->errors()->toArray()['password'][0]; + } + + return null; + } +}