GNUsocial/gnu-social
4
0
Fork 1
Code Issues Releases Wiki Activity

[ActivityPub] Fix some bugs with onStartNoticeSearch

Browse Source 
Refactored Activitypub_profile::ensure_web_finger to Activitypub_profile::ensure_webfinger
Do not throw exceptions in the handling of this event because we don't
want to stop the regular search just because we were unable to find
ActivityPub actors or notes.
This commit is contained in:
Diogo Cordeiro 2019-09-13 11:56:36 +01:00
parent 1f2f57b03b
commit 4eb4a2de00
4 changed files with 108 additions and 83 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

133
plugins/ActivityPub/ActivityPubPlugin.php
View File

@ -87,13 +87,13 @@ class ActivityPubPlugin extends Plugin
/** /**
* Returns a notice from its URL. * Returns a notice from its URL.
* *
* @author Diogo Cordeiro <diogo@fc.up.pt>
* @param string $url Notice's URL * @param string $url Notice's URL
* @param bool $grabOnline whether to try online grabbing, defaults to true * @param bool $grab_online whether to try online grabbing, defaults to true
* @return Notice|null The Notice object * @return Notice|null The Notice object
* @throws Exception This function or provides a Notice, null, or fails with exception * @throws Exception This function or provides a Notice, null, or fails with exception
* @author Diogo Cordeiro <diogo@fc.up.pt>
*/ */
public static function grab_notice_from_url(string $url, bool $grabOnline = true): ?Notice public static function grab_notice_from_url(string $url, bool $grab_online = true): ?Notice
{ {
/* Offline Grabbing */ /* Offline Grabbing */
try { try {
@ -114,7 +114,7 @@ class ActivityPubPlugin extends Plugin
} }
} }
if ($grabOnline) { if ($grab_online) {
/* Online Grabbing */ /* Online Grabbing */
$client = new HTTPClient(); $client = new HTTPClient();
$headers = []; $headers = [];
@ -319,7 +319,7 @@ class ActivityPubPlugin extends Plugin
/** /**
* Validate AP-recipients for profile page message action addition * Validate AP-recipients for profile page message action addition
* *
* @param Profile $recipient * @param Profile $recipient
* @return bool hook return value * @return bool hook return value
*/ */
@ -334,7 +334,7 @@ class ActivityPubPlugin extends Plugin
/** /**
* Mark an ap_profile object for deletion * Mark an ap_profile object for deletion
* *
* @param Profile profile being deleted * @param Profile profile being deleted
* @param array &$related objects with same profile_id to be deleted * @param array &$related objects with same profile_id to be deleted
* @return void * @return void
@ -353,7 +353,7 @@ class ActivityPubPlugin extends Plugin
} }
} catch (Exception $e) { } catch (Exception $e) {
// nothing to do // nothing to do
} }
} }
/** /**
@ -400,70 +400,91 @@ class ActivityPubPlugin extends Plugin
/** /**
* Hack the notice search-box and try to grab remote profiles or notices. * Hack the notice search-box and try to grab remote profiles or notices.
* *
* Note that, on successful grabbing, this function will redirect to the * Note that, on successful grabbing, this function will redirect to the
* new profile/notice, so URL searching is directly affected. A good solution * new profile/notice, so URL searching is directly affected. A good solution
* for this is to store the URLs in the notice text without the https/http * for this is to store the URLs in the notice text without the https/http
* prefixes. This would change the queries for URL searching and therefore we * prefixes. This would change the queries for URL searching and therefore we
* could do both search and grab. * could do both search and grab.
* *
* @param string $query search query * @param string $query search query
* @return void * @return bool hook
* @author Bruno Casteleiro <up201505347@fc.up.pt>
*/ */
public function onStartNoticeSearch(string $query): void public function onStartNoticeSearch(string $query): bool
{ {
if (!common_logged_in()) { if (!common_logged_in()) {
// early return, search-only for non-logged sessions // early return: Only allow logged users to import/search for remote actors or notes
return; return true;
} }
if (!filter_var($query, FILTER_VALIDATE_URL) && if (preg_match('!^((?:\w+\.)*\w+@(?:\w+\.)*\w+(?:\w+\-\w+)*\.\w+)$!', $query)) { // WebFinger ID found!
!preg_match('!^((?:\w+\.)*\w+@(?:\w+\.)*\w+(?:\w+\-\w+)*\.\w+)$!', $query)) { // Try to grab remote actor
// early return, not an url or webfinger ID $aprofile = self::pull_remote_profile($query);
return; if ($aprofile instanceof Activitypub_profile) {
} $url = common_local_url('userbyid', ['id' => $aprofile->getID()], null, null, false);
common_redirect($url, 303);
// someone we know about ? return true;
try {
$explorer = new Activitypub_explorer();
$profile = $explorer->lookup($query, false)[0];
if ($profile instanceof Profile) {
return;
} }
} catch (Exception $e) { } elseif (filter_var($query, FILTER_VALIDATE_URL)) { // URL found!
// nope /* Is this an ActivityPub notice? */
}
// some notice we know about ? // If we already know it, just return
try { try {
$notice = self::grab_notice_from_url($query, false); $notice = self::grab_notice_from_url($query, false); // Only check locally
if ($notice instanceof Notice) { if ($notice instanceof Notice) {
return; return true;
}
} catch (Exception $e) {
// We will next try online
} }
} catch (Exception $e) {
// nope
}
// try to grab profile // Otherwise, try to grab it
$aprofile = self::pull_remote_profile($query); try {
if ($aprofile instanceof Activitypub_profile) { $notice = self::grab_notice_from_url($query); // Unfortunately we will be trying locally again
$url = common_local_url('userbyid', ['id' => $aprofile->getID()], null, null, false); if ($notice instanceof Notice) {
common_redirect($url, 303); $url = common_local_url('shownotice', ['notice' => $notice->getID()]);
return; common_redirect($url, 303);
} }
} catch (Exception $e) {
// We will next check if this URL is an actor
}
// try to grab notice /* Is this an ActivityPub actor? */
$notice = self::grab_notice_from_url($query);
if ($notice instanceof Notice) { // If we already know it, just return
$url = common_local_url('shownotice', ['notice' => $notice->getID()]); try {
common_redirect($url, 303); $explorer = new Activitypub_explorer();
$profile = $explorer->lookup($query, false)[0]; // Only check locally
if ($profile instanceof Profile) {
return true;
}
} catch (Exception $e) {
// We will next try online
}
// Try to grab remote actor
try {
if (!isset($explorer)) {
$explorer = new Activitypub_explorer();
}
$profile = $explorer->lookup($query)[0]; // Unfortunately we will be trying locally again
if ($profile instanceof Profile) {
$url = common_local_url('userbyid', ['id' => $profile->getID()], null, null, false);
common_redirect($url, 303);
return true;
}
} catch (Exception $e) {
// Let the search run naturally
}
} }
return true;
} }
/** /**
* Make sure necessary tables are filled out. * Make sure necessary tables are filled out.
* *
* @return boolean hook true * @return bool hook true
*/ */
public function onCheckSchema() public function onCheckSchema()
{ {
@ -481,17 +502,17 @@ class ActivityPubPlugin extends Plugin
/** /**
* Get remote user's ActivityPub_profile via a identifier * Get remote user's ActivityPub_profile via a identifier
* *
* @author GNU social
* @author Diogo Cordeiro <diogo@fc.up.pt>
* @param string $arg A remote user identifier * @param string $arg A remote user identifier
* @return Activitypub_profile|null Valid profile in success | null otherwise * @return Activitypub_profile|null Valid profile in success | null otherwise
* @author GNU social
* @author Diogo Cordeiro <diogo@fc.up.pt>
*/ */
public static function pull_remote_profile($arg) public static function pull_remote_profile($arg)
{ {
if (preg_match('!^((?:\w+\.)*\w+@(?:\w+\.)*\w+(?:\w+\-\w+)*\.\w+)$!', $arg)) { if (preg_match('!^((?:\w+\.)*\w+@(?:\w+\.)*\w+(?:\w+\-\w+)*\.\w+)$!', $arg)) {
// webfinger lookup // webfinger lookup
try { try {
return Activitypub_profile::ensure_web_finger($arg); return Activitypub_profile::ensure_webfinger($arg);
} catch (Exception $e) { } catch (Exception $e) {
common_log(LOG_ERR, 'Webfinger lookup failed for ' . common_log(LOG_ERR, 'Webfinger lookup failed for ' .
$arg . ': ' . $e->getMessage()); $arg . ': ' . $e->getMessage());
@ -618,7 +639,7 @@ class ActivityPubPlugin extends Plugin
$this->log(LOG_INFO, "Checking webfinger person '$target'"); $this->log(LOG_INFO, "Checking webfinger person '$target'");
$profile = null; $profile = null;
try { try {
$aprofile = Activitypub_profile::ensure_web_finger($target); $aprofile = Activitypub_profile::ensure_webfinger($target);
$profile = $aprofile->local_profile(); $profile = $aprofile->local_profile();
} catch (Exception $e) { } catch (Exception $e) {
$this->log(LOG_ERR, "Webfinger check failed: " . $e->getMessage()); $this->log(LOG_ERR, "Webfinger check failed: " . $e->getMessage());
@ -751,7 +772,7 @@ class ActivityPubPlugin extends Plugin
/** /**
* Try to grab and store the remote profile by the given uri * Try to grab and store the remote profile by the given uri
* *
* @param string $uri * @param string $uri
* @param Profile &$profile * @param Profile &$profile
* @return bool * @return bool
@ -979,7 +1000,7 @@ class ActivityPubPlugin extends Plugin
if ($notice->reply_to) { if ($notice->reply_to) {
try { try {
$parent_notice = $notice->getParent(); $parent_notice = $notice->getParent();
try { try {
$other[] = Activitypub_profile::from_profile($parent_notice->getProfile()); $other[] = Activitypub_profile::from_profile($parent_notice->getProfile());
} catch (Exception $e) { } catch (Exception $e) {
@ -1005,7 +1026,7 @@ class ActivityPubPlugin extends Plugin
/** /**
* Notify remote followers when a user gets deleted * Notify remote followers when a user gets deleted
* *
* @param Action $action * @param Action $action
* @param User $user user being deleted * @param User $user user being deleted
*/ */

33
plugins/ActivityPub/classes/Activitypub_profile.php
View File

@ -331,15 +331,16 @@ class Activitypub_profile extends Managed_DataObject
/** /**
* Ensures a valid Activitypub_profile when provided with a valid URI. * Ensures a valid Activitypub_profile when provided with a valid URI.
* *
* @author Diogo Cordeiro <diogo@fc.up.pt>
* @param string $url * @param string $url
* @param bool $grab_online whether to try online grabbing, defaults to true
* @return Activitypub_profile * @return Activitypub_profile
* @throws Exception if it isn't possible to return an Activitypub_profile * @throws Exception if it isn't possible to return an Activitypub_profile
* @author Diogo Cordeiro <diogo@fc.up.pt>
*/ */
public static function fromUri($url) public static function fromUri($url, $grab_online = true)
{ {
try { try {
return self::from_profile(Activitypub_explorer::get_profile_from_url($url)); return self::from_profile(Activitypub_explorer::get_profile_from_url($url, $grab_online));
} catch (Exception $e) { } catch (Exception $e) {
throw new Exception('No valid ActivityPub profile found for given URI.'); throw new Exception('No valid ActivityPub profile found for given URI.');
} }
@ -347,17 +348,17 @@ class Activitypub_profile extends Managed_DataObject
/** /**
* Look up, and if necessary create, an Activitypub_profile for the remote * Look up, and if necessary create, an Activitypub_profile for the remote
* entity with the given webfinger address. * entity with the given WebFinger address.
* This should never return null -- you will either get an object or * This should never return null -- you will either get an object or
* an exception will be thrown. * an exception will be thrown.
* *
* @author GNU social * @author GNU social
* @author Diogo Cordeiro <diogo@fc.up.pt> * @author Diogo Cordeiro <diogo@fc.up.pt>
* @param string $addr webfinger address * @param string $addr WebFinger address
* @return Activitypub_profile * @return Activitypub_profile
* @throws Exception on error conditions * @throws Exception on error conditions
*/ */
public static function ensure_web_finger($addr) public static function ensure_webfinger($addr)
{ {
// Normalize $addr, i.e. add 'acct:' if missing // Normalize $addr, i.e. add 'acct:' if missing
$addr = Discovery::normalize($addr); $addr = Discovery::normalize($addr);
@ -369,12 +370,12 @@ class Activitypub_profile extends Managed_DataObject
if (is_null($uri)) { if (is_null($uri)) {
// Negative cache entry // Negative cache entry
// TRANS: Exception. // TRANS: Exception.
throw new Exception(_m('Not a valid webfinger address (via cache).')); throw new Exception(_m('Not a valid WebFinger address (via cache).'));
} }
try { try {
return self::fromUri($uri); return self::fromUri($uri);
} catch (Exception $e) { } catch (Exception $e) {
common_log(LOG_ERR, sprintf(__METHOD__ . ': Webfinger address cache inconsistent with database, did not find Activitypub_profile uri==%s', $uri)); common_log(LOG_ERR, sprintf(__METHOD__ . ': WebFinger address cache inconsistent with database, did not find Activitypub_profile uri==%s', $uri));
self::cacheSet(sprintf('activitypub_profile:webfinger:%s', $addr), false); self::cacheSet(sprintf('activitypub_profile:webfinger:%s', $addr), false);
} }
} }
@ -390,13 +391,13 @@ class Activitypub_profile extends Managed_DataObject
// @todo FIXME: Distinguish temporary failures? // @todo FIXME: Distinguish temporary failures?
self::cacheSet(sprintf('activitypub_profile:webfinger:%s', $addr), null); self::cacheSet(sprintf('activitypub_profile:webfinger:%s', $addr), null);
// TRANS: Exception. // TRANS: Exception.
throw new Exception(_m('Not a valid webfinger address.')); throw new Exception(_m('Not a valid WebFinger address.'));
} }
$hints = array_merge( $hints = array_merge(
array('webfinger' => $addr), ['webfinger' => $addr],
DiscoveryHints::fromXRD($xrd) DiscoveryHints::fromXRD($xrd)
); );
// If there's an Hcard, let's grab its info // If there's an Hcard, let's grab its info
if (array_key_exists('hcard', $hints)) { if (array_key_exists('hcard', $hints)) {
@ -419,17 +420,17 @@ class Activitypub_profile extends Managed_DataObject
} catch (Exception $e) { } catch (Exception $e) {
common_log(LOG_WARNING, "Failed creating profile from profile URL '$profileUrl': " . $e->getMessage()); common_log(LOG_WARNING, "Failed creating profile from profile URL '$profileUrl': " . $e->getMessage());
// keep looking // keep looking
// //
// @todo FIXME: This means an error discovering from profile page // @todo FIXME: This means an error discovering from profile page
// may give us a corrupt entry using the webfinger URI, which // may give us a corrupt entry using the webfinger URI, which
// will obscure the correct page-keyed profile later on. // will obscure the correct page-keyed profile later on.
} }
} }
// XXX: try hcard // XXX: try hcard
// XXX: try FOAF // XXX: try FOAF
// TRANS: Exception. %s is a webfinger address. // TRANS: Exception. %s is a WebFinger address.
throw new Exception(sprintf(_m('Could not find a valid profile for "%s".'), $addr)); throw new Exception(sprintf(_m('Could not find a valid profile for "%s".'), $addr));
} }

23
plugins/ActivityPub/lib/explorer.php
View File

@ -43,16 +43,19 @@ class Activitypub_explorer
/** /**
* Shortcut function to get a single profile from its URL. * Shortcut function to get a single profile from its URL.
* *
* @author Diogo Cordeiro <diogo@fc.up.pt>
* @param string $url * @param string $url
* @param bool $grab_online whether to try online grabbing, defaults to true
* @return Profile * @return Profile
* @throws Exception * @throws HTTP_Request2_Exception
* @throws NoProfileException
* @throws ServerException
* @author Diogo Cordeiro <diogo@fc.up.pt>
*/ */
public static function get_profile_from_url($url) public static function get_profile_from_url($url, $grab_online = true)
{ {
$discovery = new Activitypub_explorer; $discovery = new Activitypub_explorer;
// Get valid Actor object // Get valid Actor object
$actor_profile = $discovery->lookup($url); $actor_profile = $discovery->lookup($url, $grab_online);
if (!empty($actor_profile)) { if (!empty($actor_profile)) {
return $actor_profile[0]; return $actor_profile[0];
} }
@ -65,14 +68,14 @@ class Activitypub_explorer
* so that there is no erroneous data * so that there is no erroneous data
* *
* @param string $url User's url * @param string $url User's url
* @param bool $remote remote lookup? * @param bool $grab_online whether to try online grabbing, defaults to true
* @return array of Profile objects * @return array of Profile objects
* @throws HTTP_Request2_Exception * @throws HTTP_Request2_Exception
* @throws NoProfileException * @throws NoProfileException
* @throws ServerException * @throws ServerException
* @author Diogo Cordeiro <diogo@fc.up.pt> * @author Diogo Cordeiro <diogo@fc.up.pt>
*/ */
public function lookup(string $url, bool $remote = true) public function lookup(string $url, bool $grab_online = true)
{ {
if (in_array($url, ACTIVITYPUB_PUBLIC_TO)) { if (in_array($url, ACTIVITYPUB_PUBLIC_TO)) {
return []; return [];
@ -81,7 +84,7 @@ class Activitypub_explorer
common_debug('ActivityPub Explorer: Started now looking for '.$url); common_debug('ActivityPub Explorer: Started now looking for '.$url);
$this->discovered_actor_profiles = []; $this->discovered_actor_profiles = [];
return $this->_lookup($url, $remote); return $this->_lookup($url, $grab_online);
} }
/** /**
@ -90,7 +93,7 @@ class Activitypub_explorer
* $discovered_actor_profiles array * $discovered_actor_profiles array
* *
* @param string $url User's url * @param string $url User's url
* @param bool $remote remote lookup? * @param bool $grab_online whether to try online grabbing, defaults to true
* @return array of Profile objects * @return array of Profile objects
* @throws HTTP_Request2_Exception * @throws HTTP_Request2_Exception
* @throws NoProfileException * @throws NoProfileException
@ -98,13 +101,13 @@ class Activitypub_explorer
* @throws Exception * @throws Exception
* @author Diogo Cordeiro <diogo@fc.up.pt> * @author Diogo Cordeiro <diogo@fc.up.pt>
*/ */
private function _lookup(string $url, bool $remote) private function _lookup(string $url, bool $grab_online = true)
{ {
$grab_local = $this->grab_local_user($url); $grab_local = $this->grab_local_user($url);
// First check if we already have it locally and, if so, return it. // First check if we already have it locally and, if so, return it.
// If the local fetch fails and remote grab is required: store locally and return. // If the local fetch fails and remote grab is required: store locally and return.
if (!$grab_local && (!$remote || !$this->grab_remote_user($url))) { if (!$grab_local && (!$grab_online || !$this->grab_remote_user($url))) {
throw new Exception('User not found.'); throw new Exception('User not found.');
} }

2
plugins/ActivityPub/tests/Unit/ActivitypubProfileTest.php
View File

@ -101,7 +101,7 @@ class ProfileObjectTest extends TestCase
/* Test ensure_web_finger() */ /* Test ensure_web_finger() */
// TODO: Maybe elaborate on this function's tests // TODO: Maybe elaborate on this function's tests
try { try {
\Activitypub_profile::ensure_web_finger('test1@testinstance.net'); \Activitypub_profile::ensure_webfinger('test1@testinstance.net');
$this->assertTrue(false); $this->assertTrue(false);
} catch (\Exception $e) { } catch (\Exception $e) {
$this->assertTrue($e->getMessage() == 'Not a valid webfinger address.' || $this->assertTrue($e->getMessage() == 'Not a valid webfinger address.' ||