wamo/gnu-social
1
0
Fork 0
Code Issues Pull Requests Packages Projects 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 committed by Diogo Peralta Cordeiro
parent ffef85414e
commit d0e3f9c823
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.
*
* @author Diogo Cordeiro <diogo@fc.up.pt>
* @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
* @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 */
try {
@ -114,7 +114,7 @@ class ActivityPubPlugin extends Plugin
}
}
if ($grabOnline) {
if ($grab_online) {
/* Online Grabbing */
$client = new HTTPClient();
$headers = [];
@ -319,7 +319,7 @@ class ActivityPubPlugin extends Plugin
/**
* Validate AP-recipients for profile page message action addition
*
*
* @param Profile $recipient
* @return bool hook return value
*/
@ -334,7 +334,7 @@ class ActivityPubPlugin extends Plugin
/**
* Mark an ap_profile object for deletion
*
*
* @param Profile profile being deleted
* @param array &$related objects with same profile_id to be deleted
* @return void
@ -353,7 +353,7 @@ class ActivityPubPlugin extends Plugin
}
} catch (Exception $e) {
// nothing to do
}
}
}
/**
@ -400,70 +400,91 @@ class ActivityPubPlugin extends Plugin
/**
* Hack the notice search-box and try to grab remote profiles or notices.
*
*
* Note that, on successful grabbing, this function will redirect to the
* 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
* prefixes. This would change the queries for URL searching and therefore we
* could do both search and grab.
*
*
* @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()) {
// early return, search-only for non-logged sessions
return;
// early return: Only allow logged users to import/search for remote actors or notes
return true;
}
if (!filter_var($query, FILTER_VALIDATE_URL) &&
!preg_match('!^((?:\w+\.)*\w+@(?:\w+\.)*\w+(?:\w+\-\w+)*\.\w+)$!', $query)) {
// early return, not an url or webfinger ID
return;
}
// someone we know about ?
try {
$explorer = new Activitypub_explorer();
$profile = $explorer->lookup($query, false)[0];
if ($profile instanceof Profile) {
return;
if (preg_match('!^((?:\w+\.)*\w+@(?:\w+\.)*\w+(?:\w+\-\w+)*\.\w+)$!', $query)) { // WebFinger ID found!
// Try to grab remote actor
$aprofile = self::pull_remote_profile($query);
if ($aprofile instanceof Activitypub_profile) {
$url = common_local_url('userbyid', ['id' => $aprofile->getID()], null, null, false);
common_redirect($url, 303);
return true;
}
} catch (Exception $e) {
// nope
}
} elseif (filter_var($query, FILTER_VALIDATE_URL)) { // URL found!
/* Is this an ActivityPub notice? */
// some notice we know about ?
try {
$notice = self::grab_notice_from_url($query, false);
if ($notice instanceof Notice) {
return;
// If we already know it, just return
try {
$notice = self::grab_notice_from_url($query, false); // Only check locally
if ($notice instanceof Notice) {
return true;
}
} catch (Exception $e) {
// We will next try online
}
} catch (Exception $e) {
// nope
}
// try to grab profile
$aprofile = self::pull_remote_profile($query);
if ($aprofile instanceof Activitypub_profile) {
$url = common_local_url('userbyid', ['id' => $aprofile->getID()], null, null, false);
common_redirect($url, 303);
return;
}
// Otherwise, try to grab it
try {
$notice = self::grab_notice_from_url($query); // Unfortunately we will be trying locally again
if ($notice instanceof Notice) {
$url = common_local_url('shownotice', ['notice' => $notice->getID()]);
common_redirect($url, 303);
}
} catch (Exception $e) {
// We will next check if this URL is an actor
}
// try to grab notice
$notice = self::grab_notice_from_url($query);
if ($notice instanceof Notice) {
$url = common_local_url('shownotice', ['notice' => $notice->getID()]);
common_redirect($url, 303);
/* Is this an ActivityPub actor? */
// If we already know it, just return
try {
$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.
*
* @return boolean hook true
* @return bool hook true
*/
public function onCheckSchema()
{
@ -481,17 +502,17 @@ class ActivityPubPlugin extends Plugin
/**
* 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
* @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)
{
if (preg_match('!^((?:\w+\.)*\w+@(?:\w+\.)*\w+(?:\w+\-\w+)*\.\w+)$!', $arg)) {
// webfinger lookup
try {
return Activitypub_profile::ensure_web_finger($arg);
return Activitypub_profile::ensure_webfinger($arg);
} catch (Exception $e) {
common_log(LOG_ERR, 'Webfinger lookup failed for ' .
$arg . ': ' . $e->getMessage());
@ -618,7 +639,7 @@ class ActivityPubPlugin extends Plugin
$this->log(LOG_INFO, "Checking webfinger person '$target'");
$profile = null;
try {
$aprofile = Activitypub_profile::ensure_web_finger($target);
$aprofile = Activitypub_profile::ensure_webfinger($target);
$profile = $aprofile->local_profile();
} catch (Exception $e) {
$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
*
*
* @param string $uri
* @param Profile &$profile
* @return bool
@ -979,7 +1000,7 @@ class ActivityPubPlugin extends Plugin
if ($notice->reply_to) {
try {
$parent_notice = $notice->getParent();
try {
$other[] = Activitypub_profile::from_profile($parent_notice->getProfile());
} catch (Exception $e) {
@ -1005,7 +1026,7 @@ class ActivityPubPlugin extends Plugin
/**
* Notify remote followers when a user gets deleted
*
*
* @param Action $action
* @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.
*
* @author Diogo Cordeiro <diogo@fc.up.pt>
* @param string $url
* @param bool $grab_online whether to try online grabbing, defaults to true
* @return 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 {
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) {
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
* entity with the given webfinger address.
* entity with the given WebFinger address.
* This should never return null -- you will either get an object or
* an exception will be thrown.
*
* @author GNU social
* @author Diogo Cordeiro <diogo@fc.up.pt>
* @param string $addr webfinger address
* @param string $addr WebFinger address
* @return Activitypub_profile
* @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
$addr = Discovery::normalize($addr);
@ -369,12 +370,12 @@ class Activitypub_profile extends Managed_DataObject
if (is_null($uri)) {
// Negative cache entry
// 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 {
return self::fromUri($uri);
} 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);
}
}
@ -390,13 +391,13 @@ class Activitypub_profile extends Managed_DataObject
// @todo FIXME: Distinguish temporary failures?
self::cacheSet(sprintf('activitypub_profile:webfinger:%s', $addr), null);
// TRANS: Exception.
throw new Exception(_m('Not a valid webfinger address.'));
throw new Exception(_m('Not a valid WebFinger address.'));
}
$hints = array_merge(
array('webfinger' => $addr),
['webfinger' => $addr],
DiscoveryHints::fromXRD($xrd)
);
);
// If there's an Hcard, let's grab its info
if (array_key_exists('hcard', $hints)) {
@ -419,17 +420,17 @@ class Activitypub_profile extends Managed_DataObject
} catch (Exception $e) {
common_log(LOG_WARNING, "Failed creating profile from profile URL '$profileUrl': " . $e->getMessage());
// keep looking
//
// @todo FIXME: This means an error discovering from profile page
// may give us a corrupt entry using the webfinger URI, which
// will obscure the correct page-keyed profile later on.
//
// @todo FIXME: This means an error discovering from profile page
// may give us a corrupt entry using the webfinger URI, which
// will obscure the correct page-keyed profile later on.
}
}
// XXX: try hcard
// 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));
}

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.
*
* @author Diogo Cordeiro <diogo@fc.up.pt>
* @param string $url
* @param bool $grab_online whether to try online grabbing, defaults to true
* @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;
// Get valid Actor object
$actor_profile = $discovery->lookup($url);
$actor_profile = $discovery->lookup($url, $grab_online);
if (!empty($actor_profile)) {
return $actor_profile[0];
}
@ -65,14 +68,14 @@ class Activitypub_explorer
* so that there is no erroneous data
*
* @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
* @throws HTTP_Request2_Exception
* @throws NoProfileException
* @throws ServerException
* @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)) {
return [];
@ -81,7 +84,7 @@ class Activitypub_explorer
common_debug('ActivityPub Explorer: Started now looking for '.$url);
$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
*
* @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
* @throws HTTP_Request2_Exception
* @throws NoProfileException
@ -98,13 +101,13 @@ class Activitypub_explorer
* @throws Exception
* @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);
// 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 (!$grab_local && (!$remote || !$this->grab_remote_user($url))) {
if (!$grab_local && (!$grab_online || !$this->grab_remote_user($url))) {
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() */
// TODO: Maybe elaborate on this function's tests
try {
\Activitypub_profile::ensure_web_finger('test1@testinstance.net');
\Activitypub_profile::ensure_webfinger('test1@testinstance.net');
$this->assertTrue(false);
} catch (\Exception $e) {
$this->assertTrue($e->getMessage() == 'Not a valid webfinger address.' ||