Skip to content

Commit 337160e

Browse files
ara4nerikjohnston
authored andcommitted
Add a passing test for recovery from gappy device list updates (#761)
1 parent b553e8c commit 337160e

File tree

1 file changed

+202
-10
lines changed

1 file changed

+202
-10
lines changed

tests/50federation/40devicelists.pl

Lines changed: 202 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -101,18 +101,17 @@
101101

102102
assert_eq( $req->method, "GET", 'request method' );
103103

104-
$req->respond_json( {
105-
user_id => $creator_id,
106-
stream_id => 1,
107-
devices => [ {
108-
device_id => $device_id,
104+
$req->respond_json( {
105+
user_id => $creator_id,
106+
stream_id => 1,
107+
devices => [ {
108+
device_id => $device_id,
109109

110-
keys => {
110+
keys => {
111111
device_keys => {}
112112
}
113-
} ]
114-
} );
115-
113+
} ]
114+
} );
116115
Future->done(1);
117116
}),
118117
$outbound_client->send_edu(
@@ -287,6 +286,8 @@
287286

288287
use Data::Dumper;
289288
test "Device list doesn't change if remote server is down",
289+
# see https://github.com/matrix-org/synapse/issues/5441
290+
290291
requires => [
291292
$main::OUTBOUND_CLIENT,
292293
$main::INBOUND_SERVER,
@@ -362,7 +363,7 @@
362363
})
363364
);
364365

365-
# First we succesfully request the remote users keys while the remote server is up.
366+
# First we succesfully request the remote user's keys while the remote server is up.
366367
# We do this once they share a room.
367368
matrix_create_room(
368369
$local_user,
@@ -431,3 +432,194 @@
431432
Future->done(1)
432433
})
433434
};
435+
436+
# for https://github.com/matrix-org/synapse/issues/4827
437+
use Data::Dumper;
438+
test "If a device list update goes missing, the server resyncs on the next one",
439+
requires => [ local_user_fixture(),
440+
$main::INBOUND_SERVER, $main::OUTBOUND_CLIENT,
441+
federation_user_id_fixture(),
442+
room_alias_name_fixture() ],
443+
444+
check => sub {
445+
my ( $user, $inbound_server, $outbound_client, $creator_id, $room_alias_name ) = @_;
446+
447+
my ( $room_id );
448+
449+
my $local_server_name = $user->server_name;
450+
451+
my $remote_server_name = $inbound_server->server_name;
452+
my $datastore = $inbound_server->datastore;
453+
454+
my $room_alias = "#$room_alias_name:$remote_server_name";
455+
456+
my $device_id = "random_device_id";
457+
458+
my $prev_stream_id;
459+
460+
my $room = $datastore->create_room(
461+
creator => $creator_id,
462+
alias => $room_alias,
463+
);
464+
465+
my $client_keys = {
466+
user_id => $creator_id,
467+
device_id => $device_id,
468+
algorithms => ["m.olm.v1.curve25519-aes-sha2", "m.megolm.v1.aes-sha2"],
469+
# start off with no keys
470+
keys => {
471+
},
472+
signatures => {
473+
},
474+
};
475+
476+
my $client_user_devices = {
477+
user_id => $creator_id,
478+
stream_id => 1,
479+
devices => [{
480+
device_id => $device_id,
481+
keys => $client_keys,
482+
device_display_name => "Original name"
483+
}],
484+
};
485+
486+
my $client_user_keys = {
487+
device_keys => {
488+
$creator_id => {
489+
$device_id => $client_keys
490+
},
491+
},
492+
};
493+
494+
do_request_json_for( $user,
495+
method => "POST",
496+
uri => "/r0/join/$room_alias",
497+
498+
content => {},
499+
)->then( sub {
500+
Future->needs_all(
501+
$inbound_server->await_request_user_devices( $creator_id )
502+
->then( sub {
503+
my ( $req, undef ) = @_;
504+
505+
assert_eq( $req->method, "GET", 'request method' );
506+
507+
$req->respond_json( $client_user_devices );
508+
509+
Future->done(1);
510+
}),
511+
$outbound_client->send_edu(
512+
edu_type => "m.device_list_update",
513+
destination => $local_server_name,
514+
content => {
515+
user_id => $creator_id,
516+
device_id => $device_id,
517+
stream_id => 1,
518+
519+
keys => {
520+
device_keys => $client_user_keys,
521+
}
522+
}
523+
)
524+
)
525+
})->then( sub {
526+
# in stream_id 2, we add keys to the device but deliberately don't
527+
# tell the remote server about it.
528+
529+
Future->needs_all(
530+
$inbound_server->await_request_user_devices( $creator_id )->then( sub {
531+
my ( $req ) = @_;
532+
log_if_fail "await_request_user_devices after out-of-order EDU";
533+
534+
# add the missing keys from stream_id 2
535+
$client_keys->{ keys } = {
536+
"curve25519:JJQDHPZKYD" => "MAtX5CLJXvHJ4wjvMBwc53+NnMceHiFch5r4mxOnOCA",
537+
"ed25519:JJQDHPZKYD" => "LOu9tc6Sg7+mCEu3elrps3IiiotpefyaNnScTpSRQbU"
538+
};
539+
540+
# then in stream_id 3, we rename the device
541+
# await a hit to federation/query, responding with stream ID 3
542+
$client_user_devices->{ stream_id } = 3;
543+
$client_user_devices->{ devices }->[0]->{ device_display_name } = "New device name";
544+
545+
$req->respond_json($client_user_devices);
546+
Future->done(1)
547+
}),
548+
549+
# deliberately send an out of order EDU to check that we get requeried
550+
$outbound_client->send_edu(
551+
edu_type => "m.device_list_update",
552+
destination => $local_server_name,
553+
content => {
554+
user_id => $creator_id,
555+
device_id => $device_id,
556+
device_display_name => "New device name",
557+
# deliberately skip an EDU in sequence to check
558+
# that we get re-queried
559+
prev_id => [ 2 ],
560+
stream_id => 3,
561+
562+
keys => {
563+
device_keys => $client_user_keys
564+
}
565+
}
566+
)
567+
);
568+
})->then( sub {
569+
do_request_json_for( $user,
570+
method => "POST",
571+
uri => "/unstable/keys/query",
572+
content => {
573+
device_keys => {
574+
$creator_id => [ $device_id ],
575+
}
576+
}
577+
)
578+
})->then( sub {
579+
my ( $content ) = @_;
580+
581+
log_if_fail "query response", $content;
582+
583+
assert_json_keys( $content, "device_keys" );
584+
585+
my $device_keys = $content->{device_keys};
586+
assert_json_keys( $device_keys, $creator_id );
587+
588+
my $creator_keys = $device_keys->{ $creator_id };
589+
assert_json_keys( $creator_keys, $device_id );
590+
591+
my $creator_device_keys = $creator_keys->{ $device_id };
592+
assert_json_keys( $creator_device_keys, "unsigned" );
593+
594+
my $unsigned = $creator_device_keys->{unsigned};
595+
596+
# check the keys are there from stream id 2
597+
assert_json_keys( $creator_device_keys->{keys}, "curve25519:JJQDHPZKYD");
598+
assert_json_keys( $creator_device_keys->{keys}, "ed25519:JJQDHPZKYD");
599+
600+
# check the name is there from stream id 3
601+
assert_eq( $unsigned->{device_display_name}, "New device name" );
602+
603+
Future->done( 1 )
604+
});
605+
};
606+
607+
# for https://github.com/matrix-org/synapse/issues/6399
608+
# test "When a room is upgraded to E2E, device lists caches should be flushed"
609+
# in practice, if you share a room with a user, your device list should be synced
610+
# irrespective of E2E, so let's not bother testing this now
611+
612+
# for https://github.com/matrix-org/synapse/issues/6399
613+
#
614+
# If you see you have a room in common with a user, you blindly assume you
615+
# have been receiving device_list updates for them. But this fails if there
616+
# was some period where you didn't have a room in common (or if an EDU got dropped).
617+
# So instead, we should either flush the devicelist cache when we stop sharing a room
618+
# with a user, or flush it when we start sharing a room.
619+
#
620+
# test "Device lists caches should be flushed when you re-encounter a user"
621+
622+
# test "Device lists get refreshed when you encounter an unrecognised device" # for https://github.com/matrix-org/synapse/issues/5095#issuecomment-501512352
623+
# however, this doesn't help us if the keys just change
624+
625+
# test "If you send >20 device lists updates in a row, they don't get lost?" # for https://github.com/matrix-org/synapse/issues/5153

0 commit comments

Comments
 (0)