From 075df0c2ec4521f5177a5a53761ac4f6f761c764 Mon Sep 17 00:00:00 2001 From: Jorik Schellekens Date: Tue, 23 Jul 2019 15:32:55 +0100 Subject: [PATCH 1/6] UISI test --- tests/05homeserver.pl | 7 +- tests/41end-to-end-keys/06-device-lists.pl | 74 ++++++++++++++++++++++ 2 files changed, 78 insertions(+), 3 deletions(-) diff --git a/tests/05homeserver.pl b/tests/05homeserver.pl index 917dd87ac..561c95000 100644 --- a/tests/05homeserver.pl +++ b/tests/05homeserver.pl @@ -4,7 +4,7 @@ my $N_HOMESERVERS = 2; -my @servers; +our @SERVERS; # Almost like an END block, but we can't use END because we need SIGCHLD, and # see @@ -14,10 +14,11 @@ ( fmap_void { my $server = $_; $server->kill_and_await_finish; - } foreach => \@servers, concurrent => scalar @servers )->get; + } foreach => \@SERVERS, concurrent => scalar @SERVERS )->get; }; push our @EXPORT, qw( HOMESERVER_INFO ); +push @EXPORT, qw( SERVERS ); our @HOMESERVER_INFO = map { my $idx = $_; @@ -112,7 +113,7 @@ ); } - push @servers, $server; + push @SERVERS, $server; $OUTPUT->diag( "Starting server-$idx" ); Future->wait_any( diff --git a/tests/41end-to-end-keys/06-device-lists.pl b/tests/41end-to-end-keys/06-device-lists.pl index 86c6a14be..91167532a 100644 --- a/tests/41end-to-end-keys/06-device-lists.pl +++ b/tests/41end-to-end-keys/06-device-lists.pl @@ -754,3 +754,77 @@ sub sync_until_user_in_device_list Future->done(1); }); }; + +test "Device list doesn't change if remote server is down", + requires => [ local_user_fixture(), $main::API_CLIENTS[1], qw( can_upload_e2e_keys )], + + check => sub { + my ( $local_user, $http ) = @_; + + my ( $remote_user, $room_id, $first_keys_query_body ); + + # We want a completely fresh user. That's why we don't use remote_user_fixture + matrix_create_user_on_server( $http, displayname => "fla-key" )->then( sub { + ( $remote_user ) = @_; + matrix_put_e2e_keys( + $remote_user, device_keys => { updated => "keys" } + ) + })->then( sub { + matrix_put_e2e_keys( + $local_user, device_keys => { updated => "keys" } + ) + })->then( sub { + matrix_create_room( $local_user, + invite => [ $remote_user->user_id ], + ) + })->then( sub { + ( $room_id ) = @_; + + matrix_join_room( $remote_user, $room_id ); + })->then( sub { + do_request_json_for( $local_user, + method => "POST", + uri => "/unstable/keys/query", + content => { + device_keys => { + $remote_user->user_id => {} + } + } + ) + })->then( sub { + ( $first_keys_query_body ) = @_; + $_ = $main::SERVERS[1]; + $_->kill_and_await_finish + })->then( sub { + do_request_json_for( $local_user, + method => "POST", + uri => "/unstable/keys/query", + content => { + device_keys => { + $remote_user->user_id => {} + } + } + ) + })->then( sub { + my ( $second_keys_query_body ) = @_; + + assert_json_keys( $second_keys_query_body, qw( device_keys )); + + # The unsiged field is optional in the spec so we remove it from any response. + foreach ($first_keys_query_body, $second_keys_query_body) { + while (my ($user_key, $devices) = each %{$_->{ device_keys }} ) { + while (my ($device_key, $values) = each %$devices) { + delete $values->{ unsigned }; + } + } + } + + assert_deeply_eq( $second_keys_query_body, $first_keys_query_body, "/keys/query while homserver offline." ); + Future->done(1) + })->followed_by( sub { + my $f = shift; + $_ = $main::SERVERS[1]; + $_->start; + return $f; + }) + }; \ No newline at end of file From bdbca6e474296d861fdf0ff847191bb1cc10b59f Mon Sep 17 00:00:00 2001 From: Jorik Schellekens Date: Fri, 26 Jul 2019 19:52:00 +0100 Subject: [PATCH 2/6] Use sytest federation server instead of shutting down remote. --- lib/SyTest/Federation/Server.pm | 4 + tests/41end-to-end-keys/06-device-lists.pl | 74 ------------ tests/50federation/40devicelists.pl | 129 +++++++++++++++++++++ 3 files changed, 133 insertions(+), 74 deletions(-) diff --git a/lib/SyTest/Federation/Server.pm b/lib/SyTest/Federation/Server.pm index 65e2e627b..1b0d0e594 100644 --- a/lib/SyTest/Federation/Server.pm +++ b/lib/SyTest/Federation/Server.pm @@ -431,6 +431,10 @@ __PACKAGE__->mk_await_request_pair( user_devices => [qw( :user_id )], ); +__PACKAGE__->mk_await_request_pair( + user_keys_query => [qw( )], +); + sub on_request_federation_v1_send { my $self = shift; diff --git a/tests/41end-to-end-keys/06-device-lists.pl b/tests/41end-to-end-keys/06-device-lists.pl index 91167532a..86c6a14be 100644 --- a/tests/41end-to-end-keys/06-device-lists.pl +++ b/tests/41end-to-end-keys/06-device-lists.pl @@ -754,77 +754,3 @@ sub sync_until_user_in_device_list Future->done(1); }); }; - -test "Device list doesn't change if remote server is down", - requires => [ local_user_fixture(), $main::API_CLIENTS[1], qw( can_upload_e2e_keys )], - - check => sub { - my ( $local_user, $http ) = @_; - - my ( $remote_user, $room_id, $first_keys_query_body ); - - # We want a completely fresh user. That's why we don't use remote_user_fixture - matrix_create_user_on_server( $http, displayname => "fla-key" )->then( sub { - ( $remote_user ) = @_; - matrix_put_e2e_keys( - $remote_user, device_keys => { updated => "keys" } - ) - })->then( sub { - matrix_put_e2e_keys( - $local_user, device_keys => { updated => "keys" } - ) - })->then( sub { - matrix_create_room( $local_user, - invite => [ $remote_user->user_id ], - ) - })->then( sub { - ( $room_id ) = @_; - - matrix_join_room( $remote_user, $room_id ); - })->then( sub { - do_request_json_for( $local_user, - method => "POST", - uri => "/unstable/keys/query", - content => { - device_keys => { - $remote_user->user_id => {} - } - } - ) - })->then( sub { - ( $first_keys_query_body ) = @_; - $_ = $main::SERVERS[1]; - $_->kill_and_await_finish - })->then( sub { - do_request_json_for( $local_user, - method => "POST", - uri => "/unstable/keys/query", - content => { - device_keys => { - $remote_user->user_id => {} - } - } - ) - })->then( sub { - my ( $second_keys_query_body ) = @_; - - assert_json_keys( $second_keys_query_body, qw( device_keys )); - - # The unsiged field is optional in the spec so we remove it from any response. - foreach ($first_keys_query_body, $second_keys_query_body) { - while (my ($user_key, $devices) = each %{$_->{ device_keys }} ) { - while (my ($device_key, $values) = each %$devices) { - delete $values->{ unsigned }; - } - } - } - - assert_deeply_eq( $second_keys_query_body, $first_keys_query_body, "/keys/query while homserver offline." ); - Future->done(1) - })->followed_by( sub { - my $f = shift; - $_ = $main::SERVERS[1]; - $_->start; - return $f; - }) - }; \ No newline at end of file diff --git a/tests/50federation/40devicelists.pl b/tests/50federation/40devicelists.pl index 198fb5379..88a36673b 100644 --- a/tests/50federation/40devicelists.pl +++ b/tests/50federation/40devicelists.pl @@ -284,3 +284,132 @@ ) }); }; + +test "Device list doesn't change if remote server is down", + requires => [ + $main::OUTBOUND_CLIENT, + $main::INBOUND_SERVER, + $main::HOMESERVER_INFO[0], + local_user_fixture, + federation_user_id_fixture(), + qw( can_upload_e2e_keys ) + ], + + check => sub { + my ( + $outbound_client, + $inbound_server, + $local_server_info, + $local_user, + $outbound_client_user + ) = @_; + + my ( $first_keys_query_body, $second_keys_query_body ); + + my $client_user_keys = { + device_keys => { + $outbound_client_user => { + CURIOSITY_ROVER => { + user_id => $outbound_client_user, + device_id => "CURIOSITY_ROVER", + algorithms => ["fast", "and broken"], + keys => { + "curve25519:JLAFKJWSCS" => "3C5BFWi2Y8MaVvjM8M22DBmh24PmgR0nPvJOIArzgyI", + "ed25519:JLAFKJWSCS" => "lEuiRJBit0IG6nUf5pUzWTUEsRVVe/HJkoKuEww9ULI" + }, + signatures => { + $outbound_client_user => {"ed25519:JLAFKJWSCS" => "dSO80A01XiigH3uBiDVx/EjzaoycHcjq9lfQX0uWsqxl2giMIiSPR8a4d291W1ihKJL/a+myXS367WT6NAIcBA"} + }, + unsigned => { + "device_display_name" => "Curiosity Rover" + }, + }, + } + }, + }; + + Future->needs_all( + $inbound_server->await_request_user_keys_query( $outbound_client_user )->then( sub { + my ( $req ) = @_; + $req->respond_json($client_user_keys); + Future->done(1) + }), + + matrix_create_room( + $local_user, + preset => "public_chat", + room_version => "5", + )->then( sub { + my ( $room_id ) = @_; + $outbound_client->join_room( + server_name => $local_server_info->server_name, + room_id => $room_id, + user_id => $outbound_client_user, + ) + })->then( sub { + do_request_json_for( $local_user, + method => "POST", + uri => "/r0/keys/query", + content => { + device_keys => { + $outbound_client_user => {} + } + } + ) + })->then( sub { + ( $first_keys_query_body ) = @_; + delete $first_keys_query_body->{failures}; + assert_deeply_eq( $first_keys_query_body, $client_user_keys, "Local user gets federation user's keys."), + Future->done(1) + }) + )->then( sub { + my $respond_503_fun = sub { + my ( $req ) = @_; + $OUTPUT->diag("responding"); + $req->respond_json({}, code => 503); + Future->done(1) + }; + my @respond_503 = ( + $inbound_server->await_request_user_devices( $outbound_client_user )->then( sub { + my ( $req ) = @_; + $OUTPUT->diag("responding"); + $req->respond_json({}, code => 503); + Future->done(1) + }), + $inbound_server->await_request_user_keys_query()->then( sub { + my ( $req ) = @_; + $OUTPUT->diag("responding"); + $req->respond_json({}, code => 503); + Future->done(1) + }) + ); + Future->wait_any( + @respond_503, + do_request_json_for( $local_user, + method => "POST", + uri => "/r0/keys/query", + content => { + device_keys => { + $outbound_client_user => {} + } + } + )->then( sub { + ( $second_keys_query_body ) = @_; + $OUTPUT->diag("got here"); + map {$_->cancel} @respond_503; + # The unsiged field is optional in the spec so we remove it from any response. + foreach ($first_keys_query_body, $second_keys_query_body) { + while (my ($user_key, $devices) = each %{$_->{ device_keys }} ) { + while (my ($device_key, $values) = each %$devices) { + delete $values->{ unsigned }; + } + } + }; + $OUTPUT->diag(Dumper $first_keys_query_body); + $OUTPUT->diag(Dumper $second_keys_query_body); + assert_deeply_eq( $second_keys_query_body, $first_keys_query_body, "Query matches while federation server is down." ); + Future->done(1) + }) + ) + }) + }; \ No newline at end of file From 208082282089e1c764f29975539930b2a0e55f11 Mon Sep 17 00:00:00 2001 From: Jorik Schellekens Date: Mon, 29 Jul 2019 11:54:39 +0100 Subject: [PATCH 3/6] Make servers list private --- tests/05homeserver.pl | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/tests/05homeserver.pl b/tests/05homeserver.pl index 561c95000..b3c5b3849 100644 --- a/tests/05homeserver.pl +++ b/tests/05homeserver.pl @@ -2,9 +2,9 @@ use Cwd qw( abs_path ); -my $N_HOMESERVERS = 2; +my $N_HOMEservers = 2; -our @SERVERS; +my @servers; # Almost like an END block, but we can't use END because we need SIGCHLD, and # see @@ -14,11 +14,10 @@ ( fmap_void { my $server = $_; $server->kill_and_await_finish; - } foreach => \@SERVERS, concurrent => scalar @SERVERS )->get; + } foreach => \@servers, concurrent => scalar @servers )->get; }; push our @EXPORT, qw( HOMESERVER_INFO ); -push @EXPORT, qw( SERVERS ); our @HOMESERVER_INFO = map { my $idx = $_; @@ -113,7 +112,7 @@ ); } - push @SERVERS, $server; + push @servers, $server; $OUTPUT->diag( "Starting server-$idx" ); Future->wait_any( @@ -136,4 +135,4 @@ }) }, ); -} 0 .. $N_HOMESERVERS-1; +} 0 .. $N_HOMEservers-1; From cea2d2f1fda2fb8d9c4f019c7cd06b7231afd61c Mon Sep 17 00:00:00 2001 From: Jorik Schellekens Date: Mon, 29 Jul 2019 14:48:11 +0100 Subject: [PATCH 4/6] Don't converge futures. --- tests/50federation/40devicelists.pl | 103 +++++++++++++--------------- 1 file changed, 48 insertions(+), 55 deletions(-) diff --git a/tests/50federation/40devicelists.pl b/tests/50federation/40devicelists.pl index 88a36673b..a2c0d2cea 100644 --- a/tests/50federation/40devicelists.pl +++ b/tests/50federation/40devicelists.pl @@ -285,6 +285,7 @@ }); }; +use Data::Dumper; test "Device list doesn't change if remote server is down", requires => [ $main::OUTBOUND_CLIENT, @@ -307,34 +308,35 @@ my ( $first_keys_query_body, $second_keys_query_body ); my $client_user_keys = { - device_keys => { - $outbound_client_user => { - CURIOSITY_ROVER => { - user_id => $outbound_client_user, - device_id => "CURIOSITY_ROVER", - algorithms => ["fast", "and broken"], - keys => { - "curve25519:JLAFKJWSCS" => "3C5BFWi2Y8MaVvjM8M22DBmh24PmgR0nPvJOIArzgyI", - "ed25519:JLAFKJWSCS" => "lEuiRJBit0IG6nUf5pUzWTUEsRVVe/HJkoKuEww9ULI" - }, - signatures => { - $outbound_client_user => {"ed25519:JLAFKJWSCS" => "dSO80A01XiigH3uBiDVx/EjzaoycHcjq9lfQX0uWsqxl2giMIiSPR8a4d291W1ihKJL/a+myXS367WT6NAIcBA"} - }, - unsigned => { - "device_display_name" => "Curiosity Rover" - }, + user_id => $outbound_client_user, + stream_id => 3, + devices => [{ + device_id => "CURIOSITY_ROVER", + keys => { + user_id => $outbound_client_user, + device_id => "CURIOSITY_ROVER", + algorithms => ["fast", "and broken"], + keys => { + "c" => "sharp", + "b" => "flat" }, - } - }, + signatures => { + $outbound_client_user => {"ed25519:JLAFKJWSCS" => "dSO80A01XiigH3uBiDVx/EjzaoycHcjq9lfQX0uWsqxl2giMIiSPR8a4d291W1ihKJL/a+myXS367WT6NAIcBA"}, + }, + }, + device_display_name => "Curiosity Rover" + }], }; Future->needs_all( - $inbound_server->await_request_user_keys_query( $outbound_client_user )->then( sub { + $inbound_server->await_request_user_devices( $outbound_client_user )->then( sub { my ( $req ) = @_; $req->respond_json($client_user_keys); Future->done(1) }), + # First we succesfully request the remote users keys while the remote server is up. + # We do this once they share a room. matrix_create_room( $local_user, preset => "public_chat", @@ -358,58 +360,49 @@ ) })->then( sub { ( $first_keys_query_body ) = @_; - delete $first_keys_query_body->{failures}; - assert_deeply_eq( $first_keys_query_body, $client_user_keys, "Local user gets federation user's keys."), + log_if_fail Dumper $first_keys_query_body; Future->done(1) }) )->then( sub { - my $respond_503_fun = sub { - my ( $req ) = @_; - $OUTPUT->diag("responding"); - $req->respond_json({}, code => 503); - Future->done(1) - }; + # We take the remote server 'offline' and then make the same request for + # the users keys. We expect no change in the keys. my @respond_503 = ( $inbound_server->await_request_user_devices( $outbound_client_user )->then( sub { my ( $req ) = @_; - $OUTPUT->diag("responding"); + log_if_fail "Responded with 503 to /user/devices request"; $req->respond_json({}, code => 503); Future->done(1) }), $inbound_server->await_request_user_keys_query()->then( sub { my ( $req ) = @_; - $OUTPUT->diag("responding"); + log_if_fail "Responded with 503 to /user/keys/query request"; $req->respond_json({}, code => 503); Future->done(1) }) ); - Future->wait_any( - @respond_503, - do_request_json_for( $local_user, - method => "POST", - uri => "/r0/keys/query", - content => { - device_keys => { - $outbound_client_user => {} - } + do_request_json_for( $local_user, + method => "POST", + uri => "/r0/keys/query", + content => { + device_keys => { + $outbound_client_user => {} } - )->then( sub { - ( $second_keys_query_body ) = @_; - $OUTPUT->diag("got here"); - map {$_->cancel} @respond_503; - # The unsiged field is optional in the spec so we remove it from any response. - foreach ($first_keys_query_body, $second_keys_query_body) { - while (my ($user_key, $devices) = each %{$_->{ device_keys }} ) { - while (my ($device_key, $values) = each %$devices) { - delete $values->{ unsigned }; - } + } + )->then( sub { + ( $second_keys_query_body ) = @_; + map {$_->cancel} @respond_503; + # The unsiged field is optional in the spec so we remove it from any response. + foreach ($first_keys_query_body, $second_keys_query_body) { + while (my ($user_key, $devices) = each %{$_->{ device_keys }} ) { + while (my ($device_key, $values) = each %$devices) { + delete $values->{ unsigned }; } - }; - $OUTPUT->diag(Dumper $first_keys_query_body); - $OUTPUT->diag(Dumper $second_keys_query_body); - assert_deeply_eq( $second_keys_query_body, $first_keys_query_body, "Query matches while federation server is down." ); - Future->done(1) - }) - ) + } + }; + + log_if_fail (Dumper $second_keys_query_body); + assert_deeply_eq( $second_keys_query_body, $first_keys_query_body, "Query matches while federation server is down." ); + Future->done(1) + }) }) }; \ No newline at end of file From 1895626629c3926b2094235a34860f0b26e89496 Mon Sep 17 00:00:00 2001 From: Jorik Schellekens Date: Mon, 29 Jul 2019 15:13:21 +0100 Subject: [PATCH 5/6] Don't wait for keys query --- tests/50federation/40devicelists.pl | 120 +++++++++++++++++----------- 1 file changed, 74 insertions(+), 46 deletions(-) diff --git a/tests/50federation/40devicelists.pl b/tests/50federation/40devicelists.pl index a2c0d2cea..555385b80 100644 --- a/tests/50federation/40devicelists.pl +++ b/tests/50federation/40devicelists.pl @@ -305,9 +305,9 @@ $outbound_client_user ) = @_; - my ( $first_keys_query_body, $second_keys_query_body ); + my ( $first_keys_query_body, $second_keys_query_body, @respond_503 ); - my $client_user_keys = { + my $client_user_devices = { user_id => $outbound_client_user, stream_id => 3, devices => [{ @@ -328,45 +328,73 @@ }], }; - Future->needs_all( + my $client_user_keys = { + device_keys => { + $outbound_client_user => { + CURIOSITY_ROVER => { + user_id => $outbound_client_user, + device_id => "CURIOSITY_ROVER", + algorithms => ["fast", "and broken"], + keys => { + "c" => "sharp", + "b" => "flat" + }, + signatures => { + $outbound_client_user => {"ed25519:JLAFKJWSCS" => "dSO80A01XiigH3uBiDVx/EjzaoycHcjq9lfQX0uWsqxl2giMIiSPR8a4d291W1ihKJL/a+myXS367WT6NAIcBA"} + }, + unsigned => { + "device_display_name" => "Curiosity Rover" + }, + }, + }, + }, + }; + + + my @respond_with_keys = ( $inbound_server->await_request_user_devices( $outbound_client_user )->then( sub { my ( $req ) = @_; - $req->respond_json($client_user_keys); + $req->respond_json($client_user_devices); Future->done(1) }), - - # First we succesfully request the remote users keys while the remote server is up. - # We do this once they share a room. - matrix_create_room( - $local_user, - preset => "public_chat", - room_version => "5", - )->then( sub { - my ( $room_id ) = @_; - $outbound_client->join_room( - server_name => $local_server_info->server_name, - room_id => $room_id, - user_id => $outbound_client_user, - ) - })->then( sub { - do_request_json_for( $local_user, - method => "POST", - uri => "/r0/keys/query", - content => { - device_keys => { - $outbound_client_user => {} - } - } - ) - })->then( sub { - ( $first_keys_query_body ) = @_; - log_if_fail Dumper $first_keys_query_body; + $inbound_server->await_request_user_keys_query( $outbound_client_user )->then( sub { + my ( $req ) = @_; + $req->respond_json($client_user_keys); Future->done(1) }) + ); + + # First we succesfully request the remote users keys while the remote server is up. + # We do this once they share a room. + matrix_create_room( + $local_user, + preset => "public_chat", + room_version => "5", )->then( sub { + my ( $room_id ) = @_; + $outbound_client->join_room( + server_name => $local_server_info->server_name, + room_id => $room_id, + user_id => $outbound_client_user, + ) + })->then( sub { + do_request_json_for( $local_user, + method => "POST", + uri => "/r0/keys/query", + content => { + device_keys => { + $outbound_client_user => {} + } + } + ) + })->then( sub { + ( $first_keys_query_body ) = @_; + map {$_->cancel} @respond_with_keys; + log_if_fail (Dumper $first_keys_query_body); + # We take the remote server 'offline' and then make the same request for # the users keys. We expect no change in the keys. - my @respond_503 = ( + @respond_503 = ( $inbound_server->await_request_user_devices( $outbound_client_user )->then( sub { my ( $req ) = @_; log_if_fail "Responded with 503 to /user/devices request"; @@ -388,21 +416,21 @@ $outbound_client_user => {} } } - )->then( sub { - ( $second_keys_query_body ) = @_; - map {$_->cancel} @respond_503; - # The unsiged field is optional in the spec so we remove it from any response. - foreach ($first_keys_query_body, $second_keys_query_body) { - while (my ($user_key, $devices) = each %{$_->{ device_keys }} ) { - while (my ($device_key, $values) = each %$devices) { - delete $values->{ unsigned }; - } + ) + })->then( sub { + ( $second_keys_query_body ) = @_; + map {$_->cancel} @respond_503; + # The unsiged field is optional in the spec so we remove it from any response. + foreach ($first_keys_query_body, $second_keys_query_body) { + while (my ($user_key, $devices) = each %{$_->{ device_keys }} ) { + while (my ($device_key, $values) = each %$devices) { + delete $values->{ unsigned }; } - }; + } + }; - log_if_fail (Dumper $second_keys_query_body); - assert_deeply_eq( $second_keys_query_body, $first_keys_query_body, "Query matches while federation server is down." ); - Future->done(1) - }) + log_if_fail (Dumper $second_keys_query_body); + assert_deeply_eq( $second_keys_query_body, $first_keys_query_body, "Query matches while federation server is down." ); + Future->done(1) }) }; \ No newline at end of file From f05628dbfdc22abeda0231472a20e11f5d1a8c8a Mon Sep 17 00:00:00 2001 From: Jorik Schellekens Date: Mon, 29 Jul 2019 15:14:18 +0100 Subject: [PATCH 6/6] This file is should be pristine --- tests/05homeserver.pl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/05homeserver.pl b/tests/05homeserver.pl index b3c5b3849..917dd87ac 100644 --- a/tests/05homeserver.pl +++ b/tests/05homeserver.pl @@ -2,7 +2,7 @@ use Cwd qw( abs_path ); -my $N_HOMEservers = 2; +my $N_HOMESERVERS = 2; my @servers; @@ -135,4 +135,4 @@ }) }, ); -} 0 .. $N_HOMEservers-1; +} 0 .. $N_HOMESERVERS-1;