review changes

includes/controller/users_controller.php

@@ -75,7 +75,7 @@ function user_delete_controller()
             && $auth->verifyPassword($user, $request->postData('password'))
         )) {
             $valid = false;
-            error(__('Your password is incorrect. Please try it again.'));
+            error(__('auth.password.error'));
         }
 
         if ($valid) {

includes/sys_form.php

@@ -321,8 +321,9 @@ function form_password($name, $label, $disabled = false)
     return form_element(
         $label,
         sprintf(
-            '<input class="form-control" id="form_%1$s" type="password" name="%1$s" value=""%s/>',
+            '<input class="form-control" id="form_%1$s" type="password" name="%1$s" minlength="%2$s" value=""%3$s/>',
             $name,
+            config('min_password_length'),
             $disabled
         ),
         'form_' . $name

includes/view/User_view.php

@@ -7,6 +7,7 @@ use Engelsystem\Models\Worklog;
 use Engelsystem\Renderer\Renderer;
 use Illuminate\Support\Collection;
 use Illuminate\Support\Str;
+use Engelsystem\Controllers\SettingsController;
 
 /**
  * Renders user settings page
@@ -44,6 +45,7 @@ function User_settings_view(
         'pages/settings/settings.twig',
         [
             'title' => 'settings.profile',
+            'settings_menu' => app()->make(SettingsController::class)->settingsMenu(),
             'content' =>
                 msg()
                 . div('row', [

resources/lang/de_DE/additional.po

@@ -27,7 +27,10 @@ msgid "validation.email.email"
 msgstr "Die E-Mail-Adresse ist nicht gültig."
 
 msgid "validation.password.min"
-msgstr "Dein angegebenes Passwort ist zu kurz."
+msgstr "Dein neues Passwort ist zu kurz."
+
+msgid "validation.new_password.min"
+msgstr "Dein neues Passwort ist zu kurz."
 
 msgid "validation.password.confirmed"
 msgstr "Deine Passwörter stimmen nicht überein."

resources/lang/de_DE/default.po

@@ -775,7 +775,7 @@ msgid "You cannot delete yourself."
 msgstr "Du kannst Dich nicht selber löschen."
 
 #: includes/controller/users_controller.php:76
-msgid "Your password is incorrect. Please try it again."
+msgid "auth.password.error"
 msgstr "Dein Passwort stimmt nicht. Bitte probiere es nochmal."
 
 #: includes/controller/users_controller.php:85
@@ -1578,10 +1578,6 @@ msgstr "Bitte gib Deine E-Mail-Adresse ein."
 msgid "Please select your shirt size."
 msgstr "Bitte wähle Deine T-Shirt Größe."
 
-#: includes/pages/guest_login.php:124 includes/pages/user_settings.php:117
-msgid "Your passwords don't match."
-msgstr "Deine Passwörter stimmen nicht überein."
-
 #: includes/pages/guest_login.php:129
 #, php-format
 msgid "Your password is too short (please use at least %s characters)."
@@ -2491,22 +2487,6 @@ msgstr "Geplanter Abreisetag"
 msgid "Please visit the angeltypes page to manage your angeltypes."
 msgstr "Bitte benutze die Engeltypen-Seite um deine Engeltypen zu verwalten."
 
-#: includes/view/User_view.php:89
-msgid "Here you can change your password."
-msgstr "Hier kannst Du Dein Passwort ändern."
-
-#: includes/view/User_view.php:90
-msgid "Old password:"
-msgstr "Altes Passwort:"
-
-#: includes/view/User_view.php:91
-msgid "New password:"
-msgstr "Neues Passwort:"
-
-#: includes/view/User_view.php:92
-msgid "Password confirmation:"
-msgstr "Passwort wiederholen:"
-
 #: includes/view/User_view.php:96
 msgid "Here you can choose your color settings:"
 msgstr "Hier kannst Du das Aussehen auswählen:"
@@ -2896,6 +2876,9 @@ msgstr "Treffen"
 msgid "news.edit.message"
 msgstr "Nachricht"
 
+msgid "news.preview"
+msgstr "Vorschau"
+
 msgid "form.search"
 msgstr "Suchen"
 
@@ -2918,8 +2901,23 @@ msgstr "Einstellungen"
 msgid "settings.password"
 msgstr "Passwort"
 
+msgid "settings.password.info"
+msgstr "Hier kannst Du Dein Passwort ändern."
+
+msgid "settings.password.password"
+msgstr "Altes Passwort"
+
+msgid "settings.password.new_password"
+msgstr "Neues Passwort"
+
+msgid "settings.password.new_password2"
+msgstr "Passwort wiederholen"
+
+msgid "settings.password.success"
+msgstr "Passwort wurde erfolgreich geändert."
+
 msgid "settings.oauth"
-msgstr "Single-Sign-On"
+msgstr "Single Sign-On"
 
 msgid "settings.oauth.identity-provider"
 msgstr "Login-Dienst"

resources/lang/en_US/additional.po

@@ -25,7 +25,10 @@ msgid "validation.email.email"
 msgstr "This email address is not valid."
 
 msgid "validation.password.min"
-msgstr "Your password is too short."
+msgstr "Your new password is too short."
+
+msgid "validation.new_password.min"
+msgstr "Your new password is too short."
 
 msgid "validation.password.confirmed"
 msgstr "Your passwords are not equal."

resources/lang/en_US/default.po

@@ -21,6 +21,8 @@ msgstr ""
 
 #~ msgid "auth.no-password"
 #~ msgstr "Please enter a password."
+msgid "auth.password.error"
+msgstr "Your password is incorrect. Please try it again."
 
 msgid "form.submit"
 msgstr "Submit"
@@ -163,6 +165,9 @@ msgstr "Meeting"
 msgid "news.edit.message"
 msgstr "Message"
 
+msgid "news.preview"
+msgstr "Preview"
+
 msgid "form.search"
 msgstr "Search"
 
@@ -184,8 +189,23 @@ msgstr "Settings"
 msgid "settings.password"
 msgstr "Password"
 
+msgid "settings.password.info"
+msgstr "Here you can change your password."
+
+msgid "settings.password.password"
+msgstr "Old password"
+
+msgid "settings.password.new_password"
+msgstr "New passwort"
+
+msgid "settings.password.new_password2"
+msgstr "Password confirmation"
+
+msgid "settings.password.success"
+msgstr "Password was changed successfully."
+
 msgid "settings.oauth"
-msgstr "Single-Sign-On"
+msgstr "Single Sign-On"
 
 msgid "settings.oauth.identity-provider"
 msgstr "Identity provider"

resources/views/macros/base.twig

@@ -23,3 +23,7 @@
         {{ label }}
     </a>
 {% endmacro %}
+
+{% macro info(text) %}
+    <span class="help-block">{{ _self.glyphicon('info-sign') }}{{ text }}</span>
+{%- endmacro %}

resources/views/macros/form.twig

@@ -7,6 +7,7 @@
             type="{{ type|default('text') }}" class="form-control"
             id="{{ name }}" name="{{ name }}"
             value="{{ opt.value|default('')|escape('html_attr') }}"
+            {%- if opt.min is defined %} minlength="{{ opt.min }}"{% endif %}
             {%- if opt.required|default(false) %}
                 required
             {%- endif -%}
@@ -77,8 +78,3 @@
 {% macro submit(label, opt) %}
     {{ _self.button(label|default(__('form.submit')), opt|default({})|merge({'type': 'submit'})) }}
 {%- endmacro %}
-
-{% macro info(text) %}
-{% import 'macros/base.twig' as m %}
-    <span class="help-block">{{ m.glyphicon('info-sign') }}{{ text }}</span>
-{%- endmacro %}

resources/views/pages/news/edit.twig

@@ -62,7 +62,7 @@
             {% if news %}
                 <div class="row">
                     <div class="col-md-12">
-                        <h2>{{ __('Preview') }}</h2>
+                        <h2>{{ __('news.preview') }}</h2>
 
                         <div class="panel {% if not news.is_meeting %}panel-default{% else %}panel-info{% endif %}">
                             <div class="panel-heading">

resources/views/pages/password/reset-form.twig

@@ -7,8 +7,8 @@
         <form action="" enctype="multipart/form-data" method="post">
             {{ csrf() }}
 
-            {{ f.input('password', __('Password'), 'password', {'required': true}) }}
+            {{ f.input('password', __('Password'), 'password', {'min': min_length, 'required': true}) }}
-            {{ f.input('password_confirmation', __('Confirm password'), 'password', {'required': true}) }}
+            {{ f.input('password_confirmation', __('Confirm password'), 'password', {'min': min_length, 'required': true}) }}
 
             <div class="form-group">
                 {{ f.submit(__('Save')) }}

resources/views/pages/settings/password.twig

@@ -10,24 +10,24 @@
 
             <div class="row">
                 <div class="col-md-12">
-                    {{ f.info(__('Here you can change your password.')) }}
+                    {{ m.info(__('settings.password.info')) }}
                     {{ f.input(
                         'password',
-                        __('Old password:'),
+                        __('settings.password.password'),
                         'password',
                         {'required': true}
                     ) }}
                     {{ f.input(
                         'new_password',
-                        __('New password:'),
+                        __('settings.password.new_password'),
                         'password',
-                        {'required': true}
+                        {'min': min_length, 'required': true}
                     ) }}
                     {{ f.input(
                         'new_password2',
-                        __('Password confirmation:'),
+                        __('settings.password.new_password2'),
                         'password',
-                        {'required': true}
+                        {'min': min_length, 'required': true}
                     ) }}
                     {{ f.submit() }}
                 </div>

resources/views/pages/settings/settings.twig

@@ -12,13 +12,9 @@
         <div class="row">
             <div class="col-md-3 settings-menu">
                 <ul class="nav nav-pills nav-stacked">
-                    {% for url,title in {
+                    {% for url,title in settings_menu %}
-                        (url('/user-settings')): __('settings.profile'),
-                        (url('/settings/password')): __('settings.password'),
-                        (url('/settings/oauth')): __('settings.oauth'),
-                    } %}
                         <li{% if url == request.url() %} class="active"{% endif %}>
-                            <a href="{{ url }}">{{ title }}</a>
+                            <a href="{{ url }}">{{ __(title) }}</a>
                         </li>
                     {% endfor %}
                 </ul>

src/Controllers/PasswordResetController.php

@@ -103,7 +103,10 @@ class PasswordResetController extends BaseController
     {
         $this->requireToken($request);
 
-        return $this->showView('pages/password/reset-form');
+        return $this->showView(
+            'pages/password/reset-form',
+            ['min_length' => config('min_password_length')]
+        );
     }
 
     /**

src/Controllers/SettingsController.php

@@ -45,11 +45,11 @@ class SettingsController extends BaseController
         Redirector $redirector,
         Response $response
     ) {
-            $this->auth = $auth;
+        $this->auth = $auth;
-            $this->config = $config;
+        $this->config = $config;
-            $this->log = $log;
+        $this->log = $log;
-            $this->redirect = $redirector;
+        $this->redirect = $redirector;
-            $this->response = $response;
+        $this->response = $response;
     }
 
     /**
@@ -58,31 +58,38 @@ class SettingsController extends BaseController
     public function password(): Response
     {
         return $this->response->withView(
-            'pages/settings/password.twig',
+            'pages/settings/password',
-            $this->getNotifications()
+            [
+                'settings_menu' => $this->settingsMenu(),
+                'min_length' => config('min_password_length')
+
+            ] + $this->getNotifications()
         );
     }
 
     /**
+     * @param Request $request
      * @return Response
      */
     public function savePassword(Request $request): Response
     {
         $user = $this->auth->user();
 
-        if (
+        $minLength = config('min_password_length');
-            !$request->has('password')
+        $data = $this->validate($request, [
-            || !$this->auth->verifyPassword($user, $request->postData('password'))
+            'password' => 'required',
-        ) {
+            'new_password' => 'required|min:' . $minLength,
-            $this->addNotification('-> not OK. Please try again.', 'errors');
+            'new_password2' => 'required'
-        } elseif (strlen($request->postData('new_password')) < config('min_password_length')) {
+        ]);
-            $this->addNotification('Your password is to short (please use at least 6 characters).', 'errors');
-        } elseif ($request->postData('new_password') != $request->postData('new_password2')) {
-            $this->addNotification('Your passwords don\'t match.', 'errors');
-        } else {
-            $this->auth->setPassword($user, $request->postData('new_password'));
 
-            $this->addNotification('Password saved.');
+        if (!$this->auth->verifyPassword($user, $data['password'])) {
+            $this->addNotification('auth.password.error', 'errors');
+        } elseif ($data['new_password'] != $data['new_password2']) {
+            $this->addNotification('validation.password.confirmed', 'errors');
+        } else {
+            $this->auth->setPassword($user, $data['new_password']);
+
+            $this->addNotification('settings.password.success');
             $this->log->info('User set new password.');
         }
 
@@ -100,10 +107,27 @@ class SettingsController extends BaseController
         }
 
         return $this->response->withView(
-            'pages/settings/oauth.twig',
+            'pages/settings/oauth',
             [
+                'settings_menu' => $this->settingsMenu(),
                 'providers' => $providers,
             ] + $this->getNotifications(),
         );
     }
+
+    /**
+     * @return array
+     */
+    public function settingsMenu(): array
+    {
+        $menu = [
+            url('/user-settings') => 'settings.profile',
+            url('/settings/password') => 'settings.password'
+        ];
+        if (!empty(config('oauth'))) {
+            $menu[url('/settings/oauth')] = 'settings.oauth';
+        }
+
+        return $menu;
+    }
 }

tests/Unit/Controllers/PasswordResetControllerTest.php

@@ -104,6 +104,7 @@ class PasswordResetControllerTest extends TestCase
     {
         $this->initDatabase();
 
+        $this->app->instance('config', new Config(['min_password_length' => 3]));
         $user = $this->createUser();
         $token = $this->createToken($user);
         $request = new Request([], [], ['token' => $token->token]);

tests/Unit/Controllers/SettingsControllerTest.php

@@ -19,6 +19,8 @@ use Psr\Log\Test\TestLogger;
 use Engelsystem\Http\UrlGeneratorInterface;
 use Engelsystem\Http\UrlGenerator;
 use Engelsystem\Models\User\User;
+use Engelsystem\Http\Validation\Validator;
+use Engelsystem\Http\Exceptions\ValidationException;
 
 class SettingsControllerTest extends TestCase
 {
@@ -51,7 +53,7 @@ class SettingsControllerTest extends TestCase
         $this->response->expects($this->once())
         ->method('withView')
         ->willReturnCallback(function ($view, $data) {
-            $this->assertEquals('pages/settings/password.twig', $view);
+            $this->assertEquals('pages/settings/password', $view);
 
             return $this->response;
         });
@@ -73,26 +75,20 @@ class SettingsControllerTest extends TestCase
         ];
         $this->request = $this->request->withParsedBody($body);
 
-        $this->auth->expects($this->once())
+        $this->setExpects($this->auth, 'user', null, $this->user, $this->once());
-        ->method('user')
+        $this->setExpects($this->auth, 'verifyPassword', [$this->user, 'password'], true, $this->once());
-        ->willReturn($this->user);
+        $this->setExpects($this->auth, 'setPassword', [$this->user, 'newpassword'], null, $this->once());
-
+        $this->setExpects(
-        $this->auth->expects($this->once())
+            $this->response,
-        ->method('verifyPassword')
+            'redirectTo',
-        ->with($this->user, 'password')
+            ['http://localhost/settings/password'],
-        ->willReturn(true);
+            $this->response,
-
+            $this->once()
-        $this->auth->expects($this->once())
+        );
-        ->method('setPassword')
-        ->with($this->user, 'newpassword');
-
-        $this->response->expects($this->once())
-        ->method('redirectTo')
-        ->with('http://localhost/settings/password')
-        ->willReturn($this->response);
 
         /** @var SettingsController $controller */
         $controller = $this->app->make(SettingsController::class);
+        $controller->setValidator(new Validator());
         $controller->savePassword($this->request);
 
         $this->assertTrue($this->log->hasInfoThatContains('User set new password.'));
@@ -100,7 +96,7 @@ class SettingsControllerTest extends TestCase
         /** @var Session $session */
         $session = $this->app->get('session');
         $messages = $session->get('messages');
-        $this->assertEquals('Password saved.', $messages[0]);
+        $this->assertEquals('settings.password.success', $messages[0]);
     }
 
     /**
@@ -115,31 +111,26 @@ class SettingsControllerTest extends TestCase
         ];
         $this->request = $this->request->withParsedBody($body);
 
-        $this->auth->expects($this->once())
+        $this->setExpects($this->auth, 'user', null, $this->user, $this->once());
-        ->method('user')
+        $this->setExpects($this->auth, 'verifyPassword', [$this->user, 'wrongpassword'], false, $this->once());
-        ->willReturn($this->user);
+        $this->setExpects($this->auth, 'setPassword', null, null, $this->never());
-
+        $this->setExpects(
-        $this->auth->expects($this->once())
+            $this->response,
-        ->method('verifyPassword')
+            'redirectTo',
-        ->with($this->user, 'wrongpassword')
+            ['http://localhost/settings/password'],
-        ->willReturn(false);
+            $this->response,
-
+            $this->once()
-        $this->auth->expects($this->never())
+        );
-        ->method('setPassword');
-
-        $this->response->expects($this->once())
-        ->method('redirectTo')
-        ->with('http://localhost/settings/password')
-        ->willReturn($this->response);
 
         /** @var SettingsController $controller */
         $controller = $this->app->make(SettingsController::class);
+        $controller->setValidator(new Validator());
         $controller->savePassword($this->request);
 
         /** @var Session $session */
         $session = $this->app->get('session');
         $errors = $session->get('errors');
-        $this->assertEquals('-> not OK. Please try again.', $errors[0]);
+        $this->assertEquals('auth.password.error', $errors[0]);
     }
 
     /**
@@ -154,70 +145,69 @@ class SettingsControllerTest extends TestCase
         ];
         $this->request = $this->request->withParsedBody($body);
 
-        $this->auth->expects($this->once())
+        $this->setExpects($this->auth, 'user', null, $this->user, $this->once());
-        ->method('user')
+        $this->setExpects($this->auth, 'verifyPassword', [$this->user, 'password'], true, $this->once());
-        ->willReturn($this->user);
+        $this->setExpects($this->auth, 'setPassword', null, null, $this->never());
-
+        $this->setExpects(
-        $this->auth->expects($this->once())
+            $this->response,
-        ->method('verifyPassword')
+            'redirectTo',
-        ->with($this->user, 'password')
+            ['http://localhost/settings/password'],
-        ->willReturn(true);
+            $this->response,
-
+            $this->once()
-        $this->auth->expects($this->never())
+        );
-        ->method('setPassword');
-
-        $this->response->expects($this->once())
-        ->method('redirectTo')
-        ->with('http://localhost/settings/password')
-        ->willReturn($this->response);
 
         /** @var SettingsController $controller */
         $controller = $this->app->make(SettingsController::class);
+        $controller->setValidator(new Validator());
         $controller->savePassword($this->request);
 
         /** @var Session $session */
         $session = $this->app->get('session');
         $errors = $session->get('errors');
-        $this->assertEquals('Your passwords don\'t match.', $errors[0]);
+        $this->assertEquals('validation.password.confirmed', $errors[0]);
+    }
+
+    /**
+     * @return array
+     */
+    public function savePasswordValidationProvider(): array
+    {
+        return [
+            [null, 'newpassword', 'newpassword'],
+            ['password', null, 'newpassword'],
+            ['password', 'newpassword', null],
+            ['password', 'short', 'short']
+        ];
     }
 
     /**
      * @covers \Engelsystem\Controllers\SettingsController::savePassword
+     * @dataProvider savePasswordValidationProvider
+     * @param string $password
+     * @param string $new_password
+     * @param string $new_password2
      */
-    public function testSavePasswordInvalidNewPassword()
+    public function testSavePasswordValidation(
-    {
+        ?string $password,
+        ?string $newPassword,
+        ?string $newPassword2
+    ) {
         $body = [
-            'password' => 'password',
+            'password' => $password,
-            'new_password' => 'short',
+            'new_password' => $newPassword,
-            'new_password2' => 'short'
+            'new_password2' => $newPassword2
         ];
         $this->request = $this->request->withParsedBody($body);
 
-        $this->auth->expects($this->once())
+        $this->setExpects($this->auth, 'user', null, $this->user, $this->once());
-        ->method('user')
+        $this->setExpects($this->auth, 'setPassword', null, null, $this->never());
-        ->willReturn($this->user);
 
-        $this->auth->expects($this->once())
+        $this->expectException(ValidationException::class);
-        ->method('verifyPassword')
-        ->with($this->user, 'password')
-        ->willReturn(true);
-
-        $this->auth->expects($this->never())
-        ->method('setPassword');
-
-        $this->response->expects($this->once())
-        ->method('redirectTo')
-        ->with('http://localhost/settings/password')
-        ->willReturn($this->response);
 
         /** @var SettingsController $controller */
         $controller = $this->app->make(SettingsController::class);
+        $controller->setValidator(new Validator());
         $controller->savePassword($this->request);
-
-        /** @var Session $session */
-        $session = $this->app->get('session');
-        $errors = $session->get('errors');
-        $this->assertEquals('Your password is to short (please use at least 6 characters).', $errors[0]);
     }
 
     /**
@@ -231,7 +221,7 @@ class SettingsControllerTest extends TestCase
         $this->response->expects($this->once())
             ->method('withView')
             ->willReturnCallback(function ($view, $data) use ($providers) {
-                $this->assertEquals('pages/settings/oauth.twig', $view);
+                $this->assertEquals('pages/settings/oauth', $view);
                 $this->assertArrayHasKey('information', $data);
                 $this->assertArrayHasKey('providers', $data);
                 $this->assertEquals($providers, $data['providers']);
@@ -258,6 +248,40 @@ class SettingsControllerTest extends TestCase
         $controller->oauth();
     }
 
+    /**
+     * @covers \Engelsystem\Controllers\SettingsController::settingsMenu
+     */
+    public function testSettingsMenuWithOAuth()
+    {
+        $providers = ['foo' => ['lorem' => 'ipsum']];
+        config(['oauth' => $providers]);
+
+        /** @var SettingsController $controller */
+        $controller = $this->app->make(SettingsController::class);
+
+        $this->assertEquals([
+            'http://localhost/user-settings' => 'settings.profile',
+            'http://localhost/settings/password' => 'settings.password',
+            'http://localhost/settings/oauth' => 'settings.oauth'
+        ], $controller->settingsMenu());
+    }
+
+    /**
+     * @covers \Engelsystem\Controllers\SettingsController::settingsMenu
+     */
+    public function testSettingsMenuWithoutOAuth()
+    {
+        config(['oauth' => []]);
+
+        /** @var SettingsController $controller */
+        $controller = $this->app->make(SettingsController::class);
+
+        $this->assertEquals([
+            'http://localhost/user-settings' => 'settings.profile',
+            'http://localhost/settings/password' => 'settings.password'
+        ], $controller->settingsMenu());
+    }
+
     /**
      * Setup environment
      */
@@ -279,6 +303,7 @@ class SettingsControllerTest extends TestCase
         $this->app->instance(Response::class, $this->response);
 
         $this->app->bind(UrlGeneratorInterface::class, UrlGenerator::class);
+        $this->app->bind('http.urlGenerator', UrlGenerator::class);
 
         $this->log = new TestLogger();
         $this->app->instance(LoggerInterface::class, $this->log);