Item storage: Permissions aren't stored in the items anymore (#5495)

* The permission set is now used for item permissions

* Check for allow_cid, ... is superfluous. Checking for "private" is enough

* We query the permissionset

* Permissions are displayed correctly

* Changed index

* We don't store the permissions in the item table anymore

* Permission fields are now deprecated

* Reversed ...

boot.php

@@ -41,7 +41,7 @@ define('FRIENDICA_PLATFORM',     'Friendica');
 define('FRIENDICA_CODENAME',     'The Tazmans Flax-lily');
 define('FRIENDICA_VERSION',      '2018.08-dev');
 define('DFRN_PROTOCOL_VERSION',  '2.23');
-define('DB_UPDATE_VERSION',      1279);
+define('DB_UPDATE_VERSION',      1280);
 define('NEW_UPDATE_ROUTINE_VERSION', 1170);
 
 /**

database.sql

@@ -1,6 +1,6 @@
 -- ------------------------------------------
 -- Friendica 2018.08-dev (The Tazmans Flax-lily)
--- DB_UPDATE_VERSION 1279
+-- DB_UPDATE_VERSION 1280
 -- ------------------------------------------
 
 
@@ -487,13 +487,13 @@ CREATE TABLE IF NOT EXISTS `item` (
 	`mention` boolean NOT NULL DEFAULT '0' COMMENT 'The owner of this item was mentioned in it',
 	`forum_mode` tinyint unsigned NOT NULL DEFAULT 0 COMMENT '',
 	`psid` int unsigned COMMENT 'ID of the permission set of this post',
-	`allow_cid` mediumtext COMMENT 'Access Control - list of allowed contact.id \'<19><78>\'',
-	`allow_gid` mediumtext COMMENT 'Access Control - list of allowed groups',
-	`deny_cid` mediumtext COMMENT 'Access Control - list of denied contact.id',
-	`deny_gid` mediumtext COMMENT 'Access Control - list of denied groups',
 	`resource-id` varchar(32) NOT NULL DEFAULT '' COMMENT 'Used to link other tables to items, it identifies the linked resource (e.g. photo) and if set must also set resource_type',
 	`event-id` int unsigned NOT NULL DEFAULT 0 COMMENT 'Used to link to the event.id',
 	`attach` mediumtext COMMENT 'JSON structure representing attachments to this item',
+	`allow_cid` mediumtext COMMENT 'Deprecated',
+	`allow_gid` mediumtext COMMENT 'Deprecated',
+	`deny_cid` mediumtext COMMENT 'Deprecated',
+	`deny_gid` mediumtext COMMENT 'Deprecated',
 	`postopts` text COMMENT 'Deprecated',
 	`inform` mediumtext COMMENT 'Deprecated',
 	`type` varchar(20) COMMENT 'Deprecated',
@@ -545,7 +545,7 @@ CREATE TABLE IF NOT EXISTS `item` (
 	 INDEX `uid_eventid` (`uid`,`event-id`),
 	 INDEX `icid` (`icid`),
 	 INDEX `iaid` (`iaid`),
-	 INDEX `psid` (`psid`)
+	 INDEX `psid_wall` (`psid`,`wall`)
 ) DEFAULT COLLATE utf8mb4_general_ci COMMENT='Structure for all posts';
 
 --

include/security.php

@@ -12,6 +12,7 @@ use Friendica\Database\DBA;
 use Friendica\Model\Contact;
 use Friendica\Model\Group;
 use Friendica\Util\DateTimeFormat;
+use Friendica\Model\PermissionSet;
 
 /**
  * @brief Calculate the hash that is needed for the "Friendica" cookie
@@ -342,12 +343,7 @@ function item_permissions_sql($owner_id, $remote_verified = false, $groups = nul
 	 *
 	 * default permissions - anonymous user
 	 */
-	$sql = " AND `item`.allow_cid = ''
+	$sql = " AND NOT `item`.`private`";
-			 AND `item`.allow_gid = ''
-			 AND `item`.deny_cid  = ''
-			 AND `item`.deny_gid  = ''
-			 AND `item`.private != 1
-	";
 
 	// Profile owner - everything is visible
 	if ($local_user && ($local_user == $owner_id)) {
@@ -360,37 +356,15 @@ function item_permissions_sql($owner_id, $remote_verified = false, $groups = nul
 		 * If pre-verified, the caller is expected to have already
 		 * done this and passed the groups into this function.
 		 */
-		if (!$remote_verified) {
+		$set = PermissionSet::get($owner_id, $remote_user, $groups);
-			$r = q("SELECT id FROM contact WHERE id = %d AND uid = %d AND blocked = 0 LIMIT 1",
+
-				intval($remote_user),
+		if (!empty($set)) {
-				intval($owner_id)
+			$sql_set = " OR (`item`.`private` IN (1,2) AND `item`.`wall` AND `item`.`psid` IN (" . implode(',', $set) . "))";
-			);
+		} else {
-			if (DBA::isResult($r)) {
+			$sql_set = '';
-				$remote_verified = true;
-				$groups = Group::getIdsByContactId($remote_user);
-			}
 		}
-		if ($remote_verified) {
 
-			$gs = '<<>>'; // should be impossible to match
+		$sql = " AND (NOT `item`.`private`" . $sql_set . ")";
-
-			if (is_array($groups) && count($groups)) {
-				foreach ($groups as $g) {
-					$gs .= '|<' . intval($g) . '>';
-				}
-			}
-
-			$sql = sprintf(
-				" AND ( `item`.private = 0 OR ( `item`.private in (1,2) AND `item`.`wall` = 1
-				  AND ( NOT (`item`.deny_cid REGEXP '<%d>' OR `item`.deny_gid REGEXP '%s')
-				  AND ( `item`.allow_cid REGEXP '<%d>' OR `item`.allow_gid REGEXP '%s' OR ( `item`.allow_cid = '' AND `item`.allow_gid = '')))))
-				",
-				intval($remote_user),
-				DBA::escape($gs),
-				intval($remote_user),
-				DBA::escape($gs)
-			);
-		}
 	}
 
 	return $sql;

mod/lockview.php

@@ -6,6 +6,7 @@ use Friendica\App;
 use Friendica\Core\Addon;
 use Friendica\Core\L10n;
 use Friendica\Database\DBA;
+use Friendica\Model\Item;
 
 function lockview_content(App $a) {
 
@@ -17,31 +18,34 @@ function lockview_content(App $a) {
 		$item_id = (($a->argc > 2) ? intval($a->argv[2]) : 0);
 	}
 
-	if(! $item_id)
+	if (!$item_id)
 		killme();
 
 	if (!in_array($type, ['item','photo','event']))
 		killme();
 
-	$r = q("SELECT * FROM `%s` WHERE `id` = %d LIMIT 1",
+	$fields = ['uid', 'private', 'allow_cid', 'allow_gid', 'deny_cid', 'deny_gid'];
-		DBA::escape($type),
+	$condition = ['id' => $item_id];
-		intval($item_id)
+	if ($type != 'item') {
-	);
+		$item = DBA::selectFirst($type, $fields, $condition);
-	if (! DBA::isResult($r)) {
+	} else {
+		$item = Item::selectFirst($fields, $condition);
+	}
+
+	if (!DBA::isResult($item)) {
 		killme();
 	}
-	$item = $r[0];
 
 	Addon::callHooks('lockview_content', $item);
 
-	if($item['uid'] != local_user()) {
+	if ($item['uid'] != local_user()) {
 		echo L10n::t('Remote privacy information not available.') . '<br />';
 		killme();
 	}
 
 
-	if(($item['private'] == 1) && (! strlen($item['allow_cid'])) && (! strlen($item['allow_gid']))
+	if (($item['private'] == 1) && empty($item['allow_cid']) && empty($item['allow_gid'])
-		&& (! strlen($item['deny_cid'])) && (! strlen($item['deny_gid']))) {
+		&& empty($item['deny_cid']) && empty($item['deny_gid'])) {
 
 		echo L10n::t('Remote privacy information not available.') . '<br />';
 		killme();
@@ -55,7 +59,7 @@ function lockview_content(App $a) {
 	$o = L10n::t('Visible to:') . '<br />';
 	$l = [];
 
-	if(count($allowed_groups)) {
+	if (count($allowed_groups)) {
 		$r = q("SELECT `name` FROM `group` WHERE `id` IN ( %s )",
 			DBA::escape(implode(', ', $allowed_groups))
 		);
@@ -63,7 +67,7 @@ function lockview_content(App $a) {
 			foreach($r as $rr)
 				$l[] = '<b>' . $rr['name'] . '</b>';
 	}
-	if(count($allowed_users)) {
+	if (count($allowed_users)) {
 		$r = q("SELECT `name` FROM `contact` WHERE `id` IN ( %s )",
 			DBA::escape(implode(', ',$allowed_users))
 		);
@@ -73,7 +77,7 @@ function lockview_content(App $a) {
 
 	}
 
-	if(count($deny_groups)) {
+	if (count($deny_groups)) {
 		$r = q("SELECT `name` FROM `group` WHERE `id` IN ( %s )",
 			DBA::escape(implode(', ', $deny_groups))
 		);
@@ -81,7 +85,7 @@ function lockview_content(App $a) {
 			foreach($r as $rr)
 				$l[] = '<b><strike>' . $rr['name'] . '</strike></b>';
 	}
-	if(count($deny_users)) {
+	if (count($deny_users)) {
 		$r = q("SELECT `name` FROM `contact` WHERE `id` IN ( %s )",
 			DBA::escape(implode(', ',$deny_users))
 		);

src/Database/DBStructure.php

@@ -1328,17 +1328,16 @@ class DBStructure
 						"mention" => ["type" => "boolean", "not null" => "1", "default" => "0", "comment" => "The owner of this item was mentioned in it"],
 						"forum_mode" => ["type" => "tinyint unsigned", "not null" => "1", "default" => "0", "comment" => ""],
 						"psid" => ["type" => "int unsigned", "relation" => ["permissionset" => "id"], "comment" => "ID of the permission set of this post"],
-						// These fields will be replaced by the "psid" from above
+						// It has to be decided whether these fields belong to the user or the structure
-						"allow_cid" => ["type" => "mediumtext", "comment" => "Access Control - list of allowed contact.id '<19><78>'"],
-						"allow_gid" => ["type" => "mediumtext", "comment" => "Access Control - list of allowed groups"],
-						"deny_cid" => ["type" => "mediumtext", "comment" => "Access Control - list of denied contact.id"],
-						"deny_gid" => ["type" => "mediumtext", "comment" => "Access Control - list of denied groups"],
-						// It is to be decided whether these fields belong to the user or the structure
 						"resource-id" => ["type" => "varchar(32)", "not null" => "1", "default" => "", "comment" => "Used to link other tables to items, it identifies the linked resource (e.g. photo) and if set must also set resource_type"],
 						"event-id" => ["type" => "int unsigned", "not null" => "1", "default" => "0", "relation" => ["event" => "id"], "comment" => "Used to link to the event.id"],
 						// Could possibly be replaced by the "attach" table?
 						"attach" => ["type" => "mediumtext", "comment" => "JSON structure representing attachments to this item"],
 						// Deprecated fields. Will be removed in upcoming versions
+						"allow_cid" => ["type" => "mediumtext", "comment" => "Deprecated"],
+						"allow_gid" => ["type" => "mediumtext", "comment" => "Deprecated"],
+						"deny_cid" => ["type" => "mediumtext", "comment" => "Deprecated"],
+						"deny_gid" => ["type" => "mediumtext", "comment" => "Deprecated"],
 						"postopts" => ["type" => "text", "comment" => "Deprecated"],
 						"inform" => ["type" => "mediumtext", "comment" => "Deprecated"],
 						"type" => ["type" => "varchar(20)", "comment" => "Deprecated"],
@@ -1392,7 +1391,7 @@ class DBStructure
 						"uid_eventid" => ["uid","event-id"],
 						"icid" => ["icid"],
 						"iaid" => ["iaid"],
-						"psid" => ["psid"],
+						"psid_wall" => ["psid", "wall"],
 						]
 				];
 		$database["item-activity"] = [

src/Database/PostUpdate.php

@@ -70,8 +70,6 @@ class PostUpdate
 				AND `item`.`visible` AND NOT `item`.`private`
 				AND NOT `item`.`deleted` AND NOT `item`.`moderated`
 				AND `item`.`network` IN ('%s', '%s', '%s', '')
-				AND `item`.`allow_cid` = '' AND `item`.`allow_gid` = ''
-				AND `item`.`deny_cid` = '' AND `item`.`deny_gid` = ''
 				AND NOT `item`.`global`";
 
 		$r = q($query1.$query2.$query3."  ORDER BY `item`.`id` LIMIT 1",
@@ -264,7 +262,8 @@ class PostUpdate
 				$item['owner-id'] = Contact::getIdForURL($item["owner-link"], 0, false, $default);
 			}
 
-			if (empty($item['psid'])) {
+			if (!is_null($item['allow_cid']) && !is_null($item['allow_gid'])
+				&& !is_null($item['deny_cid']) && !is_null($item['deny_gid'])) {
 				$item['psid'] = PermissionSet::fetchIDForPost($item);
 			}
 

src/Model/Item.php

@@ -514,9 +514,8 @@ class Item extends BaseObject
 
 		$fields['item'] = ['id', 'uid', 'parent', 'uri', 'parent-uri', 'thr-parent', 'guid',
 			'contact-id', 'owner-id', 'author-id', 'type', 'wall', 'gravity', 'extid',
-			'created', 'edited', 'commented', 'received', 'changed',
+			'created', 'edited', 'commented', 'received', 'changed', 'psid',
-			'resource-id', 'event-id', 'tag', 'attach', 'post-type',
+			'resource-id', 'event-id', 'tag', 'attach', 'post-type', 'file',
-			'file', 'allow_cid', 'allow_gid', 'deny_cid', 'deny_gid', 'psid',
 			'private', 'pubmail', 'moderated', 'visible', 'starred', 'bookmark',
 			'unseen', 'deleted', 'origin', 'forum_mode', 'mention', 'global',
 			'id' => 'item_id', 'network', 'icid', 'iaid', 'id' => 'internal-iid',
@@ -529,6 +528,8 @@ class Item extends BaseObject
 
 		$fields['item-delivery-data'] = self::DELIVERY_DATA_FIELDLIST;
 
+		$fields['permissionset'] = ['allow_cid', 'allow_gid', 'deny_cid', 'deny_gid'];
+
 		$fields['author'] = ['url' => 'author-link', 'name' => 'author-name',
 			'thumb' => 'author-avatar', 'nick' => 'author-nick', 'network' => 'author-network'];
 
@@ -642,6 +643,10 @@ class Item extends BaseObject
 			$joins .= " LEFT JOIN `item-delivery-data` ON `item-delivery-data`.`iid` = `item`.`id`";
 		}
 
+		if (strpos($sql_commands, "`permissionset`.") !== false) {
+			$joins .= " LEFT JOIN `permissionset` ON `permissionset`.`id` = `item`.`psid`";
+		}
+
 		if ((strpos($sql_commands, "`parent-item`.") !== false) || (strpos($sql_commands, "`parent-author`.") !== false)) {
 			$joins .= " STRAIGHT_JOIN `item` AS `parent-item` ON `parent-item`.`id` = `item`.`parent`";
 		}
@@ -1648,8 +1653,7 @@ class Item extends BaseObject
 			$files = '';
 		}
 
-		// Creates the permission set
+		// Creates or assigns the permission set
-		// Currently we only store the data but don't using it
 		$item['psid'] = PermissionSet::fetchIDForPost($item);
 
 		// We are doing this outside of the transaction to avoid timing problems

src/Model/PermissionSet.php

@@ -20,8 +20,13 @@ class PermissionSet extends BaseObject
 	 * @param array $postarray The array from an item, picture or event post
 	 * @return id
 	 */
-	public static function fetchIDForPost($postarray)
+	public static function fetchIDForPost(&$postarray)
 	{
+		if (is_null($postarray['allow_cid']) || is_null($postarray['allow_gid'])
+			|| is_null($postarray['deny_cid']) || is_null($postarray['deny_gid'])) {
+			return null;
+		}
+
 		$condition = ['uid' => $postarray['uid'],
 			'allow_cid' => self::sortPermissions(defaults($postarray, 'allow_cid', '')),
 			'allow_gid' => self::sortPermissions(defaults($postarray, 'allow_gid', '')),
@@ -35,6 +40,13 @@ class PermissionSet extends BaseObject
 
 			$set = DBA::selectFirst('permissionset', ['id'], $condition);
 		}
+
+
+		$postarray['allow_cid'] = null;
+		$postarray['allow_gid'] = null;
+		$postarray['deny_cid'] = null;
+		$postarray['deny_gid'] = null;
+
 		return $set['id'];
 	}
 
@@ -56,4 +68,48 @@ class PermissionSet extends BaseObject
 
 		return '<' . implode('><', $elements) . '>';
 	}
+
+	/**
+	 * @brief Returns a permission set for a given contact
+	 *
+	 * @param integer $uid        User id whom the items belong
+	 * @param integer $contact_id Contact id of the visitor
+	 * @param array   $groups     Possibly previously fetched group ids for that contact
+	 *
+	 * @return array of permission set ids.
+	 */
+
+	static public function get($uid, $contact_id, $groups = null)
+	{
+		if (empty($groups) && DBA::exists('contact', ['id' => $contact_id, 'uid' => $uid, 'blocked' => false])) {
+			$groups = Group::getIdsByContactId($contact_id);
+		}
+
+		if (empty($groups) || !is_array($groups)) {
+			return [];
+		}
+		$group_str = '<<>>'; // should be impossible to match
+
+		foreach ($groups as $g) {
+			$group_str .= '|<' . intval($g) . '>';
+		}
+
+		$contact_str = '<' . $contact_id . '>';
+
+		$condition = ["`uid` = ? AND (`allow_cid` = '' OR`allow_cid` REGEXP ?)
+			AND (`deny_cid` = '' OR NOT `deny_cid` REGEXP ?)
+			AND (`allow_gid` = '' OR `allow_gid` REGEXP ?)
+			AND (`deny_gid` = '' OR NOT `deny_gid` REGEXP ?)",
+			$uid, $contact_str, $contact_str, $group_str, $group_str];
+
+		$ret = DBA::select('permissionset', ['id'], $condition);
+		$set = [];
+		while ($permission = DBA::fetch($ret)) {
+			$set[] = $permission['id'];
+		}
+		DBA::close($ret);
+		logger('Blubb: '.$uid.' - '.$contact_id.': '.implode(', ', $set));
+
+		return $set;
+	}
 }

src/Protocol/DFRN.php

@@ -25,6 +25,7 @@ use Friendica\Model\GContact;
 use Friendica\Model\Group;
 use Friendica\Model\Item;
 use Friendica\Model\Profile;
+use Friendica\Model\PermissionSet;
 use Friendica\Model\User;
 use Friendica\Object\Image;
 use Friendica\Util\Crypto;
@@ -123,7 +124,7 @@ class DFRN
 
 		// default permissions - anonymous user
 
-		$sql_extra = " AND `item`.`allow_cid` = '' AND `item`.`allow_gid` = '' AND `item`.`deny_cid`  = '' AND `item`.`deny_gid`  = '' ";
+		$sql_extra = " AND NOT `item`.`private` ";
 
 		$r = q(
 			"SELECT `contact`.*, `user`.`nickname`, `user`.`timezone`, `user`.`page-flags`, `user`.`account-type`
@@ -175,30 +176,14 @@ class DFRN
 
 			$contact = $r[0];
 			include_once 'include/security.php';
-			$groups = Group::getIdsByContactId($contact['id']);
 
-			if (count($groups)) {
+			$set = PermissionSet::get($owner_id, $contact['id']);
-				for ($x = 0; $x < count($groups); $x ++) {
-					$groups[$x] = '<' . intval($groups[$x]) . '>' ;
-				}
 
-				$gs = implode('|', $groups);
+			if (!empty($set)) {
+				$sql_extra = " AND `item`.`psid` IN (" . implode(',', $set) .")";
 			} else {
-				$gs = '<<>>' ; // Impossible to match
+				$sql_extra = " AND NOT `item`.`private`";
 			}
-
-			$sql_extra = sprintf(
-				"
-				AND ( `allow_cid` = '' OR     `allow_cid` REGEXP '<%d>' )
-				AND ( `deny_cid`  = '' OR NOT `deny_cid`  REGEXP '<%d>' )
-				AND ( `allow_gid` = '' OR     `allow_gid` REGEXP '%s' )
-				AND ( `deny_gid`  = '' OR NOT `deny_gid`  REGEXP '%s')
-			",
-				intval($contact['id']),
-				intval($contact['id']),
-				DBA::escape($gs),
-				DBA::escape($gs)
-			);
 		}
 
 		if ($public_feed) {
@@ -911,7 +896,7 @@ class DFRN
 			$entry->setAttribute("xmlns:statusnet", NAMESPACE_STATUSNET);
 		}
 
-		if ($item['allow_cid'] || $item['allow_gid'] || $item['deny_cid'] || $item['deny_gid']) {
+		if ($item['private']) {
 			$body = Item::fixPrivatePhotos($item['body'], $owner['uid'], $item, $cid);
 		} else {
 			$body = $item['body'];
@@ -1005,8 +990,8 @@ class DFRN
 			XML::addElement($doc, $entry, "georss:point", $item['coord']);
 		}
 
-		if (($item['private']) || strlen($item['allow_cid']) || strlen($item['allow_gid']) || strlen($item['deny_cid']) || strlen($item['deny_gid'])) {
+		if ($item['private']) {
-			XML::addElement($doc, $entry, "dfrn:private", (($item['private']) ? $item['private'] : 1));
+			XML::addElement($doc, $entry, "dfrn:private", ($item['private'] ? $item['private'] : 1));
 		}
 
 		if ($item['extid']) {