From 9f4bcbad8ae200d27cc2ba5244e2d878b8718c53 Mon Sep 17 00:00:00 2001 From: Mikael Nordfeldth Date: Tue, 1 Jul 2014 15:48:34 +0200 Subject: [PATCH] checkAuthorship events, Ostatus_profile rewrite to handle it Lost dependency of OStatus plugin for lib/microappplugin.php, whoo! also noting which plugins should be upgraded to new saveActivity support. Favorite plugin won't work with the new system just yet, it doesn't have the necessary functions to extract activity objects, but that's coming in the next (few) commits. --- lib/activityhandlerplugin.php | 61 +++++++++++-------- lib/activityutils.php | 31 ++++++++++ plugins/Blog/BlogPlugin.php | 3 + plugins/Bookmark/BookmarkPlugin.php | 2 + .../GNUsocialPhoto/GNUsocialPhotoPlugin.php | 2 + .../GNUsocialVideo/GNUsocialVideoPlugin.php | 2 + plugins/OStatus/OStatusPlugin.php | 15 +++++ plugins/OStatus/classes/Ostatus_profile.php | 57 +++++++++-------- plugins/Poll/PollPlugin.php | 2 + plugins/QnA/QnAPlugin.php | 3 + 10 files changed, 128 insertions(+), 50 deletions(-) diff --git a/lib/activityhandlerplugin.php b/lib/activityhandlerplugin.php index 6e9e23422e..c8a46ae5ef 100644 --- a/lib/activityhandlerplugin.php +++ b/lib/activityhandlerplugin.php @@ -109,19 +109,8 @@ abstract class ActivityHandlerPlugin extends Plugin * gets to figure out how to actually save it into a notice * and any additional data structures you require. * - * This will handle things received via AtomPub, OStatus - * (PuSH and Salmon transports), or ActivityStreams-based - * backup/restore of account data. - * - * You should be able to accept as input the output from your - * $this->activityObjectFromNotice(). Where applicable, try to - * use existing ActivityStreams structures and object types, - * and be liberal in accepting input from what might be other - * compatible apps. - * - * All micro-app classes must override this method. - * - * @fixme are there any standard options? + * This function is deprecated and in the future, Notice::saveActivity + * should be called from onStartHandleFeedEntryWithProfile in this class. * * @param Activity $activity * @param Profile $actor @@ -129,7 +118,36 @@ abstract class ActivityHandlerPlugin extends Plugin * * @return Notice the resulting notice */ - abstract function saveNoticeFromActivity(Activity $activity, Profile $actor, array $options=array()); + public function saveNoticeFromActivity(Activity $activity, Profile $actor, array $options=array()) + { + // Any plugin which has not implemented saveObjectFromActivity _must_ + // override this function (which will be deleted when all plugins are migrated). + + if (isset($this->oldSaveNew)) { + throw new ServerException('A function has been called for new saveActivity functionality, but is still set with an oldSaveNew configuration'); + } + + return Notice::saveActivity($activity, $actor, $options); + } + + /* + * This usually gets called from Notice::saveActivity after a Notice object has been created, + * so it contains a proper id and a uri for the object to be saved. + */ + public function onStoreActivityObject(Activity $act, Notice $stored, array $options=array(), &$object) { + // $this->oldSaveNew is there during a migration period of plugins, to start using + // Notice::saveActivity instead of Notice::saveNew + if (!$this->isMyActivity($act) || isset($this->oldSaveNew)) { + return true; + } + $object = $this->saveObjectFromActivity($act, $stored, $options); + try { + $act->context->attention = array_merge($act->context->attention, $object->getAttentionArray()); + } catch (Exception $e) { + common_debug('WARNING: Could not get attention list from object '.get_class($object).'!'); + } + return false; + } /** * Given an existing Notice object, your plugin gets to @@ -242,22 +260,18 @@ abstract class ActivityHandlerPlugin extends Plugin * Handle a posted object from PuSH * * @param Activity $activity activity to handle - * @param Ostatus_profile $oprofile Profile for the feed + * @param Profile $actor Profile for the feed * * @return boolean hook value */ - function onStartHandleFeedEntryWithProfile(Activity $activity, $oprofile, &$notice) + function onStartHandleFeedEntryWithProfile(Activity $activity, Profile $profile, &$notice) { if (!$this->isMyActivity($activity)) { return true; } - $actor = $oprofile->checkAuthorship($activity); - - if (!$actor instanceof Ostatus_profile) { - // TRANS: Client exception thrown when no author for an activity was found. - throw new ClientException(_('Cannot get author for activity.')); - } + // We are guaranteed to get a Profile back from checkAuthorship (or it throws an exception) + $profile = ActivityUtils::checkAuthorship($activity, $profile); $object = $activity->objects[0]; @@ -266,8 +280,7 @@ abstract class ActivityHandlerPlugin extends Plugin 'is_local' => Notice::REMOTE, 'source' => 'ostatus'); - // $actor is an ostatus_profile - $notice = $this->saveNoticeFromActivity($activity, $actor->localProfile(), $options); + $notice = $this->saveNoticeFromActivity($activity, $profile, $options); return false; } diff --git a/lib/activityutils.php b/lib/activityutils.php index 07d8a86141..e9d5d1fba5 100644 --- a/lib/activityutils.php +++ b/lib/activityutils.php @@ -402,4 +402,35 @@ class ActivityUtils } return $object; } + + // Check authorship by supplying a Profile as a default and letting plugins + // set it to something else if the activity's author is actually someone + // else (like with a group or peopletag feed as handled in OStatus). + // + // NOTE: Returned is not necessarily the supplied profile! For example, + // the "feed author" may be a group, but the "activity author" is a person! + static function checkAuthorship(Activity $activity, Profile $profile) + { + if (Event::handle('CheckActivityAuthorship', array($activity, &$profile))) { + // if (empty($activity->actor)), then we generated this Activity ourselves and can trust $profile + + $actor_uri = $profile->getUri(); + + if (!in_array($actor_uri, array($activity->actor->id, $activity->actor->link))) { + // A mismatch between our locally stored URI and the supplied author? + // Probably not more than a blog feed or something (with multiple authors or so) + // but log it for future inspection. + common_log(LOG_WARNING, "Got an actor '{$activity->actor->title}' ({$activity->actor->id}) on single-user feed for " . $actor_uri); + } elseif (empty($activity->actor->id)) { + // Plain without ActivityStreams actor info. + // We'll just ignore this info for now and save the update under the feed's identity. + } + } + + if (!$profile instanceof Profile) { + throw new ServerException('Could not get an author Profile for activity'); + } + + return $profile; + } } diff --git a/plugins/Blog/BlogPlugin.php b/plugins/Blog/BlogPlugin.php index 4ff904db0b..c192909d67 100644 --- a/plugins/Blog/BlogPlugin.php +++ b/plugins/Blog/BlogPlugin.php @@ -50,6 +50,9 @@ if (!defined('STATUSNET')) { class BlogPlugin extends MicroAppPlugin { + + var $oldSaveNew = true; + /** * Database schema setup * diff --git a/plugins/Bookmark/BookmarkPlugin.php b/plugins/Bookmark/BookmarkPlugin.php index 1093b435aa..0e3db1ed82 100644 --- a/plugins/Bookmark/BookmarkPlugin.php +++ b/plugins/Bookmark/BookmarkPlugin.php @@ -48,6 +48,8 @@ class BookmarkPlugin extends MicroAppPlugin const VERSION = '0.1'; const IMPORTDELICIOUS = 'BookmarkPlugin:IMPORTDELICIOUS'; + var $oldSaveNew = true; + /** * Authorization for importing delicious bookmarks * diff --git a/plugins/GNUsocialPhoto/GNUsocialPhotoPlugin.php b/plugins/GNUsocialPhoto/GNUsocialPhotoPlugin.php index 7b5c73d09f..4f4f4af827 100644 --- a/plugins/GNUsocialPhoto/GNUsocialPhotoPlugin.php +++ b/plugins/GNUsocialPhoto/GNUsocialPhotoPlugin.php @@ -33,6 +33,8 @@ if (!defined('STATUSNET')) { class GNUsocialPhotoPlugin extends MicroAppPlugin { + var $oldSaveNew = true; + function onCheckSchema() { $schema = Schema::get(); diff --git a/plugins/GNUsocialVideo/GNUsocialVideoPlugin.php b/plugins/GNUsocialVideo/GNUsocialVideoPlugin.php index ae44ee7a2d..b78ed84c42 100644 --- a/plugins/GNUsocialVideo/GNUsocialVideoPlugin.php +++ b/plugins/GNUsocialVideo/GNUsocialVideoPlugin.php @@ -33,6 +33,8 @@ if (!defined('STATUSNET')) { class GNUsocialVideoPlugin extends MicroAppPlugin { + var $oldSaveNew = true; + function onCheckSchema() { $schema = Schema::get(); diff --git a/plugins/OStatus/OStatusPlugin.php b/plugins/OStatus/OStatusPlugin.php index f799b4033d..cebf5597d3 100644 --- a/plugins/OStatus/OStatusPlugin.php +++ b/plugins/OStatus/OStatusPlugin.php @@ -1365,4 +1365,19 @@ class OStatusPlugin extends Plugin { list($mentions, $groups) = Ostatus_profile::filterAttention($actor, $attention_uris); } + + // FIXME: Maybe this shouldn't be so authoritative that it breaks other remote profile lookups? + static public function onCheckActivityAuthorship(Activity $activity, Profile &$profile) + { + try { + $oprofile = Ostatus_profile::getFromProfile($profile); + $oprofile = $oprofile->checkAuthorship($activity); + $profile = $oprofile->localProfile(); + } catch (Exception $e) { + common_log(LOG_ERR, 'Could not get a profile or check authorship ('.get_class($e).': "'.$e->getMessage().'")'); + $profile = null; + return false; + } + return true; + } } diff --git a/plugins/OStatus/classes/Ostatus_profile.php b/plugins/OStatus/classes/Ostatus_profile.php index 3d550872d6..0629d73a75 100644 --- a/plugins/OStatus/classes/Ostatus_profile.php +++ b/plugins/OStatus/classes/Ostatus_profile.php @@ -478,7 +478,7 @@ class Ostatus_profile extends Managed_DataObject // The "WithProfile" events were added later. - if (Event::handle('StartHandleFeedEntryWithProfile', array($activity, $this, &$notice)) && + if (Event::handle('StartHandleFeedEntryWithProfile', array($activity, $this->localProfile(), &$notice)) && Event::handle('StartHandleFeedEntry', array($activity))) { switch ($activity->verb) { @@ -516,10 +516,9 @@ class Ostatus_profile extends Managed_DataObject { $notice = null; - $oprofile = $this->checkAuthorship($activity); - - if (!$oprofile instanceof Ostatus_profile) { - common_log(LOG_INFO, "No author matched share activity"); + try { + $profile = ActivityUtils::checkAuthorship($activity, $this->localProfile()); + } catch (ServerException $e) { return null; } @@ -670,7 +669,7 @@ class Ostatus_profile extends Managed_DataObject if ($activity->context) { // TODO: context->attention list($options['groups'], $options['replies']) - = self::filterAttention($oprofile->localProfile(), $activity->context->attention); + = self::filterAttention($profile, $activity->context->attention); // Maintain direct reply associations // @todo FIXME: What about conversation ID? @@ -713,7 +712,7 @@ class Ostatus_profile extends Managed_DataObject $options['urls'][] = $href; } - $notice = Notice::saveNew($oprofile->profile_id, + $notice = Notice::saveNew($profile->id, $content, 'ostatus', $options); @@ -732,11 +731,7 @@ class Ostatus_profile extends Managed_DataObject { $notice = null; - $oprofile = $this->checkAuthorship($activity); - - if (!$oprofile instanceof Ostatus_profile) { - return null; - } + $profile = $this->checkAuthorship($activity, $this->localProfile()); // It's not always an ActivityObject::NOTE, but... let's just say it is. @@ -839,7 +834,7 @@ class Ostatus_profile extends Managed_DataObject if ($activity->context) { // TODO: context->attention list($options['groups'], $options['replies']) - = self::filterAttention($oprofile->localProfile(), $activity->context->attention); + = self::filterAttention($profile, $activity->context->attention); // Maintain direct reply associations // @todo FIXME: What about conversation ID? @@ -882,7 +877,7 @@ class Ostatus_profile extends Managed_DataObject } try { - $saved = Notice::saveNew($oprofile->profile_id, + $saved = Notice::saveNew($profile->id, $content, 'ostatus', $options); @@ -1077,17 +1072,17 @@ class Ostatus_profile extends Managed_DataObject return null; } - // Is it a known Ostatus profile? - $oprofile = Ostatus_profile::getKV('profile_id', $profile->id); - if ($oprofile instanceof Ostatus_profile) { + try { + $oprofile = self::getFromProfile($profile); + // We found the profile, return it! return $oprofile; - } - - // Is it a local user? - $user = User::getKV('id', $profile->id); - if ($user instanceof User) { - // @todo i18n FIXME: use sprintf and add i18n (?) - throw new OStatusShadowException($profile, "'$profile_url' is the profile for local user '{$user->nickname}'."); + } catch (NoResultException $e) { + // Could not find an OStatus profile, is it instead a local user? + $user = User::getKV('id', $profile->id); + if ($user instanceof User) { + // @todo i18n FIXME: use sprintf and add i18n (?) + throw new OStatusShadowException($profile, "'$profile_url' is the profile for local user '{$user->nickname}'."); + } } // Continue discovery; it's a remote profile @@ -1097,6 +1092,16 @@ class Ostatus_profile extends Managed_DataObject return null; } + static function getFromProfile(Profile $profile) + { + $oprofile = new Ostatus_profile(); + $oprofile->profile_id = $profile->id; + if (!$oprofile->find(true)) { + throw new NoResultException($oprofile); + } + return $oprofile; + } + /** * Look up and if necessary create an Ostatus_profile for remote entity * with the given update feed. This should never return null -- you will @@ -2130,7 +2135,7 @@ class Ostatus_profile extends Managed_DataObject return $oprofile; } - function checkAuthorship($activity) + public function checkAuthorship(Activity $activity) { if ($this->isGroup() || $this->isPeopletag()) { // A group or propletag feed will contain posts from multiple authors. @@ -2165,7 +2170,7 @@ class Ostatus_profile extends Managed_DataObject $oprofile = $this; } - return $oprofile; + return $oprofile->localProfile(); } } diff --git a/plugins/Poll/PollPlugin.php b/plugins/Poll/PollPlugin.php index de882fdf74..d52e69aabe 100644 --- a/plugins/Poll/PollPlugin.php +++ b/plugins/Poll/PollPlugin.php @@ -51,6 +51,8 @@ class PollPlugin extends MicroAppPlugin const POLL_OBJECT = 'http://activityschema.org/object/poll'; const POLL_RESPONSE_OBJECT = 'http://activityschema.org/object/poll-response'; + var $oldSaveNew = true; + /** * Database schema setup * diff --git a/plugins/QnA/QnAPlugin.php b/plugins/QnA/QnAPlugin.php index beae1cb6b2..6542e95f98 100644 --- a/plugins/QnA/QnAPlugin.php +++ b/plugins/QnA/QnAPlugin.php @@ -46,6 +46,9 @@ if (!defined('STATUSNET')) { */ class QnAPlugin extends MicroAppPlugin { + + var $oldSaveNew = true; + /** * Set up our tables (question and answer) *