From b95b81b4e5f5efb21339d3ade8cf590cc6d4dc13 Mon Sep 17 00:00:00 2001 From: Mark Date: Mon, 26 Oct 2026 10:15:26 +0100 Subject: [PATCH] test: cover remote pings and restore full coverage --- .../Api/ConversationPingControllerTest.php | 105 +++++++++++++++ .../Controllers/Api/OrgPingControllerTest.php | 63 +++++++++ .../ConversationPingControllerTest.php | 127 ++++++++++++++++++ .../Controllers/OrgPingControllerTest.php | 100 ++++++++++++++ tests/Unit/Jobs/SendRemotePushTest.php | 9 ++ 5 files changed, 404 insertions(+) create mode 100644 tests/Feature/Controllers/Api/ConversationPingControllerTest.php create mode 100644 tests/Feature/Controllers/Api/OrgPingControllerTest.php create mode 100644 tests/Feature/Controllers/ConversationPingControllerTest.php create mode 100644 tests/Feature/Controllers/OrgPingControllerTest.php diff --git a/tests/Feature/Controllers/Api/ConversationPingControllerTest.php b/tests/Feature/Controllers/Api/ConversationPingControllerTest.php new file mode 100644 index 000000000..2873ea0ee --- /dev/null +++ b/tests/Feature/Controllers/Api/ConversationPingControllerTest.php @@ -0,0 +1,105 @@ + true, 'remotes.everyone' => true]); + $this->user = User::factory()->withRemotes()->create(); + $this->keyPair = apiKeyPairFor($this->user); +}); + +it('records a ping, brings both participants out of the stash and marks it read only for its sender', function (): void { + $partner = User::factory()->withRemotes()->create(); + $previous = sendChatMessage($partner, $this->user); + $conversation = $previous->conversation; + ConversationParticipant::query()->where('conversation_id', $conversation->id)->update(['archived_at' => now()]); + $this->travel(10)->seconds(); + + $response = signedApiRequest($this->keyPair, 'POST', route('api-conversation-ping.store', $partner))->assertCreated(); + + $ping = Message::query()->latest('id')->firstOrFail(); + $response->assertJsonPath('data.id', $ping->id) + ->assertJsonPath('data.user_id', $this->user->id) + ->assertJsonPath('data.body', '') + ->assertJsonPath('data.event', 'ping'); + expect($ping->event)->toBe(OrgEvent::Ping) + ->and($ping->conversation_id)->toBe($conversation->id) + ->and($ping->client_id)->toBeUuid() + ->and($conversation->fresh()->last_message_id)->toBe($ping->id); + $this->assertDatabaseCount('messages', 2); + $this->assertDatabaseHas('conversation_participants', ['conversation_id' => $conversation->id, 'user_id' => $this->user->id, 'read_through_id' => $ping->id, 'read_at' => now(), 'archived_at' => null, 'last_message_at' => now()]); + $this->assertDatabaseHas('conversation_participants', ['conversation_id' => $conversation->id, 'user_id' => $partner->id, 'read_through_id' => $previous->id, 'archived_at' => null, 'last_message_at' => now()]); +}); + +it('requires a conversation before pinging', function (): void { + $partner = User::factory()->withRemotes()->create(); + + signedApiRequest($this->keyPair, 'POST', route('api-conversation-ping.store', $partner))->assertForbidden() + ->assertJsonPath('message', "Say hi to {$partner->mention()} before pinging them."); + + $this->assertDatabaseCount('conversations', 0); + $this->assertDatabaseCount('messages', 0); +}); + +it('hides a ping target when either person blocks the other', function (bool $senderBlocks): void { + $partner = User::factory()->withRemotes()->create(); + sendChatMessage($partner, $this->user); + $blocker = $senderBlocks ? $this->user : $partner; + $blocked = $senderBlocks ? $partner : $this->user; + $blocker->blocking()->attach($blocked); + + signedApiRequest($this->keyPair, 'POST', route('api-conversation-ping.store', $partner))->assertNotFound(); + + $this->assertDatabaseCount('messages', 1); +})->with([true, false]); + +it('rejects pings while the sender has Remotes disabled', function (): void { + $partner = User::factory()->withRemotes()->create(); + sendChatMessage($partner, $this->user); + $this->user->update(['remotes_enabled' => false]); + + signedApiRequest($this->keyPair, 'POST', route('api-conversation-ping.store', $partner))->assertNotFound(); + + $this->assertDatabaseCount('messages', 1); +}); + +it('rejects pings while the partner has Remotes disabled', function (): void { + $partner = User::factory()->withRemotes()->create(); + sendChatMessage($partner, $this->user); + $partner->update(['remotes_enabled' => false]); + + signedApiRequest($this->keyPair, 'POST', route('api-conversation-ping.store', $partner))->assertForbidden() + ->assertJsonPath('message', "You can't ping {$partner->mention()} right now."); + + $this->assertDatabaseCount('messages', 1); +}); + +it('does not ping a request before it is merged or after it is dropped', function (string $side, RemoteStatus $status): void { + $partner = User::factory()->withRemotes()->create(); + $message = sendChatMessage($partner, $this->user); + ConversationParticipant::query()->where('conversation_id', $message->conversation_id) + ->where('user_id', $side === 'sender' ? $this->user->id : $partner->id)->update(['status' => $status]); + + signedApiRequest($this->keyPair, 'POST', route('api-conversation-ping.store', $partner))->assertForbidden() + ->assertJsonPath('message', "You can't ping {$partner->mention()} right now."); + + $this->assertDatabaseCount('messages', 1); +})->with([ + 'incoming request' => ['sender', RemoteStatus::Fetched], + 'outgoing request' => ['partner', RemoteStatus::Fetched], + 'dropped request' => ['partner', RemoteStatus::Dropped], +]); + +it('returns 401 for an unsigned ping', function (): void { + $partner = User::factory()->withRemotes()->create(); + + $this->postJson(route('api-conversation-ping.store', $partner))->assertUnauthorized(); + + $this->assertDatabaseCount('messages', 0); +}); diff --git a/tests/Feature/Controllers/Api/OrgPingControllerTest.php b/tests/Feature/Controllers/Api/OrgPingControllerTest.php new file mode 100644 index 000000000..0bec867c6 --- /dev/null +++ b/tests/Feature/Controllers/Api/OrgPingControllerTest.php @@ -0,0 +1,63 @@ +user = User::factory()->withRemotes()->create(); + $this->keyPair = apiKeyPairFor($this->user); +}); + +it('records a ping from a maintainer or contributor with its sender', function (bool $contributor): void { + [$member] = followersOf($this->user, 1); + $org = createOrg($this->user, [$member]); + if ($contributor) { + $this->user = $member; + $this->keyPair = apiKeyPairFor($this->user); + } + + $response = signedApiRequest($this->keyPair, 'POST', route('api-org-ping.store', $org))->assertCreated(); + + $ping = Message::query()->latest('id')->firstOrFail(); + $response->assertJsonPath('data.id', $ping->id) + ->assertJsonPath('data.event', 'ping') + ->assertJsonPath('data.body', '') + ->assertJsonPath('data.user.username', $this->user->username); + expect($ping->event)->toBe(OrgEvent::Ping) + ->and($ping->conversation_id)->toBe($org) + ->and(Conversation::query()->findOrFail($org)->last_message_id)->toBe($ping->id); + $this->assertDatabaseCount('messages', 2); +})->with([false, true]); + +it('hides an org from a nonmember trying to ping it', function (): void { + $maintainer = User::factory()->withRemotes()->create(); + [$member] = followersOf($maintainer, 1); + $org = createOrg($maintainer, [$member]); + + signedApiRequest($this->keyPair, 'POST', route('api-org-ping.store', $org))->assertNotFound(); + + $this->assertDatabaseCount('messages', 1); +}); + +it('rejects pings from a member with Remotes disabled', function (): void { + [$member] = followersOf($this->user, 1); + $org = createOrg($this->user, [$member]); + $this->user->update(['remotes_enabled' => false]); + + signedApiRequest($this->keyPair, 'POST', route('api-org-ping.store', $org))->assertNotFound(); + + $this->assertDatabaseCount('messages', 1); +}); + +it('returns 401 for an unsigned org ping', function (): void { + [$member] = followersOf($this->user, 1); + $org = createOrg($this->user, [$member]); + + $this->postJson(route('api-org-ping.store', $org))->assertUnauthorized(); + + $this->assertDatabaseCount('messages', 1); +}); diff --git a/tests/Feature/Controllers/ConversationPingControllerTest.php b/tests/Feature/Controllers/ConversationPingControllerTest.php new file mode 100644 index 000000000..32c2f5fe7 --- /dev/null +++ b/tests/Feature/Controllers/ConversationPingControllerTest.php @@ -0,0 +1,127 @@ + true, 'remotes.everyone' => true]); + $this->user = User::factory()->withRemotes()->create(); +}); + +it('records a ping, brings both participants out of the stash and marks it read only for its sender', function (): void { + $partner = User::factory()->withRemotes()->create(); + $previous = sendChatMessage($partner, $this->user); + $conversation = $previous->conversation; + ConversationParticipant::query()->where('conversation_id', $conversation->id)->update(['archived_at' => now()]); + $this->travel(10)->seconds(); + + $response = $this->actingAs($this->user)->postJson(route('conversation-ping.store', $partner))->assertCreated(); + + $ping = Message::query()->latest('id')->firstOrFail(); + $response->assertJsonPath('message.id', $ping->id) + ->assertJsonPath('message.user_id', $this->user->id) + ->assertJsonPath('message.body', '') + ->assertJsonPath('message.event', 'ping'); + expect($ping->event)->toBe(OrgEvent::Ping) + ->and($ping->conversation_id)->toBe($conversation->id) + ->and($ping->client_id)->toBeUuid() + ->and($conversation->fresh()->last_message_id)->toBe($ping->id); + $this->assertDatabaseCount('messages', 2); + $this->assertDatabaseHas('conversation_participants', ['conversation_id' => $conversation->id, 'user_id' => $this->user->id, 'read_through_id' => $ping->id, 'read_at' => now(), 'archived_at' => null, 'last_message_at' => now()]); + $this->assertDatabaseHas('conversation_participants', ['conversation_id' => $conversation->id, 'user_id' => $partner->id, 'read_through_id' => $previous->id, 'archived_at' => null, 'last_message_at' => now()]); +}); + +it('requires a conversation before pinging', function (): void { + $partner = User::factory()->withRemotes()->create(); + + $this->actingAs($this->user)->postJson(route('conversation-ping.store', $partner))->assertForbidden() + ->assertJsonPath('message', "Say hi to {$partner->mention()} before pinging them."); + + $this->assertDatabaseCount('conversations', 0); + $this->assertDatabaseCount('messages', 0); +}); + +it('hides a ping target when either person blocks the other', function (bool $senderBlocks): void { + $partner = User::factory()->withRemotes()->create(); + sendChatMessage($partner, $this->user); + $blocker = $senderBlocks ? $this->user : $partner; + $blocked = $senderBlocks ? $partner : $this->user; + $blocker->blocking()->attach($blocked); + + $this->actingAs($this->user)->postJson(route('conversation-ping.store', $partner))->assertNotFound(); + + $this->assertDatabaseCount('messages', 1); +})->with([true, false]); + +it('rejects pings while the sender has Remotes disabled', function (): void { + $partner = User::factory()->withRemotes()->create(); + sendChatMessage($partner, $this->user); + $this->user->update(['remotes_enabled' => false]); + + $this->actingAs($this->user)->postJson(route('conversation-ping.store', $partner))->assertNotFound(); + + $this->assertDatabaseCount('messages', 1); +}); + +it('rejects pings while the partner has Remotes disabled', function (): void { + $partner = User::factory()->withRemotes()->create(); + sendChatMessage($partner, $this->user); + $partner->update(['remotes_enabled' => false]); + + $this->actingAs($this->user)->postJson(route('conversation-ping.store', $partner))->assertForbidden() + ->assertJsonPath('message', "You can't ping {$partner->mention()} right now."); + + $this->assertDatabaseCount('messages', 1); +}); + +it('does not ping a request before it is merged or after it is dropped', function (string $side, RemoteStatus $status): void { + $partner = User::factory()->withRemotes()->create(); + $message = sendChatMessage($partner, $this->user); + ConversationParticipant::query()->where('conversation_id', $message->conversation_id) + ->where('user_id', $side === 'sender' ? $this->user->id : $partner->id)->update(['status' => $status]); + + $this->actingAs($this->user)->postJson(route('conversation-ping.store', $partner))->assertForbidden() + ->assertJsonPath('message', "You can't ping {$partner->mention()} right now."); + + $this->assertDatabaseCount('messages', 1); +})->with([ + 'incoming request' => ['sender', RemoteStatus::Fetched], + 'outgoing request' => ['partner', RemoteStatus::Fetched], + 'dropped request' => ['partner', RemoteStatus::Dropped], +]); + +it('returns 401 for a guest ping', function (): void { + $partner = User::factory()->withRemotes()->create(); + + $this->postJson(route('conversation-ping.store', $partner))->assertUnauthorized(); + + $this->assertDatabaseCount('messages', 0); +}); + +it('limits a sender to three pings a minute across web and API remotes', function (): void { + $partner = User::factory()->withRemotes()->create(); + sendChatMessage($partner, $this->user); + $keyPair = apiKeyPairFor($this->user); + [$member] = followersOf($this->user, 1); + $org = createOrg($this->user, [$member]); + + $this->actingAs($this->user)->postJson(route('conversation-ping.store', $partner))->assertCreated(); + signedApiRequest($keyPair, 'POST', route('api-conversation-ping.store', $partner))->assertCreated(); + $this->postJson(route('org-ping.store', $org))->assertCreated(); + signedApiRequest($keyPair, 'POST', route('api-org-ping.store', $org))->assertTooManyRequests() + ->assertJsonPath('message', 'Three pings a minute, tops. Try again in a bit.') + ->assertHeader('Retry-After', '60'); + + expect(Message::query()->where('event', OrgEvent::Ping)->count())->toBe(3); + + $this->travel(61)->seconds(); + + signedApiRequest($keyPair, 'POST', route('api-org-ping.store', $org))->assertCreated(); + + expect(Message::query()->where('event', OrgEvent::Ping)->count())->toBe(4); +}); diff --git a/tests/Feature/Controllers/OrgPingControllerTest.php b/tests/Feature/Controllers/OrgPingControllerTest.php new file mode 100644 index 000000000..d01eb341f --- /dev/null +++ b/tests/Feature/Controllers/OrgPingControllerTest.php @@ -0,0 +1,100 @@ +user = User::factory()->withRemotes()->create(); +}); + +it('records a ping from a maintainer or contributor with its sender', function (bool $contributor): void { + [$member] = followersOf($this->user, 1); + $org = createOrg($this->user, [$member]); + if ($contributor) { + $this->user = $member; + } + + $response = $this->actingAs($this->user)->postJson(route('org-ping.store', $org))->assertCreated(); + + $ping = Message::query()->latest('id')->firstOrFail(); + $response->assertJsonPath('message.id', $ping->id) + ->assertJsonPath('message.event', 'ping') + ->assertJsonPath('message.body', '') + ->assertJsonPath('message.user.username', $this->user->username); + expect($ping->event)->toBe(OrgEvent::Ping) + ->and($ping->conversation_id)->toBe($org) + ->and(Conversation::query()->findOrFail($org)->last_message_id)->toBe($ping->id); + $this->assertDatabaseCount('messages', 2); +})->with([false, true]); + +it('hides an org from a nonmember trying to ping it', function (): void { + $maintainer = User::factory()->withRemotes()->create(); + [$member] = followersOf($maintainer, 1); + $org = createOrg($maintainer, [$member]); + + $this->actingAs($this->user)->postJson(route('org-ping.store', $org))->assertNotFound(); + + $this->assertDatabaseCount('messages', 1); +}); + +it('rejects pings from a member with Remotes disabled', function (): void { + [$member] = followersOf($this->user, 1); + $org = createOrg($this->user, [$member]); + $this->user->update(['remotes_enabled' => false]); + + $this->actingAs($this->user)->postJson(route('org-ping.store', $org))->assertNotFound(); + + $this->assertDatabaseCount('messages', 1); +}); + +it('returns 401 for a guest org ping', function (): void { + [$member] = followersOf($this->user, 1); + $org = createOrg($this->user, [$member]); + auth()->logout(); + + $this->postJson(route('org-ping.store', $org))->assertUnauthorized(); + + $this->assertDatabaseCount('messages', 1); +}); + +it('queues a delayed push for each subscribed member except the sender', function (): void { + [$first, $second, $unsubscribed] = followersOf($this->user, 3); + $org = createOrg($this->user, [$first, $second, $unsubscribed]); + $outsider = User::factory()->withRemotes()->create(); + foreach ([$this->user, $first, $second, $outsider] as $user) { + PushSubscription::factory()->create(['user_id' => $user->id]); + } + + $keys = VAPID::createVapidKeys(); + config(['services.web_push.public_key' => $keys['publicKey'], 'services.web_push.private_key' => $keys['privateKey']]); + Queue::fake([SendRemotePush::class]); + + $response = $this->actingAs($this->user)->postJson(route('org-ping.store', $org))->assertCreated(); + + Queue::assertPushed(SendRemotePush::class, 2); + foreach ([$first, $second] as $recipient) { + Queue::assertPushed(SendRemotePush::class, fn (SendRemotePush $job): bool => $job->messageId === $response->json('message.id') + && $job->userId === $recipient->id && $job->afterCommit === true && $job->delay->equalTo(now()->addSeconds(10))); + } +}); + +it('does not queue a ping push when either web push key is missing', function (string $missingKey): void { + [$member] = followersOf($this->user, 1); + $org = createOrg($this->user, [$member]); + PushSubscription::factory()->create(['user_id' => $member->id]); + $keys = VAPID::createVapidKeys(); + config(['services.web_push.public_key' => $keys['publicKey'], 'services.web_push.private_key' => $keys['privateKey'], 'services.web_push.'.$missingKey => null]); + Queue::fake([SendRemotePush::class]); + + $this->actingAs($this->user)->postJson(route('org-ping.store', $org))->assertCreated(); + + Queue::assertNothingPushed(); +})->with(['public_key', 'private_key']); diff --git a/tests/Unit/Jobs/SendRemotePushTest.php b/tests/Unit/Jobs/SendRemotePushTest.php index b4a515d6c..051b06ecb 100644 --- a/tests/Unit/Jobs/SendRemotePushTest.php +++ b/tests/Unit/Jobs/SendRemotePushTest.php @@ -59,6 +59,15 @@ Http::assertNothingSent(); }); +it('delivers a ping event as an empty push without message data', function (): void { + $this->message->update(['event' => OrgEvent::Ping, 'body' => '']); + + new SendRemotePush($this->message->id, $this->recipient->id)->handle(resolve(RemotePush::class)); + + Http::assertSentCount(1); + Http::assertSent(fn (Request $request): bool => $request->body() === ''); +}); + it('rechecks eligibility at delivery time', function (string $change): void { match ($change) { 'read' => $this->participant->update(['read_through_id' => $this->message->id]),