From 6e31b8d6a54dca2f345aef961539ddae904c0395 Mon Sep 17 00:00:00 2001 From: Hypolite Petovan Date: Fri, 30 Dec 2022 01:45:04 -0500 Subject: [PATCH] Avoid return type exeption in HTTPSignature->post - Prefer passing the owner record array instead of just the uid - +4/-7 calls to User::getOwnerDataById --- src/Core/Protocol.php | 22 +++--- src/Model/Contact/Introduction.php | 5 +- src/Protocol/ActivityPub/Delivery.php | 27 +++++-- src/Protocol/ActivityPub/Transmitter.php | 96 ++++++++++-------------- src/Util/HTTPSignature.php | 26 +++---- src/Worker/APDelivery.php | 10 ++- src/Worker/Contact/RevokeFollow.php | 10 ++- 7 files changed, 101 insertions(+), 95 deletions(-) diff --git a/src/Core/Protocol.php b/src/Core/Protocol.php index e4490da732..98eb386c34 100644 --- a/src/Core/Protocol.php +++ b/src/Core/Protocol.php @@ -174,12 +174,12 @@ class Protocol * Sends an unfollow message. Does not remove the contact * * @param array $contact Target public contact (uid = 0) array - * @param array $user Source local user array + * @param array $owner Source owner-view record * @return bool|null true if successful, false if not, null if no remote action was performed * @throws HTTPException\InternalServerErrorException * @throws \ImagickException */ - public static function unfollow(array $contact, array $user): ?bool + public static function unfollow(array $contact, array $owner): ?bool { if (empty($contact['network'])) { Logger::notice('Contact has got no network, we quit here', ['id' => $contact['id']]); @@ -203,24 +203,24 @@ class Protocol 'uri-id' => 0, ]; - $slap = OStatus::salmon($item, $user); + $slap = OStatus::salmon($item, $owner); if (empty($contact['notify'])) { Logger::notice('OStatus/DFRN Contact is missing notify, we quit here', ['id' => $contact['id']]); return null; } - return Salmon::slapper($user, $contact['notify'], $slap) === 0; + return Salmon::slapper($owner, $contact['notify'], $slap) === 0; } elseif ($protocol == Protocol::DIASPORA) { - return Diaspora::sendUnshare($user, $contact) > 0; + return Diaspora::sendUnshare($owner, $contact) > 0; } elseif ($protocol == Protocol::ACTIVITYPUB) { - return ActivityPub\Transmitter::sendContactUndo($contact['url'], $contact['id'], $user['uid']); + return ActivityPub\Transmitter::sendContactUndo($contact['url'], $contact['id'], $owner); } // Catch-all hook for connector addons $hook_data = [ 'contact' => $contact, - 'uid' => $user['uid'], + 'uid' => $owner['uid'], 'result' => null, ]; Hook::callAll('unfollow', $hook_data); @@ -232,12 +232,12 @@ class Protocol * Revoke an incoming follow from the provided contact * * @param array $contact Target public contact (uid == 0) array - * @param int $uid Source local user id + * @param array $owner Source owner-view record * @return bool|null true if successful, false if not, null if no action was performed * @throws \Friendica\Network\HTTPException\InternalServerErrorException * @throws \ImagickException */ - public static function revokeFollow(array $contact, int $uid): ?bool + public static function revokeFollow(array $contact, array $owner): ?bool { if (empty($contact['network'])) { throw new \InvalidArgumentException('Missing network key in contact array'); @@ -249,13 +249,13 @@ class Protocol } if ($protocol == Protocol::ACTIVITYPUB) { - return ActivityPub\Transmitter::sendContactReject($contact['url'], $contact['hub-verify'], $uid); + return ActivityPub\Transmitter::sendContactReject($contact['url'], $contact['hub-verify'], $owner); } // Catch-all hook for connector addons $hook_data = [ 'contact' => $contact, - 'uid' => $uid, + 'uid' => $owner['uid'], 'result' => null, ]; Hook::callAll('revoke_follow', $hook_data); diff --git a/src/Model/Contact/Introduction.php b/src/Model/Contact/Introduction.php index 7b2f7ebc50..902726413c 100644 --- a/src/Model/Contact/Introduction.php +++ b/src/Model/Contact/Introduction.php @@ -122,7 +122,10 @@ class Introduction } if ($protocol == Protocol::ACTIVITYPUB) { - ActivityPub\Transmitter::sendContactReject($contact['url'], $contact['hub-verify'], $contact['uid']); + $owner = User::getOwnerDataById($contact['uid']); + if ($owner) { + ActivityPub\Transmitter::sendContactReject($contact['url'], $contact['hub-verify'], $owner); + } } } } diff --git a/src/Protocol/ActivityPub/Delivery.php b/src/Protocol/ActivityPub/Delivery.php index 785c626ce7..d0721bcf12 100644 --- a/src/Protocol/ActivityPub/Delivery.php +++ b/src/Protocol/ActivityPub/Delivery.php @@ -29,6 +29,7 @@ use Friendica\Model\Contact; use Friendica\Model\GServer; use Friendica\Model\Item; use Friendica\Model\Post; +use Friendica\Model\User; use Friendica\Protocol\ActivityPub; use Friendica\Protocol\Delivery as ProtocolDelivery; use Friendica\Util\HTTPSignature; @@ -48,8 +49,15 @@ class Delivery $serverfail = false; foreach ($posts as $post) { + $owner = User::getOwnerDataById($post['uid']); + if (!$owner) { + Post\Delivery::remove($post['uri-id'], $inbox); + Post\Delivery::incrementFailed($post['uri-id'], $inbox); + continue; + } + if (!$serverfail) { - $result = self::deliverToInbox($post['command'], 0, $inbox, $post['uid'], $post['receivers'], $post['uri-id']); + $result = self::deliverToInbox($post['command'], 0, $inbox, $owner, $post['receivers'], $post['uri-id']); if ($result['serverfailure']) { // In a timeout situation we assume that every delivery to that inbox will time out. @@ -75,13 +83,16 @@ class Delivery * @param string $cmd * @param integer $item_id * @param string $inbox - * @param integer $uid + * @param array $owner Sender owner-view record * @param array $receivers * @param integer $uri_id * @return array */ - public static function deliverToInbox(string $cmd, int $item_id, string $inbox, int $uid, array $receivers, int $uri_id): array + public static function deliverToInbox(string $cmd, int $item_id, string $inbox, array $owner, array $receivers, int $uri_id): array { + /** @var int $uid */ + $uid = $owner['uid']; + if (empty($item_id) && !empty($uri_id) && !empty($uid)) { $item = Post::selectFirst(['id', 'parent', 'origin', 'gravity', 'verb'], ['uri-id' => $uri_id, 'uid' => [$uid, 0]], ['order' => ['uid' => true]]); if (empty($item['id'])) { @@ -104,21 +115,21 @@ class Delivery if ($cmd == ProtocolDelivery::MAIL) { $data = ActivityPub\Transmitter::createActivityFromMail($item_id); if (!empty($data)) { - $success = HTTPSignature::transmit($data, $inbox, $uid); + $success = HTTPSignature::transmit($data, $inbox, $owner); } } elseif ($cmd == ProtocolDelivery::SUGGESTION) { - $success = ActivityPub\Transmitter::sendContactSuggestion($uid, $inbox, $item_id); + $success = ActivityPub\Transmitter::sendContactSuggestion($owner, $inbox, $item_id); } elseif ($cmd == ProtocolDelivery::RELOCATION) { // @todo Implementation pending } elseif ($cmd == ProtocolDelivery::REMOVAL) { - $success = ActivityPub\Transmitter::sendProfileDeletion($uid, $inbox); + $success = ActivityPub\Transmitter::sendProfileDeletion($owner, $inbox); } elseif ($cmd == ProtocolDelivery::PROFILEUPDATE) { - $success = ActivityPub\Transmitter::sendProfileUpdate($uid, $inbox); + $success = ActivityPub\Transmitter::sendProfileUpdate($owner, $inbox); } else { $data = ActivityPub\Transmitter::createCachedActivityFromItem($item_id); if (!empty($data)) { $timestamp = microtime(true); - $response = HTTPSignature::post($data, $inbox, $uid); + $response = HTTPSignature::post($data, $inbox, $owner); $runtime = microtime(true) - $timestamp; $success = $response->isSuccess(); $serverfail = $response->isTimeout(); diff --git a/src/Protocol/ActivityPub/Transmitter.php b/src/Protocol/ActivityPub/Transmitter.php index 66ee9e4b5c..9a68bc3351 100644 --- a/src/Protocol/ActivityPub/Transmitter.php +++ b/src/Protocol/ActivityPub/Transmitter.php @@ -138,7 +138,7 @@ class Transmitter return false; } - $success = self::sendContactUndo($url, $contact['id'], 0); + $success = self::sendContactUndo($url, $contact['id'], User::getSystemAccount()); if ($success || $force) { Contact::update(['rel' => Contact::NOTHING], ['id' => $contact['id']]); @@ -1903,16 +1903,14 @@ class Transmitter /** * Transmits a contact suggestion to a given inbox * - * @param integer $uid User ID + * @param array $owner Sender owner-view record * @param string $inbox Target inbox * @param integer $suggestion_id Suggestion ID * @return boolean was the transmission successful? - * @throws \Friendica\Network\HTTPException\InternalServerErrorException + * @throws \Exception */ - public static function sendContactSuggestion(int $uid, string $inbox, int $suggestion_id): bool + public static function sendContactSuggestion(array $owner, string $inbox, int $suggestion_id): bool { - $owner = User::getOwnerDataById($uid); - $suggestion = DI::fsuggest()->selectOneById($suggestion_id); $data = [ @@ -1929,22 +1927,20 @@ class Transmitter $signed = LDSignature::sign($data, $owner); - Logger::info('Deliver profile deletion for user ' . $uid . ' to ' . $inbox . ' via ActivityPub'); - return HTTPSignature::transmit($signed, $inbox, $uid); + Logger::info('Deliver profile deletion for user ' . $owner['uid'] . ' to ' . $inbox . ' via ActivityPub'); + return HTTPSignature::transmit($signed, $inbox, $owner); } /** * Transmits a profile relocation to a given inbox * - * @param integer $uid User ID - * @param string $inbox Target inbox + * @param array $owner Sender owner-view record + * @param string $inbox Target inbox * @return boolean was the transmission successful? - * @throws \Friendica\Network\HTTPException\InternalServerErrorException + * @throws \Exception */ - public static function sendProfileRelocation(int $uid, string $inbox): bool + public static function sendProfileRelocation(array $owner, string $inbox): bool { - $owner = User::getOwnerDataById($uid); - $data = [ '@context' => ActivityPub::CONTEXT, 'id' => DI::baseUrl() . '/activity/' . System::createGUID(), @@ -1959,29 +1955,22 @@ class Transmitter $signed = LDSignature::sign($data, $owner); - Logger::info('Deliver profile relocation for user ' . $uid . ' to ' . $inbox . ' via ActivityPub'); - return HTTPSignature::transmit($signed, $inbox, $uid); + Logger::info('Deliver profile relocation for user ' . $owner['uid'] . ' to ' . $inbox . ' via ActivityPub'); + return HTTPSignature::transmit($signed, $inbox, $owner); } /** * Transmits a profile deletion to a given inbox * - * @param integer $uid User ID - * @param string $inbox Target inbox + * @param array $owner Sender owner-view record + * @param string $inbox Target inbox * @return boolean was the transmission successful? - * @throws \Friendica\Network\HTTPException\InternalServerErrorException + * @throws \Exception */ - public static function sendProfileDeletion(int $uid, string $inbox): bool + public static function sendProfileDeletion(array $owner, string $inbox): bool { - $owner = User::getOwnerDataById($uid); - - if (empty($owner)) { - Logger::error('No owner data found, the deletion message cannot be processed.', ['user' => $uid]); - return false; - } - if (empty($owner['uprvkey'])) { - Logger::error('No private key for owner found, the deletion message cannot be processed.', ['user' => $uid]); + Logger::error('No private key for owner found, the deletion message cannot be processed.', ['user' => $owner['uid']]); return false; } @@ -1997,30 +1986,29 @@ class Transmitter $signed = LDSignature::sign($data, $owner); - Logger::info('Deliver profile deletion for user ' . $uid . ' to ' . $inbox . ' via ActivityPub'); - return HTTPSignature::transmit($signed, $inbox, $uid); + Logger::info('Deliver profile deletion for user ' . $owner['uid'] . ' to ' . $inbox . ' via ActivityPub'); + return HTTPSignature::transmit($signed, $inbox, $owner); } /** * Transmits a profile change to a given inbox * - * @param integer $uid User ID - * @param string $inbox Target inbox + * @param array $owner Sender owner-view record + * @param string $inbox Target inbox * @return boolean was the transmission successful? * @throws HTTPException\InternalServerErrorException * @throws HTTPException\NotFoundException * @throws \ImagickException */ - public static function sendProfileUpdate(int $uid, string $inbox): bool + public static function sendProfileUpdate(array $owner, string $inbox): bool { - $owner = User::getOwnerDataById($uid); $profile = APContact::getByURL($owner['url']); $data = ['@context' => ActivityPub::CONTEXT, 'id' => DI::baseUrl() . '/activity/' . System::createGUID(), 'type' => 'Update', 'actor' => $owner['url'], - 'object' => self::getProfile($uid), + 'object' => self::getProfile($owner['uid']), 'published' => DateTimeFormat::utcNow(DateTimeFormat::ATOM), 'instrument' => self::getService(), 'to' => [$profile['followers']], @@ -2028,8 +2016,8 @@ class Transmitter $signed = LDSignature::sign($data, $owner); - Logger::info('Deliver profile update for user ' . $uid . ' to ' . $inbox . ' via ActivityPub'); - return HTTPSignature::transmit($signed, $inbox, $uid); + Logger::info('Deliver profile update for user ' . $owner['uid'] . ' to ' . $inbox . ' via ActivityPub'); + return HTTPSignature::transmit($signed, $inbox, $owner); } /** @@ -2075,7 +2063,7 @@ class Transmitter Logger::info('Sending activity ' . $activity . ' to ' . $target . ' for user ' . $uid); $signed = LDSignature::sign($data, $owner); - return HTTPSignature::transmit($signed, $profile['inbox'], $uid); + return HTTPSignature::transmit($signed, $profile['inbox'], $owner); } /** @@ -2131,7 +2119,7 @@ class Transmitter Logger::info('Sending follow ' . $object . ' to ' . $target . ' for user ' . $uid); $signed = LDSignature::sign($data, $owner); - return HTTPSignature::transmit($signed, $profile['inbox'], $uid); + return HTTPSignature::transmit($signed, $profile['inbox'], $owner); } /** @@ -2153,6 +2141,11 @@ class Transmitter } $owner = User::getOwnerDataById($uid); + if (!$owner) { + Logger::notice('No user found for actor', ['uid' => $uid]); + return; + } + $data = [ '@context' => ActivityPub::CONTEXT, 'id' => DI::baseUrl() . '/activity/' . System::createGUID(), @@ -2171,7 +2164,7 @@ class Transmitter Logger::debug('Sending accept to ' . $target . ' for user ' . $uid . ' with id ' . $id); $signed = LDSignature::sign($data, $owner); - HTTPSignature::transmit($signed, $profile['inbox'], $uid); + HTTPSignature::transmit($signed, $profile['inbox'], $owner); } /** @@ -2179,12 +2172,12 @@ class Transmitter * * @param string $target Target profile * @param string $objectId Object id - * @param int $uid User ID + * @param array $owner Sender owner-view record * @return bool Operation success * @throws HTTPException\InternalServerErrorException * @throws \ImagickException */ - public static function sendContactReject(string $target, string $objectId, int $uid): bool + public static function sendContactReject(string $target, string $objectId, array $owner): bool { $profile = APContact::getByURL($target); if (empty($profile['inbox'])) { @@ -2192,12 +2185,6 @@ class Transmitter return false; } - $owner = User::getOwnerDataById($uid); - if (empty($owner)) { - Logger::notice('No user found for actor', ['uid' => $uid]); - return false; - } - $data = [ '@context' => ActivityPub::CONTEXT, 'id' => DI::baseUrl() . '/activity/' . System::createGUID(), @@ -2213,10 +2200,10 @@ class Transmitter 'to' => [$profile['url']], ]; - Logger::debug('Sending reject to ' . $target . ' for user ' . $uid . ' with id ' . $objectId); + Logger::debug('Sending reject to ' . $target . ' for user ' . $owner['uid'] . ' with id ' . $objectId); $signed = LDSignature::sign($data, $owner); - return HTTPSignature::transmit($signed, $profile['inbox'], $uid); + return HTTPSignature::transmit($signed, $profile['inbox'], $owner); } /** @@ -2224,13 +2211,13 @@ class Transmitter * * @param string $target Target profile * @param integer $cid Contact id - * @param integer $uid User ID + * @param array $owner Sender owner-view record * @return bool success * @throws \Friendica\Network\HTTPException\InternalServerErrorException * @throws \ImagickException * @throws \Exception */ - public static function sendContactUndo(string $target, int $cid, int $uid): bool + public static function sendContactUndo(string $target, int $cid, array $owner): bool { $profile = APContact::getByURL($target); if (empty($profile['inbox'])) { @@ -2245,7 +2232,6 @@ class Transmitter $objectId = DI::baseUrl() . '/activity/' . System::createGUID(); - $owner = User::getOwnerDataById($uid); $data = [ '@context' => ActivityPub::CONTEXT, 'id' => $objectId, @@ -2261,10 +2247,10 @@ class Transmitter 'to' => [$profile['url']], ]; - Logger::info('Sending undo to ' . $target . ' for user ' . $uid . ' with id ' . $objectId); + Logger::info('Sending undo to ' . $target . ' for user ' . $owner['uid'] . ' with id ' . $objectId); $signed = LDSignature::sign($data, $owner); - return HTTPSignature::transmit($signed, $profile['inbox'], $uid); + return HTTPSignature::transmit($signed, $profile['inbox'], $owner); } /** diff --git a/src/Util/HTTPSignature.php b/src/Util/HTTPSignature.php index 225a2bcf20..47000f5c74 100644 --- a/src/Util/HTTPSignature.php +++ b/src/Util/HTTPSignature.php @@ -267,21 +267,14 @@ class HTTPSignature /** * Post given data to a target for a user, returns the result class * - * @param array $data Data that is about to be send - * @param string $target The URL of the inbox - * @param integer $uid User id of the sender + * @param array $data Data that is about to be sent + * @param string $target The URL of the inbox + * @param array $owner Sender owner-view record * * @return ICanHandleHttpResponses - * @throws \Friendica\Network\HTTPException\InternalServerErrorException */ - public static function post(array $data, string $target, int $uid): ICanHandleHttpResponses + public static function post(array $data, string $target, array $owner): ICanHandleHttpResponses { - $owner = User::getOwnerDataById($uid); - - if (!$owner) { - return null; - } - $content = json_encode($data, JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE); // Header data that is about to be signed. @@ -319,16 +312,15 @@ class HTTPSignature /** * Transmit given data to a target for a user * - * @param array $data Data that is about to be send - * @param string $target The URL of the inbox - * @param integer $uid User id of the sender + * @param array $data Data that is about to be sent + * @param string $target The URL of the inbox + * @param array $owner Sender owner-vew record * * @return boolean Was the transmission successful? - * @throws \Friendica\Network\HTTPException\InternalServerErrorException */ - public static function transmit(array $data, string $target, int $uid): bool + public static function transmit(array $data, string $target, array $owner): bool { - $postResult = self::post($data, $target, $uid); + $postResult = self::post($data, $target, $owner); $return_code = $postResult->getReturnCode(); return ($return_code >= 200) && ($return_code <= 299); diff --git a/src/Worker/APDelivery.php b/src/Worker/APDelivery.php index 203c409761..16c8290382 100644 --- a/src/Worker/APDelivery.php +++ b/src/Worker/APDelivery.php @@ -24,6 +24,7 @@ namespace Friendica\Worker; use Friendica\Core\Logger; use Friendica\Core\Worker; use Friendica\Model\Post; +use Friendica\Model\User; use Friendica\Protocol\ActivityPub; class APDelivery { @@ -69,7 +70,14 @@ class APDelivery $drop = false; $uri_ids = $result['uri_ids']; } else { - $result = ActivityPub\Delivery::deliverToInbox($cmd, $item_id, $inbox, $uid, $receivers, $uri_id); + $owner = User::getOwnerDataById($uid); + if (!$owner) { + Post\Delivery::remove($uri_id, $inbox); + Post\Delivery::incrementFailed($uri_id, $inbox); + return; + } + + $result = ActivityPub\Delivery::deliverToInbox($cmd, $item_id, $inbox, $owner, $receivers, $uri_id); $success = $result['success']; $drop = $result['drop']; $uri_ids = [$uri_id]; diff --git a/src/Worker/Contact/RevokeFollow.php b/src/Worker/Contact/RevokeFollow.php index 726a69b8da..f44ac59dfb 100644 --- a/src/Worker/Contact/RevokeFollow.php +++ b/src/Worker/Contact/RevokeFollow.php @@ -24,6 +24,8 @@ namespace Friendica\Worker\Contact; use Friendica\Core\Protocol; use Friendica\Core\Worker; use Friendica\Model\Contact; +use Friendica\Model\User; +use Friendica\Network\HTTPException; class RevokeFollow { @@ -43,8 +45,12 @@ class RevokeFollow return; } - $result = Protocol::revokeFollow($contact, $uid); - if ($result === false) { + $owner = User::getOwnerDataById($uid, false); + if (empty($owner)) { + return; + } + + if (!Protocol::revokeFollow($contact, $owner)) { Worker::defer(); } }