Merge pull request #3639 from pixelfed/staging

Federation bugfixes

CHANGELOG.md

@@ -67,6 +67,12 @@
 - Fix cache invalidation in AdminSettingsController when updating rules ([fe6787f7](https://github.com/pixelfed/pixelfed/commit/fe6787f7))
 - Update SearchApiService, improve account/webfinger results ([533f7165](https://github.com/pixelfed/pixelfed/commit/533f7165))
 - Update NotificationService, fix account attribute ([949b7bb6](https://github.com/pixelfed/pixelfed/commit/949b7bb6))
+- Update DeleteWorker, remove cache lock ([6d6a033a](https://github.com/pixelfed/pixelfed/commit/6d6a033a))
+- Fix SearchApiV2Service, improve webfinger condition ([9d31f73b](https://github.com/pixelfed/pixelfed/commit/9d31f73b))
+- Update inbox handler, upsert statuses to fix duplicate bug. Fixes #2670, #2961, #3556 ([2c20d9e3](https://github.com/pixelfed/pixelfed/commit/2c20d9e3))
+- Update AP helpers, remove cache lock from profileUpdateOrCreate method and move webfinger + key_id to unique constraints to fix sql duplicate errors ([bc2bbc14](https://github.com/pixelfed/pixelfed/commit/bc2bbc14))
+- Add migrations to fix webfinger profiles ([66aa8bf9](https://github.com/pixelfed/pixelfed/commit/66aa8bf9))
+- Update ap helpers, move remote_url constraint ([acd8f5bb](https://github.com/pixelfed/pixelfed/commit/acd8f5bb))
 -  ([](https://github.com/pixelfed/pixelfed/commit/))
 
 ## [v0.11.3 (2022-05-09)](https://github.com/pixelfed/pixelfed/compare/v0.11.2...v0.11.3)

app/Jobs/InboxPipeline/DeleteWorker.php

@@ -72,39 +72,31 @@ class DeleteWorker implements ShouldQueue
 				'b:' . base64_encode($actor) :
 				'h:' . hash('sha256', $actor);
 
-			$lockKey = 'ap:inbox:actor-delete-exists:lock:' . $hash;
-			Cache::lock($lockKey, 30)->block(15, function () use(
-				$headers,
-				$payload,
-				$actor,
-				$hash
-			) {
-				$key = 'ap:inbox:actor-delete-exists:' . $hash;
-				$actorDelete = Cache::remember($key, now()->addMinutes(15), function() use($actor) {
-					return Profile::whereRemoteUrl($actor)
-						->whereNotNull('domain')
-						->exists();
-				});
-				if($actorDelete) {
-					if($this->verifySignature($headers, $payload) == true) {
-						Cache::set($key, false);
-						$profile = Profile::whereNotNull('domain')
-							->whereNull('status')
-							->whereRemoteUrl($actor)
-							->first();
-						if($profile) {
-							DeleteRemoteProfilePipeline::dispatch($profile)->onQueue('delete');
-						}
-						return 1;
-					} else {
-						// Signature verification failed, exit.
-						return 1;
+			$key = 'ap:inbox:actor-delete-exists:' . $hash;
+			$actorDelete = Cache::remember($key, now()->addMinutes(15), function() use($actor) {
+				return Profile::whereRemoteUrl($actor)
+					->whereNotNull('domain')
+					->exists();
+			});
+			if($actorDelete) {
+				if($this->verifySignature($headers, $payload) == true) {
+					Cache::set($key, false);
+					$profile = Profile::whereNotNull('domain')
+						->whereNull('status')
+						->whereRemoteUrl($actor)
+						->first();
+					if($profile) {
+						DeleteRemoteProfilePipeline::dispatch($profile)->onQueue('delete');
 					}
+					return 1;
 				} else {
-					// Remote user doesn't exist, exit early.
+					// Signature verification failed, exit.
 					return 1;
 				}
-			});
+			} else {
+				// Remote user doesn't exist, exit early.
+				return 1;
+			}
 
 			return 1;
 		}

app/Services/SearchApiV2Service.php

@@ -33,7 +33,6 @@ class SearchApiV2Service
 		$q = urldecode($query->input('q'));
 
 		if($query->has('resolve') && 
-			$query->resolve == true && 
 			( Str::startsWith($q, 'https://') ||
 			  Str::substrCount($q, '@') >= 1)
 		) {
@@ -203,7 +202,7 @@ class SearchApiV2Service
 				return $default;
 			}
 
-			if(Str::substrCount($query, '@') == 1 && strpos($query, '@') !== 0) {
+			if(!Str::startsWith($query, 'http') && Str::substrCount($query, '@') == 1 && strpos($query, '@') !== 0) {
 				try {
 					$res = WebfingerService::lookup('@' . $query, $mastodonMode);
 				} catch (\Exception $e) {

app/Services/WebfingerService.php

@@ -56,6 +56,9 @@ class WebfingerService
 			->first();
 
 		$profile = Helpers::profileFetch($link);
+		if(!$profile) {
+			return;
+		}
 		return $mastodonMode ?
 			AccountService::getMastodon($profile->id, true) :
 			AccountService::get($profile->id);

app/Status.php

@@ -28,7 +28,7 @@ class Status extends Model
 	 */
 	protected $dates = ['deleted_at'];
 
-	protected $fillable = ['profile_id', 'visibility', 'in_reply_to_id', 'reblog_of_id', 'type'];
+	protected $guarded = [];
 
 	const STATUS_TYPES = [
 		'text',

app/Util/ActivityPub/Helpers.php

@@ -474,22 +474,25 @@ class Helpers {
 				return;
 			}
 
-			$status = new Status;
-			$status->profile_id = $pid;
-			$status->url = $url;
-			$status->uri = $url;
-			$status->object_url = $id;
-			$status->caption = strip_tags($activity['content']);
-			$status->rendered = Purify::clean($activity['content']);
-			$status->created_at = Carbon::parse($ts)->tz('UTC');
-			$status->in_reply_to_id = $reply_to;
-			$status->local = false;
-			$status->is_nsfw = $cw;
-			$status->scope = $scope;
-			$status->visibility = $scope;
-			$status->cw_summary = $cw == true && isset($activity['summary']) ?
-				Purify::clean(strip_tags($activity['summary'])) : null;
-			$status->save();
+            $status = Status::updateOrCreate(
+                [
+                    'uri' => $url
+                ], [
+                    'profile_id' => $pid,
+                    'url' => $url,
+                    'object_url' => $id,
+                    'caption' => strip_tags($activity['content']),
+                    'rendered' => Purify::clean($activity['content']),
+                    'created_at' => Carbon::parse($ts)->tz('UTC'),
+                    'in_reply_to_id' => $reply_to,
+                    'local' => false,
+                    'is_nsfw' => $cw,
+                    'scope' => $scope,
+                    'visibility' => $scope,
+                    'cw_summary' => ($cw == true && isset($activity['summary']) ?
+                        Purify::clean(strip_tags($activity['summary'])) : null)
+                ]
+            );
 
 			if($reply_to == null) {
 				self::importNoteAttachment($activity, $status);
@@ -717,75 +720,66 @@ class Helpers {
 
 	public static function profileUpdateOrCreate($url)
 	{
-		$hash = base64_encode($url);
-		$key = 'ap:profile:by_url:' . $hash;
-		$lock = Cache::lock($key, 30);
-		$profile = null;
-
-		try {
-			$lock->block(5);
+		$res = self::fetchProfileFromUrl($url);
+		if(!$res || isset($res['id']) == false) {
+			return;
+		}
+		$domain = parse_url($res['id'], PHP_URL_HOST);
+		if(!isset($res['preferredUsername']) && !isset($res['nickname'])) {
+			return;
+		}
+		$username = (string) Purify::clean($res['preferredUsername'] ?? $res['nickname']);
+		if(empty($username)) {
+			return;
+		}
+		$remoteUsername = $username;
+		$webfinger = "@{$username}@{$domain}";
+
+		if(!self::validateUrl($res['inbox'])) {
+            return;
+        }
+		if(!self::validateUrl($res['id'])) {
+            return;
+        }
+
+		$profile = DB::transaction(function() use($domain, $webfinger, $res) {
+			$instance = Instance::updateOrCreate([
+				'domain' => $domain
+			]);
+			if($instance->wasRecentlyCreated == true) {
+				\App\Jobs\InstancePipeline\FetchNodeinfoPipeline::dispatch($instance)->onQueue('low');
+			}
+
+			$profile = Profile::updateOrCreate(
+				[
+					'domain' => strtolower($domain),
+					'username' => Purify::clean($webfinger),
+					'webfinger' => Purify::clean($webfinger),
+					'key_id' => $res['publicKey']['id'],
+				],
+				[
+					'remote_url' => $res['id'],
+					'name' => isset($res['name']) ? Purify::clean($res['name']) : 'user',
+					'bio' => isset($res['summary']) ? Purify::clean($res['summary']) : null,
+					'sharedInbox' => isset($res['endpoints']) && isset($res['endpoints']['sharedInbox']) ? $res['endpoints']['sharedInbox'] : null,
+					'inbox_url' => $res['inbox'],
+					'outbox_url' => isset($res['outbox']) ? $res['outbox'] : null,
+					'public_key' => $res['publicKey']['publicKeyPem'],
+				]
+			);
 
-			$res = self::fetchProfileFromUrl($url);
-			if(isset($res['id']) == false) {
-				return;
-			}
-			$domain = parse_url($res['id'], PHP_URL_HOST);
-			if(!isset($res['preferredUsername']) && !isset($res['nickname'])) {
-				return;
-			}
-			$username = (string) Purify::clean($res['preferredUsername'] ?? $res['nickname']);
-			if(empty($username)) {
-				return;
+			if( $profile->last_fetched_at == null ||
+				$profile->last_fetched_at->lt(now()->subHours(24))
+			) {
+				RemoteAvatarFetch::dispatch($profile);
 			}
-			$remoteUsername = $username;
-			$webfinger = "@{$username}@{$domain}";
-
-			abort_if(!self::validateUrl($res['inbox']), 400);
-			abort_if(!self::validateUrl($res['id']), 400);
-
-			$profile = DB::transaction(function() use($domain, $webfinger, $res) {
-				$instance = Instance::updateOrCreate([
-					'domain' => $domain
-				]);
-				if($instance->wasRecentlyCreated == true) {
-					\App\Jobs\InstancePipeline\FetchNodeinfoPipeline::dispatch($instance)->onQueue('low');
-				}
-
-				$profile = Profile::updateOrCreate(
-					[
-						'domain' => strtolower($domain),
-						'username' => Purify::clean($webfinger),
-						'remote_url' => $res['id'],
-					],
-					[
-						'name' => isset($res['name']) ? Purify::clean($res['name']) : 'user',
-						'bio' => isset($res['summary']) ? Purify::clean($res['summary']) : null,
-						'sharedInbox' => isset($res['endpoints']) && isset($res['endpoints']['sharedInbox']) ? $res['endpoints']['sharedInbox'] : null,
-						'inbox_url' => $res['inbox'],
-						'outbox_url' => isset($res['outbox']) ? $res['outbox'] : null,
-						'public_key' => $res['publicKey']['publicKeyPem'],
-						'key_id' => $res['publicKey']['id'],
-						'webfinger' => Purify::clean($webfinger),
-					]
-				);
-
-				if( $profile->last_fetched_at == null ||
-					$profile->last_fetched_at->lt(now()->subHours(24))
-				) {
-					RemoteAvatarFetch::dispatch($profile);
-				}
-				$profile->last_fetched_at = now();
-				$profile->save();
-				return $profile;
-			});
-
+			$profile->last_fetched_at = now();
+			$profile->save();
 			return $profile;
-		} catch (LockTimeoutException $e) {
-		} finally {
-		    optional($lock)->release();
-		}
+		});
 
 		return $profile;
+
 	}
 
 	public static function profileFetch($url)

database/migrations/2022_09_01_000000_fix_webfinger_profile_duplicate_accounts.php

@@ -0,0 +1,59 @@
+<?php
+
+use Illuminate\Database\Migrations\Migration;
+use Illuminate\Database\Schema\Blueprint;
+use Illuminate\Support\Facades\Schema;
+use App\Profile;
+use App\Jobs\DeletePipeline\DeleteRemoteProfilePipeline;
+
+class FixWebfingerProfileDuplicateAccounts extends Migration
+{
+    /**
+     * Run the migrations.
+     *
+     * @return void
+     */
+    public function up()
+    {
+        if(Profile::count() === 0) {
+            return;
+        }
+
+        Profile::whereNotNull('domain')
+            ->where('username', 'not like', '@%')
+            ->chunk(200, function($profiles) {
+                foreach($profiles as $profile) {
+                    $exists = Profile::whereUsername("@{$profile->username}@{$profile->domain}")->first();
+                    if($exists) {
+                        $exists->username = null;
+                        $exists->domain = null;
+                        $exists->webfinger = null;
+                        $exists->save();
+                        DeleteRemoteProfilePipeline::dispatch($exists);
+
+                        $profile->username = "@{$profile->username}@{$profile->domain}";
+                        if(!$profile->webfinger) {
+                            $profile->webfinger = "@{$profile->username}@{$profile->domain}";
+                        }
+                        $profile->save();
+                    } else {
+                        $profile->username = "@{$profile->username}@{$profile->domain}";
+                        if(!$profile->webfinger) {
+                            $profile->webfinger = "@{$profile->username}@{$profile->domain}";
+                        }
+                        $profile->save();
+                    }
+                }
+            });
+    }
+
+    /**
+     * Reverse the migrations.
+     *
+     * @return void
+     */
+    public function down()
+    {
+        //
+    }
+}

database/migrations/2022_09_01_043002_generate_missing_profile_webfinger.php

@@ -0,0 +1,38 @@
+<?php
+
+use Illuminate\Database\Migrations\Migration;
+use Illuminate\Database\Schema\Blueprint;
+use Illuminate\Support\Facades\Schema;
+use App\Profile;
+
+class GenerateMissingProfileWebfinger extends Migration
+{
+    /**
+     * Run the migrations.
+     *
+     * @return void
+     */
+    public function up()
+    {
+        Profile::whereNotNull('domain')
+            ->whereNull('webfinger')
+            ->chunk(200, function($profiles) {
+                foreach($profiles as $profile) {
+                    if(substr($profile->username, 0, 1) === "@") {
+                        $profile->webfinger = $profile->username;
+                        $profile->save();
+                    }
+                }
+            });
+    }
+
+    /**
+     * Reverse the migrations.
+     *
+     * @return void
+     */
+    public function down()
+    {
+        //
+    }
+}