aboutsummaryrefslogtreecommitdiff
path: root/engine
diff options
context:
space:
mode:
authorBrett Profitt <brett.profitt@gmail.com>2011-08-30 20:52:12 -0700
committerBrett Profitt <brett.profitt@gmail.com>2011-08-30 20:52:12 -0700
commit9a53ddf57cdbf557b0d4f21d0fdf01b4b92569c4 (patch)
tree8ef2736c0aa6491ff3fd47d52e78594570e7da71 /engine
parente32bfa450e9a07f6f6575b84d18eb63bd72969f4 (diff)
downloadelgg-9a53ddf57cdbf557b0d4f21d0fdf01b4b92569c4.tar.gz
elgg-9a53ddf57cdbf557b0d4f21d0fdf01b4b92569c4.tar.bz2
Fixes #3543. Ported access collections fix to master.
Diffstat (limited to 'engine')
-rw-r--r--engine/lib/access.php190
-rw-r--r--engine/tests/api/access_collections.php269
2 files changed, 382 insertions, 77 deletions
diff --git a/engine/lib/access.php b/engine/lib/access.php
index cde3d256f..1a4a22162 100644
--- a/engine/lib/access.php
+++ b/engine/lib/access.php
@@ -410,6 +410,43 @@ function get_write_access_array($user_id = 0, $site_id = 0, $flush = false) {
return $tmp_access_array;
}
+
+/**
+ * Can the user write to the access collection?
+ *
+ * Hook into the access:collections:write, user to change this.
+ *
+ * Respects access control disabling for admin users and {@see elgg_set_ignore_access()}
+ *
+ * @see get_write_access_array()
+ *
+ * @param int $collection_id The collection id
+ * @param mixed $user_guid The user GUID to check for. Defaults to logged in user.
+ * @return bool
+ */
+function can_edit_access_collection($collection_id, $user_guid = null) {
+ if ($user_guid) {
+ $user = get_entity((int) $user_guid);
+ } else {
+ $user = get_loggedin_user();
+ }
+
+ $collection = get_access_collection($collection_id);
+
+ if (!($user instanceof ElggUser) || !$collection) {
+ return false;
+ }
+
+ $write_access = get_write_access_array($user->getGUID(), null, true);
+
+ // don't ignore access when checking users.
+ if ($user_guid) {
+ return array_key_exists($collection_id, $write_access);
+ } else {
+ return elgg_get_ignore_access() || array_key_exists($collection_id, $write_access);
+ }
+}
+
/**
* Creates a new access collection.
*
@@ -483,37 +520,30 @@ function create_access_collection($name, $owner_guid = 0, $site_guid = 0) {
function update_access_collection($collection_id, $members) {
global $CONFIG;
- $collection_id = (int) $collection_id;
- $members = (is_array($members)) ? $members : array();
+ $acl = get_access_collection($collection_id);
- $collections = get_write_access_array();
-
- if (array_key_exists($collection_id, $collections)) {
- $cur_members = get_members_of_access_collection($collection_id, true);
- $cur_members = (is_array($cur_members)) ? $cur_members : array();
+ if (!$acl) {
+ return false;
+ }
+ $members = (is_array($members)) ? $members : array();
- $remove_members = array_diff($cur_members, $members);
- $add_members = array_diff($members, $cur_members);
+ $cur_members = get_members_of_access_collection($collection_id, true);
+ $cur_members = (is_array($cur_members)) ? $cur_members : array();
- $params = array(
- 'collection_id' => $collection_id,
- 'members' => $members,
- 'add_members' => $add_members,
- 'remove_members' => $remove_members
- );
+ $remove_members = array_diff($cur_members, $members);
+ $add_members = array_diff($members, $cur_members);
- foreach ($add_members as $guid) {
- add_user_to_access_collection($guid, $collection_id);
- }
+ $result = true;
- foreach ($remove_members as $guid) {
- remove_user_from_access_collection($guid, $collection_id);
- }
+ foreach ($add_members as $guid) {
+ $result = $result && add_user_to_access_collection($guid, $collection_id);
+ }
- return true;
+ foreach ($remove_members as $guid) {
+ $result = $result && remove_user_from_access_collection($guid, $collection_id);
}
- return false;
+ return $result;
}
/**
@@ -527,27 +557,26 @@ function update_access_collection($collection_id, $members) {
* @see update_access_collection()
*/
function delete_access_collection($collection_id) {
+ global $CONFIG;
+
$collection_id = (int) $collection_id;
- $collections = get_write_access_array(null, null, TRUE);
$params = array('collection_id' => $collection_id);
if (!elgg_trigger_plugin_hook('access:collections:deletecollection', 'collection', $params, true)) {
return false;
}
- if (array_key_exists($collection_id, $collections)) {
- global $CONFIG;
- $query = "delete from {$CONFIG->dbprefix}access_collection_membership"
- . " where access_collection_id = {$collection_id}";
- delete_data($query);
+ // Deleting membership doesn't affect result of deleting ACL.
+ $q = "DELETE FROM {$CONFIG->dbprefix}access_collection_membership
+ WHERE access_collection_id = {$collection_id}";
+ delete_data($q);
+
+ $q = "DELETE FROM {$CONFIG->dbprefix}access_collections
+ WHERE id = {$collection_id}";
+ $result = delete_data($q);
- $query = "delete from {$CONFIG->dbprefix}access_collections where id = {$collection_id}";
- delete_data($query);
- return true;
- } else {
- return false;
- }
+ return $result;
}
/**
@@ -584,45 +613,34 @@ function get_access_collection($collection_id) {
* @see remove_user_from_access_collection()
*/
function add_user_to_access_collection($user_guid, $collection_id) {
+ global $CONFIG;
+
$collection_id = (int) $collection_id;
$user_guid = (int) $user_guid;
- $collections = get_write_access_array();
+ $user = get_user($user_guid);
- if (!($collection = get_access_collection($collection_id))) {
- return false;
- }
+ $collection = get_access_collection($collection_id);
- $user = get_user($user_guid);
- if (!$user) {
+ if (!($user instanceof Elgguser) || !$collection) {
return false;
}
- // to add someone to a collection, the user must be a member of the collection or
- // no one must own it
- if ((array_key_exists($collection_id, $collections) || $collection->owner_guid == 0)) {
- $result = true;
- } else {
- $result = false;
- }
-
$params = array(
'collection_id' => $collection_id,
- 'collection' => $collection,
'user_guid' => $user_guid
);
- $result = elgg_trigger_plugin_hook('access:collections:add_user', 'collection', $params, $result);
+ $result = elgg_trigger_plugin_hook('access:collections:add_user', 'collection', $params, true);
if ($result == false) {
return false;
}
try {
- global $CONFIG;
- $query = "insert into {$CONFIG->dbprefix}access_collection_membership"
- . " set access_collection_id = {$collection_id}, user_guid = {$user_guid}";
- insert_data($query);
+ $q = "INSERT INTO {$CONFIG->dbprefix}access_collection_membership
+ SET access_collection_id = {$collection_id},
+ user_guid = {$user_guid}";
+ insert_data($q);
} catch (DatabaseException $e) {
- // nothing.
return false;
}
@@ -640,34 +658,32 @@ function add_user_to_access_collection($user_guid, $collection_id) {
* @return true|false Depending on success
*/
function remove_user_from_access_collection($user_guid, $collection_id) {
+ global $CONFIG;
+
$collection_id = (int) $collection_id;
$user_guid = (int) $user_guid;
- $collections = get_write_access_array();
- $user = $user = get_user($user_guid);
+ $user = get_user($user_guid);
+
+ $collection = get_access_collection($collection_id);
- if (!($collection = get_access_collection($collection_id))) {
+ if (!($user instanceof Elgguser) || !$collection) {
return false;
}
- if ((array_key_exists($collection_id, $collections) || $collection->owner_guid == 0) && $user) {
- global $CONFIG;
- $params = array(
- 'collection_id' => $collection_id,
- 'user_guid' => $user_guid
- );
-
- if (!elgg_trigger_plugin_hook('access:collections:remove_user', 'collection', $params, true)) {
- return false;
- }
-
- delete_data("delete from {$CONFIG->dbprefix}access_collection_membership "
- . "where access_collection_id = {$collection_id} and user_guid = {$user_guid}");
-
- return true;
+ $params = array(
+ 'collection_id' => $collection_id,
+ 'user_guid' => $user_guid
+ );
+ if (!elgg_trigger_plugin_hook('access:collections:remove_user', 'collection', $params, true)) {
+ return false;
}
- return false;
+ $q = "DELETE FROM {$CONFIG->dbprefix}access_collection_membership
+ WHERE access_collection_id = {$collection_id}
+ AND user_guid = {$user_guid}";
+
+ return delete_data($q);
}
/**
@@ -939,8 +955,17 @@ function access_init() {
* @since 1.7.0
* @elgg_event_handler permissions_check all
*/
-function elgg_override_permissions_hook() {
- $user_guid = elgg_get_logged_in_user_guid();
+function elgg_override_permissions_hook($hook, $type, $value, $params) {
+ $user = elgg_extract('user', $params);
+ if (!$user) {
+ $user = elgg_get_logged_in_user();
+ }
+
+ if (!$user instanceof ElggUser) {
+ return false;
+ }
+
+ $user_guid = $user->guid;
// check for admin
if ($user_guid && elgg_is_admin_user($user_guid)) {
@@ -956,9 +981,20 @@ function elgg_override_permissions_hook() {
return NULL;
}
+/**
+ * Runs unit tests for the entities object.
+ */
+function access_test($hook, $type, $value, $params) {
+ global $CONFIG;
+ $value[] = $CONFIG->path . 'engine/tests/api/access_collections.php';
+ return $value;
+}
+
// This function will let us know when 'init' has finished
elgg_register_event_handler('init', 'system', 'access_init', 9999);
// For overrided permissions
elgg_register_plugin_hook_handler('permissions_check', 'all', 'elgg_override_permissions_hook');
elgg_register_plugin_hook_handler('container_permissions_check', 'all', 'elgg_override_permissions_hook');
+
+elgg_register_plugin_hook_handler('unit_test', 'system', 'access_test'); \ No newline at end of file
diff --git a/engine/tests/api/access_collections.php b/engine/tests/api/access_collections.php
new file mode 100644
index 000000000..1e61c45bb
--- /dev/null
+++ b/engine/tests/api/access_collections.php
@@ -0,0 +1,269 @@
+<?php
+/**
+ * Access Collections tests
+ *
+ * @package Elgg
+ * @subpackage Test
+ */
+class ElggCoreAccessCollectionsTest extends ElggCoreUnitTest {
+
+ /**
+ * Called before each test object.
+ */
+ public function __construct() {
+ parent::__construct();
+
+ $this->dbPrefix = get_config("dbprefix");
+
+ $user = new ElggUser();
+ $user->username = 'test_user_' . rand();
+ $user->email = 'fake_email@fake.com' . rand();
+ $user->name = 'fake user';
+ $user->access_id = ACCESS_PUBLIC;
+ $user->salt = generate_random_cleartext_password();
+ $user->password = generate_user_password($user, rand());
+ $user->owner_guid = 0;
+ $user->container_guid = 0;
+ $user->save();
+
+ $this->user = $user;
+ }
+
+ /**
+ * Called before each test method.
+ */
+ public function setUp() {
+
+ }
+
+ /**
+ * Called after each test method.
+ */
+ public function tearDown() {
+ // do not allow SimpleTest to interpret Elgg notices as exceptions
+ $this->swallowErrors();
+ }
+
+ /**
+ * Called after each test object.
+ */
+ public function __destruct() {
+ // all __destruct() code should go above here
+ $this->user->delete();
+ parent::__destruct();
+ }
+
+ public function testCreateGetDeleteACL() {
+ global $DB_QUERY_CACHE;
+
+ $acl_name = 'test access collection';
+ $acl_id = create_access_collection($acl_name);
+
+ $this->assertTrue(is_int($acl_id));
+
+ $q = "SELECT * FROM {$this->dbPrefix}access_collections WHERE id = $acl_id";
+ $acl = get_data_row($q);
+
+ $this->assertEqual($acl->id, $acl_id);
+
+ if ($acl) {
+ $DB_QUERY_CACHE = array();
+
+ $this->assertEqual($acl->name, $acl_name);
+
+ $result = delete_access_collection($acl_id);
+ $this->assertTrue($result);
+
+ $q = "SELECT * FROM {$this->dbPrefix}access_collections WHERE id = $acl_id";
+ $data = get_data($q);
+ $this->assertFalse($data);
+ }
+ }
+
+ public function testAddRemoveUserToACL() {
+ $acl_id = create_access_collection('test acl');
+
+ $result = add_user_to_access_collection($this->user->guid, $acl_id);
+ $this->assertTrue($result);
+
+ if ($result) {
+ $result = remove_user_from_access_collection($this->user->guid, $acl_id);
+ $this->assertTrue($result);
+ }
+
+ delete_access_collection($acl_id);
+ }
+
+ public function testUpdateACL() {
+ // another fake user to test with
+ $user = new ElggUser();
+ $user->username = 'test_user_' . rand();
+ $user->email = 'fake_email@fake.com' . rand();
+ $user->name = 'fake user';
+ $user->access_id = ACCESS_PUBLIC;
+ $user->salt = generate_random_cleartext_password();
+ $user->password = generate_user_password($user, rand());
+ $user->owner_guid = 0;
+ $user->container_guid = 0;
+ $user->save();
+
+ $acl_id = create_access_collection('test acl');
+
+ $member_lists = array(
+ // adding
+ array(
+ $this->user->guid,
+ $user->guid
+ ),
+ // removing one, keeping one.
+ array(
+ $user->guid
+ ),
+ // removing one, adding one
+ array(
+ $this->user->guid,
+ ),
+ // removing all.
+ array()
+ );
+
+ foreach ($member_lists as $members) {
+ $result = update_access_collection($acl_id, $members);
+ $this->assertTrue($result);
+
+ if ($result) {
+ $q = "SELECT * FROM {$this->dbPrefix}access_collection_membership
+ WHERE access_collection_id = $acl_id";
+ $data = get_data($q);
+
+ if (count($members) == 0) {
+ $this->assertFalse($data);
+ } else {
+ $this->assertEqual(count($members), count($data));
+ }
+ foreach ($data as $row) {
+ $this->assertTrue(in_array($row->user_guid, $members));
+ }
+ }
+ }
+
+ delete_access_collection($acl_id);
+ $user->delete();
+ }
+
+ public function testCanEditACL() {
+ $acl_id = create_access_collection('test acl', $this->user->guid);
+
+ // should be true since it's the owner
+ $result = can_edit_access_collection($acl_id, $this->user->guid);
+ $this->assertTrue($result);
+
+ // should be true since IA is on.
+ $ia = elgg_set_ignore_access(true);
+ $result = can_edit_access_collection($acl_id);
+ $this->assertTrue($result);
+ elgg_set_ignore_access($ia);
+
+ // should be false since IA is off
+ $ia = elgg_set_ignore_access(false);
+ $result = can_edit_access_collection($acl_id);
+ $this->assertFalse($result);
+ elgg_set_ignore_access($ia);
+
+ delete_access_collection($acl_id);
+ }
+
+ public function testCanEditACLHook() {
+ // if only we supported closures!
+ global $acl_test_info;
+
+ $acl_id = create_access_collection('test acl');
+
+ $acl_test_info = array(
+ 'acl_id' => $acl_id,
+ 'user' => $this->user
+ );
+
+ function test_acl_access_hook($hook, $type, $value, $params) {
+ global $acl_test_info;
+ if ($params['user_id'] == $acl_test_info['user']->guid) {
+ $acl = get_access_collection($acl_test_info['acl_id']);
+ $value[$acl->id] = $acl->name;
+ }
+
+ return $value;
+ }
+
+ register_plugin_hook('access:collections:write', 'all', 'test_acl_access_hook');
+
+ // enable security since we usually run as admin
+ $ia = elgg_set_ignore_access(false);
+ $result = can_edit_access_collection($acl_id, $this->user->guid);
+ $this->assertTrue($result);
+ $ia = elgg_set_ignore_access($ia);
+
+ unregister_plugin_hook('access:collections:write', 'all', 'test_acl_access_hook');
+ }
+
+ // groups interface
+ // only runs if the groups plugin is enabled because implementation is split between
+ // core and the plugin.
+ public function testCreateDeleteGroupACL() {
+ if (!is_plugin_enabled('groups')) {
+ return;
+ }
+
+ $group = new ElggGroup();
+ $group->name = 'Test group';
+ $group->save();
+ $acl = get_access_collection($group->group_acl);
+
+ // ACLs are owned by groups
+ $this->assertEqual($acl->owner_guid, $group->guid);
+
+ // removing group and acl
+ $this->assertTrue($group->delete());
+
+ $acl = get_access_collection($group->group_acl);
+ $this->assertFalse($acl);
+
+ $group->delete();
+ }
+
+ public function testJoinLeaveGroupACL() {
+ if (!is_plugin_enabled('groups')) {
+ return;
+ }
+
+ $group = new ElggGroup();
+ $group->name = 'Test group';
+ $group->save();
+
+ $result = $group->join($this->user);
+ $this->assertTrue($result);
+
+ // disable security since we run as admin
+ $ia = elgg_set_ignore_access(false);
+
+ // need to set the page owner to emulate being in a group context.
+ // this is kinda hacky.
+ elgg_set_page_owner_guid($group->getGUID());
+
+ if ($result) {
+ $can_edit = can_edit_access_collection($group->group_acl, $this->user->guid);
+ $this->assertTrue($can_edit);
+ }
+
+ $result = $group->leave($this->user);
+ $this->assertTrue($result);
+
+ if ($result) {
+ $can_edit = can_edit_access_collection($group->group_acl, $this->user->guid);
+ $this->assertFalse($can_edit);
+ }
+
+ elgg_set_ignore_access($ia);
+
+ $group->delete();
+ }
+}