From 986106a8f7d0b58a2b2834b082301a426bc98999 Mon Sep 17 00:00:00 2001 From: Michael Vogel Date: Thu, 26 Jul 2018 01:14:55 +0200 Subject: [PATCH] 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 | 2 +- database.sql | 12 ++++---- include/security.php | 44 ++++++--------------------- mod/lockview.php | 32 +++++++++++--------- src/Database/DBStructure.php | 13 ++++---- src/Database/PostUpdate.php | 5 ++-- src/Model/Item.php | 14 +++++---- src/Model/PermissionSet.php | 58 +++++++++++++++++++++++++++++++++++- src/Protocol/DFRN.php | 35 +++++++--------------- 9 files changed, 118 insertions(+), 97 deletions(-) diff --git a/boot.php b/boot.php index 426d0b1d79..bc1bb43c39 100644 --- a/boot.php +++ b/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); /** diff --git a/database.sql b/database.sql index d963ce9720..6dc3b407bf 100644 --- a/database.sql +++ b/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'; -- diff --git a/include/security.php b/include/security.php index 82cac3f97c..cf5b03d04c 100644 --- a/include/security.php +++ b/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 = '' - AND `item`.allow_gid = '' - AND `item`.deny_cid = '' - AND `item`.deny_gid = '' - AND `item`.private != 1 - "; + $sql = " AND NOT `item`.`private`"; // 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) { - $r = q("SELECT id FROM contact WHERE id = %d AND uid = %d AND blocked = 0 LIMIT 1", - intval($remote_user), - intval($owner_id) - ); - if (DBA::isResult($r)) { - $remote_verified = true; - $groups = Group::getIdsByContactId($remote_user); - } - } - if ($remote_verified) { - - $gs = '<<>>'; // should be impossible to match + $set = PermissionSet::get($owner_id, $remote_user, $groups); - 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) - ); + if (!empty($set)) { + $sql_set = " OR (`item`.`private` IN (1,2) AND `item`.`wall` AND `item`.`psid` IN (" . implode(',', $set) . "))"; + } else { + $sql_set = ''; } + + $sql = " AND (NOT `item`.`private`" . $sql_set . ")"; } return $sql; diff --git a/mod/lockview.php b/mod/lockview.php index b1c3e37767..893764c4d9 100644 --- a/mod/lockview.php +++ b/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", - DBA::escape($type), - intval($item_id) - ); - if (! DBA::isResult($r)) { + $fields = ['uid', 'private', 'allow_cid', 'allow_gid', 'deny_cid', 'deny_gid']; + $condition = ['id' => $item_id]; + if ($type != 'item') { + $item = DBA::selectFirst($type, $fields, $condition); + } 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.') . '
'; killme(); } - if(($item['private'] == 1) && (! strlen($item['allow_cid'])) && (! strlen($item['allow_gid'])) - && (! strlen($item['deny_cid'])) && (! strlen($item['deny_gid']))) { + if (($item['private'] == 1) && empty($item['allow_cid']) && empty($item['allow_gid']) + && empty($item['deny_cid']) && empty($item['deny_gid'])) { echo L10n::t('Remote privacy information not available.') . '
'; killme(); @@ -55,7 +59,7 @@ function lockview_content(App $a) { $o = L10n::t('Visible to:') . '
'; $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[] = '' . $rr['name'] . ''; } - 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[] = '' . $rr['name'] . ''; } - if(count($deny_users)) { + if (count($deny_users)) { $r = q("SELECT `name` FROM `contact` WHERE `id` IN ( %s )", DBA::escape(implode(', ',$deny_users)) ); diff --git a/src/Database/DBStructure.php b/src/Database/DBStructure.php index d03268374b..5ff15227b4 100644 --- a/src/Database/DBStructure.php +++ b/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 - "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 + // It has 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"] = [ diff --git a/src/Database/PostUpdate.php b/src/Database/PostUpdate.php index 43bbcce022..3b78b94fcd 100644 --- a/src/Database/PostUpdate.php +++ b/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); } diff --git a/src/Model/Item.php b/src/Model/Item.php index e2dc28e833..932db885d0 100644 --- a/src/Model/Item.php +++ b/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', - 'resource-id', 'event-id', 'tag', 'attach', 'post-type', - 'file', 'allow_cid', 'allow_gid', 'deny_cid', 'deny_gid', 'psid', + 'created', 'edited', 'commented', 'received', 'changed', 'psid', + 'resource-id', 'event-id', 'tag', 'attach', 'post-type', 'file', '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 - // Currently we only store the data but don't using it + // Creates or assigns the permission set $item['psid'] = PermissionSet::fetchIDForPost($item); // We are doing this outside of the transaction to avoid timing problems diff --git a/src/Model/PermissionSet.php b/src/Model/PermissionSet.php index 2b34f1e32a..20848c04d3 100644 --- a/src/Model/PermissionSet.php +++ b/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; + } } diff --git a/src/Protocol/DFRN.php b/src/Protocol/DFRN.php index eda85023fd..32ce506cc8 100644 --- a/src/Protocol/DFRN.php +++ b/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)) { - for ($x = 0; $x < count($groups); $x ++) { - $groups[$x] = '<' . intval($groups[$x]) . '>' ; - } + $set = PermissionSet::get($owner_id, $contact['id']); - $gs = implode('|', $groups); + if (!empty($set)) { + $sql_extra = " AND `item`.`psid` IN (" . implode(',', $set) .")"; } else { - $gs = '<<>>' ; // Impossible to match - } - - $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) - ); + $sql_extra = " AND NOT `item`.`private`"; + } } 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'])) { - XML::addElement($doc, $entry, "dfrn:private", (($item['private']) ? $item['private'] : 1)); + if ($item['private']) { + XML::addElement($doc, $entry, "dfrn:private", ($item['private'] ? $item['private'] : 1)); } if ($item['extid']) { -- 2.39.5