Skip to content

Commit 03131d7

Browse files
akirkpfefferle
akirk
and
pfefferle
authored
Ensure that direct messages are only sent when the user is the recipient (#1102)
* Ensure that direct messages are only sent when the user is the recipient * Add test reply * Only assume a DM when the user is in the to * changelog * simplify * Use User->get_id * Use Actors::get_by_id --------- Co-authored-by: Matthias Pfefferle <[email protected]>
1 parent 9d9ec68 commit 03131d7

File tree

4 files changed

+142
-69
lines changed

4 files changed

+142
-69
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1414

1515
### Improved
1616

17+
* Direct Messages: Test for the user being in the to field
1718
* Direct Messages: Improve HTML to e-mail text conversion
1819

1920
## [4.5.1] - 2024-12-18

‎includes/class-mailer.php‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
namespace Activitypub;
99

1010
use Activitypub\Collection\Actors;
11+
use Activitypub\Model\User;
1112

1213
/**
1314
* Mailer Class.
@@ -148,8 +149,12 @@ public static function new_follower( $notification ) {
148149
* @param int $user_id The id of the local blog-user.
149150
*/
150151
public static function direct_message( $activity, $user_id ) {
151-
// Check if Activity is public or not.
152-
if ( is_activity_public( $activity ) ) {
152+
if (
153+
is_activity_public( $activity ) ||
154+
// Only accept messages that have the user in the "to" field.
155+
empty( $activity['to'] ) ||
156+
! in_array( Actors::get_by_id( $user_id )->get_id(), (array) $activity['to'], true )
157+
) {
153158
return;
154159
}
155160

‎readme.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,7 @@ For reasons of data protection, it is not possible to see the followers of other
137137
* Added: A filter to allow modifying the ActivityPub preview template
138138
* Added: `@mentions` in the JSON representation of the reply
139139
* Improved: HTML to e-mail text conversion
140+
* Improved: Direct Messages: Test for the user being in the to field
140141

141142
= 4.5.1 =
142143

‎tests/includes/class-test-mailer.php‎

Lines changed: 133 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
namespace Activitypub\Tests;
99

1010
use Activitypub\Mailer;
11+
use Activitypub\Collection\Actors;
1112
use Activitypub\Notification;
1213
use WP_UnitTestCase;
1314

@@ -210,21 +211,121 @@ public function test_init() {
210211
$this->assertEquals( 10, has_action( 'activitypub_notification_follow', array( Mailer::class, 'new_follower' ) ) );
211212
}
212213

214+
/**
215+
* Data provider for direct message notification.
216+
*
217+
* @return array
218+
*/
219+
public function direct_message_provider() {
220+
return array(
221+
'to' => array(
222+
true,
223+
array(
224+
'actor' => 'https://example.com/author',
225+
'object' => array(
226+
'content' => 'Test direct message',
227+
),
228+
'to' => array( 'user_url' ),
229+
),
230+
),
231+
'none' => array(
232+
false,
233+
array(
234+
'actor' => 'https://example.com/author',
235+
'object' => array(
236+
'content' => 'Test direct message',
237+
),
238+
),
239+
),
240+
'public+reply' => array(
241+
false,
242+
array(
243+
'actor' => 'https://example.com/author',
244+
'object' => array(
245+
'content' => 'Test public reply',
246+
'inReplyTo' => 'https://example.com/post/1',
247+
),
248+
'to' => array( 'https://www.w3.org/ns/activitystreams#Public' ),
249+
),
250+
),
251+
'public+reply+cc' => array(
252+
false,
253+
array(
254+
'actor' => 'https://example.com/author',
255+
'object' => array(
256+
'content' => 'Test public reply',
257+
'inReplyTo' => 'https://example.com/post/1',
258+
),
259+
'to' => array( 'https://www.w3.org/ns/activitystreams#Public' ),
260+
'cc' => array( 'user_url' ),
261+
),
262+
),
263+
'public+followers' => array(
264+
false,
265+
array(
266+
'actor' => 'https://example.com/author',
267+
'object' => array(
268+
'content' => 'Test public activity',
269+
'inReplyTo' => null,
270+
),
271+
'to' => array( 'https://www.w3.org/ns/activitystreams#Public' ),
272+
'cc' => array( 'https://example.com/followers' ),
273+
),
274+
),
275+
'followers' => array(
276+
false,
277+
array(
278+
'actor' => 'https://example.com/author',
279+
'object' => array(
280+
'content' => 'Test activity just to followers',
281+
'inReplyTo' => null,
282+
),
283+
'to' => array( 'https://example.com/followers' ),
284+
),
285+
),
286+
'reply+cc' => array(
287+
false,
288+
array(
289+
'actor' => 'https://example.com/author',
290+
'object' => array(
291+
'content' => 'Reply activity to me and to followers',
292+
'inReplyTo' => 'https://example.com/post/1',
293+
),
294+
'to' => array( 'https://example.com/followers' ),
295+
'cc' => array( 'user_url' ),
296+
),
297+
),
298+
);
299+
}
300+
213301
/**
214302
* Test direct message notification.
215303
*
304+
* @param bool $send_email Whether email should be sent.
305+
* @param array $activity Activity object.
306+
* @dataProvider direct_message_provider
216307
* @covers ::direct_message
217308
*/
218-
public function test_direct_message() {
309+
public function test_direct_message( $send_email, $activity ) {
219310
$user_id = self::$user_id;
220311
$mock = new \MockAction();
221312

222-
$activity = array(
223-
'actor' => 'https://example.com/author',
224-
'object' => array(
225-
'content' => 'Test direct message',
226-
),
227-
);
313+
// We need to replace back in the user URL because the user_id is not available in the data provider.
314+
$replace = function ( $url ) use ( $user_id ) {
315+
if ( 'user_url' === $url ) {
316+
return Actors::get_by_id( $user_id )->get_id();
317+
318+
}
319+
return $url;
320+
};
321+
322+
foreach ( $activity as $key => $value ) {
323+
if ( is_array( $value ) ) {
324+
$activity[ $key ] = array_map( $replace, $value );
325+
} else {
326+
$activity[ $key ] = $replace( $value );
327+
}
328+
}
228329

229330
// Mock remote metadata.
230331
add_filter(
@@ -238,69 +339,33 @@ function () {
238339
);
239340
add_filter( 'wp_mail', array( $mock, 'filter' ), 1 );
240341

241-
// Capture email.
242-
add_filter(
243-
'wp_mail',
244-
function ( $args ) use ( $user_id ) {
245-
$this->assertStringContainsString( 'Direct Message', $args['subject'] );
246-
$this->assertStringContainsString( 'Test Sender', $args['subject'] );
247-
$this->assertStringContainsString( 'Test direct message', $args['message'] );
248-
$this->assertStringContainsString( 'https://example.com/author', $args['message'] );
249-
$this->assertEquals( get_user_by( 'id', $user_id )->user_email, $args['to'] );
250-
return $args;
251-
}
252-
);
342+
if ( $send_email ) {
343+
// Capture email.
344+
add_filter(
345+
'wp_mail',
346+
function ( $args ) use ( $user_id, $activity ) {
347+
$this->assertStringContainsString( 'Direct Message', $args['subject'] );
348+
$this->assertStringContainsString( 'Test Sender', $args['subject'] );
349+
$this->assertStringContainsString( $activity['object']['content'], $args['message'] );
350+
$this->assertStringContainsString( 'https://example.com/author', $args['message'] );
351+
$this->assertEquals( get_user_by( 'id', $user_id )->user_email, $args['to'] );
352+
return $args;
353+
}
354+
);
355+
} else {
356+
add_filter(
357+
'wp_mail',
358+
function ( $args ) {
359+
$this->fail( 'Email should not be sent for public activity' );
360+
return $args;
361+
}
362+
);
363+
364+
}
253365

254366
Mailer::direct_message( $activity, $user_id );
255367

256-
// Test public activity (should not send email).
257-
$public_activity = array(
258-
'actor' => 'https://example.com/author',
259-
'object' => array(
260-
'content' => 'Test public reply',
261-
'inReplyTo' => 'https://example.com/post/1',
262-
),
263-
'to' => array( 'https://www.w3.org/ns/activitystreams#Public' ),
264-
);
265-
266-
// Reset email capture.
267-
remove_all_filters( 'wp_mail' );
268-
add_filter( 'wp_mail', array( $mock, 'filter' ), 1 );
269-
add_filter(
270-
'wp_mail',
271-
function ( $args ) {
272-
$this->fail( 'Email should not be sent for public activity' );
273-
return $args;
274-
}
275-
);
276-
277-
Mailer::direct_message( $public_activity, $user_id );
278-
279-
// Test public activity (should not send email).
280-
$public_activity = array(
281-
'actor' => 'https://example.com/author',
282-
'object' => array(
283-
'content' => 'Test public activity',
284-
'inReplyTo' => null,
285-
),
286-
'to' => array( 'https://www.w3.org/ns/activitystreams#Public' ),
287-
'cc' => array( 'https://example.com/followers' ),
288-
);
289-
290-
// Reset email capture.
291-
remove_all_filters( 'wp_mail' );
292-
add_filter( 'wp_mail', array( $mock, 'filter' ), 1 );
293-
add_filter(
294-
'wp_mail',
295-
function ( $args ) {
296-
$this->fail( 'Email should not be sent for public activity' );
297-
return $args;
298-
}
299-
);
300-
301-
Mailer::direct_message( $public_activity, $user_id );
302-
303-
$this->assertEquals( 1, $mock->get_call_count() );
368+
$this->assertEquals( $send_email ? 1 : 0, $mock->get_call_count() );
304369

305370
// Clean up.
306371
remove_all_filters( 'pre_get_remote_metadata_by_actor' );
@@ -343,6 +408,7 @@ public function test_direct_message_text( $text, $expected ) {
343408
'object' => array(
344409
'content' => $text,
345410
),
411+
'to' => array( Actors::get_by_id( $user_id )->get_id() ),
346412
);
347413

348414
// Mock remote metadata.

0 commit comments

Comments
 (0)