From bfd59d9e544028a1ea041806fd60e112f3a90167 Mon Sep 17 00:00:00 2001 From: Frederic Guillot Date: Wed, 18 May 2016 21:27:36 -0400 Subject: Reset failed login counter and unlock user when changing password --- ChangeLog | 1 + app/Controller/UserCredentialController.php | 109 ++++++++++++++++++ app/Controller/UserModificationController.php | 69 ++++++++++++ app/Controller/UserViewController.php | 141 +++--------------------- app/ServiceProvider/AuthenticationProvider.php | 2 +- app/ServiceProvider/RouteProvider.php | 6 +- app/Template/user_credential/authentication.php | 27 +++++ app/Template/user_credential/password.php | 23 ++++ app/Template/user_modification/show.php | 35 ++++++ app/Template/user_view/authentication.php | 27 ----- app/Template/user_view/edit.php | 35 ------ app/Template/user_view/password.php | 26 ----- app/Template/user_view/show.php | 4 + app/Template/user_view/sidebar.php | 16 +-- 14 files changed, 294 insertions(+), 227 deletions(-) create mode 100644 app/Controller/UserCredentialController.php create mode 100644 app/Controller/UserModificationController.php create mode 100644 app/Template/user_credential/authentication.php create mode 100644 app/Template/user_credential/password.php create mode 100644 app/Template/user_modification/show.php delete mode 100644 app/Template/user_view/authentication.php delete mode 100644 app/Template/user_view/edit.php delete mode 100644 app/Template/user_view/password.php diff --git a/ChangeLog b/ChangeLog index 1defcb9a..94da950e 100644 --- a/ChangeLog +++ b/ChangeLog @@ -10,6 +10,7 @@ New features: Improvements: +* Reset failed login counter and unlock user when changing password * Task do not open anymore in a new window on the Gantt chart * Do not display task progress for tasks with no start/end date * Use Gulp and Bower to manage assets diff --git a/app/Controller/UserCredentialController.php b/app/Controller/UserCredentialController.php new file mode 100644 index 00000000..3310aaa8 --- /dev/null +++ b/app/Controller/UserCredentialController.php @@ -0,0 +1,109 @@ +getUser(); + + return $this->response->html($this->helper->layout->user('user_credential/password', array( + 'values' => $values + array('id' => $user['id']), + 'errors' => $errors, + 'user' => $user, + ))); + } + + /** + * Save new password + * + * @throws \Kanboard\Core\Controller\AccessForbiddenException + * @throws \Kanboard\Core\Controller\PageNotFoundException + */ + public function savePassword() + { + $user = $this->getUser(); + $values = $this->request->getValues(); + + list($valid, $errors) = $this->userValidator->validatePasswordModification($values); + + if ($valid) { + if ($this->user->update($values)) { + $this->flash->success(t('Password modified successfully.')); + $this->userLocking->resetFailedLogin($user['username']); + } else { + $this->flash->failure(t('Unable to change the password.')); + } + + return $this->response->redirect($this->helper->url->to('UserViewController', 'show', array('user_id' => $user['id']))); + } + + return $this->changePassword($values, $errors); + } + + /** + * Display a form to edit authentication + * + * @access public + * @param array $values + * @param array $errors + * @throws \Kanboard\Core\Controller\AccessForbiddenException + * @throws \Kanboard\Core\Controller\PageNotFoundException + */ + public function changeAuthentication(array $values = array(), array $errors = array()) + { + $user = $this->getUser(); + + if (empty($values)) { + $values = $user; + unset($values['password']); + } + + return $this->response->html($this->helper->layout->user('user_credential/authentication', array( + 'values' => $values, + 'errors' => $errors, + 'user' => $user, + ))); + } + + /** + * Save authentication + * + * @throws \Kanboard\Core\Controller\AccessForbiddenException + * @throws \Kanboard\Core\Controller\PageNotFoundException + */ + public function saveAuthentication() + { + $user = $this->getUser(); + $values = $this->request->getValues() + array('disable_login_form' => 0, 'is_ldap_user' => 0); + list($valid, $errors) = $this->userValidator->validateModification($values); + + if ($valid) { + if ($this->user->update($values)) { + $this->flash->success(t('User updated successfully.')); + } else { + $this->flash->failure(t('Unable to update your user.')); + } + + return $this->response->redirect($this->helper->url->to('UserCredentialController', 'changeAuthentication', array('user_id' => $user['id']))); + } + + return $this->changeAuthentication($values, $errors); + } +} diff --git a/app/Controller/UserModificationController.php b/app/Controller/UserModificationController.php new file mode 100644 index 00000000..0a50eb5b --- /dev/null +++ b/app/Controller/UserModificationController.php @@ -0,0 +1,69 @@ +getUser(); + + if (empty($values)) { + $values = $user; + unset($values['password']); + } + + return $this->response->html($this->helper->layout->user('user_modification/show', array( + 'values' => $values, + 'errors' => $errors, + 'user' => $user, + 'timezones' => $this->timezone->getTimezones(true), + 'languages' => $this->language->getLanguages(true), + 'roles' => $this->role->getApplicationRoles(), + ))); + } + + /** + * Save user information + */ + public function save() + { + $user = $this->getUser(); + $values = $this->request->getValues(); + + if (! $this->userSession->isAdmin()) { + if (isset($values['role'])) { + unset($values['role']); + } + } + + list($valid, $errors) = $this->userValidator->validateModification($values); + + if ($valid) { + if ($this->user->update($values)) { + $this->flash->success(t('User updated successfully.')); + } else { + $this->flash->failure(t('Unable to update your user.')); + } + + return $this->response->redirect($this->helper->url->to('UserViewController', 'show', array('user_id' => $user['id']))); + } + + return $this->show($values, $errors); + } +} diff --git a/app/Controller/UserViewController.php b/app/Controller/UserViewController.php index dc03f419..b299e35b 100644 --- a/app/Controller/UserViewController.php +++ b/app/Controller/UserViewController.php @@ -29,7 +29,7 @@ class UserViewController extends BaseController $this->response->html($this->helper->layout->app('user_view/profile', array( 'title' => $user['name'] ?: $user['username'], - 'user' => $user, + 'user' => $user, ))); } @@ -42,7 +42,7 @@ class UserViewController extends BaseController { $user = $this->getUser(); $this->response->html($this->helper->layout->user('user_view/show', array( - 'user' => $user, + 'user' => $user, 'timezones' => $this->timezone->getTimezones(true), 'languages' => $this->language->getLanguages(true), ))); @@ -67,7 +67,7 @@ class UserViewController extends BaseController $this->response->html($this->helper->layout->user('user_view/timesheet', array( 'subtask_paginator' => $subtask_paginator, - 'user' => $user, + 'user' => $user, ))); } @@ -81,7 +81,7 @@ class UserViewController extends BaseController $user = $this->getUser(); $this->response->html($this->helper->layout->user('user_view/password_reset', array( 'tokens' => $this->passwordReset->getAll($user['id']), - 'user' => $user, + 'user' => $user, ))); } @@ -95,7 +95,7 @@ class UserViewController extends BaseController $user = $this->getUser(); $this->response->html($this->helper->layout->user('user_view/last', array( 'last_logins' => $this->lastLogin->getAll($user['id']), - 'user' => $user, + 'user' => $user, ))); } @@ -109,7 +109,7 @@ class UserViewController extends BaseController $user = $this->getUser(); $this->response->html($this->helper->layout->user('user_view/sessions', array( 'sessions' => $this->rememberMeSession->getAll($user['id']), - 'user' => $user, + 'user' => $user, ))); } @@ -143,11 +143,11 @@ class UserViewController extends BaseController } return $this->response->html($this->helper->layout->user('user_view/notifications', array( - 'projects' => $this->projectUserRole->getProjectsByUser($user['id'], array(ProjectModel::ACTIVE)), + 'projects' => $this->projectUserRole->getProjectsByUser($user['id'], array(ProjectModel::ACTIVE)), 'notifications' => $this->userNotification->readSettings($user['id']), - 'types' => $this->userNotificationType->getTypes(), - 'filters' => $this->userNotificationFilter->getFilters(), - 'user' => $user, + 'types' => $this->userNotificationType->getTypes(), + 'filters' => $this->userNotificationFilter->getFilters(), + 'user' => $user, ))); } @@ -168,7 +168,7 @@ class UserViewController extends BaseController } $this->response->html($this->helper->layout->user('user_view/integrations', array( - 'user' => $user, + 'user' => $user, 'values' => $this->userMetadata->getAll($user['id']), ))); } @@ -183,7 +183,7 @@ class UserViewController extends BaseController $user = $this->getUser(); $this->response->html($this->helper->layout->user('user_view/external', array( 'last_logins' => $this->lastLogin->getAll($user['id']), - 'user' => $user, + 'user' => $user, ))); } @@ -200,7 +200,7 @@ class UserViewController extends BaseController if ($switch === 'enable' || $switch === 'disable') { $this->checkCSRFParam(); - if ($this->user->{$switch.'PublicAccess'}($user['id'])) { + if ($this->user->{$switch . 'PublicAccess'}($user['id'])) { $this->flash->success(t('User updated successfully.')); } else { $this->flash->failure(t('Unable to update this user.')); @@ -210,121 +210,8 @@ class UserViewController extends BaseController } return $this->response->html($this->helper->layout->user('user_view/share', array( - 'user' => $user, + 'user' => $user, 'title' => t('Public access'), ))); } - - /** - * Password modification - * - * @access public - */ - public function password() - { - $user = $this->getUser(); - $values = array('id' => $user['id']); - $errors = array(); - - if ($this->request->isPost()) { - $values = $this->request->getValues(); - list($valid, $errors) = $this->userValidator->validatePasswordModification($values); - - if ($valid) { - if ($this->user->update($values)) { - $this->flash->success(t('Password modified successfully.')); - } else { - $this->flash->failure(t('Unable to change the password.')); - } - - return $this->response->redirect($this->helper->url->to('UserViewController', 'show', array('user_id' => $user['id']))); - } - } - - return $this->response->html($this->helper->layout->user('user_view/password', array( - 'values' => $values, - 'errors' => $errors, - 'user' => $user, - ))); - } - - /** - * Display a form to edit a user - * - * @access public - */ - public function edit() - { - $user = $this->getUser(); - $values = $user; - $errors = array(); - - unset($values['password']); - - if ($this->request->isPost()) { - $values = $this->request->getValues(); - - if (! $this->userSession->isAdmin()) { - if (isset($values['role'])) { - unset($values['role']); - } - } - - list($valid, $errors) = $this->userValidator->validateModification($values); - - if ($valid) { - if ($this->user->update($values)) { - $this->flash->success(t('User updated successfully.')); - } else { - $this->flash->failure(t('Unable to update your user.')); - } - - return $this->response->redirect($this->helper->url->to('UserViewController', 'show', array('user_id' => $user['id']))); - } - } - - return $this->response->html($this->helper->layout->user('user_view/edit', array( - 'values' => $values, - 'errors' => $errors, - 'user' => $user, - 'timezones' => $this->timezone->getTimezones(true), - 'languages' => $this->language->getLanguages(true), - 'roles' => $this->role->getApplicationRoles(), - ))); - } - - /** - * Display a form to edit authentication - * - * @access public - */ - public function authentication() - { - $user = $this->getUser(); - $values = $user; - $errors = array(); - - unset($values['password']); - - if ($this->request->isPost()) { - $values = $this->request->getValues() + array('disable_login_form' => 0, 'is_ldap_user' => 0); - list($valid, $errors) = $this->userValidator->validateModification($values); - - if ($valid) { - if ($this->user->update($values)) { - $this->flash->success(t('User updated successfully.')); - } else { - $this->flash->failure(t('Unable to update your user.')); - } - - return $this->response->redirect($this->helper->url->to('UserViewController', 'authentication', array('user_id' => $user['id']))); - } - } - - return $this->response->html($this->helper->layout->user('user_view/authentication', array( - 'values' => $values, - 'errors' => $errors, - 'user' => $user, - ))); - } } diff --git a/app/ServiceProvider/AuthenticationProvider.php b/app/ServiceProvider/AuthenticationProvider.php index 193929c7..1ac4656c 100644 --- a/app/ServiceProvider/AuthenticationProvider.php +++ b/app/ServiceProvider/AuthenticationProvider.php @@ -143,7 +143,7 @@ class AuthenticationProvider implements ServiceProviderInterface $acl->add('UserCreationController', '*', Role::APP_ADMIN); $acl->add('UserListController', '*', Role::APP_ADMIN); $acl->add('UserStatusController', '*', Role::APP_ADMIN); - $acl->add('UserViewController', array('authentication'), Role::APP_ADMIN); + $acl->add('UserCredentialController', array('changeAuthentication', 'saveAuthentication'), Role::APP_ADMIN); return $acl; } diff --git a/app/ServiceProvider/RouteProvider.php b/app/ServiceProvider/RouteProvider.php index 2bf3b6db..eb567e46 100644 --- a/app/ServiceProvider/RouteProvider.php +++ b/app/ServiceProvider/RouteProvider.php @@ -149,13 +149,13 @@ class RouteProvider implements ServiceProviderInterface $container['route']->addRoute('user/show/:user_id/timesheet', 'UserViewController', 'timesheet'); $container['route']->addRoute('user/show/:user_id/last-logins', 'UserViewController', 'lastLogin'); $container['route']->addRoute('user/show/:user_id/sessions', 'UserViewController', 'sessions'); - $container['route']->addRoute('user/:user_id/edit', 'UserViewController', 'edit'); - $container['route']->addRoute('user/:user_id/password', 'UserViewController', 'password'); + $container['route']->addRoute('user/:user_id/edit', 'UserModificationController', 'show'); + $container['route']->addRoute('user/:user_id/password', 'UserCredentialController', 'changePassword'); $container['route']->addRoute('user/:user_id/share', 'UserViewController', 'share'); $container['route']->addRoute('user/:user_id/notifications', 'UserViewController', 'notifications'); $container['route']->addRoute('user/:user_id/accounts', 'UserViewController', 'external'); $container['route']->addRoute('user/:user_id/integrations', 'UserViewController', 'integrations'); - $container['route']->addRoute('user/:user_id/authentication', 'UserViewController', 'authentication'); + $container['route']->addRoute('user/:user_id/authentication', 'UserCredentialController', 'changeAuthentication'); $container['route']->addRoute('user/:user_id/2fa', 'twofactor', 'index'); $container['route']->addRoute('user/:user_id/avatar', 'AvatarFile', 'show'); diff --git a/app/Template/user_credential/authentication.php b/app/Template/user_credential/authentication.php new file mode 100644 index 00000000..fbe2e915 --- /dev/null +++ b/app/Template/user_credential/authentication.php @@ -0,0 +1,27 @@ +
+

+
+
+ form->csrf() ?> + + form->hidden('id', $values) ?> + form->hidden('username', $values) ?> + + hook->render('template:user:authentication:form', array('values' => $values, 'errors' => $errors, 'user' => $user)) ?> + + form->checkbox('is_ldap_user', t('Remote user'), 1, isset($values['is_ldap_user']) && $values['is_ldap_user'] == 1) ?> + form->checkbox('disable_login_form', t('Disallow login form'), 1, isset($values['disable_login_form']) && $values['disable_login_form'] == 1) ?> + +
+ + + url->link(t('cancel'), 'UserViewController', 'show', array('user_id' => $user['id'])) ?> +
+ +
+
    +
    • +
    • +
+
+
diff --git a/app/Template/user_credential/password.php b/app/Template/user_credential/password.php new file mode 100644 index 00000000..5a6e4403 --- /dev/null +++ b/app/Template/user_credential/password.php @@ -0,0 +1,23 @@ +
+

+
+ +
+ form->hidden('id', $values) ?> + form->csrf() ?> + + form->label(t('Current password for the user "%s"', $this->user->getFullname()), 'current_password') ?> + form->password('current_password', $values, $errors) ?> + + form->label(t('New password for the user "%s"', $this->user->getFullname($user)), 'password') ?> + form->password('password', $values, $errors) ?> + + form->label(t('Confirmation'), 'confirmation') ?> + form->password('confirmation', $values, $errors) ?> + +
+ + + url->link(t('cancel'), 'UserViewController', 'show', array('user_id' => $user['id'])) ?> +
+
diff --git a/app/Template/user_modification/show.php b/app/Template/user_modification/show.php new file mode 100644 index 00000000..396d550d --- /dev/null +++ b/app/Template/user_modification/show.php @@ -0,0 +1,35 @@ +
+

+
+
+ + form->csrf() ?> + + form->hidden('id', $values) ?> + + form->label(t('Username'), 'username') ?> + form->text('username', $values, $errors, array('required', isset($values['is_ldap_user']) && $values['is_ldap_user'] == 1 ? 'readonly' : '', 'maxlength="50"')) ?> + + form->label(t('Name'), 'name') ?> + form->text('name', $values, $errors) ?> + + form->label(t('Email'), 'email') ?> + form->email('email', $values, $errors) ?> + + form->label(t('Timezone'), 'timezone') ?> + form->select('timezone', $timezones, $values, $errors) ?> + + form->label(t('Language'), 'language') ?> + form->select('language', $languages, $values, $errors) ?> + + user->isAdmin()): ?> + form->label(t('Role'), 'role') ?> + form->select('role', $roles, $values, $errors) ?> + + +
+ + + url->link(t('cancel'), 'UserViewController', 'show', array('user_id' => $user['id'])) ?> +
+
diff --git a/app/Template/user_view/authentication.php b/app/Template/user_view/authentication.php deleted file mode 100644 index 44643388..00000000 --- a/app/Template/user_view/authentication.php +++ /dev/null @@ -1,27 +0,0 @@ -
-

-
-
- form->csrf() ?> - - form->hidden('id', $values) ?> - form->hidden('username', $values) ?> - - hook->render('template:user:authentication:form', array('values' => $values, 'errors' => $errors, 'user' => $user)) ?> - - form->checkbox('is_ldap_user', t('Remote user'), 1, isset($values['is_ldap_user']) && $values['is_ldap_user'] == 1) ?> - form->checkbox('disable_login_form', t('Disallow login form'), 1, isset($values['disable_login_form']) && $values['disable_login_form'] == 1) ?> - -
- - - url->link(t('cancel'), 'UserViewController', 'show', array('user_id' => $user['id'])) ?> -
- -
-
    -
    • -
    • -
-
-
diff --git a/app/Template/user_view/edit.php b/app/Template/user_view/edit.php deleted file mode 100644 index 18947905..00000000 --- a/app/Template/user_view/edit.php +++ /dev/null @@ -1,35 +0,0 @@ -
-

-
-
- - form->csrf() ?> - - form->hidden('id', $values) ?> - - form->label(t('Username'), 'username') ?> - form->text('username', $values, $errors, array('required', isset($values['is_ldap_user']) && $values['is_ldap_user'] == 1 ? 'readonly' : '', 'maxlength="50"')) ?> - - form->label(t('Name'), 'name') ?> - form->text('name', $values, $errors) ?> - - form->label(t('Email'), 'email') ?> - form->email('email', $values, $errors) ?> - - form->label(t('Timezone'), 'timezone') ?> - form->select('timezone', $timezones, $values, $errors) ?> - - form->label(t('Language'), 'language') ?> - form->select('language', $languages, $values, $errors) ?> - - user->isAdmin()): ?> - form->label(t('Role'), 'role') ?> - form->select('role', $roles, $values, $errors) ?> - - -
- - - url->link(t('cancel'), 'UserViewController', 'show', array('user_id' => $user['id'])) ?> -
-
diff --git a/app/Template/user_view/password.php b/app/Template/user_view/password.php deleted file mode 100644 index 32ff9d5c..00000000 --- a/app/Template/user_view/password.php +++ /dev/null @@ -1,26 +0,0 @@ -
-

-
- -
- - form->hidden('id', $values) ?> - form->csrf() ?> - -
- form->label(t('Current password for the user "%s"', $this->user->getFullname()), 'current_password') ?> - form->password('current_password', $values, $errors) ?> -
- - form->label(t('New password for the user "%s"', $this->user->getFullname($user)), 'password') ?> - form->password('password', $values, $errors) ?> - - form->label(t('Confirmation'), 'confirmation') ?> - form->password('confirmation', $values, $errors) ?> - -
- - - url->link(t('cancel'), 'UserViewController', 'show', array('user_id' => $user['id'])) ?> -
-
diff --git a/app/Template/user_view/show.php b/app/Template/user_view/show.php index df0affb8..390a1e45 100644 --- a/app/Template/user_view/show.php +++ b/app/Template/user_view/show.php @@ -15,6 +15,10 @@
  • user->getRoleName($user['role']) ?>
    • +
    • + +
  • dt->datetime($user['lock_expiration_date']) ?>
    • +
    diff --git a/app/Template/user_view/sidebar.php b/app/Template/user_view/sidebar.php index 9a25df16..5eb0457b 100644 --- a/app/Template/user_view/sidebar.php +++ b/app/Template/user_view/sidebar.php @@ -33,9 +33,9 @@