From 73af7be061146f3eb462e70c2019dcd61ca4609d Mon Sep 17 00:00:00 2001 From: Diogo Cordeiro Date: Tue, 10 Dec 2019 22:27:32 +0000 Subject: [PATCH] [ActivityPub] Properly handle Actor URIs by using events correctly This should fix nulls on explorer lookups inputed by postman after generate_followers/getSubscribers, that I think were caused by calling common_profile_uri that, curiously, only handles local profiles --- lib/util/util.php | 3 +- plugins/ActivityPub/ActivityPubPlugin.php | 50 +++++-------------- .../ActivityPub/actions/apactorfollowers.php | 2 +- .../ActivityPub/actions/apactorfollowing.php | 2 +- plugins/ActivityPub/actions/apactoroutbox.php | 2 +- .../classes/Activitypub_profile.php | 22 ++++---- .../ActivityPub/classes/Activitypub_rsa.php | 2 +- plugins/ActivityPub/lib/httpsignature.php | 2 +- .../lib/models/Activitypub_announce.php | 2 +- .../lib/models/Activitypub_follow.php | 6 +-- .../lib/models/Activitypub_message.php | 2 +- .../lib/models/Activitypub_notice.php | 4 +- plugins/ActivityPub/lib/postman.php | 4 +- 13 files changed, 37 insertions(+), 66 deletions(-) diff --git a/lib/util/util.php b/lib/util/util.php index c4f24f33a5..fb631bb68d 100644 --- a/lib/util/util.php +++ b/lib/util/util.php @@ -2330,7 +2330,8 @@ function common_profile_uri($profile) $user = User::getKV('id', $profile->id); if ($user instanceof User) { $uri = $user->getUri(); - } + } // FIXME: might be a remote profile, by this function name, I would guess it would be fine to call this + // On the other hand, there's Profile->getUri Event::handle('EndCommonProfileURI', [$profile, &$uri]); } } diff --git a/plugins/ActivityPub/ActivityPubPlugin.php b/plugins/ActivityPub/ActivityPubPlugin.php index 9c48278f84..0df868f408 100644 --- a/plugins/ActivityPub/ActivityPubPlugin.php +++ b/plugins/ActivityPub/ActivityPubPlugin.php @@ -60,29 +60,22 @@ class ActivityPubPlugin extends Plugin /** * Returns a Actor's URI from its local $profile * Works both for local and remote users. + * This is a discovery event but it seems more logical to have it separated. + * This ensures that Profile->getUri() will always return the intended for a remote AP profile. * * @param Profile $profile Actor's local profile - * @return string Actor's URI - * @throws Exception + * @param string &$uri I/O Actor's URI * @author Diogo Cordeiro + * @return bool event hook */ - public static function actor_uri($profile) + public function onStartGetProfileUri(Profile $profile, &$uri): bool { - return common_profile_uri($profile); - } - - /** - * Returns a Actor's URL from its local $profile - * Works both for local and remote users. - * - * @param Profile $profile Actor's local profile - * @return string Actor's URL - * @throws Exception - * @author Diogo Cordeiro - */ - public static function actor_url($profile) - { - return ActivityPubPlugin::actor_uri($profile)."/"; + $aprofile = Activitypub_profile::getKV('profile_id', $profile->id); + if ($aprofile instanceof Activitypub_profile) { + $uri = $aprofile->getUri(); + return false; + } + return true; } /** @@ -621,7 +614,7 @@ class ActivityPubPlugin extends Plugin if ($object->isPerson()) { $link = new XML_XRD_Element_Link( 'self', - ActivityPubPlugin::actor_uri($object->getProfile()), + $object->getProfile()->getUri(), 'application/activity+json' ); $xrd->links[] = clone($link); @@ -743,25 +736,6 @@ class ActivityPubPlugin extends Plugin * Discovery Events * ********************************************************/ - /** - * Profile URI for remote profiles. - * - * @author GNU social - * @author Diogo Cordeiro - * @param Profile $profile - * @param string $uri in/out - * @return mixed hook return code - */ - public function onStartGetProfileUri(Profile $profile, &$uri) - { - $aprofile = Activitypub_profile::getKV('profile_id', $profile->id); - if ($aprofile instanceof Activitypub_profile) { - $uri = $aprofile->getUri(); - return false; - } - return true; - } - /** * Profile from URI. * diff --git a/plugins/ActivityPub/actions/apactorfollowers.php b/plugins/ActivityPub/actions/apactorfollowers.php index 710877fc60..74df047d18 100644 --- a/plugins/ActivityPub/actions/apactorfollowers.php +++ b/plugins/ActivityPub/actions/apactorfollowers.php @@ -122,7 +122,7 @@ class apActorFollowersAction extends ManagedAction /* Get followers' URLs */ foreach ($sub as $s) { - $subs[] = ActivityPubPlugin::actor_uri($s); + $subs[] = $s->getUri(); } } catch (NoResultException $e) { // Just let the exception go on its merry way diff --git a/plugins/ActivityPub/actions/apactorfollowing.php b/plugins/ActivityPub/actions/apactorfollowing.php index 8cf4ab51a4..3382d40692 100644 --- a/plugins/ActivityPub/actions/apactorfollowing.php +++ b/plugins/ActivityPub/actions/apactorfollowing.php @@ -122,7 +122,7 @@ class apActorFollowingAction extends ManagedAction /* Get followed' URLs */ foreach ($sub as $s) { - $subs[] = ActivityPubPlugin::actor_uri($s); + $subs[] = $s->getUri(); } } catch (NoResultException $e) { // Just let the exception go on its merry way diff --git a/plugins/ActivityPub/actions/apactoroutbox.php b/plugins/ActivityPub/actions/apactoroutbox.php index 2c3449949a..3762e80a72 100644 --- a/plugins/ActivityPub/actions/apactoroutbox.php +++ b/plugins/ActivityPub/actions/apactoroutbox.php @@ -123,7 +123,7 @@ class apActorOutboxAction extends ManagedAction // TODO: Handle other types if ($note->object_type == 'http://activitystrea.ms/schema/1.0/note') { $notices[] = Activitypub_create::create_to_array( - ActivityPubPlugin::actor_uri($note->getProfile()), + $note->getProfile()->getUri(), Activitypub_notice::notice_to_array($note) ); } diff --git a/plugins/ActivityPub/classes/Activitypub_profile.php b/plugins/ActivityPub/classes/Activitypub_profile.php index 954bf69706..fc241ca246 100644 --- a/plugins/ActivityPub/classes/Activitypub_profile.php +++ b/plugins/ActivityPub/classes/Activitypub_profile.php @@ -86,7 +86,7 @@ class Activitypub_profile extends Managed_DataObject */ public static function profile_to_array($profile) { - $uri = ActivityPubPlugin::actor_uri($profile); + $uri = $profile->getUri(); $id = $profile->getID(); $rsa = new Activitypub_rsa(); $public_key = $rsa->ensure_public_key($profile); @@ -589,20 +589,16 @@ class Activitypub_profile extends Managed_DataObject } $subs = Subscription::getSubscriberIDs($profile->id, $offset, $limit); - try { - $profiles = []; + $profiles = []; - $users = User::multiGet('id', $subs); - foreach ($users->fetchAll() as $user) { - $profiles[$user->id] = $user->getProfile(); - } + $users = User::multiGet('id', $subs); + foreach ($users->fetchAll() as $user) { + $profiles[$user->id] = $user->getProfile(); + } - $ap_profiles = Activitypub_profile::multiGet('profile_id', $subs); - foreach ($ap_profiles->fetchAll() as $ap) { - $profiles[$ap->getID()] = $ap->local_profile(); - } - } catch (NoResultException $e) { - return $e->obj; + $ap_profiles = Activitypub_profile::multiGet('profile_id', $subs); + foreach ($ap_profiles->fetchAll() as $ap) { + $profiles[$ap->getID()] = $ap->local_profile(); } if ($cache) { diff --git a/plugins/ActivityPub/classes/Activitypub_rsa.php b/plugins/ActivityPub/classes/Activitypub_rsa.php index 2cc1555587..bd32090c3c 100644 --- a/plugins/ActivityPub/classes/Activitypub_rsa.php +++ b/plugins/ActivityPub/classes/Activitypub_rsa.php @@ -115,7 +115,7 @@ class Activitypub_rsa extends Managed_DataObject // ASSERT: This should never happen, but try to recover! common_log(LOG_ERR, "Activitypub_rsa: An impossible thing has happened... Please let the devs know that it entered in line 116 at Activitypub_rsa.php"); if ($fetch) { - $res = Activitypub_explorer::get_remote_user_activity(ActivityPubPlugin::actor_uri($profile)); + $res = Activitypub_explorer::get_remote_user_activity($profile->getUri()); Activitypub_rsa::update_public_key($profile, $res['publicKey']['publicKeyPem']); return self::ensure_public_key($profile, false); } else { diff --git a/plugins/ActivityPub/lib/httpsignature.php b/plugins/ActivityPub/lib/httpsignature.php index 7f33790592..02d4db30fc 100644 --- a/plugins/ActivityPub/lib/httpsignature.php +++ b/plugins/ActivityPub/lib/httpsignature.php @@ -36,7 +36,7 @@ class HttpSignature $key = openssl_pkey_get_private($actor_private_key); openssl_sign($stringToSign, $signature, $key, OPENSSL_ALGO_SHA256); $signature = base64_encode($signature); - $signatureHeader = 'keyId="' . ActivityPubPlugin::actor_uri($user).'#public-key' . '",headers="' . $signedHeaders . '",algorithm="rsa-sha256",signature="' . $signature . '"'; + $signatureHeader = 'keyId="' . $user->getUri() . '#public-key' . '",headers="' . $signedHeaders . '",algorithm="rsa-sha256",signature="' . $signature . '"'; unset($headers['(request-target)']); $headers['Signature'] = $signatureHeader; diff --git a/plugins/ActivityPub/lib/models/Activitypub_announce.php b/plugins/ActivityPub/lib/models/Activitypub_announce.php index 6c57892c65..7ffa6541f3 100644 --- a/plugins/ActivityPub/lib/models/Activitypub_announce.php +++ b/plugins/ActivityPub/lib/models/Activitypub_announce.php @@ -46,7 +46,7 @@ class Activitypub_announce */ public static function announce_to_array(Profile $actor, Notice $notice): array { - $actor_uri = ActivityPubPlugin::actor_uri($actor); + $actor_uri = $actor->getUri(); $notice_url = Activitypub_notice::getUrl($notice); $to = [common_local_url('apActorFollowers', ['id' => $actor->getID()])]; diff --git a/plugins/ActivityPub/lib/models/Activitypub_follow.php b/plugins/ActivityPub/lib/models/Activitypub_follow.php index a79bbc32b6..c4bcc5d304 100644 --- a/plugins/ActivityPub/lib/models/Activitypub_follow.php +++ b/plugins/ActivityPub/lib/models/Activitypub_follow.php @@ -85,13 +85,13 @@ class Activitypub_follow if (!Subscription::exists($actor_profile, $object_profile)) { Subscription::start($actor_profile, $object_profile); Activitypub_profile::subscribeCacheUpdate($actor_profile, $object_profile); - common_debug('ActivityPubPlugin: Accepted Follow request from '.ActivityPubPlugin::actor_uri($actor_profile).' to '.$object); + common_debug('ActivityPubPlugin: Accepted Follow request from '.$actor_profile->getUri().' to '.$object); } else { - common_debug('ActivityPubPlugin: Received a repeated Follow request from '.ActivityPubPlugin::actor_uri($actor_profile).' to '.$object); + common_debug('ActivityPubPlugin: Received a repeated Follow request from '.$actor_profile->getUri().' to '.$object); } // Notify remote instance that we have accepted their request - common_debug('ActivityPubPlugin: Notifying remote instance that we have accepted their Follow request request from '.ActivityPubPlugin::actor_uri($actor_profile).' to '.$object); + common_debug('ActivityPubPlugin: Notifying remote instance that we have accepted their Follow request request from '.$actor_profile->getUri().' to '.$object); $postman = new Activitypub_postman($object_profile, [$actor_aprofile]); $postman->accept_follow($id); } diff --git a/plugins/ActivityPub/lib/models/Activitypub_message.php b/plugins/ActivityPub/lib/models/Activitypub_message.php index 65dda4ccb2..e7d9bdceea 100644 --- a/plugins/ActivityPub/lib/models/Activitypub_message.php +++ b/plugins/ActivityPub/lib/models/Activitypub_message.php @@ -63,7 +63,7 @@ class Activitypub_message 'id' => common_local_url('showmessage', ['message' => $message->getID()]), 'type' => 'Note', 'published' => str_replace(' ', 'T', $message->created).'Z', - 'attributedTo' => ActivityPubPlugin::actor_uri($from), + 'attributedTo' => $from->getUri(), 'to' => $to, 'cc' => [], 'content' => $message->getRendered(), diff --git a/plugins/ActivityPub/lib/models/Activitypub_notice.php b/plugins/ActivityPub/lib/models/Activitypub_notice.php index da61c2a72c..fb8204d2b3 100644 --- a/plugins/ActivityPub/lib/models/Activitypub_notice.php +++ b/plugins/ActivityPub/lib/models/Activitypub_notice.php @@ -85,7 +85,7 @@ class Activitypub_notice 'type' => 'Note', 'published' => str_replace(' ', 'T', $notice->getCreated()) . 'Z', 'url' => self::getUrl($notice), - 'attributedTo' => ActivityPubPlugin::actor_uri($profile), + 'attributedTo' => $profile->getUri(), 'to' => $to, 'cc' => $cc, 'conversation' => $notice->getConversationUrl(), @@ -205,7 +205,7 @@ class Activitypub_notice foreach ($mentions_profiles as $mp) { if (!$mp->hasBlocked($actor_profile)) { - $act->context->attention[ActivityPubPlugin::actor_uri($mp)] = 'http://activitystrea.ms/schema/1.0/person'; + $act->context->attention[$mp->getUri()] = 'http://activitystrea.ms/schema/1.0/person'; } } diff --git a/plugins/ActivityPub/lib/postman.php b/plugins/ActivityPub/lib/postman.php index 95baa12038..1cb79ccf57 100644 --- a/plugins/ActivityPub/lib/postman.php +++ b/plugins/ActivityPub/lib/postman.php @@ -58,7 +58,7 @@ class Activitypub_postman $this->actor = $from; $this->to = $to; - $this->actor_uri = ActivityPubPlugin::actor_uri($this->actor); + $this->actor_uri = $this->actor->getUri(); $this->client = new HTTPClient(); } @@ -364,7 +364,7 @@ class Activitypub_postman public function delete_note($notice) { $data = Activitypub_delete::delete_to_array( - ActivityPubPlugin::actor_uri($notice->getProfile()), + $notice->getProfile()->getUri(), Activitypub_notice::getUrl($notice) ); $errors = [];