From 22ea03847ab8251a2f068b801599043014834e98 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Mon, 1 Feb 2010 21:27:01 -0800 Subject: Localize validation errors. --- modules/user/controllers/admin_users.php | 39 ++++++++++++++++++++++---------- modules/user/controllers/users.php | 1 + 2 files changed, 28 insertions(+), 12 deletions(-) (limited to 'modules/user/controllers') diff --git a/modules/user/controllers/admin_users.php b/modules/user/controllers/admin_users.php index 03d9858b..48847433 100644 --- a/modules/user/controllers/admin_users.php +++ b/modules/user/controllers/admin_users.php @@ -287,16 +287,22 @@ class Admin_Users_Controller extends Admin_Controller { $form = new Forge( "admin/users/edit_user/$user->id", "", "post", array("id" => "g-edit-user-form")); $group = $form->group("edit_user")->label(t("Edit user")); - $group->input("name")->label(t("Username"))->id("g-username")->value($user->name); - $group->inputs["name"]->error_messages( - "conflict", t("There is already a user with that username")); - $group->input("full_name")->label(t("Full name"))->id("g-fullname")->value($user->full_name); - self::_add_locale_dropdown($group, $user); - $group->password("password")->label(t("Password"))->id("g-password"); + $group->input("name")->label(t("Username"))->id("g-username")->value($user->name) + ->error_messages("conflict", t("There is already a user with that username")); + $group->input("full_name")->label(t("Full name"))->id("g-fullname")->value($user->full_name) + ->error_messages("length", t("This name is too long")); + $group->password("password")->label(t("Password"))->id("g-password") + ->error_messages("min_length", t("This password is too short")); $group->password("password2")->label(t("Confirm password"))->id("g-password2") + ->error_messages("matches", t("The passwords you entered do not match")) ->matches($group->password); - $group->input("email")->label(t("Email"))->id("g-email")->value($user->email); - $group->input("url")->label(t("URL"))->id("g-url")->value($user->url); + $group->input("email")->label(t("Email"))->id("g-email")->value($user->email) + ->error_messages("required", t("You must enter a valid email address")) + ->error_messages("length", t("This email address is too long")) + ->error_messages("email", t("You must enter a valid email address")); + $group->input("url")->label(t("URL"))->id("g-url")->value($user->url) + ->error_messages("url", t("You must enter a valid URL")); + self::_add_locale_dropdown($group, $user); $group->checkbox("admin")->label(t("Admin"))->id("g-admin")->checked($user->admin); module::event("user_edit_form_admin", $user, $form); @@ -308,13 +314,22 @@ class Admin_Users_Controller extends Admin_Controller { $form = new Forge("admin/users/add_user", "", "post", array("id" => "g-add-user-form")); $group = $form->group("add_user")->label(t("Add user")); $group->input("name")->label(t("Username"))->id("g-username") + ->error_messages("required", t("A name is required")) + ->error_messages("length", t("This name is too long")) ->error_messages("conflict", t("There is already a user with that username")); - $group->input("full_name")->label(t("Full name"))->id("g-fullname"); - $group->password("password")->label(t("Password"))->id("g-password"); + $group->input("full_name")->label(t("Full name"))->id("g-fullname") + ->error_messages("length", t("This name is too long")); + $group->password("password")->label(t("Password"))->id("g-password") + ->error_messages("min_length", t("This password is too short")); $group->password("password2")->label(t("Confirm password"))->id("g-password2") + ->error_messages("matches", t("The passwords you entered do not match")) ->matches($group->password); - $group->input("email")->label(t("Email"))->id("g-email"); - $group->input("url")->label(t("URL"))->id("g-url"); + $group->input("email")->label(t("Email"))->id("g-email") + ->error_messages("required", t("You must enter a valid email address")) + ->error_messages("length", t("This email address is too long")) + ->error_messages("email", t("You must enter a valid email address")); + $group->input("url")->label(t("URL"))->id("g-url") + ->error_messages("url", t("You must enter a valid URL")); self::_add_locale_dropdown($group); $group->checkbox("admin")->label(t("Admin"))->id("g-admin"); diff --git a/modules/user/controllers/users.php b/modules/user/controllers/users.php index d0c67dd1..43a92b44 100644 --- a/modules/user/controllers/users.php +++ b/modules/user/controllers/users.php @@ -90,6 +90,7 @@ class Users_Controller extends Controller { ->error_messages("matches", t("The passwords you entered do not match")); $group->input("email")->label(t("Email"))->id("g-email")->value($user->email) ->error_messages("email", t("You must enter a valid email address")) + ->error_messages("length", t("Your email address is too long")) ->error_messages("required", t("You must enter a valid email address")); $group->input("url")->label(t("URL"))->id("g-url")->value($user->url); -- cgit v1.2.3 From 6e1b761b12e13566875804c33efe2ae130ffa32e Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Tue, 2 Feb 2010 21:36:01 -0800 Subject: Require the current password to change your password. Fixes ticket #585. Separate out the password change form from the regular edit user form. Require the old password to enter a new one. While I'm at it, roll the password strength javascript into a Form_Script element so that we can get rid of the old view (which incidentally fixes a bug where the password strength meter would go away on form errors). --- modules/gallery/views/user_profile.html.php | 7 ++- modules/user/controllers/users.php | 78 +++++++++++++++++++++++------ modules/user/helpers/user.php | 6 +++ modules/user/views/user_form.html.php | 7 --- 4 files changed, 75 insertions(+), 23 deletions(-) delete mode 100644 modules/user/views/user_form.html.php (limited to 'modules/user/controllers') diff --git a/modules/gallery/views/user_profile.html.php b/modules/gallery/views/user_profile.html.php index f35f8c3f..78e1c579 100644 --- a/modules/gallery/views/user_profile.html.php +++ b/modules/gallery/views/user_profile.html.php @@ -57,13 +57,16 @@ - id}") ?>"> + id}") ?>"> + id}") ?>"> + + - \ No newline at end of file + diff --git a/modules/user/controllers/users.php b/modules/user/controllers/users.php index 43a92b44..c11f22ff 100644 --- a/modules/user/controllers/users.php +++ b/modules/user/controllers/users.php @@ -20,7 +20,6 @@ class Users_Controller extends Controller { public function update($id) { $user = user::lookup($id); - if ($user->guest || $user->id != identity::active_user()->id) { access::forbidden(); } @@ -29,9 +28,6 @@ class Users_Controller extends Controller { try { $valid = $form->validate(); $user->full_name = $form->edit_user->full_name->value; - if ($form->edit_user->password->value) { - $user->password = $form->edit_user->password->value; - } $user->email = $form->edit_user->email->value; $user->url = $form->edit_user->url->value; @@ -57,7 +53,38 @@ class Users_Controller extends Controller { $user->save(); module::event("user_edit_form_completed", $user, $form); - message::success(t("User information updated.")); + message::success(t("User information updated")); + print json_encode( + array("result" => "success", + "resource" => url::site("users/{$user->id}"))); + } else { + print json_encode(array("result" => "error", "form" => (string) $form)); + } + } + + public function change_password($id) { + $user = user::lookup($id); + if ($user->guest || $user->id != identity::active_user()->id) { + access::forbidden(); + } + + $form = $this->_get_change_password_form($user); + try { + $valid = $form->validate(); + $user->password = $form->change_password->password->value; + $user->validate(); + } catch (ORM_Validation_Exception $e) { + // Translate ORM validation errors into form error messages + foreach ($e->validation->errors() as $key => $error) { + $form->change_password->inputs[$key]->add_error($error, 1); + } + $valid = false; + } + + if ($valid) { + $user->save(); + module::event("user_change_password_form_completed", $user, $form); + message::success(t("Password changed")); print json_encode( array("result" => "success", "resource" => url::site("users/{$user->id}"))); @@ -72,22 +99,45 @@ class Users_Controller extends Controller { access::forbidden(); } - $v = new View("user_form.html"); - $v->form = $this->_get_edit_form($user); - print $v; + print $this->_get_edit_form($user); + } + + public function form_change_password($id) { + $user = user::lookup($id); + if ($user->guest || $user->id != identity::active_user()->id) { + access::forbidden(); + } + + print $this->_get_change_password_form($user); + } + + private function _get_change_password_form($user) { + $form = new Forge( + "users/change_password/$user->id", "", "post", array("id" => "g-change-password-user-form")); + $group = $form->group("change_password")->label(t("Change your password")); + $group->password("old_password")->label(t("Old password"))->id("g-password") + ->callback("user::valid_password") + ->error_messages("invalid", t("Incorrect password")); + $group->password("password")->label(t("New password"))->id("g-password") + ->error_messages("min_length", t("Your new password is too short")); + $group->script("") + ->text( + '$("form").ready(function(){$(\'input[name="password"]\').user_password_strength();});'); + $group->password("password2")->label(t("Confirm new password"))->id("g-password2") + ->matches($group->password) + ->error_messages("matches", t("The passwords you entered do not match")); + + module::event("user_change_password_form", $user, $form); + $group->submit("")->value(t("Save")); + return $form; } private function _get_edit_form($user) { $form = new Forge("users/update/$user->id", "", "post", array("id" => "g-edit-user-form")); - $group = $form->group("edit_user")->label(t("Edit User: %name", array("name" => $user->name))); + $group = $form->group("edit_user")->label(t("Edit your profile")); $group->input("full_name")->label(t("Full Name"))->id("g-fullname")->value($user->full_name) ->error_messages("length", t("Your name is too long")); self::_add_locale_dropdown($group, $user); - $group->password("password")->label(t("Password"))->id("g-password") - ->error_messages("min_length", t("Your password is too short")); - $group->password("password2")->label(t("Confirm Password"))->id("g-password2") - ->matches($group->password) - ->error_messages("matches", t("The passwords you entered do not match")); $group->input("email")->label(t("Email"))->id("g-email")->value($user->email) ->error_messages("email", t("You must enter a valid email address")) ->error_messages("length", t("Your email address is too long")) diff --git a/modules/user/helpers/user.php b/modules/user/helpers/user.php index 3561021f..7ceca6a5 100644 --- a/modules/user/helpers/user.php +++ b/modules/user/helpers/user.php @@ -70,6 +70,12 @@ class user_Core { return false; } + static function valid_password($password_input) { + if (!user::is_correct_password(identity::active_user(), $password_input->value)) { + $password_input->add_error("invalid", 1); + } + } + /** * Create the hashed passwords. * @param string $password a plaintext password diff --git a/modules/user/views/user_form.html.php b/modules/user/views/user_form.html.php deleted file mode 100644 index 4ce2b532..00000000 --- a/modules/user/views/user_form.html.php +++ /dev/null @@ -1,7 +0,0 @@ - - - -- cgit v1.2.3 From 99a7f470b93d35717f8d5979d05da6cf05a1dd20 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Tue, 2 Feb 2010 21:48:01 -0800 Subject: Protect password changes against brute force attacks. --- modules/gallery/helpers/auth.php | 10 ++++++++-- modules/gallery/helpers/gallery_event.php | 12 ++++++++++-- modules/user/controllers/users.php | 12 ++++++++++-- 3 files changed, 28 insertions(+), 6 deletions(-) (limited to 'modules/user/controllers') diff --git a/modules/gallery/helpers/auth.php b/modules/gallery/helpers/auth.php index 16f8915a..717cf40a 100644 --- a/modules/gallery/helpers/auth.php +++ b/modules/gallery/helpers/auth.php @@ -78,10 +78,16 @@ class auth_Core { } } + static function validate_too_many_failed_password_changes($password_input) { + if (self::too_many_failed_logins(identity::active_user()->name)) { + $password_input->add_error("too_many_failed_password_changes", 1); + } + } + /** * Record a failed login for this user */ - static function record_failed_login($name) { + static function record_failed_auth_attempts($name) { $failed_login = ORM::factory("failed_login") ->where("name", "=", $name) ->find(); @@ -96,7 +102,7 @@ class auth_Core { /** * Clear any failed logins for this user */ - static function record_successful_login($user) { + static function clear_failed_logins($user) { db::build() ->delete("failed_logins") ->where("name", "=", $user->name) diff --git a/modules/gallery/helpers/gallery_event.php b/modules/gallery/helpers/gallery_event.php index 6479e2c3..7b538c49 100644 --- a/modules/gallery/helpers/gallery_event.php +++ b/modules/gallery/helpers/gallery_event.php @@ -110,11 +110,19 @@ class gallery_event_Core { graphics::choose_default_toolkit(); module::clear_var("gallery", "choose_default_tookit"); } - auth::record_successful_login($user); + auth::clear_failed_auth_attempts($user); } static function user_login_failed($name) { - auth::record_failed_login($name); + auth::record_failed_auth_attempts($name); + } + + static function user_password_changed($user) { + auth::clear_failed_auth_attempts($user); + } + + static function user_password_change_failed($name) { + auth::record_failed_auth_attempts($name); } static function item_index_data($item, $data) { diff --git a/modules/user/controllers/users.php b/modules/user/controllers/users.php index c11f22ff..166ff8b2 100644 --- a/modules/user/controllers/users.php +++ b/modules/user/controllers/users.php @@ -77,7 +77,7 @@ class Users_Controller extends Controller { // Translate ORM validation errors into form error messages foreach ($e->validation->errors() as $key => $error) { $form->change_password->inputs[$key]->add_error($error, 1); - } + } $valid = false; } @@ -85,10 +85,14 @@ class Users_Controller extends Controller { $user->save(); module::event("user_change_password_form_completed", $user, $form); message::success(t("Password changed")); + module::event("user_password_change", $user); print json_encode( array("result" => "success", "resource" => url::site("users/{$user->id}"))); } else { + log::warning("user", t("Failed password change for %name", array("name" => $user->name))); + $name = $user->name; + module::event("user_password_change_failed", $name); print json_encode(array("result" => "error", "form" => (string) $form)); } } @@ -116,8 +120,12 @@ class Users_Controller extends Controller { "users/change_password/$user->id", "", "post", array("id" => "g-change-password-user-form")); $group = $form->group("change_password")->label(t("Change your password")); $group->password("old_password")->label(t("Old password"))->id("g-password") + ->callback("auth::validate_too_many_failed_password_changes") ->callback("user::valid_password") - ->error_messages("invalid", t("Incorrect password")); + ->error_messages("invalid", t("Incorrect password")) + ->error_messages( + "too_many_failed_password_changes", + t("Too many incorrect passwords. Try again later")); $group->password("password")->label(t("New password"))->id("g-password") ->error_messages("min_length", t("Your new password is too short")); $group->script("") -- cgit v1.2.3 From f631c2a0e5d1de4d17478993fc0cac2c9a989df2 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Sat, 6 Feb 2010 09:30:25 -0800 Subject: Fix up Admin_Users_Controller() form handling now that user_form.html is gone. Fixes ticket #1005. --- modules/user/controllers/admin_users.php | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) (limited to 'modules/user/controllers') diff --git a/modules/user/controllers/admin_users.php b/modules/user/controllers/admin_users.php index 48847433..df3d96c9 100644 --- a/modules/user/controllers/admin_users.php +++ b/modules/user/controllers/admin_users.php @@ -60,9 +60,7 @@ class Admin_Users_Controller extends Admin_Controller { } public function add_user_form() { - $v = new View("user_form.html"); - $v->form = $this->_get_user_add_form_admin(); - print $v; + print $this->_get_user_add_form_admin(); } public function delete_user($id) { @@ -147,13 +145,7 @@ class Admin_Users_Controller extends Admin_Controller { throw new Kohana_404_Exception(); } - $v = new View("user_form.html"); - $v->form = $this->_get_user_edit_form_admin($user); - // Don't allow the user to control their own admin bit, else you can lock yourself out - if ($user->id == identity::active_user()->id) { - $v->form->edit_user->admin->disabled(1); - } - print $v; + print $this->_get_user_edit_form_admin($user); } public function add_user_to_group($user_id, $group_id) { @@ -293,6 +285,9 @@ class Admin_Users_Controller extends Admin_Controller { ->error_messages("length", t("This name is too long")); $group->password("password")->label(t("Password"))->id("g-password") ->error_messages("min_length", t("This password is too short")); + $group->script("") + ->text( + '$("form").ready(function(){$(\'input[name="password"]\').user_password_strength();});'); $group->password("password2")->label(t("Confirm password"))->id("g-password2") ->error_messages("matches", t("The passwords you entered do not match")) ->matches($group->password); @@ -305,6 +300,11 @@ class Admin_Users_Controller extends Admin_Controller { self::_add_locale_dropdown($group, $user); $group->checkbox("admin")->label(t("Admin"))->id("g-admin")->checked($user->admin); + // Don't allow the user to control their own admin bit, else you can lock yourself out + if ($user->id == identity::active_user()->id) { + $group->admin->disabled(1); + } + module::event("user_edit_form_admin", $user, $form); $group->submit("")->value(t("Modify User")); return $form; @@ -321,6 +321,9 @@ class Admin_Users_Controller extends Admin_Controller { ->error_messages("length", t("This name is too long")); $group->password("password")->label(t("Password"))->id("g-password") ->error_messages("min_length", t("This password is too short")); + $group->script("") + ->text( + '$("form").ready(function(){$(\'input[name="password"]\').user_password_strength();});'); $group->password("password2")->label(t("Confirm password"))->id("g-password2") ->error_messages("matches", t("The passwords you entered do not match")) ->matches($group->password); -- cgit v1.2.3