From b5454547e94e166130eca099a88eebbda9a72357 Mon Sep 17 00:00:00 2001 From: Hypolite Petovan Date: Sun, 19 May 2019 17:57:53 -0400 Subject: [PATCH 1/8] Add query string to API "call not implemented" log message --- include/api.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/api.php b/include/api.php index eccd77675e..0ca0bf838c 100644 --- a/include/api.php +++ b/include/api.php @@ -361,7 +361,7 @@ function api_call(App $a) } } - Logger::warning(API_LOG_PREFIX . 'not implemented', ['module' => 'api', 'action' => 'call']); + Logger::warning(API_LOG_PREFIX . 'not implemented', ['module' => 'api', 'action' => 'call', 'query' => $a->query_string]); throw new NotImplementedException(); } catch (HTTPException $e) { header("HTTP/1.1 {$e->getCode()} {$e->httpdesc}"); From 956ae6241d8872f0b7074fc7693e7658edd4fc52 Mon Sep 17 00:00:00 2001 From: Hypolite Petovan Date: Sun, 19 May 2019 18:18:14 -0400 Subject: [PATCH 2/8] Add exception message chain, string trace and original object to JsonLD normalize error logging --- src/Util/JsonLD.php | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/Util/JsonLD.php b/src/Util/JsonLD.php index 69973f4feb..926fa1437d 100644 --- a/src/Util/JsonLD.php +++ b/src/Util/JsonLD.php @@ -68,9 +68,16 @@ class JsonLD } catch (Exception $e) { $normalized = false; - Logger::error('normalise error'); - // Sooner or later we should log some details as well - but currently this leads to memory issues - // Logger::log('normalise error:' . substr(print_r($e, true), 0, 10000), Logger::DEBUG); + $messages = []; + $currentException = $e; + do { + $messages[] = $currentException->getMessage(); + } while($currentException = $currentException->getPrevious()); + + Logger::warning('JsonLD normalize error'); + Logger::notice('JsonLD normalize error', ['messages' => $messages]); + Logger::info('JsonLD normalize error', ['trace' => $e->getTraceAsString()]); + Logger::debug('JsonLD normalize error', ['jsonobj' => $jsonobj]); } return $normalized; From 3114754f4b7b7b14ffe5749bc92c0e6034d58cae Mon Sep 17 00:00:00 2001 From: Hypolite Petovan Date: Sun, 19 May 2019 18:43:19 -0400 Subject: [PATCH 3/8] Refactor Model\Contact::addRelationship - Remove unused parameter $item - Replace q() call with DBA::insert - Update check on $contact that can only be an array now - Add moethod doc block --- src/Model/Contact.php | 45 ++++++++++++++++---------- src/Protocol/ActivityPub/Processor.php | 4 +-- src/Protocol/DFRN.php | 5 ++- src/Protocol/OStatus.php | 9 +----- 4 files changed, 33 insertions(+), 30 deletions(-) diff --git a/src/Model/Contact.php b/src/Model/Contact.php index ce3aeac560..e75ac0394a 100644 --- a/src/Model/Contact.php +++ b/src/Model/Contact.php @@ -2120,7 +2120,17 @@ class Contact extends BaseObject return $contact; } - public static function addRelationship($importer, $contact, $datarray, $item = '', $sharing = false, $note = '') { + /** + * @param array $importer Owner (local user) data + * @param array $contact Existing owner-specific contact data we want to expand the relationship with. Optional. + * @param array $datarray An item-like array with at least the 'author-id' and 'author-url' keys for the contact. Mandatory. + * @param bool $sharing True: Contact is now sharing with Owner; False: Contact is now following Owner (default) + * @param string $note Introduction additional message + * @throws \Friendica\Network\HTTPException\InternalServerErrorException + * @throws \ImagickException + */ + public static function addRelationship(array $importer, array $contact, array $datarray, $sharing = false, $note = '') + { // Should always be set if (empty($datarray['author-id'])) { return; @@ -2139,7 +2149,7 @@ class Contact extends BaseObject $nick = $pub_contact['nick']; $network = $pub_contact['network']; - if (is_array($contact)) { + if (!empty($contact)) { // Make sure that the existing contact isn't archived self::unmarkForArchival($contact); @@ -2155,28 +2165,30 @@ class Contact extends BaseObject ActivityPub\Transmitter::sendContactAccept($contact['url'], $contact['hub-verify'], $importer['uid']); } - // send email notification to owner? } else { $protocol = self::getProtocol($url, $network); + // send email notification to owner? if (DBA::exists('contact', ['nurl' => Strings::normaliseLink($url), 'uid' => $importer['uid'], 'pending' => true])) { Logger::log('ignoring duplicated connection request from pending contact ' . $url); return; } // create contact record - q("INSERT INTO `contact` (`uid`, `created`, `url`, `nurl`, `name`, `nick`, `photo`, `network`, `rel`, - `blocked`, `readonly`, `pending`, `writable`) - VALUES (%d, '%s', '%s', '%s', '%s', '%s', '%s', '%s', %d, 0, 0, 1, 1)", - intval($importer['uid']), - DBA::escape(DateTimeFormat::utcNow()), - DBA::escape($url), - DBA::escape(Strings::normaliseLink($url)), - DBA::escape($name), - DBA::escape($nick), - DBA::escape($photo), - DBA::escape($network), - intval(self::FOLLOWER) - ); + DBA::insert('contact', [ + 'uid' => $importer['uid'], + 'created' => DateTimeFormat::utcNow(), + 'url' => $url, + 'nurl' => Strings::normaliseLink($url), + 'name' => $name, + 'nick' => $nick, + 'photo' => $photo, + 'network' => $network, + 'rel' => self::FOLLOWER, + 'blocked' => 0, + 'readonly' => 0, + 'pending' => 1, + 'writable' => 1, + ]); $contact_record = [ 'id' => DBA::lastInsertId(), @@ -2220,7 +2232,6 @@ class Contact extends BaseObject 'verb' => ($sharing ? ACTIVITY_FRIEND : ACTIVITY_FOLLOW), 'otype' => 'intro' ]); - } } elseif (DBA::isResult($user) && in_array($user['page-flags'], [User::PAGE_FLAGS_SOAPBOX, User::PAGE_FLAGS_FREELOVE, User::PAGE_FLAGS_COMMUNITY])) { $condition = ['uid' => $importer['uid'], 'url' => $url, 'pending' => true]; diff --git a/src/Protocol/ActivityPub/Processor.php b/src/Protocol/ActivityPub/Processor.php index 045c48a4c1..c63300e84b 100644 --- a/src/Protocol/ActivityPub/Processor.php +++ b/src/Protocol/ActivityPub/Processor.php @@ -530,7 +530,7 @@ class Processor DBA::update('contact', ['hub-verify' => $activity['id'], 'protocol' => Protocol::ACTIVITYPUB], ['id' => $cid]); $contact = DBA::selectFirst('contact', [], ['id' => $cid, 'network' => Protocol::NATIVE_SUPPORT]); } else { - $contact = false; + $contact = []; } $item = ['author-id' => Contact::getIdForURL($activity['actor']), @@ -541,7 +541,7 @@ class Processor // Ensure that the contact has got the right network type self::switchContact($item['author-id']); - Contact::addRelationship($owner, $contact, $item, '', false, $note); + $result = Contact::addRelationship($owner, $contact, $item, false, $note); $cid = Contact::getIdForURL($activity['actor'], $uid); if (empty($cid)) { return; diff --git a/src/Protocol/DFRN.php b/src/Protocol/DFRN.php index f74b572949..01159b1bae 100644 --- a/src/Protocol/DFRN.php +++ b/src/Protocol/DFRN.php @@ -2247,13 +2247,12 @@ class DFRN // The functions below are partly used by ostatus.php as well - where we have this variable $r = q("SELECT * FROM `contact` WHERE `id` = %d", intval($importer["id"])); $contact = $r[0]; - $nickname = $contact["nick"]; // Big question: Do we need these functions? They were part of the "consume_feed" function. // This function once was responsible for DFRN and OStatus. if (activity_match($item["verb"], ACTIVITY_FOLLOW)) { Logger::log("New follower"); - Contact::addRelationship($importer, $contact, $item, $nickname); + Contact::addRelationship($importer, $contact, $item); return false; } if (activity_match($item["verb"], ACTIVITY_UNFOLLOW)) { @@ -2263,7 +2262,7 @@ class DFRN } if (activity_match($item["verb"], ACTIVITY_REQ_FRIEND)) { Logger::log("New friend request"); - Contact::addRelationship($importer, $contact, $item, $nickname, true); + Contact::addRelationship($importer, $contact, $item, true); return false; } if (activity_match($item["verb"], ACTIVITY_UNFRIEND)) { diff --git a/src/Protocol/OStatus.php b/src/Protocol/OStatus.php index dec5c4c80b..180ef4e372 100644 --- a/src/Protocol/OStatus.php +++ b/src/Protocol/OStatus.php @@ -417,13 +417,6 @@ class OStatus $author = self::fetchAuthor($xpath, $entry, $importer, $contact, $stored); } - $value = XML::getFirstNodeValue($xpath, 'atom:author/poco:preferredUsername/text()', $entry); - if ($value != "") { - $nickname = $value; - } else { - $nickname = $author["author-name"]; - } - $item = array_merge($header, $author); $item["uri"] = XML::getFirstNodeValue($xpath, 'atom:id/text()', $entry); @@ -463,7 +456,7 @@ class OStatus } if ($item["verb"] == ACTIVITY_FOLLOW) { - Contact::addRelationship($importer, $contact, $item, $nickname); + Contact::addRelationship($importer, $contact, $item); continue; } From 10dfe4347b491b2b41c6c5b62f0c6ad8a2391ecc Mon Sep 17 00:00:00 2001 From: Hypolite Petovan Date: Sun, 19 May 2019 18:44:57 -0400 Subject: [PATCH 4/8] Ensure follow reject/accept object IDs are strings to avoid JsonLD normalize errors in ActivityPub\Transmitter --- src/Protocol/ActivityPub/Transmitter.php | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/Protocol/ActivityPub/Transmitter.php b/src/Protocol/ActivityPub/Transmitter.php index 7fc5e2df06..ed792db16e 100644 --- a/src/Protocol/ActivityPub/Transmitter.php +++ b/src/Protocol/ActivityPub/Transmitter.php @@ -1538,13 +1538,16 @@ class Transmitter 'id' => System::baseUrl() . '/activity/' . System::createGUID(), 'type' => 'Accept', 'actor' => $owner['url'], - 'object' => ['id' => $id, 'type' => 'Follow', + 'object' => [ + 'id' => (string)$id, + 'type' => 'Follow', 'actor' => $profile['url'], - 'object' => $owner['url']], + 'object' => $owner['url'] + ], 'instrument' => self::getService(), 'to' => [$profile['url']]]; - Logger::log('Sending accept to ' . $target . ' for user ' . $uid . ' with id ' . $id, Logger::DEBUG); + Logger::debug('Sending accept to ' . $target . ' for user ' . $uid . ' with id ' . $id); $signed = LDSignature::sign($data, $owner); HTTPSignature::transmit($signed, $profile['inbox'], $uid); @@ -1568,13 +1571,16 @@ class Transmitter 'id' => System::baseUrl() . '/activity/' . System::createGUID(), 'type' => 'Reject', 'actor' => $owner['url'], - 'object' => ['id' => $id, 'type' => 'Follow', + 'object' => [ + 'id' => (string)$id, + 'type' => 'Follow', 'actor' => $profile['url'], - 'object' => $owner['url']], + 'object' => $owner['url'] + ], 'instrument' => self::getService(), 'to' => [$profile['url']]]; - Logger::log('Sending reject to ' . $target . ' for user ' . $uid . ' with id ' . $id, Logger::DEBUG); + Logger::debug('Sending reject to ' . $target . ' for user ' . $uid . ' with id ' . $id); $signed = LDSignature::sign($data, $owner); HTTPSignature::transmit($signed, $profile['inbox'], $uid); From ea77f214d7dd7206562b8d0687ed483bc1908ec9 Mon Sep 17 00:00:00 2001 From: Hypolite Petovan Date: Sun, 19 May 2019 18:46:29 -0400 Subject: [PATCH 5/8] Add return value to Model\Contact::addRelationship to remove protocol-specific code from it --- src/Model/Contact.php | 26 +++++++++----------------- src/Protocol/ActivityPub/Processor.php | 7 +++++++ 2 files changed, 16 insertions(+), 17 deletions(-) diff --git a/src/Model/Contact.php b/src/Model/Contact.php index e75ac0394a..18b60d2121 100644 --- a/src/Model/Contact.php +++ b/src/Model/Contact.php @@ -2126,6 +2126,7 @@ class Contact extends BaseObject * @param array $datarray An item-like array with at least the 'author-id' and 'author-url' keys for the contact. Mandatory. * @param bool $sharing True: Contact is now sharing with Owner; False: Contact is now following Owner (default) * @param string $note Introduction additional message + * @return bool|null True: follow request is accepted; False: relationship is rejected; Null: relationship is pending * @throws \Friendica\Network\HTTPException\InternalServerErrorException * @throws \ImagickException */ @@ -2133,14 +2134,14 @@ class Contact extends BaseObject { // Should always be set if (empty($datarray['author-id'])) { - return; + return false; } $fields = ['url', 'name', 'nick', 'photo', 'network']; $pub_contact = DBA::selectFirst('contact', $fields, ['id' => $datarray['author-id']]); if (!DBA::isResult($pub_contact)) { // Should never happen - return; + return false; } $url = defaults($datarray, 'author-link', $pub_contact['url']); @@ -2153,26 +2154,20 @@ class Contact extends BaseObject // Make sure that the existing contact isn't archived self::unmarkForArchival($contact); - $protocol = self::getProtocol($url, $contact['network']); - if (($contact['rel'] == self::SHARING) || ($sharing && $contact['rel'] == self::FOLLOWER)) { DBA::update('contact', ['rel' => self::FRIEND, 'writable' => true, 'pending' => false], ['id' => $contact['id'], 'uid' => $importer['uid']]); } - if ($protocol == Protocol::ACTIVITYPUB) { - ActivityPub\Transmitter::sendContactAccept($contact['url'], $contact['hub-verify'], $importer['uid']); - } - + return true; } else { - $protocol = self::getProtocol($url, $network); - // send email notification to owner? if (DBA::exists('contact', ['nurl' => Strings::normaliseLink($url), 'uid' => $importer['uid'], 'pending' => true])) { Logger::log('ignoring duplicated connection request from pending contact ' . $url); - return; + return null; } + // create contact record DBA::insert('contact', [ 'uid' => $importer['uid'], @@ -2237,14 +2232,11 @@ class Contact extends BaseObject $condition = ['uid' => $importer['uid'], 'url' => $url, 'pending' => true]; DBA::update('contact', ['pending' => false], $condition); - $contact = DBA::selectFirst('contact', ['url', 'network', 'hub-verify'], ['id' => $contact_record['id']]); - $protocol = self::getProtocol($contact['url'], $contact['network']); - - if ($protocol == Protocol::ACTIVITYPUB) { - ActivityPub\Transmitter::sendContactAccept($contact['url'], $contact['hub-verify'], $importer['uid']); - } + return true; } } + + return null; } public static function removeFollower($importer, $contact, array $datarray = [], $item = "") diff --git a/src/Protocol/ActivityPub/Processor.php b/src/Protocol/ActivityPub/Processor.php index c63300e84b..8280ccfb78 100644 --- a/src/Protocol/ActivityPub/Processor.php +++ b/src/Protocol/ActivityPub/Processor.php @@ -542,6 +542,13 @@ class Processor self::switchContact($item['author-id']); $result = Contact::addRelationship($owner, $contact, $item, false, $note); + if ($result === false) { + ActivityPub\Transmitter::sendContactReject($item['author-link'], $item['author-id'], $owner['uid']); + return; + }elseif ($result === true) { + ActivityPub\Transmitter::sendContactAccept($item['author-link'], $item['author-id'], $owner['uid']); + } + $cid = Contact::getIdForURL($activity['actor'], $uid); if (empty($cid)) { return; From 62dfcbb07436b69ae3965458cd0dd0855992d33f Mon Sep 17 00:00:00 2001 From: Hypolite Petovan Date: Sun, 19 May 2019 18:46:58 -0400 Subject: [PATCH 6/8] Prevent contact relationships with node- or user-level blocked contacts --- src/Model/Contact.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/Model/Contact.php b/src/Model/Contact.php index 18b60d2121..625c66b199 100644 --- a/src/Model/Contact.php +++ b/src/Model/Contact.php @@ -2137,13 +2137,18 @@ class Contact extends BaseObject return false; } - $fields = ['url', 'name', 'nick', 'photo', 'network']; + $fields = ['url', 'name', 'nick', 'photo', 'network', 'blocked']; $pub_contact = DBA::selectFirst('contact', $fields, ['id' => $datarray['author-id']]); if (!DBA::isResult($pub_contact)) { // Should never happen return false; } + // Contact is blocked on node-level or user-level + if (!empty($pub_contact['blocked']) || !empty($contact['blocked'])) { + return false; + } + $url = defaults($datarray, 'author-link', $pub_contact['url']); $name = $pub_contact['name']; $photo = $pub_contact['photo']; From 7bbaf0757f1549707787f8a5578e032d13c6c177 Mon Sep 17 00:00:00 2001 From: Hypolite Petovan Date: Mon, 20 May 2019 16:33:09 -0400 Subject: [PATCH 7/8] Revert sending ActivityPub follow reject on Model\Contact::addRelationship failure --- src/Protocol/ActivityPub/Processor.php | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/Protocol/ActivityPub/Processor.php b/src/Protocol/ActivityPub/Processor.php index 8280ccfb78..fd84f494e2 100644 --- a/src/Protocol/ActivityPub/Processor.php +++ b/src/Protocol/ActivityPub/Processor.php @@ -542,10 +542,7 @@ class Processor self::switchContact($item['author-id']); $result = Contact::addRelationship($owner, $contact, $item, false, $note); - if ($result === false) { - ActivityPub\Transmitter::sendContactReject($item['author-link'], $item['author-id'], $owner['uid']); - return; - }elseif ($result === true) { + if ($result === true) { ActivityPub\Transmitter::sendContactAccept($item['author-link'], $item['author-id'], $owner['uid']); } From 14a78807d7d9b5a0f6ae8fd10ab3b4476a410b9d Mon Sep 17 00:00:00 2001 From: Hypolite Petovan Date: Mon, 20 May 2019 16:34:17 -0400 Subject: [PATCH 8/8] Use self::isBlocked and self:isBlockedByUser in Model\Contact::addRelationship --- src/Model/Contact.php | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/Model/Contact.php b/src/Model/Contact.php index 625c66b199..a0bbdd1049 100644 --- a/src/Model/Contact.php +++ b/src/Model/Contact.php @@ -2144,8 +2144,8 @@ class Contact extends BaseObject return false; } - // Contact is blocked on node-level or user-level - if (!empty($pub_contact['blocked']) || !empty($contact['blocked'])) { + // Contact is blocked at node-level + if (self::isBlocked($datarray['author-id'])) { return false; } @@ -2156,6 +2156,11 @@ class Contact extends BaseObject $network = $pub_contact['network']; if (!empty($contact)) { + // Contact is blocked at user-level + if (self::isBlockedByUser($contact['id'], $importer['id'])) { + return false; + } + // Make sure that the existing contact isn't archived self::unmarkForArchival($contact);