From 8a6556b30bc34d69284df6246f4010a8835f3bc2 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Fri, 17 Jul 2009 08:14:08 -0700 Subject: Fix a bug where moved items don't properly inherit permissions from their new target. After each move, recalculate the permissions for the new parent's hierarchy. Fixes ticket #552 --- modules/gallery/tests/Access_Helper_Test.php | 36 ++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) (limited to 'modules/gallery/tests/Access_Helper_Test.php') diff --git a/modules/gallery/tests/Access_Helper_Test.php b/modules/gallery/tests/Access_Helper_Test.php index d71bf971..1352b493 100644 --- a/modules/gallery/tests/Access_Helper_Test.php +++ b/modules/gallery/tests/Access_Helper_Test.php @@ -324,4 +324,40 @@ class Access_Helper_Test extends Unit_Test_Case { $this->assert_false(file_exists($album->resize_path() . "/.htaccess")); $this->assert_false(file_exists($album->thumb_path() . "/.htaccess")); } + + public function moved_items_inherit_new_permissions_test() { + user::set_active(user::lookup_by_name("admin")); + + $root = ORM::factory("item", 1); + $public_album = album::create($root, rand(), "public album"); + $public_photo = photo::create($public_album, MODPATH . "gallery/images/gallery.png", "", ""); + access::allow(group::everybody(), "view", $public_album); + + $root->reload(); // Account for MPTT changes + + $private_album = album::create($root, rand(), "private album"); + access::deny(group::everybody(), "view", $private_album); + $private_photo = photo::create($private_album, MODPATH . "gallery/images/gallery.png", "", ""); + + // Make sure that we now have a public photo and private photo. + $this->assert_true(access::group_can(group::everybody(), "view", $public_photo)); + $this->assert_false(access::group_can(group::everybody(), "view", $private_photo)); + + // Swap the photos + item::move($public_photo, $private_album); + $private_album->reload(); // Reload to get new MPTT pointers and cached perms. + $public_album->reload(); + $private_photo->reload(); + $public_photo->reload(); + + item::move($private_photo, $public_album); + $private_album->reload(); // Reload to get new MPTT pointers and cached perms. + $public_album->reload(); + $private_photo->reload(); + $public_photo->reload(); + + // Make sure that the public_photo is now private, and the private_photo is now public. + $this->assert_false(access::group_can(group::everybody(), "view", $public_photo)); + $this->assert_true(access::group_can(group::everybody(), "view", $private_photo)); + } } -- cgit v1.2.3 From d4104a23ec504e451e7a96f9798ea4ed695e9d97 Mon Sep 17 00:00:00 2001 From: Tim Almdal Date: Wed, 22 Jul 2009 15:16:56 -0700 Subject: Add explicit unit tests for access::user_can --- modules/gallery/helpers/access.php | 1 + modules/gallery/tests/Access_Helper_Test.php | 37 ++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+) (limited to 'modules/gallery/tests/Access_Helper_Test.php') diff --git a/modules/gallery/helpers/access.php b/modules/gallery/helpers/access.php index 65316a8a..956b4e5c 100644 --- a/modules/gallery/helpers/access.php +++ b/modules/gallery/helpers/access.php @@ -209,6 +209,7 @@ class access_Core { $access = model_cache::get("access_intent", $album->id, "item_id"); $access->__set("{$perm_name}_{$group->id}", $value); $access->save(); + $album->reload(); if ($perm_name == "view") { self::_update_access_view_cache($group, $album); diff --git a/modules/gallery/tests/Access_Helper_Test.php b/modules/gallery/tests/Access_Helper_Test.php index 1352b493..59cec453 100644 --- a/modules/gallery/tests/Access_Helper_Test.php +++ b/modules/gallery/tests/Access_Helper_Test.php @@ -64,6 +64,43 @@ class Access_Helper_Test extends Unit_Test_Case { $this->assert_false(array_key_exists("access_test_{$group->id}", $fields)); } + public function user_can_access_test() { + $access_test = group::create("access_test"); + + $root = ORM::factory("item", 1); + access::allow($access_test, "view", $root); + + $item = album::create($root, rand(), "test album"); + + access::deny(group::everybody(), "view", $item); + access::deny(group::registered_users(), "view", $item); + + $user = user::create("access_test", "Access Test", ""); + foreach ($user->groups as $group) { + $user->remove($group); + } + $user->add($access_test); + $user->save(); + + $this->assert_true(access::user_can($user, "view", $item), "Should be able to view"); + } + + public function user_can_no_access_test() { + $root = ORM::factory("item", 1); + $item = album::create($root, rand(), "test album"); + + access::deny(group::everybody(), "view", $item); + access::deny(group::registered_users(), "view", $item); + + $user = user::create("access_test", "Access Test", ""); + foreach ($user->groups as $group) { + $user->remove($group); + } + $user->save(); + + $this->assert_false(access::user_can($user, "view", $item), "Should be unable 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"); -- cgit v1.2.3 From 0b97cfd6f098be08be5f3cf1dbca1cce580ae330 Mon Sep 17 00:00:00 2001 From: Tim Almdal Date: Sun, 26 Jul 2009 09:29:29 -0700 Subject: Changed access::user_can to force the owner of an item to have view permission on the parent. Added a whitelist of allowable owner permissions. If the requested permission is view and the user requesting access is the owner, check that they have view permission to the parent. --- modules/gallery/helpers/access.php | 17 +++++++++-- modules/gallery/tests/Access_Helper_Test.php | 42 ++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 3 deletions(-) (limited to 'modules/gallery/tests/Access_Helper_Test.php') diff --git a/modules/gallery/helpers/access.php b/modules/gallery/helpers/access.php index 2faa922b..4f737c7f 100644 --- a/modules/gallery/helpers/access.php +++ b/modules/gallery/helpers/access.php @@ -95,13 +95,24 @@ class access_Core { return false; } - if ($user->admin && $item->owner_id == $user->id) { + if ($user->admin) { return true; } - $resource = $perm_name == "view" ? - $item : model_cache::get("access_cache", $item->id, "item_id"); + 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"; 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 59cec453..737ed8a6 100644 --- a/modules/gallery/tests/Access_Helper_Test.php +++ b/modules/gallery/tests/Access_Helper_Test.php @@ -101,6 +101,48 @@ class Access_Helper_Test extends Unit_Test_Case { $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"); -- cgit v1.2.3 From 5fd82a2edea41209a6936f89c56bbd53083ed182 Mon Sep 17 00:00:00 2001 From: Tim Almdal Date: Mon, 27 Jul 2009 11:13:20 -0700 Subject: 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(-) (limited to 'modules/gallery/tests/Access_Helper_Test.php') diff --git a/modules/gallery/helpers/access.php b/modules/gallery/helpers/access.php index 79394d35..8c6f5d54 100644 --- a/modules/gallery/helpers/access.php +++ b/modules/gallery/helpers/access.php @@ -95,24 +95,13 @@ class access_Core { 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 737ed8a6..59cec453 100644 --- a/modules/gallery/tests/Access_Helper_Test.php +++ b/modules/gallery/tests/Access_Helper_Test.php @@ -101,48 +101,6 @@ class Access_Helper_Test extends Unit_Test_Case { $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"); -- cgit v1.2.3