Test that remote device list is refetched on leave/rejoin (#793)

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
matrix-org · Feb 6, 2020 · 4172585 · 4172585
1 parent 4b6cc83
commit 4172585
Show file tree

Hide file tree

Showing 7 changed files with 195 additions and 41 deletions.
diff --git a/lib/SyTest/Federation/Client.pm b/lib/SyTest/Federation/Client.pm
@@ -271,6 +271,8 @@ sub join_room
 
          $room->insert_event( $member_event );
 
+         $store->{rooms_by_id}{ $room->room_id } = $room;
+
          Future->done( $room );
       });
    });

diff --git a/lib/SyTest/Federation/Datastore.pm b/lib/SyTest/Federation/Datastore.pm
@@ -301,7 +301,10 @@ sub get_auth_chain_events
    while( @event_ids ) {
       my $event = $events_by_id{shift @event_ids};
 
-      my @auth_ids = map { $_->[0] } @{ $event->{auth_events} };
+      my $room = $self->get_room( $event->{room_id} ) or
+         croak "Unknown room $event->{room_id}";
+
+      my @auth_ids = @{ $room->event_ids_from_refs( $event->{auth_events} ) };
 
       foreach my $id ( @auth_ids ) {
          next if $events_by_id{$id};

diff --git a/lib/SyTest/Federation/Room.pm b/lib/SyTest/Federation/Room.pm
@@ -321,6 +321,10 @@ sub insert_outlier_event
 
    croak "Event not ref" unless ref $event;
 
+   my $event_id = $self->id_for_event( $event );
+
+   $self->{datastore}->put_event( $event_id, $event );
+
    if( defined $event->{state_key} ) {
       $self->{current_state}{ join "\0", $event->{type}, $event->{state_key} }
          = $event;

diff --git a/lib/SyTest/Federation/Server.pm b/lib/SyTest/Federation/Server.pm
@@ -274,10 +274,13 @@ sub on_request_federation_v1_make_join
          404, "Not found", [ Content_length => 0 ], "",
       ) );
 
+   my $room_version = $room->room_version;
+
    Future->done( json => {
       event => $room->make_join_protoevent(
          user_id => $user_id,
       ),
+      room_version => "$room_version",
    } );
 }
 
@@ -308,7 +311,7 @@ sub on_request_federation_v2_send_join
    my $event = $req->body_from_json;
 
    my @auth_chain = $store->get_auth_chain_events(
-      map { $_->[0] } @{ $event->{auth_events} }
+      @{ $room->event_ids_from_refs( $event->{auth_events} ) }
    );
    my @state_events = $room->current_state_events;
 
@@ -477,6 +480,15 @@ sub on_request_federation_v1_send
 
    # A PDU is an event
    foreach my $event ( @{ $body->{pdus} } ) {
+      my $room = $self->{datastore}->get_room( $event->{room_id} );
+
+      if ( $room ) {
+         my $event_id = $room->id_for_event( $event );
+         $room->insert_event( $event );
+      } else {
+         warn "Unknown room ${ $event->{room_id} }"
+      }
+
       next if $self->on_event( $event );
 
       warn "TODO: Unhandled incoming event of type '$event->{type}'";
@@ -545,4 +557,41 @@ sub on_event
    return 1;
 }
 
+=head2 await_request_v1_send_join_reject_v2
+
+   $fut = $server->await_request_v1_send_join_reject_v2( $room_id );
+   my ( $request, $room_id, $event_id ) = $fut->get;
+
+Awaits inbound request for /v1/send_join endpoint while rejecting inbound
+requests to /v2/send_join. Using the C<await_request_v1_send_join> standard
+has the problem that C<SyTest::Federation::Server> will handle /v2/send_join
+appropriately unless overriden, and so remote servers that use v2 will never
+call v1 endpoint in such a case.
+
+=cut
+
+sub await_request_v1_send_join_reject_v2 {
+   my $self = shift;
+   my ( $room_id ) = @_;
+
+   my $v2_fut = $self->await_request_v2_send_join( $room_id )
+   ->then( sub {
+      my ( $req, $room_id, $event_id ) = @_;
+      $req->respond( HTTP::Response->new(
+         404, "Not found", [ Content_length => 0 ], "",
+      ) );
+
+      Future->done
+   });
+
+   $self->await_request_v1_send_join( $room_id )
+   ->then( sub {
+      my ( $req, $room_id, $event_id ) = @_;
+
+      $v2_fut->cancel();
+
+      Future->done( $req, $room_id, $event_id )
+   })
+}
+
 1;
diff --git a/tests/50federation/30room-join.pl b/tests/50federation/30room-join.pl
@@ -63,30 +63,22 @@ sub assert_is_valid_pdu {
          my $local_server_name = $inbound_server->server_name;
          my $datastore         = $inbound_server->datastore;
 
-         my $room = SyTest::Federation::Room->new(
-            datastore => $datastore,
-         );
-
-         $room->create_initial_events(
-            server  => $inbound_server,
+         my $room_alias = "#50fed-room-alias:$local_server_name";
+         my $room = $datastore->create_room(
             creator => $creator_id,
+            alias   => $room_alias,
          );
 
          my $room_id = $room->room_id;
 
-         my $room_alias = "#50fed-room-alias:$local_server_name";
-         $datastore->{room_aliases}{$room_alias} = $room_id;
-
          my $await_request_send_join;
 
          if( $versionprefix eq "v1" ) {
-            # If we only expect a response on the v1 endpoint, the homeserver will try to
-            # hit the v2 one, get a 404 from SyTest (because we didn't call
-            # await_request_v2_send_join), then fall back to the v1 endpoint. We rely on
-            # that 404 response from SyTest and that fallback mechanism to test that the
-            # homeserver can query the v1 endpoint, and correctly handles responses from
-            # it.
-            $await_request_send_join = $inbound_server->await_request_v1_send_join( $room_id );
+            # We need to use the `_reject_v2` form here as otherwise SyTest
+            # will respond to /v2/send_join and v1 endpoint will never get
+            # called.
+            $await_request_send_join =
+               $inbound_server->await_request_v1_send_join_reject_v2($room_id );
          }
          elsif( $versionprefix eq "v2" ) {
             $await_request_send_join = $inbound_server->await_request_v2_send_join( $room_id );
@@ -811,19 +803,14 @@ sub assert_is_valid_pdu {
       my $local_server_name = $inbound_server->server_name;
       my $datastore         = $inbound_server->datastore;
 
-      my $room = SyTest::Federation::Room->new(
-         datastore => $datastore,
-      );
-
-      $room->create_initial_events(
+      my $room_alias = "#no_create_event:$local_server_name";
+      my $room = $datastore->create_room(
          creator => $creator_id,
+         alias   => $room_alias,
       );
 
       my $room_id = $room->room_id;
 
-      my $room_alias = "#no_create_event:$local_server_name";
-      $datastore->{room_aliases}{$room_alias} = $room_id;
-
       Future->needs_all(
          $inbound_server->await_request_make_join( $room_id, $user->user_id )->then( sub {
             my ( $req, $room_id, $user_id ) = @_;
@@ -839,10 +826,12 @@ sub assert_is_valid_pdu {
                event => $proto,
             } );
 
+            log_if_fail "make_join resp", $proto;
+
             Future->done;
          }),
 
-         $inbound_server->await_request_send_join( $room_id )->then( sub {
+         $inbound_server->await_request_v1_send_join_reject_v2( $room_id )->then( sub {
             my ( $req, $room_id, $event_id ) = @_;
 
             $req->method eq "PUT" or
@@ -901,20 +890,16 @@ sub assert_is_valid_pdu {
       my $local_server_name = $inbound_server->server_name;
       my $datastore         = $inbound_server->datastore;
 
-      my $room = SyTest::Federation::Room->new(
-         datastore => $datastore,
-      );
-
-      $room->create_initial_events(
+      my $room_alias = "#no_create_event:$local_server_name";
+      my $room = $datastore->create_room(
          creator => $creator_id,
+         alias   => $room_alias,
+
          room_version => 'sytest-room-ver',
       );
 
       my $room_id = $room->room_id;
 
-      my $room_alias = "#inconsistent-room-ver:$local_server_name";
-      $datastore->{room_aliases}{$room_alias} = $room_id;
-
       Future->needs_all(
          $inbound_server->await_request_make_join( $room_id, $user->user_id )->then( sub {
             my ( $req, $room_id, $user_id ) = @_;
@@ -933,7 +918,7 @@ sub assert_is_valid_pdu {
             Future->done;
          }),
 
-         $inbound_server->await_request_send_join( $room_id )->then( sub {
+         $inbound_server->await_request_v2_send_join( $room_id )->then( sub {
             my ( $req, $room_id, $event_id ) = @_;
 
             $req->method eq "PUT" or
@@ -943,15 +928,14 @@ sub assert_is_valid_pdu {
             log_if_fail "send_join event", $event;
 
             my @auth_chain = $datastore->get_auth_chain_events(
-               map { $_->[0] } @{ $event->{auth_events} }
+               @{ $room->event_ids_from_refs( $event->{auth_events} ) }
             );
 
             $req->respond_json(
-               # /v1/send_join has an extraneous [200, ...] wrapper (see MSC1802)
-               my $response = [ 200, {
+               my $response = {
                   auth_chain => \@auth_chain,
                   state      => [ $room->current_state_events ],
-               } ]
+               }
             );
 
             log_if_fail "send_join response", $response;

diff --git a/tests/50federation/40devicelists.pl b/tests/50federation/40devicelists.pl
@@ -266,6 +266,119 @@
    };
 
 
+test "Server correctly resyncs when server leaves and rejoins a room",
+   requires => [ $main::INBOUND_SERVER, federated_rooms_fixture() ],
+
+   check => sub {
+      my ( $inbound_server, $user, $federated_user_id, $room ) = @_;
+
+      # At first the server shares a room with the federated user, who at that
+      # point has a single device. The server will then leave and then rejoin
+      # the room. In the mean time the federated user has added a device, but
+      # won't have poked the server as they didn't share a room.
+      #
+      # When the server rejoins the subsequent calls by clients to fetch keys
+      # should result in the server resyncing the device lists.
+      my $device_id1 = "random_device_id1";
+      my $device_id2 = "random_device_id2";
+
+      Future->needs_all(
+         $inbound_server->await_request_user_devices( $federated_user_id )
+         ->then( sub {
+            my ( $req, undef ) = @_;
+
+            assert_eq( $req->method, "GET", 'request method' );
+
+            $req->respond_json( {
+               user_id   => $federated_user_id,
+               stream_id => 1,
+               devices   => [
+                  {
+                     device_id => $device_id1,
+                     keys      => { device_keys => {} },
+                  },
+               ],
+            } );
+            Future->done(1);
+         }),
+         do_request_json_for( $user,
+            method  => "POST",
+            uri     => "/r0/keys/query",
+            content => {
+               device_keys => {
+                  $federated_user_id => [],
+               },
+            },
+         ),
+      )->then( sub {
+         my ( $first, $content ) = @_;
+
+         log_if_fail "first query response", $content;
+
+         assert_json_keys( $content, "device_keys" );
+
+         my $device_keys = $content->{device_keys};
+         assert_json_keys( $device_keys, $federated_user_id );
+
+         my $alice_keys = $device_keys->{ $federated_user_id };
+         assert_json_keys( $alice_keys, ( $device_id1 ) );
+
+         matrix_leave_room( $user, $room->room_id )
+      })->then( sub {
+         matrix_join_room( $user, $room->room_id,
+            server_name => $inbound_server->server_name,
+         )
+      })->then( sub {
+         Future->needs_all(
+            $inbound_server->await_request_user_devices( $federated_user_id )
+            ->then( sub {
+               my ( $req, undef ) = @_;
+
+               assert_eq( $req->method, "GET", 'request method' );
+
+               $req->respond_json( {
+                  user_id   => $federated_user_id,
+                  stream_id => 1,
+                  devices   => [
+                     {
+                        device_id => $device_id1,
+                        keys      => { device_keys => {} },
+                     },
+                     {
+                        device_id => $device_id2,
+                        keys      => { device_keys => {} },
+                     },
+                  ],
+               } );
+               Future->done(1);
+            }),
+            do_request_json_for( $user,
+               method  => "POST",
+               uri     => "/r0/keys/query",
+               content => {
+                  device_keys => {
+                     $federated_user_id => [],
+                  },
+               },
+            ),
+         )
+      })->then( sub {
+         my ( $first, $content ) = @_;
+
+         log_if_fail "second query response", $content;
+
+         assert_json_keys( $content, "device_keys" );
+
+         my $device_keys = $content->{device_keys};
+         assert_json_keys( $device_keys, $federated_user_id );
+
+         my $alice_keys = $device_keys->{ $federated_user_id };
+         assert_json_keys( $alice_keys, ( $device_id1, $device_id2 ) );
+
+         Future->done( 1 )
+      });
+   };
+
 test "Local device key changes get to remote servers with correct prev_id",
    requires => [ local_user_fixtures( 2 ), $main::INBOUND_SERVER, federation_user_id_fixture(), room_alias_name_fixture() ],
 

diff --git a/tests/50federation/50no-deextrem-outliers.pl b/tests/50federation/50no-deextrem-outliers.pl
@@ -51,7 +51,6 @@
          ->on_done( sub {
             my ( $pl_event_b ) = @_;
             $pl_event_b_id = $room->id_for_event( $pl_event_b );
-            $room->insert_event( $pl_event_b );
          }),
       )->then( sub {
          my %state_before_c = %{ $room->{current_state} };