From 6a40e0a341d84e792990b22b10375b80e41bf6ba Mon Sep 17 00:00:00 2001
From: Bharat Mediratta <bharat@menalto.com>
Date: Tue, 9 Feb 2010 16:04:36 -0800
Subject: Revise the "review your permission" text to my liking.

---
 modules/g2_import/views/admin_g2_import.html.php | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'modules')

diff --git a/modules/g2_import/views/admin_g2_import.html.php b/modules/g2_import/views/admin_g2_import.html.php
index 3b7afec2..5c520172 100644
--- a/modules/g2_import/views/admin_g2_import.html.php
+++ b/modules/g2_import/views/admin_g2_import.html.php
@@ -13,7 +13,7 @@
       </p>
       <ul>
         <li>
-          <?= t("Please <b>review album permissions</b> after the import! Permissions are imported, but user specific and item specific permissions are not supported in Gallery 3 and thus ignored.") ?>
+          <?= t("Gallery 3 does not support per-user / per-item permissions.  <b>Review permissions after your import is done.</b>") ?>
         </li>
         <li>
           <?= t("The only supported file formats are JPG, PNG and GIF, FLV and MP4.  Other formats will be skipped.") ?>
-- 
cgit v1.2.3


From 1ada27916fa4575f6b093db17f4165d8cce16088 Mon Sep 17 00:00:00 2001
From: Bharat Mediratta <bharat@menalto.com>
Date: Thu, 11 Feb 2010 05:24:16 -0800
Subject: Use the admin/users/edit_user_form version of the user editing form
 right after initial install so that we're not requiring the user to re-enter
 the auto-generated password to change their password and email.

Fixes ticket #1007
---
 modules/gallery/views/welcome_message.html.php | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

(limited to 'modules')

diff --git a/modules/gallery/views/welcome_message.html.php b/modules/gallery/views/welcome_message.html.php
index caeeff66..4d6ed726 100644
--- a/modules/gallery/views/welcome_message.html.php
+++ b/modules/gallery/views/welcome_message.html.php
@@ -15,12 +15,15 @@
   </p>
 
   <p>
-    <a href="<?= url::site("user_profile/show/{$user->id}") ?>"
+    <a href="<?= url::site("admin/users/edit_user_form/{$user->id}") ?>"
       title="<?= t("Edit your profile")->for_html_attr() ?>"
       id="g-after-install-change-password-link"
       class="g-button ui-state-default ui-corners-all">
       <?= t("Change password and email now") ?>
     </a>
+    <script type="text/javascript">
+      $("#g-after-install-change-password-link").gallery_dialog();
+    </script>
   </p>
 
   <p>
-- 
cgit v1.2.3


From cd98f85260efd90cc93db78ee1efed997d0221c2 Mon Sep 17 00:00:00 2001
From: Andy Staudacher <andy.st@gmail.com>
Date: Thu, 11 Feb 2010 13:11:31 -0800
Subject: Fix for ticket 1010: Don't leak valid user names in "forgot password"
 form. Includes fixes for user forms as well (edit user / email / password).

---
 modules/user/controllers/password.php | 44 +++++++++++++++++++----------------
 modules/user/controllers/users.php    | 12 +++++-----
 2 files changed, 30 insertions(+), 26 deletions(-)

(limited to 'modules')

diff --git a/modules/user/controllers/password.php b/modules/user/controllers/password.php
index 07fdc1ed..c6d7e889 100644
--- a/modules/user/controllers/password.php
+++ b/modules/user/controllers/password.php
@@ -19,12 +19,19 @@
  */
 class Password_Controller extends Controller {
   public function reset() {
+    $form = self::_reset_form();
     if (request::method() == "post") {
       // @todo separate the post from get parts of this function
       access::verify_csrf();
-      $this->_send_reset();
+      // Basic validation (was some user name specified?)
+      if ($form->validate()) {
+        $this->_send_reset($form);
+      } else {
+        print json_encode(array("result" => "error",
+                                "form" => (string) $form));
+      }
     } else {
-      print $this->_reset_form();
+      print $form;
     }
   }
 
@@ -41,19 +48,9 @@ class Password_Controller extends Controller {
     }
   }
 
-  private function _send_reset() {
-    $form = $this->_reset_form();
-
-    $valid = $form->validate();
-    if ($valid) {
-      $user = user::lookup_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) {
+  private function _send_reset($form) {
+    $user = user::lookup_by_name($form->reset->inputs["name"]->value);
+    if ($user && !empty($user->email)) {
       $user->hash = md5(rand());
       $user->save();
       $message = new View("reset_password.html");
@@ -71,22 +68,29 @@ class Password_Controller extends Controller {
       log::success(
         "user",
         t("Password reset email sent for user %name", array("name" => $user->name)));
-    } else {
+    } else if (!$user) {
       // 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");
+          "user", t("Password reset email requested for bogus user"));
+    } else  {
+      log::warning(
+          "user", t("Password reset failed for %user_name (has no email address on record).",
+                    array("user_name" => $user->name)));
     }
 
+    // Always pretend that an email has been sent to avoid leaking
+    // information on what user names are actually real.
     message::success(t("Password reset email sent"));
     print json_encode(
       array("result" => "success"));
   }
 
-  private function _reset_form() {
+  private static 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->input("name")->label(t("Username"))->id("g-name")->class(null)
+      ->rules("required")
+      ->error_messages("required", t("You must enter a user name"));
     $group->submit("")->value(t("Reset"));
 
     return $form;
diff --git a/modules/user/controllers/users.php b/modules/user/controllers/users.php
index 0730f391..cd7d271f 100644
--- a/modules/user/controllers/users.php
+++ b/modules/user/controllers/users.php
@@ -20,7 +20,7 @@
 class Users_Controller extends Controller {
   public function update($id) {
     $user = user::lookup($id);
-    if ($user->guest || $user->id != identity::active_user()->id) {
+    if (!$user || $user->guest || $user->id != identity::active_user()->id) {
       access::forbidden();
     }
 
@@ -63,7 +63,7 @@ class Users_Controller extends Controller {
 
   public function change_password($id) {
     $user = user::lookup($id);
-    if ($user->guest || $user->id != identity::active_user()->id) {
+    if (!$user || $user->guest || $user->id != identity::active_user()->id) {
       access::forbidden();
     }
 
@@ -99,7 +99,7 @@ class Users_Controller extends Controller {
 
   public function change_email($id) {
     $user = user::lookup($id);
-    if ($user->guest || $user->id != identity::active_user()->id) {
+    if (!$user || $user->guest || $user->id != identity::active_user()->id) {
       access::forbidden();
     }
 
@@ -134,7 +134,7 @@ class Users_Controller extends Controller {
 
   public function form_edit($id) {
     $user = user::lookup($id);
-    if ($user->guest || $user->id != identity::active_user()->id) {
+    if (!$user || $user->guest || $user->id != identity::active_user()->id) {
       access::forbidden();
     }
 
@@ -143,7 +143,7 @@ class Users_Controller extends Controller {
 
   public function form_change_password($id) {
     $user = user::lookup($id);
-    if ($user->guest || $user->id != identity::active_user()->id) {
+    if (!$user || $user->guest || $user->id != identity::active_user()->id) {
       access::forbidden();
     }
 
@@ -152,7 +152,7 @@ class Users_Controller extends Controller {
 
   public function form_change_email($id) {
     $user = user::lookup($id);
-    if ($user->guest || $user->id != identity::active_user()->id) {
+    if (!$user || $user->guest || $user->id != identity::active_user()->id) {
       access::forbidden();
     }
 
-- 
cgit v1.2.3


From 6353a7c2decd62098ebc96951c38c9aade44fc4c Mon Sep 17 00:00:00 2001
From: Andy Staudacher <andy.st@gmail.com>
Date: Thu, 11 Feb 2010 14:28:32 -0800
Subject: Security: Fix leaking of album / photo names. Reject previous fix for
 ticket 1009. Side effect: Renaming auth::required_login() to login_page().

---
 modules/gallery/controllers/albums.php | 12 +++++++++---
 modules/gallery/controllers/movies.php |  7 ++-----
 modules/gallery/controllers/photos.php | 11 ++++-------
 modules/gallery/helpers/access.php     |  7 ++++++-
 modules/gallery/helpers/auth.php       |  7 ++++---
 5 files changed, 25 insertions(+), 19 deletions(-)

(limited to 'modules')

diff --git a/modules/gallery/controllers/albums.php b/modules/gallery/controllers/albums.php
index e1985cfb..c2b474ee 100644
--- a/modules/gallery/controllers/albums.php
+++ b/modules/gallery/controllers/albums.php
@@ -26,12 +26,18 @@ class Albums_Controller extends Items_Controller {
     if (!is_object($album)) {
       // show() must be public because we route to it in url::parse_url(), so make
       // sure that we're actually receiving an object
-      Kohana::show_404();
+      throw new Kohana_404_Exception();
     }
 
     if (!access::can("view", $album)) {
-      print auth::require_login();
-      return;
+      if ($album->id == 1) {
+        // Even show the login page to logged in users.
+        // It's a better user experience than a "Dang" error page.
+        print auth::login_page();
+        return;
+      } else {
+        access::required("view", $album);
+      }
     }
 
     $page_size = module::get_var("gallery", "page_size", 9);
diff --git a/modules/gallery/controllers/movies.php b/modules/gallery/controllers/movies.php
index 8041066e..78a56e81 100644
--- a/modules/gallery/controllers/movies.php
+++ b/modules/gallery/controllers/movies.php
@@ -22,13 +22,10 @@ class Movies_Controller extends Items_Controller {
     if (!is_object($movie)) {
       // show() must be public because we route to it in url::parse_url(), so make
       // sure that we're actually receiving an object
-      Kohana::show_404();
+      throw new Kohana_404_Exception();
     }
 
-    if (!access::can("view", $movie)) {
-      print auth::require_login();
-      return;
-    }
+    access::required("view", $movie);
 
     $where = array(array("type", "!=", "album"));
     $position = $movie->parent()->get_position($movie, $where);
diff --git a/modules/gallery/controllers/photos.php b/modules/gallery/controllers/photos.php
index 778e9ae7..f2d47eec 100644
--- a/modules/gallery/controllers/photos.php
+++ b/modules/gallery/controllers/photos.php
@@ -22,14 +22,11 @@ class Photos_Controller extends Items_Controller {
     if (!is_object($photo)) {
       // show() must be public because we route to it in url::parse_url(), so make
       // sure that we're actually receiving an object
-      Kohana::show_404();
+      throw new Kohana_404_Exception();
     }
-
-    if (!access::can("view", $photo)) {
-      print auth::require_login();
-      return;
-    }
-
+  
+    access::required("view", $photo);
+  
     $where = array(array("type", "!=", "album"));
     $position = $photo->parent()->get_position($photo, $where);
     if ($position > 1) {
diff --git a/modules/gallery/helpers/access.php b/modules/gallery/helpers/access.php
index 29b981e8..7e8b079a 100644
--- a/modules/gallery/helpers/access.php
+++ b/modules/gallery/helpers/access.php
@@ -118,7 +118,12 @@ class access_Core {
    */
   static function required($perm_name, $item) {
     if (!self::can($perm_name, $item)) {
-      self::forbidden();
+      if ($perm_name == "view") {
+        // Treat as if the item didn't exist, don't leak any information.
+        throw new Kohana_404_Exception();
+      } else {
+        self::forbidden();
+      }
     }
   }
 
diff --git a/modules/gallery/helpers/auth.php b/modules/gallery/helpers/auth.php
index f5454f85..8b0ce470 100644
--- a/modules/gallery/helpers/auth.php
+++ b/modules/gallery/helpers/auth.php
@@ -132,15 +132,16 @@ class auth_Core {
   }
 
   /**
-   * Redirect to the login page.
+   * Returns the themed login page.
    */
-  static function require_login() {
+  static function login_page($continue_url=null) {
     $view = new Theme_View("page.html", "other", "login");
     $view->page_title = t("Log in to Gallery");
     $view->content = new View("login_ajax.html");
     $view->content->form = auth::get_login_form("login/auth_html");
     // Avoid anti-phishing protection by passing the url as session variable.
-    Session::instance()->set("continue_url", url::current(true));
+    $continue_url or $continue_url = url::current(true);
+    Session::instance()->set("continue_url", $continue_url);
     return $view;
   }
 }
\ No newline at end of file
-- 
cgit v1.2.3


From dc94f6e45a7d45747582cd0ab99439330cd844f1 Mon Sep 17 00:00:00 2001
From: Andy Staudacher <andy.st@gmail.com>
Date: Thu, 11 Feb 2010 14:35:05 -0800
Subject: Include user name in logging message for failed password reset. As
 Bharat points out, t() ensures that parameters are escaped for XSS.

---
 modules/user/controllers/password.php | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

(limited to 'modules')

diff --git a/modules/user/controllers/password.php b/modules/user/controllers/password.php
index c6d7e889..2f8dd990 100644
--- a/modules/user/controllers/password.php
+++ b/modules/user/controllers/password.php
@@ -49,7 +49,8 @@ class Password_Controller extends Controller {
   }
 
   private function _send_reset($form) {
-    $user = user::lookup_by_name($form->reset->inputs["name"]->value);
+    $user_name = $form->reset->inputs["name"]->value;
+    $user = user::lookup_by_name($user_name);
     if ($user && !empty($user->email)) {
       $user->hash = md5(rand());
       $user->save();
@@ -71,7 +72,8 @@ class Password_Controller extends Controller {
     } else if (!$user) {
       // Don't include the username here until you're sure that it's XSS safe
       log::warning(
-          "user", t("Password reset email requested for bogus user"));
+                   "user", t("Password reset email requested for user %user_name, which does not exist.",
+                             array("user_name" => $user_name)));
     } else  {
       log::warning(
           "user", t("Password reset failed for %user_name (has no email address on record).",
-- 
cgit v1.2.3


From cd45c94fe69f24b0f80a79e4b9c402763739ecfc Mon Sep 17 00:00:00 2001
From: Andy Staudacher <andy.st@gmail.com>
Date: Thu, 11 Feb 2010 15:59:17 -0800
Subject: Get rid of unnecessary view file.

---
 modules/user/controllers/password.php              | 7 +++----
 modules/user/views/confirm_reset_password.html.php | 2 --
 2 files changed, 3 insertions(+), 6 deletions(-)
 delete mode 100644 modules/user/views/confirm_reset_password.html.php

(limited to 'modules')

diff --git a/modules/user/controllers/password.php b/modules/user/controllers/password.php
index 2f8dd990..f5190974 100644
--- a/modules/user/controllers/password.php
+++ b/modules/user/controllers/password.php
@@ -116,20 +116,19 @@ class Password_Controller extends Controller {
       "mistyped", t("The password and the confirm password must match"));
     $group->submit("")->value(t("Update"));
 
-    $template->content = new View("confirm_reset_password.html");
-    $template->content->form = $form;
+    $template->content = $form;
     return $template;
   }
 
   private function _change_password() {
     $view = $this->_new_password_form();
-    if ($view->content->form->validate()) {
+    if ($view->content->validate()) {
       $user = user::lookup_by_hash(Input::instance()->post("hash"));
       if (empty($user)) {
         throw new Exception("@todo FORBIDDEN", 503);
       }
 
-      $user->password = $view->content->form->reset->password->value;
+      $user->password = $view->content->reset->password->value;
       $user->hash = null;
       $user->save();
       message::success(t("Password reset successfully"));
diff --git a/modules/user/views/confirm_reset_password.html.php b/modules/user/views/confirm_reset_password.html.php
deleted file mode 100644
index 4993189e..00000000
--- a/modules/user/views/confirm_reset_password.html.php
+++ /dev/null
@@ -1,2 +0,0 @@
-<?php defined("SYSPATH") or die("No direct script access.") ?>
-<?= $form ?>
\ No newline at end of file
-- 
cgit v1.2.3