]> gitweb.fluxo.info Git - lorea/elgg.git/commitdiff
Fixes #3543. Ported access collections fix to master.
authorBrett Profitt <brett.profitt@gmail.com>
Wed, 31 Aug 2011 03:52:12 +0000 (20:52 -0700)
committerBrett Profitt <brett.profitt@gmail.com>
Wed, 31 Aug 2011 03:52:12 +0000 (20:52 -0700)
actions/friends/collections/add.php
actions/friends/collections/delete.php
actions/friends/collections/edit.php
engine/lib/access.php
engine/tests/api/access_collections.php [new file with mode: 0644]
languages/en.php

index 8ec6a085f8189ef6f605de92224137351a29558f..1e2bc1d5cebce05021104e1d172155767403ff31 100644 (file)
@@ -9,28 +9,24 @@
 $collection_name = get_input('collection_name');
 $friends = get_input('friends_collection');
 
-//first check to make sure that a collection name has been set and create the new colection
-if ($collection_name) {
+if (!$collection_name) {
+       register_error(elgg_echo("friends:nocollectionname"));
+       forward(REFERER);
+}
 
-       //create the collection
-       $create_collection = create_access_collection($collection_name, elgg_get_logged_in_user_guid());
+$id = create_access_collection($collection_name);
 
-       //if the collection was created and the user passed some friends from the form, add them
-       if ($create_collection && (!empty($friends))) {
-               //add friends to the collection
-               foreach ($friends as $friend) {
-                       add_user_to_access_collection($friend, $create_collection);
-               }
+if ($id) {
+       $result = update_access_collection($id, $friends);
+       if ($result) {
+               system_message(elgg_echo("friends:collectionadded"));
+               // go to the collections page
+               forward("pg/collections/" . get_loggedin_user()->username);
+       } else {
+               register_error(elgg_echo("friends:nocollectionname"));
+               forward(REFERER);
        }
-
-       // Success message
-       system_message(elgg_echo("friends:collectionadded"));
-       // Forward to the collections page
-       forward("collections/" . elgg_get_logged_in_user_entity()->username);
-
 } else {
        register_error(elgg_echo("friends:nocollectionname"));
-
-       // Forward to the add collection page
-       forward("collections/add");
-}
+       forward(REFERER);
+}
\ No newline at end of file
index fe719d74bf849fe9078e77bc5312e96d220157cf..ff8f1fb55ef2c6efa4e15581e25a50aacc553040 100644 (file)
@@ -8,29 +8,16 @@
 
 $collection_id = (int) get_input('collection');
 
-// Check to see that the access collection exist and grab its owner
-$get_collection = get_access_collection($collection_id);
-
-if ($get_collection) {
-
-       if ($get_collection->owner_guid == elgg_get_logged_in_user_guid()) {
-
-               $delete_collection = delete_access_collection($collection_id);
+// check the ACL exists and we can edit
+if (!can_edit_access_collection($collection_id)) {
+       register_error(elgg_echo("friends:collectiondeletefailed"));
+       forward(REFERER);
+}
 
-               // Success message
-               if ($delete_collection) {
-                       system_message(elgg_echo("friends:collectiondeleted"));
-               } else {
-                       register_error(elgg_echo("friends:collectiondeletefailed"));
-               }
-       } else {
-               // Failure message
-               register_error(elgg_echo("friends:collectiondeletefailed"));
-       }
+if (delete_access_collection($collection_id)) {
+       system_message(elgg_echo("friends:collectiondeleted"));
 } else {
-       // Failure message
        register_error(elgg_echo("friends:collectiondeletefailed"));
 }
 
-// Forward to the collections page
-forward("collections/" . elgg_get_logged_in_user_entity()->username);
+forward(REFERER);
index b7fb716f2a3e90b3efdc71ec9b7984e4a57cdd04..9eb5e1eab647a62daa6193c52206de747d194ca3 100644 (file)
@@ -9,7 +9,15 @@
 $collection_id = get_input('collection_id');
 $friends = get_input('friend');
 
-//chech the collection exists and the current user owners it
-update_access_collection($collection_id, $friends);
+// check it exists and we can edit
+if (!can_edit_access_collection($collection_id)) {
+       system_message(elgg_echo('friends:collection:edit_failed'));
+}
 
-exit;
+if (update_access_collection($collection_id, $friends)) {
+       system_message(elgg_echo('friends:collections:edited'));
+} else {
+       system_message(elgg_echo('friends:collection:edit_failed'));
+}
+
+forward(REFERER);
\ No newline at end of file
index cde3d256f5f5e702760deb3bc7f141512f6c00ee..1a4a2216276b7b4671a92d90029eff1469822af1 100644 (file)
@@ -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 (file)
index 0000000..1e61c45
--- /dev/null
@@ -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();
+       }
+}
index da5f0ef811da90ad53fa5a7a411ae876827c5175..ab3c523def095b6859593284b59dc365aeacdc4d 100644 (file)
@@ -344,6 +344,8 @@ $english = array(
        'friends:nocollectionname' => "You need to give your collection a name before it can be created.",
        'friends:collections:members' => "Collection members",
        'friends:collections:edit' => "Edit collection",
+       'friends:collections:edited' => "Saved collection",
+       'friends:collection:edit_failed' => 'Could not save collection.',
 
        'friendspicker:chararray' => 'ABCDEFGHIJKLMNOPQRSTUVWXYZ',