From 1ec1f19f1a388ed8f49af13d0de77e714c286811 Mon Sep 17 00:00:00 2001 From: Tim Almdal Date: Mon, 27 Jul 2009 11:12:27 -0700 Subject: [PATCH 1/3] Back out fixes for ticket #452 Revert "It helps to save before committing :-)" This reverts commit 0d76d6fd77f53e9e92a9a013cd112c69217f3ceb. --- modules/gallery/helpers/access.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/modules/gallery/helpers/access.php b/modules/gallery/helpers/access.php index fbe0b55067..79394d35bc 100644 --- a/modules/gallery/helpers/access.php +++ b/modules/gallery/helpers/access.php @@ -99,6 +99,7 @@ static function user_can($user, $perm_name, $item) { return true; } + print "Before owner id check\n"; if ($item->owner_id == $user->id && in_array($perm_name, array("view_full", "edit", "add"))) { return true; @@ -109,7 +110,9 @@ static function user_can($user, $perm_name, $item) { } else { $resource = model_cache::get("access_cache", $item->id, "item_id"); } + print Kohana::debug($resource->as_array()) . "\n"; foreach ($user->groups as $group) { + print "$group->name\n"; if ($resource->__get("{$perm_name}_{$group->id}") === self::ALLOW) { return true; } From 5fd82a2edea41209a6936f89c56bbd53083ed182 Mon Sep 17 00:00:00 2001 From: Tim Almdal Date: Mon, 27 Jul 2009 11:13:20 -0700 Subject: [PATCH 2/3] Back out the fix for ticket #452 Revert "Changed access::user_can to force the owner of an item to have" This reverts commit 0b97cfd6f098be08be5f3cf1dbca1cce580ae330. --- modules/gallery/helpers/access.php | 17 ++------ modules/gallery/tests/Access_Helper_Test.php | 42 -------------------- 2 files changed, 3 insertions(+), 56 deletions(-) diff --git a/modules/gallery/helpers/access.php b/modules/gallery/helpers/access.php index 79394d35bc..8c6f5d544b 100644 --- a/modules/gallery/helpers/access.php +++ b/modules/gallery/helpers/access.php @@ -95,24 +95,13 @@ static function user_can($user, $perm_name, $item) { return false; } - if ($user->admin) { + if ($user->admin && $item->owner_id == $user->id) { return true; } - print "Before owner id check\n"; - if ($item->owner_id == $user->id && - in_array($perm_name, array("view_full", "edit", "add"))) { - return true; - } - - if ($perm_name == "view") { - $resource = $item->owner_id == $user->id ? $item->parent() : $item; - } else { - $resource = model_cache::get("access_cache", $item->id, "item_id"); - } - print Kohana::debug($resource->as_array()) . "\n"; + $resource = $perm_name == "view" ? + $item : model_cache::get("access_cache", $item->id, "item_id"); foreach ($user->groups as $group) { - print "$group->name\n"; if ($resource->__get("{$perm_name}_{$group->id}") === self::ALLOW) { return true; } diff --git a/modules/gallery/tests/Access_Helper_Test.php b/modules/gallery/tests/Access_Helper_Test.php index 737ed8a6ff..59cec453e8 100644 --- a/modules/gallery/tests/Access_Helper_Test.php +++ b/modules/gallery/tests/Access_Helper_Test.php @@ -101,48 +101,6 @@ public function user_can_no_access_test() { $this->assert_false(access::user_can($user, "view", $item), "Should be unable to view"); } - public function owner_can_view_album_test() { - $user = user::create("access_test", "Access Test", ""); - foreach ($user->groups as $group) { - $user->remove($group); - } - $user->save(); - - $root = ORM::factory("item", 1); - $item = album::create($root, rand(), "test album", $user->id); - - $this->assert_true(access::user_can($user, "view", $item), "Should be able to view"); - } - - public function owner_can_view_photo_test() { - $user = user::create("access_test", "Access Test", ""); - foreach ($user->groups as $group) { - $user->remove($group); - } - $user->save(); - - $root = ORM::factory("item", 1); - $album = album::create($root, rand(), "test album", $user->id); - $item = photo::create($album, MODPATH . "gallery/images/gallery.png", "", "", null, $user->id); - - $this->assert_true(access::user_can($user, "view", $item), "Should be able to view"); - } - - public function owner_cant_view_photo_test() { - $user = user::create("access_test", "Access Test", ""); - foreach ($user->groups as $group) { - $user->remove($group); - } - $user->save(); - - $root = ORM::factory("item", 1); - $album = album::create($root, rand(), "test album"); - access::deny(group::everybody(), "view", $album); - $item = photo::create($album, MODPATH . "gallery/images/gallery.png", "", "", null, $user->id); - - $this->assert_false(access::user_can($user, "view", $item), "Should not be able to view"); - } - public function adding_and_removing_items_adds_ands_removes_rows_test() { $root = ORM::factory("item", 1); $item = album::create($root, rand(), "test album"); From 4edf86f0ebfedbbdfda3daf71ed55a461edf9c6c Mon Sep 17 00:00:00 2001 From: Tim Almdal Date: Mon, 27 Jul 2009 11:14:03 -0700 Subject: [PATCH 3/3] Revert "Fix for ticket #452" This reverts commit 809e52d80cbf3beb75b238fddb0da3951fb9a8e7. --- modules/gallery/helpers/access.php | 2 +- modules/gallery/models/item.php | 22 ++++++++++++++++++---- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/modules/gallery/helpers/access.php b/modules/gallery/helpers/access.php index 8c6f5d544b..949aea840d 100644 --- a/modules/gallery/helpers/access.php +++ b/modules/gallery/helpers/access.php @@ -95,7 +95,7 @@ static function user_can($user, $perm_name, $item) { return false; } - if ($user->admin && $item->owner_id == $user->id) { + if ($user->admin) { return true; } diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php index 4556138036..d9dd88f58e 100644 --- a/modules/gallery/models/item.php +++ b/modules/gallery/models/item.php @@ -38,17 +38,31 @@ public function viewable() { if (user::active()->admin) { $this->view_restrictions = array(); } else { - $this->view_restrictions["owner_id"] = user::active()->id; foreach (user::group_ids() as $id) { - $this->view_restrictions["view_$id"] = access::ALLOW; + // Separate the first restriction from the rest to make it easier for us to formulate + // our where clause below + if (empty($this->view_restrictions)) { + $this->view_restrictions[0] = "view_$id"; + } else { + $this->view_restrictions[1]["view_$id"] = access::ALLOW; + } } } } + switch (count($this->view_restrictions)) { + case 0: + break; - if (!empty($this->view_restrictions)) { + case 1: + $this->where($this->view_restrictions[0], access::ALLOW); + break; + + default: $this->open_paren(); - $this->orwhere($this->view_restrictions); + $this->where($this->view_restrictions[0], access::ALLOW); + $this->orwhere($this->view_restrictions[1]); $this->close_paren(); + break; } return $this;