From 1ada27916fa4575f6b093db17f4165d8cce16088 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta 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/gallery') 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 @@

- id}") ?>" + id}") ?>" title="for_html_attr() ?>" id="g-after-install-change-password-link" class="g-button ui-state-default ui-corners-all"> +

-- cgit v1.2.3 From 6353a7c2decd62098ebc96951c38c9aade44fc4c Mon Sep 17 00:00:00 2001 From: Andy Staudacher 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/gallery') 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 3439671bcfb99c1884285e4b4e53295f044e688f Mon Sep 17 00:00:00 2001 From: Tim Almdal Date: Fri, 12 Feb 2010 09:52:57 -0800 Subject: 1) Add a depth parameter to retrieving an item thru the rest api 2) Standardize the structure of members so that client programs can consistently parse the return information. 3) Added a summary parameter so that client programs can easily determine if the information returned is summary (item type, item title) or the full meal deal --- modules/gallery/helpers/item_rest.php | 14 ++++-------- modules/gallery/models/item.php | 41 +++++++++++++++++++++++++++++------ 2 files changed, 38 insertions(+), 17 deletions(-) (limited to 'modules/gallery') diff --git a/modules/gallery/helpers/item_rest.php b/modules/gallery/helpers/item_rest.php index c0fc422a..72230d8b 100644 --- a/modules/gallery/helpers/item_rest.php +++ b/modules/gallery/helpers/item_rest.php @@ -30,6 +30,9 @@ class item_rest_Core { * name= * only return items where the name contains this substring * + * depth= + * return the children to the depth specified. + * * random=true * return a single random item * @@ -70,16 +73,7 @@ class item_rest_Core { $orm->where("type", "IN", explode(",", $p->type)); } - $members = array(); - foreach ($orm->find_all() as $child) { - $members[] = rest::url("item", $child); - } - - return array( - "url" => $request->url, - "entity" => $item->as_restful_array(), - "members" => $members, - "relationships" => rest::relationships("item", $item)); + return $item->as_restful_array(isset($p->depth) ? $p->depth : 0); } static function put($request) { diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php index dbd56fa2..a1be4fbc 100644 --- a/modules/gallery/models/item.php +++ b/modules/gallery/models/item.php @@ -918,22 +918,49 @@ class Item_Model extends ORM_MPTT { /** * Same as ORM::as_array() but convert id fields into their RESTful form. */ - public function as_restful_array() { + public function as_restful_array($depth=0, $level=0) { // Convert item ids to rest URLs for consistency - $data = $this->as_array(); + $data = array("url" => rest::url("item", $this), + "entity" => $this->as_array(), + "members" => array(), + "relationships" => array()); + if ($tmp = $this->parent()) { - $data["parent"] = rest::url("item", $tmp); + $data["entity"]["parent"] = rest::url("item", $tmp); } - unset($data["parent_id"]); + unset($data["entity"]["parent_id"]); if ($tmp = $this->album_cover()) { - $data["album_cover"] = rest::url("item", $tmp); + $data["entity"]["album_cover"] = rest::url("item", $tmp); } - unset($data["album_cover_item_id"]); + unset($data["entity"]["album_cover_item_id"]); // Elide some internal-only data that is going to cause confusion in the client. foreach (array("relative_path_cache", "relative_url_cache", "left_ptr", "right_ptr") as $key) { - unset($data[$key]); + unset($data["entity"][$key]); + } + + // check that we have given enough information. At this point we don't + // return relationships and we give enough information to determine how to handle + // the children. + $summarize = $depth < $level; + if (!$summarize) { + $data["relationships"] = rest::relationships("item", $this); + } + + $next_level = $level + 1; + foreach ($this->children() as $child) { + if ($summarize) { + $data["members"][] = array("url" => rest::url("item", $child), + "entity" => array("title" => $child->title, + "type" => $child->type), + "members" => array(), + "summary" => true, + "relationships" => array()); + } else { + $data["members"][] = $child->as_restful_array($depth, $next_level); + } } + $data["summary"] = $summarize; return $data; } } -- cgit v1.2.3 From ce71ea6aa7eac72e54b1a9d7722c87beb61327de Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Fri, 12 Feb 2010 04:53:26 -0800 Subject: Revert "1) Add a depth parameter to retrieving an item thru the rest api" This reverts commit 3439671bcfb99c1884285e4b4e53295f044e688f. --- modules/gallery/helpers/item_rest.php | 14 ++++++++---- modules/gallery/models/item.php | 41 ++++++----------------------------- 2 files changed, 17 insertions(+), 38 deletions(-) (limited to 'modules/gallery') diff --git a/modules/gallery/helpers/item_rest.php b/modules/gallery/helpers/item_rest.php index 72230d8b..c0fc422a 100644 --- a/modules/gallery/helpers/item_rest.php +++ b/modules/gallery/helpers/item_rest.php @@ -30,9 +30,6 @@ class item_rest_Core { * name= * only return items where the name contains this substring * - * depth= - * return the children to the depth specified. - * * random=true * return a single random item * @@ -73,7 +70,16 @@ class item_rest_Core { $orm->where("type", "IN", explode(",", $p->type)); } - return $item->as_restful_array(isset($p->depth) ? $p->depth : 0); + $members = array(); + foreach ($orm->find_all() as $child) { + $members[] = rest::url("item", $child); + } + + return array( + "url" => $request->url, + "entity" => $item->as_restful_array(), + "members" => $members, + "relationships" => rest::relationships("item", $item)); } static function put($request) { diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php index a1be4fbc..dbd56fa2 100644 --- a/modules/gallery/models/item.php +++ b/modules/gallery/models/item.php @@ -918,49 +918,22 @@ class Item_Model extends ORM_MPTT { /** * Same as ORM::as_array() but convert id fields into their RESTful form. */ - public function as_restful_array($depth=0, $level=0) { + public function as_restful_array() { // Convert item ids to rest URLs for consistency - $data = array("url" => rest::url("item", $this), - "entity" => $this->as_array(), - "members" => array(), - "relationships" => array()); - + $data = $this->as_array(); if ($tmp = $this->parent()) { - $data["entity"]["parent"] = rest::url("item", $tmp); + $data["parent"] = rest::url("item", $tmp); } - unset($data["entity"]["parent_id"]); + unset($data["parent_id"]); if ($tmp = $this->album_cover()) { - $data["entity"]["album_cover"] = rest::url("item", $tmp); + $data["album_cover"] = rest::url("item", $tmp); } - unset($data["entity"]["album_cover_item_id"]); + unset($data["album_cover_item_id"]); // Elide some internal-only data that is going to cause confusion in the client. foreach (array("relative_path_cache", "relative_url_cache", "left_ptr", "right_ptr") as $key) { - unset($data["entity"][$key]); - } - - // check that we have given enough information. At this point we don't - // return relationships and we give enough information to determine how to handle - // the children. - $summarize = $depth < $level; - if (!$summarize) { - $data["relationships"] = rest::relationships("item", $this); - } - - $next_level = $level + 1; - foreach ($this->children() as $child) { - if ($summarize) { - $data["members"][] = array("url" => rest::url("item", $child), - "entity" => array("title" => $child->title, - "type" => $child->type), - "members" => array(), - "summary" => true, - "relationships" => array()); - } else { - $data["members"][] = $child->as_restful_array($depth, $next_level); - } + unset($data[$key]); } - $data["summary"] = $summarize; return $data; } } -- cgit v1.2.3 From d53f6d0e052fb455059170a311640fcd06cad798 Mon Sep 17 00:00:00 2001 From: Andy Staudacher Date: Fri, 12 Feb 2010 16:40:44 -0800 Subject: Fix for tickets 1009 and 603: Show a themed error page to guests / registered users (not to admins though). And show a login form to guests for 404 (incl. insufficient view permissions) errors. --- modules/gallery/controllers/albums.php | 13 +-- modules/gallery/controllers/items.php | 2 +- modules/gallery/controllers/movies.php | 2 +- modules/gallery/controllers/photos.php | 2 +- modules/gallery/helpers/access.php | 2 +- modules/gallery/helpers/auth.php | 14 --- modules/gallery/libraries/MY_Kohana_Exception.php | 59 +++++++++- modules/gallery/views/error.html.php | 12 ++ modules/gallery/views/error_404.html.php | 21 ++++ modules/gallery/views/kohana_error_page.php | 127 ---------------------- 10 files changed, 97 insertions(+), 157 deletions(-) create mode 100644 modules/gallery/views/error.html.php create mode 100644 modules/gallery/views/error_404.html.php delete mode 100644 modules/gallery/views/kohana_error_page.php (limited to 'modules/gallery') diff --git a/modules/gallery/controllers/albums.php b/modules/gallery/controllers/albums.php index c2b474ee..1cc3b1ec 100644 --- a/modules/gallery/controllers/albums.php +++ b/modules/gallery/controllers/albums.php @@ -26,19 +26,10 @@ 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 - throw new Kohana_404_Exception(); + Event::run('system.404'); } - if (!access::can("view", $album)) { - 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); - } - } + access::required("view", $album); $page_size = module::get_var("gallery", "page_size", 9); $input = Input::instance(); diff --git a/modules/gallery/controllers/items.php b/modules/gallery/controllers/items.php index f261e3a9..0bd47b2d 100644 --- a/modules/gallery/controllers/items.php +++ b/modules/gallery/controllers/items.php @@ -21,7 +21,7 @@ class Items_Controller extends Controller { public function __call($function, $args) { $item = ORM::factory("item", (int)$function); if (!$item->loaded()) { - throw new Kohana_404_Exception(); + Event::run('system.404'); } // Redirect to the more specific resource type, since it will render diff --git a/modules/gallery/controllers/movies.php b/modules/gallery/controllers/movies.php index 78a56e81..1dbcb481 100644 --- a/modules/gallery/controllers/movies.php +++ b/modules/gallery/controllers/movies.php @@ -22,7 +22,7 @@ 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 - throw new Kohana_404_Exception(); + Event::run('system.404'); } access::required("view", $movie); diff --git a/modules/gallery/controllers/photos.php b/modules/gallery/controllers/photos.php index f2d47eec..2a77aea4 100644 --- a/modules/gallery/controllers/photos.php +++ b/modules/gallery/controllers/photos.php @@ -22,7 +22,7 @@ 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 - throw new Kohana_404_Exception(); + Event::run('system.404'); } access::required("view", $photo); diff --git a/modules/gallery/helpers/access.php b/modules/gallery/helpers/access.php index 7e8b079a..c4c100ca 100644 --- a/modules/gallery/helpers/access.php +++ b/modules/gallery/helpers/access.php @@ -120,7 +120,7 @@ class access_Core { if (!self::can($perm_name, $item)) { if ($perm_name == "view") { // Treat as if the item didn't exist, don't leak any information. - throw new Kohana_404_Exception(); + Event::run('system.404'); } else { self::forbidden(); } diff --git a/modules/gallery/helpers/auth.php b/modules/gallery/helpers/auth.php index 8b0ce470..c3e9e6e9 100644 --- a/modules/gallery/helpers/auth.php +++ b/modules/gallery/helpers/auth.php @@ -130,18 +130,4 @@ class auth_Core { $session->set("admin_area_activity_timestamp", time()); return false; } - - /** - * Returns the themed login page. - */ - 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. - $continue_url or $continue_url = url::current(true); - Session::instance()->set("continue_url", $continue_url); - return $view; - } } \ No newline at end of file diff --git a/modules/gallery/libraries/MY_Kohana_Exception.php b/modules/gallery/libraries/MY_Kohana_Exception.php index 1c40091a..d6f1f467 100644 --- a/modules/gallery/libraries/MY_Kohana_Exception.php +++ b/modules/gallery/libraries/MY_Kohana_Exception.php @@ -33,6 +33,63 @@ class Kohana_Exception extends Kohana_Exception_Core { if ($e instanceof ORM_Validation_Exception) { Kohana_Log::add("error", "Validation errors: " . print_r($e->validation->errors(), 1)); } - return parent::handle($e); + try { + $user = identity::active_user(); + $try_themed_view = $user && !$user->admin; + } catch (Exception $e2) { + $try_themed_view = false; + } + + if ($try_themed_view) { + try { + return self::_show_themed_error_page($e); + } catch (Exception $e3) { + Kohana_Log::add("error", "Exception in exception handling code: " . self::text($e3)); + return parent::handle($e); + } + } else { + return parent::handle($e); + } + } + + /** + * Shows a themed error page. + * @see Kohana_Exception::handle + */ + private static function _show_themed_error_page(Exception $e) { + // Create a text version of the exception + $error = Kohana_Exception::text($e); + + // Add this exception to the log + Kohana_Log::add('error', $error); + + // Manually save logs after exceptions + Kohana_Log::save(); + + if (!headers_sent()) { + if ($e instanceof Kohana_Exception) { + $e->sendHeaders(); + } else { + header("HTTP/1.1 500 Internal Server Error"); + } + } + + $view = new Theme_View("page.html", "other", "error"); + if ($e instanceof Kohana_404_Exception) { + $view->page_title = t("Dang... Page not found!"); + $view->content = new View("error_404.html"); + $user = identity::active_user(); + $view->content->is_guest = $user && $user->guest; + if ($view->content->is_guest) { + $view->content->login_form = new View("login_ajax.html"); + $view->content->login_form->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)); + } + } else { + $view->page_title = t("Dang... Something went wrong!"); + $view->content = new View("error.html"); + } + print $view; } } \ No newline at end of file diff --git a/modules/gallery/views/error.html.php b/modules/gallery/views/error.html.php new file mode 100644 index 00000000..5d81b651 --- /dev/null +++ b/modules/gallery/views/error.html.php @@ -0,0 +1,12 @@ + +

+

+ +

+

+ +

+

+ +

+
\ No newline at end of file diff --git a/modules/gallery/views/error_404.html.php b/modules/gallery/views/error_404.html.php new file mode 100644 index 00000000..e5846e65 --- /dev/null +++ b/modules/gallery/views/error_404.html.php @@ -0,0 +1,21 @@ + +
+

+ +

+ +

+ +

+

+ + +

+ + +

+ + +

+ +
\ No newline at end of file diff --git a/modules/gallery/views/kohana_error_page.php b/modules/gallery/views/kohana_error_page.php deleted file mode 100644 index b9fdcc19..00000000 --- a/modules/gallery/views/kohana_error_page.php +++ /dev/null @@ -1,127 +0,0 @@ - - - - - - <?= t("Something went wrong!") ?> - - - - admin ?> -
-

- -

-

- -

- -

- -

- -
- -
-

- -

- - - - - - - getTraceAsString(); ?> - - - - - - -- cgit v1.2.3 From e88e976fc4255d323a1f5919453604ecf467ed0d Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Fri, 12 Feb 2010 13:49:14 -0800 Subject: Tighten up the text. --- modules/gallery/views/error_404.html.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'modules/gallery') diff --git a/modules/gallery/views/error_404.html.php b/modules/gallery/views/error_404.html.php index e5846e65..4b037a79 100644 --- a/modules/gallery/views/error_404.html.php +++ b/modules/gallery/views/error_404.html.php @@ -15,7 +15,7 @@

- +

\ No newline at end of file -- cgit v1.2.3 From 8412aeb1337c4060a77af7c94441ca8cbc6bb4ad Mon Sep 17 00:00:00 2001 From: Andy Staudacher Date: Fri, 12 Feb 2010 19:05:44 -0800 Subject: For consistency, use straight Kohana_404_Exception instead of the event system. --- modules/gallery/controllers/albums.php | 2 +- modules/gallery/controllers/items.php | 2 +- modules/gallery/controllers/movies.php | 2 +- modules/gallery/controllers/photos.php | 2 +- modules/gallery/helpers/access.php | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) (limited to 'modules/gallery') diff --git a/modules/gallery/controllers/albums.php b/modules/gallery/controllers/albums.php index 1cc3b1ec..036dade0 100644 --- a/modules/gallery/controllers/albums.php +++ b/modules/gallery/controllers/albums.php @@ -26,7 +26,7 @@ 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 - Event::run('system.404'); + throw new Kohana_404_Exception(); } access::required("view", $album); diff --git a/modules/gallery/controllers/items.php b/modules/gallery/controllers/items.php index 0bd47b2d..f261e3a9 100644 --- a/modules/gallery/controllers/items.php +++ b/modules/gallery/controllers/items.php @@ -21,7 +21,7 @@ class Items_Controller extends Controller { public function __call($function, $args) { $item = ORM::factory("item", (int)$function); if (!$item->loaded()) { - Event::run('system.404'); + throw new Kohana_404_Exception(); } // Redirect to the more specific resource type, since it will render diff --git a/modules/gallery/controllers/movies.php b/modules/gallery/controllers/movies.php index 1dbcb481..78a56e81 100644 --- a/modules/gallery/controllers/movies.php +++ b/modules/gallery/controllers/movies.php @@ -22,7 +22,7 @@ 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 - Event::run('system.404'); + throw new Kohana_404_Exception(); } access::required("view", $movie); diff --git a/modules/gallery/controllers/photos.php b/modules/gallery/controllers/photos.php index 2a77aea4..f2d47eec 100644 --- a/modules/gallery/controllers/photos.php +++ b/modules/gallery/controllers/photos.php @@ -22,7 +22,7 @@ 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 - Event::run('system.404'); + throw new Kohana_404_Exception(); } access::required("view", $photo); diff --git a/modules/gallery/helpers/access.php b/modules/gallery/helpers/access.php index c4c100ca..7e8b079a 100644 --- a/modules/gallery/helpers/access.php +++ b/modules/gallery/helpers/access.php @@ -120,7 +120,7 @@ class access_Core { if (!self::can($perm_name, $item)) { if ($perm_name == "view") { // Treat as if the item didn't exist, don't leak any information. - Event::run('system.404'); + throw new Kohana_404_Exception(); } else { self::forbidden(); } -- cgit v1.2.3 From 2dad1d7cd15bed3c89de11cadf4b3b5c43134f69 Mon Sep 17 00:00:00 2001 From: Andy Staudacher Date: Fri, 12 Feb 2010 20:59:26 -0800 Subject: Some HTML validation fixes (don't render empty