diff --git a/src/app/Console/Commands/PasswordRetentionCommand.php b/src/app/Console/Commands/PasswordRetentionCommand.php new file mode 100644 --- /dev/null +++ b/src/app/Console/Commands/PasswordRetentionCommand.php @@ -0,0 +1,84 @@ +join('user_settings', 'users.id', '=', 'user_settings.user_id') + ->withEnvTenantContext('users') + ->where('user_settings.key', 'max_password_age') + ->cursor(); + + foreach ($accounts as $account) { + // For all users in the account (get the password update date)... + $account->users(false) + ->addSelect( + DB::raw("(select value from user_settings" + . " where users.id = user_settings.user_id and user_settings.key = 'password_update'" + . ") as password_update") + ) + ->get() + ->each(function ($user) use ($account) { + // Skip incomplete or suspended users + if (!$user->isImapReady() || $user->isSuspended()) { + return; + } + + // If the password was never updated use the user creation time + if (!empty($user->password_update)) { + $lastUpdate = new Carbon($user->password_update); + } else { + $lastUpdate = $user->created_at; + } + + $nextUpdate = $lastUpdate->copy()->addMonthsWithoutOverflow($account->max_age); + $diff = Carbon::now()->diffInDays($nextUpdate, false); + + // The password already expired, do nothing + if ($diff <= 0) { + return; + } + + if ($warnedOn = $user->getSetting('password_expiration_warning')) { + $warnedOn = new Carbon($warnedOn); + } + + // The password expires in 14 days or less + if ($diff <= 14) { + // Send a warning if it wasn't sent yet or 7 days passed since the last warning. + // Which means that we send the email 14 and 7 days before the password expires. + if (empty($warnedOn) || $warnedOn->diffInDays(Carbon::now(), false) > 7) { + \App\Jobs\Password\RetentionEmailJob::dispatch($user, $nextUpdate->toDateString()); + } + } + }); + } + } +} diff --git a/src/app/Console/Kernel.php b/src/app/Console/Kernel.php --- a/src/app/Console/Kernel.php +++ b/src/app/Console/Kernel.php @@ -19,6 +19,10 @@ // This command imports countries and the current set of IPv4 and IPv6 networks allocated to countries. $schedule->command('data:import')->dailyAt('05:00'); + // This notifies users about coming password expiration + $schedule->command('password:retention')->dailyAt('06:00'); + + // These apply wallet charges $schedule->command('wallet:charge')->dailyAt('00:00'); $schedule->command('wallet:charge')->dailyAt('04:00'); $schedule->command('wallet:charge')->dailyAt('08:00'); diff --git a/src/app/Http/Controllers/API/PasswordPolicyController.php b/src/app/Http/Controllers/API/PasswordPolicyController.php --- a/src/app/Http/Controllers/API/PasswordPolicyController.php +++ b/src/app/Http/Controllers/API/PasswordPolicyController.php @@ -25,9 +25,15 @@ $policy = new Password($owner); $rules = $policy->rules(true); + // Get the account's password retention config + $config = [ + 'max_password_age' => $owner->getSetting('max_password_age'), + ]; + return response()->json([ 'list' => array_values($rules), 'count' => count($rules), + 'config' => $config, ]); } diff --git a/src/app/Jobs/Password/RetentionEmailJob.php b/src/app/Jobs/Password/RetentionEmailJob.php new file mode 100644 --- /dev/null +++ b/src/app/Jobs/Password/RetentionEmailJob.php @@ -0,0 +1,68 @@ +user = $user; + $this->expiresOn = $expiresOn; + } + + /** + * Execute the job. + * + * @return void + */ + public function handle() + { + // TODO: Should we check if the password didn't update since + // the job has been created? + + \App\Mail\Helper::sendMail( + new PasswordExpirationReminder($this->user, $this->expiresOn), + $this->user->tenant_id, + ['to' => $this->user->email] + ); + + // Remember when we sent the email notification + $this->user->setSetting('password_expiration_warning', \now()->toDateTimeString()); + } +} diff --git a/src/app/Mail/PasswordExpirationReminder.php b/src/app/Mail/PasswordExpirationReminder.php new file mode 100644 --- /dev/null +++ b/src/app/Mail/PasswordExpirationReminder.php @@ -0,0 +1,82 @@ +user = $user; + $this->expiresOn = $expiresOn; + } + + /** + * Build the message. + * + * @return $this + */ + public function build() + { + $appName = Tenant::getConfig($this->user->tenant_id, 'app.name'); + $supportUrl = Tenant::getConfig($this->user->tenant_id, 'app.support_url'); + $href = Utils::serviceUrl('profile', $this->user->tenant_id); + + $params = [ + 'site' => $appName, + 'date' => $this->expiresOn, + 'link' => sprintf('%s', $href, $href), + 'username' => $this->user->name(true), + ]; + + $this->view('emails.html.password_expiration_reminder') + ->text('emails.plain.password_expiration_reminder') + ->subject(\trans('mail.passwordexpiration-subject', $params)) + ->with($params); + + return $this; + } + + /** + * Render the mail template with fake data + * + * @param string $type Output format ('html' or 'text') + * + * @return string HTML or Plain Text output + */ + public static function fakeRender(string $type = 'html'): string + { + $user = new User([ + 'email' => 'test@' . \config('app.domain'), + ]); + + $mail = new self($user, now()->copy()->addDays(14)->toDateString()); + + return Helper::render($mail, $type); + } +} 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 @@ -296,6 +296,14 @@ */ private static function saveOldPassword(User $user, string $password): void { + // Remember the timestamp of the last password change and unset the last warning date + $user->setSettings([ + 'password_expiration_warning' => null, + // Note: We could get this from user_passwords table, but only if the policy + // enables storing of old passwords there. + '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 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 @@ -11,12 +11,13 @@ */ public function getConfig(): array { - $config = []; + $settings = $this->getSettings(['greylist_enabled', 'password_policy', 'max_password_age']); - // 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'); + $config = [ + 'greylist_enabled' => $settings['greylist_enabled'] !== 'false', + 'max_password_age' => $settings['max_password_age'], + 'password_policy' => $settings['password_policy'], + ]; return $config; } @@ -34,7 +35,9 @@ foreach ($config as $key => $value) { if ($key == 'greylist_enabled') { - $this->setSetting('greylist_enabled', $value ? 'true' : 'false'); + $this->setSetting($key, $value ? 'true' : 'false'); + } elseif ($key == 'max_password_age') { + $this->setSetting($key, intval($value) > 0 ? (int) $value : null); } elseif ($key == 'password_policy') { // Validate the syntax and make sure min and max is included if ( @@ -54,7 +57,7 @@ } } - $this->setSetting('password_policy', $value); + $this->setSetting($key, $value); } else { $errors[$key] = \trans('validation.invalid-config-parameter'); } diff --git a/src/resources/lang/en/mail.php b/src/resources/lang/en/mail.php --- a/src/resources/lang/en/mail.php +++ b/src/resources/lang/en/mail.php @@ -65,6 +65,9 @@ 'passwordreset-body3' => "You can also click the link below:", 'passwordreset-body4' => "If you did not make such a request, you can either ignore this message or get in touch with us about this incident.", + 'passwordexpiration-subject' => ":site password expires on :date", + 'passwordexpiration-body' => "Your password will expire on :date. You can change it here:", + 'paymentmandatedisabled-subject' => ":site Auto-payment Problem", 'paymentmandatedisabled-body' => "Your :site account balance is negative " . "and the configured amount for automatically topping up the balance does not cover " 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 @@ -162,6 +162,7 @@ 'general' => "General", 'lastname' => "Last Name", 'name' => "Name", + 'months' => "months", 'none' => "none", 'or' => "or", 'password' => "Password", @@ -352,6 +353,12 @@ 'new' => "New resource", ], + 'settings' => [ + 'password-policy' => "Password Policy", + 'password-retention' => "Password Retention", + 'password-max-age' => "Require a password change every", + ], + 'shf' => [ 'aliases-none' => "This shared folder has no email aliases.", 'create' => "Create folder", diff --git a/src/resources/views/emails/html/password_expiration_reminder.blade.php b/src/resources/views/emails/html/password_expiration_reminder.blade.php new file mode 100644 --- /dev/null +++ b/src/resources/views/emails/html/password_expiration_reminder.blade.php @@ -0,0 +1,16 @@ + + + + + + +

{{ __('mail.header', ['name' => $username]) }}

+ +

{{ __('mail.passwordexpiration-body', ['site' => $site, 'date' => $date]) }} + +

{!! $link !!}

+ +

{{ __('mail.footer1') }}

+

{{ __('mail.footer2', ['site' => $site]) }}

+ + diff --git a/src/resources/views/emails/plain/password_expiration_reminder.blade.php b/src/resources/views/emails/plain/password_expiration_reminder.blade.php new file mode 100644 --- /dev/null +++ b/src/resources/views/emails/plain/password_expiration_reminder.blade.php @@ -0,0 +1,9 @@ +{!! __('mail.header', ['name' => $username]) !!} + +{!! __('mail.passwordexpiration-body', ['site' => $site, 'date' => $date]) !!} + +{!! $link !!} + +-- +{!! __('mail.footer1') !!} +{!! __('mail.footer2', ['site' => $site]) !!} diff --git a/src/resources/vue/Settings.vue b/src/resources/vue/Settings.vue --- a/src/resources/vue/Settings.vue +++ b/src/resources/vue/Settings.vue @@ -8,7 +8,7 @@
- +
  • @@ -25,6 +25,22 @@
+
+ +
+
    +
  • + + + +
  • +
+
+
{{ $t('btn.submit') }}
@@ -37,6 +53,7 @@ export default { data() { return { + config: [], passwordPolicy: [] } }, @@ -52,6 +69,7 @@ if (response.data.list) { this.passwordPolicy = response.data.list + this.config = response.data.config } }) .catch(this.$root.errorHandler) @@ -76,6 +94,7 @@ submit() { this.$root.clearFormValidation($('#settings form')) + let max_password_age = $('#max_password_age:checked').length ? $('#max_password_age_value').val() : 0 let password_policy = []; $('#password_policy > li > input:checked').each((i, element) => { @@ -89,7 +108,10 @@ password_policy.push(entry) }) - let post = { password_policy: password_policy.join(',') } + let post = { + max_password_age, + password_policy: password_policy.join(','), + } axios.post('/api/v4/users/' + this.wallet.user_id + '/config', post) .then(response => { diff --git a/src/tests/Browser/SettingsTest.php b/src/tests/Browser/SettingsTest.php --- a/src/tests/Browser/SettingsTest.php +++ b/src/tests/Browser/SettingsTest.php @@ -58,6 +58,7 @@ { $john = $this->getTestUser('john@kolab.org'); $john->setSetting('password_policy', 'min:5,max:100,lower'); + $john->setSetting('max_password_age', null); $this->browse(function (Browser $browser) { $browser->click('@links .link-settings') @@ -98,10 +99,22 @@ ->click('li:nth-child(3) input[type=checkbox]') ->click('li:nth-child(4) input[type=checkbox]'); }) + ->assertSeeIn('@form .row:nth-child(2) > label', 'Password Retention') + ->with('@form #password_retention', function (Browser $browser) { + $browser->assertElementsCount('li', 1) + ->assertSeeIn('li:nth-child(1) label', 'Require a password change every') + ->assertNotChecked('li:nth-child(1) input[type=checkbox]') + ->assertSelected('li:nth-child(1) select', 3) + ->assertSelectHasOptions('li:nth-child(1) select', [3, 6, 9, 12]) + // change the policy + ->check('li:nth-child(1) input[type=checkbox]') + ->select('li:nth-child(1) select', 6); + }) ->click('button[type=submit]') ->assertToast(Toast::TYPE_SUCCESS, 'User settings updated successfully.'); }); $this->assertSame('min:11,max:120,upper', $john->getSetting('password_policy')); + $this->assertSame('6', $john->getSetting('max_password_age')); } } diff --git a/src/tests/Feature/Console/PasswordRetentionTest.php b/src/tests/Feature/Console/PasswordRetentionTest.php new file mode 100644 --- /dev/null +++ b/src/tests/Feature/Console/PasswordRetentionTest.php @@ -0,0 +1,117 @@ +deleteTestUser('user1@retention.com'); + $this->deleteTestUser('user2@retention.com'); + $keys = ['password_update', 'max_password_age', 'password_expiration_warning']; + \App\UserSetting::whereIn('key', $keys)->delete(); + } + + /** + * {@inheritDoc} + */ + public function tearDown(): void + { + $this->deleteTestUser('user1@retention.com'); + $this->deleteTestUser('user2@retention.com'); + $keys = ['password_update', 'max_password_age', 'password_expiration_warning']; + \App\UserSetting::whereIn('key', $keys)->delete(); + + parent::tearDown(); + } + + /** + * Test the command + * + * @group mollie + */ + public function testHandle(): void + { + Queue::fake(); + + // Create some sample account + $status = User::STATUS_IMAP_READY | User::STATUS_LDAP_READY; + $owner = $this->getTestUser('user1@retention.com', ['status' => $status]); + $user = $this->getTestUser('user2@retention.com', ['status' => $status]); + $package_kolab = \App\Package::withEnvTenantContext()->where('title', 'kolab')->first(); + $owner->assignPackage($package_kolab); + $owner->assignPackage($package_kolab, $user); + + $owner->created_at = now()->copy()->subMonths(3); + $owner->save(); + $user->created_at = now()->copy()->subMonths(3); + $user->save(); + + Queue::fake(); + + // Test with no policy + $code = \Artisan::call("password:retention"); + $output = trim(\Artisan::output()); + $this->assertSame(0, $code); + $this->assertSame("", $output); + + Queue::assertNothingPushed(); + + // Test with the policy, the passwords expired already + $owner->setSetting('max_password_age', '2'); + $user->setSetting('password_update', now()->copy()->subMonths(2)); + + $code = \Artisan::call("password:retention"); + $output = trim(\Artisan::output()); + $this->assertSame(0, $code); + $this->assertSame("", $output); + + Queue::assertNothingPushed(); + + // $user's password is about to expire in 14 days + $user->setSetting('password_update', now()->copy()->subMonthsWithoutOverflow(2)->addDays(14)); + // $owner's password is about to expire in 7 days + $owner->created_at = now()->copy()->subMonthsWithoutOverflow(2)->addDays(7); + $owner->save(); + + $code = \Artisan::call("password:retention"); + $output = trim(\Artisan::output()); + $this->assertSame(0, $code); + $this->assertSame("", $output); + + Queue::assertPushed(RetentionEmailJob::class, 2); + Queue::assertPushed(RetentionEmailJob::class, function ($job) use ($user) { + $job_user = TestCase::getObjectProperty($job, 'user'); + return $job_user->id === $user->id; + }); + Queue::assertPushed(RetentionEmailJob::class, function ($job) use ($owner) { + $job_user = TestCase::getObjectProperty($job, 'user'); + return $job_user->id === $owner->id; + }); + + // Test password_expiration_warning, + // $owner was already warned today and $user 8 days ago + Queue::fake(); + $owner->setSetting('password_expiration_warning', now()->toDateTimeString()); + $user->setSetting('password_expiration_warning', now()->copy()->subDays(8)->toDateTimeString()); + + $code = \Artisan::call("password:retention"); + $this->assertSame(0, $code); + + Queue::assertPushed(RetentionEmailJob::class, 1); + Queue::assertPushed(RetentionEmailJob::class, function ($job) use ($user) { + $job_user = TestCase::getObjectProperty($job, 'user'); + return $job_user->id === $user->id; + }); + } +} diff --git a/src/tests/Feature/Controller/PasswordPolicyTest.php b/src/tests/Feature/Controller/PasswordPolicyTest.php --- a/src/tests/Feature/Controller/PasswordPolicyTest.php +++ b/src/tests/Feature/Controller/PasswordPolicyTest.php @@ -77,6 +77,7 @@ $jack = $this->getTestUser('jack@kolab.org'); $john = $this->getTestUser('john@kolab.org'); $john->setSetting('password_policy', 'min:8,max:255,special'); + $john->setSetting('max_password_age', 6); // Get available policy rules $response = $this->actingAs($john)->get('/api/v4/password-policy'); @@ -84,9 +85,10 @@ $response->assertStatus(200); - $this->assertCount(2, $json); + $this->assertCount(3, $json); $this->assertSame(7, $json['count']); $this->assertCount(7, $json['list']); + $this->assertSame(['max_password_age' => '6'], $json['config']); $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']); @@ -112,9 +114,10 @@ $response->assertStatus(200); - $this->assertCount(2, $json); + $this->assertCount(3, $json); $this->assertSame(7, $json['count']); $this->assertCount(7, $json['list']); + $this->assertSame(['max_password_age' => '6'], $json['config']); $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']); 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 @@ -532,6 +532,7 @@ $john->setSetting('greylist_enabled', null); $john->setSetting('password_policy', null); + $john->setSetting('max_password_age', null); // Test unknown user id $post = ['greylist_enabled' => 1]; @@ -567,7 +568,12 @@ $this->assertNull($john->fresh()->getSetting('greylist_enabled')); // Test some valid data - $post = ['greylist_enabled' => 1, 'password_policy' => 'min:10,max:255,upper,lower,digit,special']; + $post = [ + 'greylist_enabled' => 1, + 'password_policy' => 'min:10,max:255,upper,lower,digit,special', + 'max_password_age' => 6, + ]; + $response = $this->actingAs($john)->post("/api/v4/users/{$john->id}/config", $post); $response->assertStatus(200); @@ -577,8 +583,9 @@ $this->assertSame('success', $json['status']); $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')); + $this->assertSame('true', $john->getSetting('greylist_enabled')); + $this->assertSame('min:10,max:255,upper,lower,digit,special', $john->getSetting('password_policy')); + $this->assertSame('6', $john->getSetting('max_password_age')); // Test some valid data, acting as another account controller $ned = $this->getTestUser('ned@kolab.org'); diff --git a/src/tests/Feature/Jobs/Password/RetentionEmailJobTest.php b/src/tests/Feature/Jobs/Password/RetentionEmailJobTest.php new file mode 100644 --- /dev/null +++ b/src/tests/Feature/Jobs/Password/RetentionEmailJobTest.php @@ -0,0 +1,73 @@ +deleteTestUser('PasswordRetention@UserAccount.com'); + } + + /** + * {@inheritDoc} + * + * @return void + */ + public function tearDown(): void + { + $this->deleteTestUser('PasswordRetention@UserAccount.com'); + + parent::tearDown(); + } + + /** + * Test job handle + * + * @return void + */ + public function testHandle() + { + $user = $this->getTestUser('PasswordRetention@UserAccount.com'); + $expiresOn = now()->copy()->addDays(7)->toDateString(); + + Mail::fake(); + + // Assert that no jobs were pushed... + Mail::assertNothingSent(); + + $job = new RetentionEmailJob($user, $expiresOn); + $job->handle(); + + $this->assertMatchesRegularExpression( + '/^' . now()->format('Y-m-d') . ' [0-9]{2}:[0-9]{2}:[0-9]{2}$/', + $user->getSetting('password_expiration_warning') + ); + + // Assert the email sending job was pushed once + Mail::assertSent(PasswordExpirationReminder::class, 1); + + // Assert the mail was sent to the code's email + Mail::assertSent(PasswordExpirationReminder::class, function ($mail) use ($user) { + return $mail->hasTo($user->email); + }); + + // Assert sender + Mail::assertSent(PasswordExpirationReminder::class, function ($mail) { + return $mail->hasFrom(\config('mail.from.address'), \config('mail.from.name')) + && $mail->hasReplyTo(\config('mail.reply_to.address'), \config('mail.reply_to.name')); + }); + } +} 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 @@ -375,12 +375,18 @@ ); // Update the user, test the password change + $user->setSetting('password_expiration_warning', '2020-10-10 10:10:10'); $oldPassword = $user->password; $user->password = 'test123'; $user->save(); $this->assertNotEquals($oldPassword, $user->password); $this->assertSame(0, $user->passwords()->count()); + $this->assertNull($user->getSetting('password_expiration_warning')); + $this->assertMatchesRegularExpression( + '/^' . now()->format('Y-m-d') . ' [0-9]{2}:[0-9]{2}:[0-9]{2}$/', + $user->getSetting('password_update') + ); Queue::assertPushed(\App\Jobs\User\UpdateJob::class, 1); Queue::assertPushed( @@ -457,8 +463,9 @@ $john = $this->getTestUser('john@kolab.org'); $john->setSetting('greylist_enabled', null); $john->setSetting('password_policy', null); + $john->setSetting('max_password_age', null); - // Greylist_enabled + // greylist_enabled $this->assertSame(true, $john->getConfig()['greylist_enabled']); $result = $john->setConfig(['greylist_enabled' => false, 'unknown' => false]); @@ -473,7 +480,22 @@ $this->assertSame(true, $john->getConfig()['greylist_enabled']); $this->assertSame('true', $john->getSetting('greylist_enabled')); - // Password_policy + // max_apssword_age + $this->assertSame(null, $john->getConfig()['max_password_age']); + + $result = $john->setConfig(['max_password_age' => -1]); + + $this->assertSame([], $result); + $this->assertSame(null, $john->getConfig()['max_password_age']); + $this->assertSame(null, $john->getSetting('max_password_age')); + + $result = $john->setConfig(['max_password_age' => 12]); + + $this->assertSame([], $result); + $this->assertSame('12', $john->getConfig()['max_password_age']); + $this->assertSame('12', $john->getSetting('max_password_age')); + + // password_policy $result = $john->setConfig(['password_policy' => true]); $this->assertSame(['password_policy' => "Specified password policy is invalid."], $result); diff --git a/src/tests/Unit/Mail/PasswordExpirationReminderTest.php b/src/tests/Unit/Mail/PasswordExpirationReminderTest.php new file mode 100644 --- /dev/null +++ b/src/tests/Unit/Mail/PasswordExpirationReminderTest.php @@ -0,0 +1,43 @@ + 'User Name', + ]); + + $expiresOn = now()->copy()->addDays(7)->toDateString(); + + $mail = $this->renderMail(new PasswordExpirationReminder($user, $expiresOn)); + + $html = $mail['html']; + $plain = $mail['plain']; + + $url = Utils::serviceUrl('profile'); + $link = "$url"; + $appName = \config('app.name'); + + $this->assertSame("$appName password expires on $expiresOn", $mail['subject']); + + $this->assertStringStartsWith('', $html); + $this->assertTrue(strpos($html, $link) > 0); + $this->assertTrue(strpos($html, $user->name(true)) > 0); + $this->assertTrue(strpos($html, $expiresOn) > 0); + + $this->assertStringStartsWith("Dear " . $user->name(true), $plain); + $this->assertTrue(strpos($plain, $link) > 0); + $this->assertTrue(strpos($plain, $expiresOn) > 0); + } +}