From c0683845048a0610316b5f3aaa9111793273f420 Mon Sep 17 00:00:00 2001 From: Tim Almdal Date: Tue, 6 Oct 2009 11:20:51 -0700 Subject: Encapsulate the user and group model in Gallery_User and Gallery_Group classes which extend the User_Definition and Group_Definition classes defined in the Identity API --- modules/user/models/user.php | 21 +-------------------- 1 file changed, 1 insertion(+), 20 deletions(-) (limited to 'modules/user/models') diff --git a/modules/user/models/user.php b/modules/user/models/user.php index 55562f34..1993bd05 100644 --- a/modules/user/models/user.php +++ b/modules/user/models/user.php @@ -51,16 +51,6 @@ class User_Model extends ORM { module::event("user_deleted", $old); } - /** - * Return a url to the user's avatar image. - * @param integer $size the target size of the image (default 80px) - * @return string a url - */ - public function avatar_url($size=80, $default=null) { - return sprintf("http://www.gravatar.com/avatar/%s.jpg?s=%d&r=pg%s", - md5($this->email), $size, $default ? "&d=" . urlencode($default) : ""); - } - public function save() { if (!$this->loaded) { $created = 1; @@ -73,13 +63,4 @@ class User_Model extends ORM { } return $this; } - - /** - * Return the best version of the user's name. Either their specified full name, or fall back - * to the user name. - * @return string - */ - public function display_name() { - return empty($this->full_name) ? $this->name : $this->full_name; - } -} \ No newline at end of file +} -- cgit v1.2.3 From bc241e44c2e4d10ac19ccc32a40c90426672d963 Mon Sep 17 00:00:00 2001 From: Tim Almdal Date: Fri, 16 Oct 2009 07:41:33 -0700 Subject: Cleanup merge of user/group helpers into Identity interface. Reduce redundant code in the user module and remove references to the Identity helper from the user module as the user module should be able to access things directly. Simplify the get_user_list api method to just accept an array of ids to return user objects for. --- modules/gallery/helpers/movie.php | 2 +- modules/gallery/helpers/photo.php | 2 +- modules/gallery/libraries/Identity.php | 18 +-- modules/gallery/libraries/drivers/Identity.php | 41 +------ modules/notification/helpers/notification.php | 8 +- modules/user/controllers/admin_users.php | 18 +-- modules/user/controllers/users.php | 2 +- modules/user/helpers/group.php | 34 ++++-- modules/user/helpers/user.php | 56 ++++------ .../user/libraries/drivers/Identity/Gallery.php | 124 +++++---------------- modules/user/models/user.php | 19 ++++ 11 files changed, 113 insertions(+), 211 deletions(-) (limited to 'modules/user/models') diff --git a/modules/gallery/helpers/movie.php b/modules/gallery/helpers/movie.php index 32a27646..bc0efa01 100644 --- a/modules/gallery/helpers/movie.php +++ b/modules/gallery/helpers/movie.php @@ -77,7 +77,7 @@ class movie_Core { $movie->title = $title; $movie->description = $description; $movie->name = $name; - $movie->owner_id = $owner_id ? $owner_id : Identity::active(); + $movie->owner_id = $owner_id ? $owner_id : Identity::active()->id; $movie->width = $movie_info[0]; $movie->height = $movie_info[1]; $movie->mime_type = strtolower($pi["extension"]) == "mp4" ? "video/mp4" : "video/x-flv"; diff --git a/modules/gallery/helpers/photo.php b/modules/gallery/helpers/photo.php index cf316819..ad23e322 100644 --- a/modules/gallery/helpers/photo.php +++ b/modules/gallery/helpers/photo.php @@ -76,7 +76,7 @@ class photo_Core { $photo->title = $title; $photo->description = $description; $photo->name = $name; - $photo->owner_id = $owner_id ? $owner_id : Identity::active(); + $photo->owner_id = $owner_id ? $owner_id : Identity::active()->id; $photo->width = $image_info[0]; $photo->height = $image_info[1]; $photo->mime_type = empty($image_info['mime']) ? "application/unknown" : $image_info['mime']; diff --git a/modules/gallery/libraries/Identity.php b/modules/gallery/libraries/Identity.php index 229d0da9..fb553de6 100644 --- a/modules/gallery/libraries/Identity.php +++ b/modules/gallery/libraries/Identity.php @@ -191,22 +191,8 @@ class Identity_Core { /** * @see Identity_Driver::get_user_list. */ - static function get_user_list($filter=array()) { - return self::instance()->driver->get_user_list($filter); - } - - /** - * @see Identity_Driver::get_group_list. - */ - static function get_group_list($filter=array()) { - return self::instance()->driver->get_group_list($filter); - } - - /** - * @see Identity_Driver::get_edit_rules. - */ - static function get_edit_rules($object_type) { - return self::instance()->driver->get_edit_rules($object_type); + static function get_user_list($ids) { + return self::instance()->driver->get_user_list($ids); } static function get_login_form($url) { diff --git a/modules/gallery/libraries/drivers/Identity.php b/modules/gallery/libraries/drivers/Identity.php index 0b789908..a9e1a75b 100644 --- a/modules/gallery/libraries/drivers/Identity.php +++ b/modules/gallery/libraries/drivers/Identity.php @@ -83,37 +83,11 @@ interface Identity_Driver { /** * List the users - * @param mixed options to apply to the selection of the user - * currently supported: - * "orderby" => array(, "ASC|DESC") - * "in" => array(, array(values, ...)) - * "where" => array(, value) - * follows Kohana syntax where it could contain the first - * half of a logical expression (i.e. "field IS NOT") - * @return array the group list. + * @param array array of ids to return the user objects for + * @return array the user list. */ - public function get_user_list($filter=array()); + public function get_user_list($ids); - /** - * List the groups - * @param mixed options to apply to the selection of the group - * currently supported: - * "orderby" => array(, "ASC|DESC") - * "in" => array(, array(values, ...)) - * "where" => array(, value) - * follows Kohana syntax where it could contain the first - * half of a logical expression (i.e. "field IS NOT") - * @return array the group list. - */ - public function get_group_list($filter=array()); - - /** - * Return the edit rules associated with an group. - * - * @param string $object_type to return rules for ("user"|"group") - * @return stdClass containing the rules - */ - public function get_edit_rules($object_type); } // End Identity Driver Definition /** @@ -205,19 +179,14 @@ abstract class User_Definition { * @param integer $size the target size of the image (default 80px) * @return string a url */ - public function avatar_url($size=80, $default=null) { - return sprintf("http://www.gravatar.com/avatar/%s.jpg?s=%d&r=pg%s", - md5($this->user->email), $size, $default ? "&d=" . urlencode($default) : ""); - } + abstract public function avatar_url($size=80, $default=null); /** * Return the best version of the user's name. Either their specified full name, or fall back * to the user name. * @return string */ - public function display_name() { - return empty($this->user->full_name) ? $this->user->name : $this->user->full_name; - } + abstract public function display_name(); /** * Return the internal user object without the wrapper. diff --git a/modules/notification/helpers/notification.php b/modules/notification/helpers/notification.php index 4c58bc29..2e22eeae 100644 --- a/modules/notification/helpers/notification.php +++ b/modules/notification/helpers/notification.php @@ -67,8 +67,7 @@ class notification { } static function get_subscribers($item) { - // @todo only return distinct email addresses - $subscriber_ids = array(); + $subscriber_ids = array(); foreach (ORM::factory("subscription") ->select("user_id") ->join("items", "subscriptions.item_id", "items.id") @@ -79,12 +78,11 @@ class notification { $subscriber_ids[] = $subscriber->user_id; } - $users = Identity::get_user_list(array("in" => array("id", $subscriber_ids), - "where" => array("email IS NOT" => null))); + $users = Identity::get_user_list($subscriber_ids); $subscribers = array(); foreach ($users as $user) { - if (access::user_can($user, "view", $item)) { + if (access::user_can($user, "view", $item) && !empty($user->email)) { $subscribers[$user->email] = 1; } } diff --git a/modules/user/controllers/admin_users.php b/modules/user/controllers/admin_users.php index 3465c4b1..fed872a5 100644 --- a/modules/user/controllers/admin_users.php +++ b/modules/user/controllers/admin_users.php @@ -21,8 +21,12 @@ class Admin_Users_Controller extends Admin_Controller { public function index() { $view = new Admin_View("admin.html"); $view->content = new View("admin_users.html"); - $view->content->users = user::get_user_list(array("orderby" => array("name" => "ASC"))); - $view->content->groups = group::get_group_list(array("orderby" => array("name" => "ASC"))); + $view->content->users = ORM::factory("user") + ->orderby("name", "ASC") + ->find_all(); + $view->content->groups = ORM::factory("group") + ->orderby("name", "ASC") + ->find_all(); print $view; } @@ -303,7 +307,7 @@ class Admin_Users_Controller extends Admin_Controller { $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->checkbox("admin")->label(t("Admin"))->id("g-admin")->checked($user->admin); - $form->add_rules_from(user::get_edit_rules()); + $form->add_rules_from($user); $form->edit_user->password->rules("-required"); module::event("user_edit_form_admin", $user, $form); @@ -325,8 +329,7 @@ class Admin_Users_Controller extends Admin_Controller { $group->input("url")->label(t("URL"))->id("g-url"); self::_add_locale_dropdown($group); $group->checkbox("admin")->label(t("Admin"))->id("g-admin"); - $user = ORM::factory("user"); - $form->add_rules_from(user::get_edit_rules()); + $form->add_rules_from(ORM::factory("user")); module::event("user_add_form_admin", $user, $form); $group->submit("")->value(t("Add User")); @@ -366,7 +369,7 @@ class Admin_Users_Controller extends Admin_Controller { $form_group->inputs["name"]->error_messages( "in_use", t("There is already a group with that name")); $form_group->submit("")->value(t("Save")); - $form->add_rules_from(group::get_edit_rules()); + $form->add_rules_from($group); return $form; } @@ -378,8 +381,7 @@ class Admin_Users_Controller extends Admin_Controller { $form_group->inputs["name"]->error_messages( "in_use", t("There is already a group with that name")); $form_group->submit("")->value(t("Add Group")); - $group = ORM::factory("group"); - $form->add_rules_from(group::get_edit_rules()); + $form->add_rules_from(ORM::factory("group")); return $form; } diff --git a/modules/user/controllers/users.php b/modules/user/controllers/users.php index 6e666ba3..ebce1d8d 100644 --- a/modules/user/controllers/users.php +++ b/modules/user/controllers/users.php @@ -77,7 +77,7 @@ class Users_Controller extends Controller { ->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); - $form->add_rules_from(user::get_edit_rules()); + $form->add_rules_from($user); module::event("user_edit_form", $user, $form); $group->submit("")->value(t("Save")); diff --git a/modules/user/helpers/group.php b/modules/user/helpers/group.php index 295e5f50..cf5c050f 100644 --- a/modules/user/helpers/group.php +++ b/modules/user/helpers/group.php @@ -28,7 +28,14 @@ class group_Core { * @see Identity_Driver::create. */ static function create($name) { - return Identity::instance()->create_group($name); + $group = ORM::factory("group")->where("name", $name)->find(); + if ($group->loaded) { + throw new Exception("@todo GROUP_ALREADY_EXISTS $name"); + } + + $group->name = $name; + $group->save(); + return $group; } /** @@ -51,7 +58,7 @@ class group_Core { * @return Group_Definition the group object, or null if the id was invalid. */ static function lookup($id) { - return Identity::instance()->lookup_group_by_field("id", $id); + return self::lookup_by_field("id", $id); } /** @@ -60,20 +67,23 @@ class group_Core { * @return Group_Definition the group object, or null if the name was invalid. */ static function lookup_by_name($name) { - return Identity::instance()->lookup_group_by_field("name", $name); + return self::lookup_by_field("name", $name); } /** * @see Identity_Driver::get_group_list. */ - static function get_group_list($filter=array()) { - return Identity::instance()->get_group_list($filter); - } - - /** - * @see Identity_Driver::get_edit_rules. - */ - static function get_edit_rules() { - return Identity::instance()->get_edit_rules("group"); + static function lookup_by_field($field_name, $value) { + try { + $user = model_cache::get("group", $value, $field_name); + if ($user->loaded) { + return $user; + } + } catch (Exception $e) { + if (strpos($e->getMessage(), "MISSING_MODEL") === false) { + throw $e; + } + } + return null; } } diff --git a/modules/user/helpers/user.php b/modules/user/helpers/user.php index 394f8185..fa7b320f 100644 --- a/modules/user/helpers/user.php +++ b/modules/user/helpers/user.php @@ -28,28 +28,37 @@ class user_Core { * @see Identity_Driver::guest. */ static function guest() { - return Identity::guest(); + return model_cache::get("user", 1); } /** * @see Identity_Driver::create_user. */ static function create($name, $full_name, $password) { - return Identity::create_user($name, $full_name, $password); - } + $user = ORM::factory("user")->where("name", $name)->find(); + if ($user->loaded) { + throw new Exception("@todo USER_ALREADY_EXISTS $name"); + } - /** - * @see Identity_Driver::is_correct_password. - */ - static function is_correct_password($user, $password) { - return Identity::is_correct_password($user, $password); + $user->name = $name; + $user->full_name = $full_name; + $user->password = $password; + + // Required groups + $user->add(group::everybody()); + $user->add(group::registered_users()); + + $user->save(); + return $user; } /** * @see Identity_Driver::hash_password. */ static function hash_password($password) { - return Identity::hash_password($password); + require_once(MODPATH . "user/lib/PasswordHash.php"); + $hashGenerator = new PasswordHash(10, true); + return $hashGenerator->HashPassword($password); } /** @@ -58,7 +67,7 @@ class user_Core { * @return User_Definition the user object, or null if the id was invalid. */ static function lookup($id) { - return self::_lookup_user_by_field("id", $id); + return self::lookup_by_field("id", $id); } /** @@ -67,33 +76,10 @@ class user_Core { * @return User_Definition the user object, or null if the name was invalid. */ static function lookup_by_name($name) { - return self::_lookup_user_by_field("name", $name); - } - - /** - * Look up a user by hash. - * @param string $name the user name - * @return User_Definition the user object, or null if the name was invalid. - */ - static function lookup_by_hash($hash) { - return self::_lookup_user_by_field("hash", $hash); - } - - /** - * @see Identity_Driver::get_user_list. - */ - static function get_user_list($filter=array()) { - return Identity::get_user_list($filter); - } - - /** - * @see Identity_Driver::get_edit_rules. - */ - static function get_edit_rules() { - return Identity::get_edit_rules("user"); + return self::lookup_by_field("name", $name); } - private static function _lookup_user_by_field($field_name, $value) { + static function lookup_by_field($field_name, $value) { try { $user = model_cache::get("user", $value, $field_name); if ($user->loaded) { diff --git a/modules/user/libraries/drivers/Identity/Gallery.php b/modules/user/libraries/drivers/Identity/Gallery.php index 013497b6..77db11a3 100644 --- a/modules/user/libraries/drivers/Identity/Gallery.php +++ b/modules/user/libraries/drivers/Identity/Gallery.php @@ -25,28 +25,14 @@ class Identity_Gallery_Driver implements Identity_Driver { * @see Identity_Driver::guest. */ public function guest() { - return new Gallery_User(model_cache::get("user", 1)); + return new Gallery_User(user::guest()); } /** * @see Identity_Driver::create_user. */ public function create_user($name, $full_name, $password) { - $user = ORM::factory("user")->where("name", $name)->find(); - if ($user->loaded) { - throw new Exception("@todo USER_ALREADY_EXISTS $name"); - } - - $user->name = $name; - $user->full_name = $full_name; - $user->password = $password; - - // Required groups - $user->add($this->everybody()->_uncloaked()); - $user->add($this->registered_users()->_uncloaked()); - - $user->save(); - return new Gallery_User($user); + return new Gallery_User(user::create($name, $full_name, $password)); } /** @@ -84,126 +70,58 @@ class Identity_Gallery_Driver implements Identity_Driver { * @see Identity_Driver::hash_password. */ public function hash_password($password) { - require_once(MODPATH . "user/lib/PasswordHash.php"); - $hashGenerator = new PasswordHash(10, true); - return $hashGenerator->HashPassword($password); + return user::hash_password($password); } /** * @see Identity_Driver::lookup_user_by_field. */ public function lookup_user_by_field($field_name, $value) { - try { - $user = model_cache::get("user", $value, $field_name); - if ($user->loaded) { - return new Gallery_User($user); - } - } catch (Exception $e) { - if (strpos($e->getMessage(), "MISSING_MODEL") === false) { - throw $e; - } - } - return null; + return new Gallery_User(user::lookup_by_field($field_name, $value)); } /** * @see Identity_Driver::create_group. */ public function create_group($name) { - $group = ORM::factory("group")->where("name", $name)->find(); - if ($group->loaded) { - throw new Exception("@todo GROUP_ALREADY_EXISTS $name"); - } - - $group->name = $name; - $group->save(); - - return new Gallery_Group($group); + return new Gallery_Group(group::create($name)); } /** * @see Identity_Driver::everybody. */ public function everybody() { - return new Gallery_Group(model_cache::get("group", 1)); + return new Gallery_Group(group::everybody()); } /** * @see Identity_Driver::registered_users. */ public function registered_users() { - return new Gallery_Group(model_cache::get("group", 2)); + return new Gallery_Group(group::registered_users()); } /** * @see Identity_Driver::lookup_group_by_field. */ public function lookup_group_by_field($field_name, $value) { - try { - $group = model_cache::get("group", $value, $field_name); - if ($group->loaded) { - return new Gallery_Group($group); - } - } catch (Exception $e) { - if (strpos($e->getMessage(), "MISSING_MODEL") === false) { - throw $e; - } - } - return null; + return new Gallery_Group(group::lookup_by_field($field_name, $value)); } /** * @see Identity_Driver::get_user_list. */ - public function get_user_list($filter=array()) { - $results = $this->_do_search("user", $filter); + public function get_user_list($ids) { + $results = ORM::factory("user") + ->in("id", ids) + ->find_all() + ->as_array();; $users = array(); - foreach ($results->as_array() as $user) { + foreach ($results as $user) { $users[] = new Gallery_User($user); } return $users; } - - /** - * @see Identity_Driver::get_group_list. - */ - public function get_group_list($filter=array()) { - $results = $this->_do_search("group", $filter); - $groups = array(); - foreach ($results->as_array() as $group) { - $groups[] = new Gallery_Group($group); - } - return $groups; - } - - /** - * @see Identity_Driver::get_edit_rules. - */ - public function get_edit_rules($object_type) { - return (object)ORM::factory($object_type)->rules; - } - - /** - * Build the query based on the supplied filters for the specified model. - * @param string $object_type to return rules for ("user"|"group") - * @param mixed $filters options to apply to the selection. - */ - private function _do_search($object_type, $filter) { - $object = ORM::factory($object_type); - - foreach ($filter as $method => $args) { - switch ($method) { - case "in": - $object->in($args[0], $args[1]); - break; - default: - $object->$method($args); - } - } - - return $object->find_all(); - } - } // End Identity Gallery Driver /** @@ -217,6 +135,20 @@ class Gallery_User extends User_Definition { $this->user = $user; } + /** + * @see User_Definition::avatar_url + */ + public function avatar_url($size=80, $default=null) { + return $this->user->avatar_url($size, $default); + } + + /** + * @see User_Definition::display_name + */ + public function display_name() { + return $this->user->display_name(); + } + public function save() { $this->user->save(); } diff --git a/modules/user/models/user.php b/modules/user/models/user.php index 1993bd05..d99603b2 100644 --- a/modules/user/models/user.php +++ b/modules/user/models/user.php @@ -51,6 +51,16 @@ class User_Model extends ORM { module::event("user_deleted", $old); } + /** + * Return a url to the user's avatar image. + * @param integer $size the target size of the image (default 80px) + * @return string a url + */ + public function avatar_url($size=80, $default=null) { + return sprintf("http://www.gravatar.com/avatar/%s.jpg?s=%d&r=pg%s", + md5($this->email), $size, $default ? "&d=" . urlencode($default) : ""); + } + public function save() { if (!$this->loaded) { $created = 1; @@ -63,4 +73,13 @@ class User_Model extends ORM { } return $this; } + + /** + * Return the best version of the user's name. Either their specified full name, or fall back + * to the user name. + * @return string + */ + public function display_name() { + return empty($this->full_name) ? $this->name : $this->full_name; + } } -- cgit v1.2.3 From 098b57bf18112d0a3173dbe28b5ed76782431ff7 Mon Sep 17 00:00:00 2001 From: Tim Almdal Date: Mon, 19 Oct 2009 12:53:44 -0700 Subject: Simplify the user interface by moving the password reset functionality into the user module Bagging the User_Definition and Group_Definition abstract classes and replacing them with interfaces with the same names. Make sure all the unit tests work. --- modules/gallery/controllers/password.php | 133 -------------- modules/gallery/helpers/access.php | 4 +- modules/gallery/libraries/Identity.php | 34 +--- modules/gallery/libraries/drivers/Identity.php | 196 ++------------------- modules/gallery/tests/Albums_Controller_Test.php | 1 + modules/gallery/tests/Photos_Controller_Test.php | 2 +- modules/gallery/views/admin_identity.html.php | 6 +- modules/gallery/views/reset_password.html.php | 17 -- modules/user/controllers/password.php | 133 ++++++++++++++ modules/user/helpers/group.php | 4 +- modules/user/helpers/user.php | 15 +- .../user/libraries/drivers/Identity/Gallery.php | 99 ++--------- modules/user/models/group.php | 2 +- modules/user/models/user.php | 2 +- modules/user/views/admin_users.html.php | 2 +- modules/user/views/reset_password.html.php | 17 ++ 16 files changed, 206 insertions(+), 461 deletions(-) delete mode 100644 modules/gallery/controllers/password.php delete mode 100644 modules/gallery/views/reset_password.html.php create mode 100644 modules/user/controllers/password.php create mode 100644 modules/user/views/reset_password.html.php (limited to 'modules/user/models') diff --git a/modules/gallery/controllers/password.php b/modules/gallery/controllers/password.php deleted file mode 100644 index ce6d67b1..00000000 --- a/modules/gallery/controllers/password.php +++ /dev/null @@ -1,133 +0,0 @@ -_send_reset(); - } else { - print $this->_reset_form(); - } - } - - public function do_reset() { - if (request::method() == "post") { - $this->_change_password(); - } else { - $user = Identity::lookup_user_by_hash(Input::instance()->get("key")); - if (!empty($user)) { - print $this->_new_password_form($user->hash); - } else { - throw new Exception("@todo FORBIDDEN", 503); - } - } - } - - private function _send_reset() { - $form = $this->_reset_form(); - - $valid = $form->validate(); - if ($valid) { - $user = Identity::lookup_user_by_name($form->reset->inputs["name"]->value); - if (!$user->loaded || empty($user->email)) { - $form->reset->inputs["name"]->add_error("no_email", 1); - $valid = false; - } - } - - if ($valid) { - $user->hash = md5(rand()); - $user->save(); - $message = new View("reset_password.html"); - $message->confirm_url = url::abs_site("password/do_reset?key=$user->hash"); - $message->user = $user; - - Sendmail::factory() - ->to($user->email) - ->subject(t("Password Reset Request")) - ->header("Mime-Version", "1.0") - ->header("Content-type", "text/html; charset=iso-8859-1") - ->message($message->render()) - ->send(); - - log::success( - "user", - t("Password reset email sent for user %name", array("name" => $user->name))); - } else { - // Don't include the username here until you're sure that it's XSS safe - log::warning( - "user", "Password reset email requested for bogus user"); - } - - message::success(t("Password reset email sent")); - print json_encode( - array("result" => "success")); - } - - private function _reset_form() { - $form = new Forge(url::current(true), "", "post", array("id" => "g-reset-form")); - $group = $form->group("reset")->label(t("Reset Password")); - $group->input("name")->label(t("Username"))->id("g-name")->class(null)->rules("required"); - $group->inputs["name"]->error_messages("no_email", t("No email, unable to reset password")); - $group->submit("")->value(t("Reset")); - - return $form; - } - - private function _new_password_form($hash=null) { - $template = new Theme_View("page.html", "reset"); - - $form = new Forge("password/do_reset", "", "post", array("id" => "g-change-password-form")); - $group = $form->group("reset")->label(t("Change Password")); - $hidden = $group->hidden("hash"); - if (!empty($hash)) { - $hidden->value($hash); - } - $group->password("password")->label(t("Password"))->id("g-password") - ->rules("required|length[1,40]"); - $group->password("password2")->label(t("Confirm Password"))->id("g-password2") - ->matches($group->password); - $group->inputs["password2"]->error_messages( - "mistyped", t("The password and the confirm password must match")); - $group->submit("")->value(t("Update")); - - $template->content = $form; - return $template; - } - - private function _change_password() { - $view = $this->_new_password_form(); - if ($view->content->validate()) { - $user = Identity::lookup_user_by_hash(Input::instance()->get("key")); - if (empty($user)) { - throw new Exception("@todo FORBIDDEN", 503); - } - - $user->password = $view->content->reset->password->value; - $user->hash = null; - $user->save(); - message::success(t("Password reset successfully")); - url::redirect(item::root()->abs_url()); - } else { - print $view; - } - } -} \ No newline at end of file diff --git a/modules/gallery/helpers/access.php b/modules/gallery/helpers/access.php index 21f4de81..fba161e3 100644 --- a/modules/gallery/helpers/access.php +++ b/modules/gallery/helpers/access.php @@ -197,8 +197,8 @@ class access_Core { * @param Item_Model $item * @param boolean $value */ - private static function _set(Group_Model $group, $perm_name, $album, $value) { - if (get_class($group) != "Group_Model") { + private static function _set(Group_Definition $group, $perm_name, $album, $value) { + if (!($group instanceof Group_Definition)) { throw new Exception("@todo PERMISSIONS_ONLY_WORK_ON_GROUPS"); } if (!$album->loaded) { diff --git a/modules/gallery/libraries/Identity.php b/modules/gallery/libraries/Identity.php index 9e5f0bb5..e77fd2d2 100644 --- a/modules/gallery/libraries/Identity.php +++ b/modules/gallery/libraries/Identity.php @@ -133,37 +133,17 @@ class Identity_Core { } /** - * @see Identity_Driver::hash_password. - */ - static function hash_password($password) { - return self::instance()->driver->hash_password($password); - } - - /** - * Look up a user by id. - * @param integer $id the user id - * @return User_Definition the user object, or null if the id was invalid. + * @see Identity_Driver::lookup_user. */ static function lookup_user($id) { - return self::instance()->driver->lookup_user_by_field("id", $id); + return self::instance()->driver->lookup_user($id); } /** - * Look up a user by name. - * @param integer $name the user name - * @return User_Definition the user object, or null if the name was invalid. + * @see Identity_Driver::lookup_user_by_name. */ static function lookup_user_by_name($name) { - return self::instance()->driver->lookup_user_by_field("name", $name); - } - - /** - * Look up a user by hash. - * @param string $name the user name - * @return User_Definition the user object, or null if the name was invalid. - */ - static function lookup_user_by_hash($hash) { - return self::instance()->driver->lookup_user_by_field("hash", $hash); + return self::instance()->driver->lookup_user_by_name($name); } /** @@ -188,12 +168,10 @@ class Identity_Core { } /** - * Look up a group by name. - * @param integer $id the group name - * @return Group_Definition the group object, or null if the name was invalid. + * @see Identity_Driver::lookup_group_by_name. */ static function lookup_group_by_name($name) { - return self::instance()->driver->lookup_group_by_field("name", $name); + return self::instance()->driver->lookup_group_by_name($name); } /** diff --git a/modules/gallery/libraries/drivers/Identity.php b/modules/gallery/libraries/drivers/Identity.php index a9e1a75b..6ab001cb 100644 --- a/modules/gallery/libraries/drivers/Identity.php +++ b/modules/gallery/libraries/drivers/Identity.php @@ -45,19 +45,18 @@ interface Identity_Driver { public function is_correct_password($user, $password); /** - * Create the hashed passwords. - * @param string $password a plaintext password - * @return string hashed password + * Look up a user by id. + * @param integer id + * @return User_Definition the user object, or null if the name was invalid. */ - public function hash_password($password); + public function lookup_user($id); /** - * Look up a user by by search the specified field. - * @param string search field - * @param string search value - * @return User_Definition the user object, or null if the name was invalid. + * Look up a user by name. + * @param string name + * @return User_Definition the user object, or null if the name was invalid. */ - public function lookup_user_by_field($field, $value); + public function lookup_user_by_name($name); /** * Create a new group. @@ -90,181 +89,6 @@ interface Identity_Driver { } // End Identity Driver Definition -/** - * User Data wrapper - */ -abstract class User_Definition { - protected $user; - public function __get($column) { - switch ($column) { - case "id": - case "name": - case "full_name": - case "password": - case "login_count": - case "last_login": - case "email": - case "admin": - case "guest": - case "hash": - case "url": - case "locale": - case "groups": - case "hashed_password": - return $this->user->$column; - default: - throw new Exception("@todo UNSUPPORTED FIELD: $column"); - break; - } - } - - public function __set($column, $value) { - switch ($column) { - case "id": - case "groups": - throw new Exception("@todo READ ONLY FIELD: $column"); - break; - case "name": - case "full_name": - case "hashed_password": - case "password": - case "login_count": - case "last_login": - case "email": - case "admin": - case "guest": - case "hash": - case "url": - case "locale": - $this->user->$column = $value; - break; - default: - throw new Exception("@todo UNSUPPORTED FIELD: $column"); - break; - } - } - - public function __isset($column) { - return isset($this->user->$column); - } - - public function __unset($column) { - switch ($column) { - case "id": - case "groups": - throw new Exception("@todo READ ONLY FIELD: $column"); - break; - case "name": - case "full_name": - case "password": - case "login_count": - case "last_login": - case "email": - case "admin": - case "guest": - case "hash": - case "url": - case "locale": - case "hashed_password": - unset($this->user->$column); - break; - default: - throw new Exception("@todo UNSUPPORTED FIELD: $column"); - break; - } - } - - /** - * Return a url to the user's avatar image. - * @param integer $size the target size of the image (default 80px) - * @return string a url - */ - abstract public function avatar_url($size=80, $default=null); - - /** - * Return the best version of the user's name. Either their specified full name, or fall back - * to the user name. - * @return string - */ - abstract public function display_name(); - - /** - * Return the internal user object without the wrapper. - * This method is used by implementing classes to access the internal user object. - * Consider it pseudo private and only declared public as PHP as not internal or friend modifier - */ - public function _uncloaked() { - return $this->user; - } - - abstract public function save(); - abstract public function delete(); -} - -/** - * Group Data wrapper - */ -abstract class Group_Definition { - protected $group; - - public function __get($column) { - switch ($column) { - case "id": - case "name": - case "special": - case "users": - return $this->group->$column; - default: - throw new Exception("@todo UNSUPPORTED FIELD: $column"); - break; - } - } - - public function __set($column, $value) { - switch ($column) { - case "id": - case "users": - throw new Exception("@todo READ ONLY FIELD: $column"); - break; - case "name": - case "special": - $this->group->$column = $value; - default: - throw new Exception("@todo UNSUPPORTED FIELD: $column"); - break; - } - } - - public function __isset($column) { - return isset($this->group->$column); - } - - public function __unset($column) { - switch ($column) { - case "id": - case "users": - throw new Exception("@todo READ ONLY FIELD: $column"); - break; - case "name": - case "special": - unset($this->group->$column); - default: - throw new Exception("@todo UNSUPPORTED FIELD: $column"); - break; - } - } - - /** - * Return the internal group object without the wrapper. - * This method is used by implementing classes to access the internal group object. - * Consider it pseudo private and only declared public as PHP as not internal or friend modifier - */ - public function _uncloaked() { - return $this->group; - } +interface Group_Definition {} - abstract public function save(); - abstract public function delete(); - abstract public function add($user); - abstract public function remove($user); -} +interface User_Definition {} diff --git a/modules/gallery/tests/Albums_Controller_Test.php b/modules/gallery/tests/Albums_Controller_Test.php index 046cb5ad..fa46d924 100644 --- a/modules/gallery/tests/Albums_Controller_Test.php +++ b/modules/gallery/tests/Albums_Controller_Test.php @@ -43,6 +43,7 @@ class Albums_Controller_Test extends Unit_Test_Case { $_POST["column"] = "weight"; $_POST["direction"] = "ASC"; $_POST["csrf"] = access::csrf_token(); + $_POST["slug"] = "new_name"; $_POST["_method"] = "put"; access::allow(Identity::everybody(), "edit", $root); diff --git a/modules/gallery/tests/Photos_Controller_Test.php b/modules/gallery/tests/Photos_Controller_Test.php index cdb4ae4f..59c3f78a 100644 --- a/modules/gallery/tests/Photos_Controller_Test.php +++ b/modules/gallery/tests/Photos_Controller_Test.php @@ -31,7 +31,7 @@ class Photos_Controller_Test extends Unit_Test_Case { $root = ORM::factory("item", 1); $photo = photo::create( $root, MODPATH . "gallery/tests/test.jpg", "test.jpeg", - "test", "test", Session::active_user(), "slug"); + "test", "test", Session::active_user()->id, "slug"); $orig_name = $photo->name; $_POST["filename"] = "test.jpeg"; diff --git a/modules/gallery/views/admin_identity.html.php b/modules/gallery/views/admin_identity.html.php index dcf1dbc1..1405cacb 100644 --- a/modules/gallery/views/admin_identity.html.php +++ b/modules/gallery/views/admin_identity.html.php @@ -15,11 +15,11 @@ height:165, modal: true, overlay: { - backgroundColor: '#000', - opacity: 0.5 + backgroundColor: '#000', + opacity: 0.5 }, buttons: { - "Continue": function() { + "Continue": function() { $("##g-dialog form").submit(); }, Cancel: function() { diff --git a/modules/gallery/views/reset_password.html.php b/modules/gallery/views/reset_password.html.php deleted file mode 100644 index 92ca4917..00000000 --- a/modules/gallery/views/reset_password.html.php +++ /dev/null @@ -1,17 +0,0 @@ - - - - <?= t("Password Reset Request") ?> - - -

-

- $user->full_name ? $user->full_name : $user->name)) ?> -

-

- %site_url. If you made this request, you can confirm it by clicking this link. If you didn't request this password reset, it's ok to ignore this mail.", - array("site_url" => html::mark_clean(url::base(false, "http")), - "confirm_url" => $confirm_url)) ?> -

- - diff --git a/modules/user/controllers/password.php b/modules/user/controllers/password.php new file mode 100644 index 00000000..a8f1c5ca --- /dev/null +++ b/modules/user/controllers/password.php @@ -0,0 +1,133 @@ +_send_reset(); + } else { + print $this->_reset_form(); + } + } + + public function do_reset() { + if (request::method() == "post") { + $this->_change_password(); + } else { + $user = user::lookup_user_by_field("hash", Input::instance()->get("key")); + if (!empty($user)) { + print $this->_new_password_form($user->hash); + } else { + throw new Exception("@todo FORBIDDEN", 503); + } + } + } + + private function _send_reset() { + $form = $this->_reset_form(); + + $valid = $form->validate(); + if ($valid) { + $user = Identity::lookup_user_by_name($form->reset->inputs["name"]->value); + if (!$user->loaded || empty($user->email)) { + $form->reset->inputs["name"]->add_error("no_email", 1); + $valid = false; + } + } + + if ($valid) { + $user->hash = md5(rand()); + $user->save(); + $message = new View("reset_password.html"); + $message->confirm_url = url::abs_site("password/do_reset?key=$user->hash"); + $message->user = $user; + + Sendmail::factory() + ->to($user->email) + ->subject(t("Password Reset Request")) + ->header("Mime-Version", "1.0") + ->header("Content-type", "text/html; charset=iso-8859-1") + ->message($message->render()) + ->send(); + + log::success( + "user", + t("Password reset email sent for user %name", array("name" => $user->name))); + } else { + // Don't include the username here until you're sure that it's XSS safe + log::warning( + "user", "Password reset email requested for bogus user"); + } + + message::success(t("Password reset email sent")); + print json_encode( + array("result" => "success")); + } + + private function _reset_form() { + $form = new Forge(url::current(true), "", "post", array("id" => "g-reset-form")); + $group = $form->group("reset")->label(t("Reset Password")); + $group->input("name")->label(t("Username"))->id("g-name")->class(null)->rules("required"); + $group->inputs["name"]->error_messages("no_email", t("No email, unable to reset password")); + $group->submit("")->value(t("Reset")); + + return $form; + } + + private function _new_password_form($hash=null) { + $template = new Theme_View("page.html", "reset"); + + $form = new Forge("password/do_reset", "", "post", array("id" => "g-change-password-form")); + $group = $form->group("reset")->label(t("Change Password")); + $hidden = $group->hidden("hash"); + if (!empty($hash)) { + $hidden->value($hash); + } + $group->password("password")->label(t("Password"))->id("g-password") + ->rules("required|length[1,40]"); + $group->password("password2")->label(t("Confirm Password"))->id("g-password2") + ->matches($group->password); + $group->inputs["password2"]->error_messages( + "mistyped", t("The password and the confirm password must match")); + $group->submit("")->value(t("Update")); + + $template->content = $form; + return $template; + } + + private function _change_password() { + $view = $this->_new_password_form(); + if ($view->content->validate()) { + $user = user::lookup_user_by_field("hash", Input::instance()->get("key")); + if (empty($user)) { + throw new Exception("@todo FORBIDDEN", 503); + } + + $user->password = $view->content->reset->password->value; + $user->hash = null; + $user->save(); + message::success(t("Password reset successfully")); + url::redirect(item::root()->abs_url()); + } else { + print $view; + } + } +} \ No newline at end of file diff --git a/modules/user/helpers/group.php b/modules/user/helpers/group.php index cf5c050f..8ad52564 100644 --- a/modules/user/helpers/group.php +++ b/modules/user/helpers/group.php @@ -42,14 +42,14 @@ class group_Core { * @see Identity_Driver::everbody. */ static function everybody() { - return Identity::instance()->everybody(); + return model_cache::get("group", 1); } /** * @see Identity_Driver::registered_users. */ static function registered_users() { - return Identity::instance()->everybody(); + return model_cache::get("group", 2); } /** diff --git a/modules/user/helpers/user.php b/modules/user/helpers/user.php index fa7b320f..5ef2b726 100644 --- a/modules/user/helpers/user.php +++ b/modules/user/helpers/user.php @@ -25,14 +25,21 @@ */ class user_Core { /** - * @see Identity_Driver::guest. + * Return the guest user. + * + * @return User_Model the user object */ static function guest() { return model_cache::get("user", 1); } /** - * @see Identity_Driver::create_user. + * Create a new user. + * + * @param string $name + * @param string $full_name + * @param string $password + * @return User_Definition the user object */ static function create($name, $full_name, $password) { $user = ORM::factory("user")->where("name", $name)->find(); @@ -53,7 +60,9 @@ class user_Core { } /** - * @see Identity_Driver::hash_password. + * Hash the password to the internal value + * @param string $password the user password + * @param string The hashed equivalent */ static function hash_password($password) { require_once(MODPATH . "user/lib/PasswordHash.php"); diff --git a/modules/user/libraries/drivers/Identity/Gallery.php b/modules/user/libraries/drivers/Identity/Gallery.php index 77db11a3..f405b710 100644 --- a/modules/user/libraries/drivers/Identity/Gallery.php +++ b/modules/user/libraries/drivers/Identity/Gallery.php @@ -25,14 +25,14 @@ class Identity_Gallery_Driver implements Identity_Driver { * @see Identity_Driver::guest. */ public function guest() { - return new Gallery_User(user::guest()); + return user::guest(); } /** * @see Identity_Driver::create_user. */ public function create_user($name, $full_name, $password) { - return new Gallery_User(user::create($name, $full_name, $password)); + return user::create($name, $full_name, $password); } /** @@ -67,122 +67,55 @@ class Identity_Gallery_Driver implements Identity_Driver { } /** - * @see Identity_Driver::hash_password. + * @see Identity_Driver::lookup_user. */ - public function hash_password($password) { - return user::hash_password($password); + public function lookup_user($id) { + return user::lookup_by_field("id", $id); } /** - * @see Identity_Driver::lookup_user_by_field. + * @see Identity_Driver::lookup_user_by_name. */ - public function lookup_user_by_field($field_name, $value) { - return new Gallery_User(user::lookup_by_field($field_name, $value)); + public function lookup_user_by_name($name) { + return user::lookup_by_field("name", $name); } /** * @see Identity_Driver::create_group. */ public function create_group($name) { - return new Gallery_Group(group::create($name)); + return group::create($name); } /** * @see Identity_Driver::everybody. */ public function everybody() { - return new Gallery_Group(group::everybody()); + return group::everybody(); } /** * @see Identity_Driver::registered_users. */ public function registered_users() { - return new Gallery_Group(group::registered_users()); + return group::registered_users(); } /** - * @see Identity_Driver::lookup_group_by_field. + * @see Identity_Driver::lookup_group_by_name. */ - public function lookup_group_by_field($field_name, $value) { - return new Gallery_Group(group::lookup_by_field($field_name, $value)); + static function lookup_group_by_name($name) { + return group::lookup_by_field("name", $name); } /** * @see Identity_Driver::get_user_list. */ public function get_user_list($ids) { - $results = ORM::factory("user") + return ORM::factory("user") ->in("id", ids) ->find_all() - ->as_array();; - $users = array(); - foreach ($results as $user) { - $users[] = new Gallery_User($user); - } - return $users; + ->as_array(); } } // End Identity Gallery Driver -/** - * User Data wrapper - */ -class Gallery_User extends User_Definition { - /* - * Not for general user, allows the back-end to easily create the interface object - */ - function __construct($user) { - $this->user = $user; - } - - /** - * @see User_Definition::avatar_url - */ - public function avatar_url($size=80, $default=null) { - return $this->user->avatar_url($size, $default); - } - - /** - * @see User_Definition::display_name - */ - public function display_name() { - return $this->user->display_name(); - } - - public function save() { - $this->user->save(); - } - - public function delete() { - $this->user->delete(); - } - -} - -/** - * Group Data wrapper - */ -class Gallery_Group extends Group_Definition { - /* - * Not for general user, allows the back-end to easily create the interface object - */ - function __construct($group) { - $this->group = $group; - } - - public function save() { - $this->group->save(); - } - - public function delete() { - $this->group->delete(); - } - - public function add($user) { - $this->group->add($user->_uncloaked()); - } - - public function remove($user) { - $this->group->remove($user->_uncloaked()); - } -} diff --git a/modules/user/models/group.php b/modules/user/models/group.php index 8af78012..4432fc69 100644 --- a/modules/user/models/group.php +++ b/modules/user/models/group.php @@ -17,7 +17,7 @@ * along with this program; if not, write to the Free Software * Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA 02110-1301, USA. */ -class Group_Model extends ORM { +class Group_Model extends ORM implements Group_Definition { protected $has_and_belongs_to_many = array("users"); var $rules = array( diff --git a/modules/user/models/user.php b/modules/user/models/user.php index d99603b2..c51fc720 100644 --- a/modules/user/models/user.php +++ b/modules/user/models/user.php @@ -17,7 +17,7 @@ * along with this program; if not, write to the Free Software * Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA 02110-1301, USA. */ -class User_Model extends ORM { +class User_Model extends ORM implements User_Definition { protected $has_and_belongs_to_many = array("groups"); var $rules = array( diff --git a/modules/user/views/admin_users.html.php b/modules/user/views/admin_users.html.php index 7c54d93d..ee8d413c 100644 --- a/modules/user/views/admin_users.html.php +++ b/modules/user/views/admin_users.html.php @@ -91,7 +91,7 @@ open_text="" class="g-panel-link g-button ui-state-default ui-corner-all ui-icon-left"> - id != $user->id && !$user->guest): ?> + id != $user->id && !$user->guest): ?> id") ?>" class="g-dialog-link g-button ui-state-default ui-corner-all ui-icon-left"> diff --git a/modules/user/views/reset_password.html.php b/modules/user/views/reset_password.html.php new file mode 100644 index 00000000..92ca4917 --- /dev/null +++ b/modules/user/views/reset_password.html.php @@ -0,0 +1,17 @@ + + + + <?= t("Password Reset Request") ?> + + +

+

+ $user->full_name ? $user->full_name : $user->name)) ?> +

+

+ %site_url. If you made this request, you can confirm it by clicking this link. If you didn't request this password reset, it's ok to ignore this mail.", + array("site_url" => html::mark_clean(url::base(false, "http")), + "confirm_url" => $confirm_url)) ?> +

+ + -- cgit v1.2.3 From 1d3651b3292a2ea4b63a6c19bc0a4f58b9efac10 Mon Sep 17 00:00:00 2001 From: Tim Almdal Date: Wed, 28 Oct 2009 17:13:23 -0700 Subject: Make the user name a required field. Fixes ticket #852 --- modules/user/models/user.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'modules/user/models') diff --git a/modules/user/models/user.php b/modules/user/models/user.php index c51fc720..184ce70a 100644 --- a/modules/user/models/user.php +++ b/modules/user/models/user.php @@ -21,7 +21,7 @@ class User_Model extends ORM implements User_Definition { protected $has_and_belongs_to_many = array("groups"); var $rules = array( - "name" => "length[1,32]", + "name" => "required|length[1,32]", "full_name" => "length[0,255]", "email" => "valid_email|length[1,255]", "password" => "length[1,40]", -- cgit v1.2.3