Fix push URLs to match the spec. (#986)

matrix-org · Dec 4, 2020 · fd84283 · fd84283
1 parent 136e441
commit fd84283
Show file tree

Hide file tree

Showing 3 changed files with 51 additions and 40 deletions.
diff --git a/tests/14account/01change-password.pl b/tests/14account/01change-password.pl
@@ -160,7 +160,7 @@ sub matrix_set_password
                pushkey             => "a_push_key",
                lang                => "en",
                data                => {
-                  url => "https://dummy.url/is/dummy",
+                  url => "https://dummy.url/_matrix/push/v1/notify",
                },
             },
          );
@@ -197,7 +197,7 @@ sub matrix_set_password
             pushkey             => "a_push_key",
             lang                => "en",
             data                => {
-               url => "https://dummy.url/is/dummy",
+               url => "https://dummy.url/_matrix/push/v1/notify",
             },
          },
       )->then( sub {

diff --git a/tests/61push/01message-pushed.pl b/tests/61push/01message-pushed.pl
@@ -1,3 +1,6 @@
+# Per the specification HTTP pushers must point to the following location.
+my $PUSH_LOCATION = "/_matrix/push/v1/notify";
+
 sub matrix_set_pusher {
    my ( $user, $location ) = @_;
 
@@ -82,15 +85,15 @@ sub matrix_set_pusher {
          # message received may be due to Bob joining rather than the
          # message that Bob sent.
          matrix_set_pusher(
-            $alice, $test_server_info->client_location . "/alice_push",
+            $alice, $test_server_info->client_location . $PUSH_LOCATION,
          )->SyTest::pass_on_done( "Alice's pusher created" )
       })->then( sub {
          # Bob sends a message that should be pushed to Alice, since it is
          # in a "1:1" room with Alice
 
          Future->needs_all(
             # TODO(check that the HTTP poke is actually the poke we wanted)
-            await_http_request( "/alice_push", sub {
+            await_http_request( $PUSH_LOCATION, sub {
                my ( $request ) = @_;
                my $body = $request->body_from_json;
 
@@ -132,7 +135,7 @@ sub matrix_set_pusher {
          pass "Alice was pushed";  # Alice has gone down the stairs
 
          Future->needs_all(
-            await_http_request( "/alice_push", sub {
+            await_http_request( $PUSH_LOCATION, sub {
                my ( $request ) = @_;
                my $body = $request->body_from_json;
 
@@ -177,14 +180,14 @@ sub matrix_set_pusher {
       my $room_id;
 
       matrix_set_pusher(
-         $alice, $test_server_info->client_location . "/alice_push",
+         $alice, $test_server_info->client_location . $PUSH_LOCATION,
       )->then( sub {
          matrix_create_room( $bob, visibility => "private" );
       })->then( sub {
          ( $room_id ) = @_;
 
          Future->needs_all(
-            await_http_request( "/alice_push", sub {
+            await_http_request( $PUSH_LOCATION, sub {
                my ( $request ) = @_;
                my $body = $request->body_from_json;
 
@@ -218,7 +221,7 @@ sub matrix_set_pusher {
 
 =head2 setup_push
 
-   setup_push( $alice, $bob, $test_server_info, $loc )
+   setup_push( $alice, $bob, $test_server_info )
 
 Sets up push for $alice and creates a room with $alice and $bob. Returns a
 future with the room_id of the newly created room.
@@ -227,10 +230,10 @@ =head2 setup_push
 
 sub setup_push
 {
-   my ( $alice, $bob, $test_server_info, $loc ) = @_;
+   my ( $alice, $bob, $test_server_info ) = @_;
    my $room_id;
 
-   my $target = $test_server_info->client_location . $loc;
+   my $target = $test_server_info->client_location . $PUSH_LOCATION;
    matrix_set_pusher(
       $alice, $target,
    )->then( sub {
@@ -253,7 +256,7 @@ sub setup_push
 
       log_if_fail "Joined room $room_id; waiting for push to start working";
 
-      wait_for_pusher_to_work( $bob, $room_id, $loc );
+      wait_for_pusher_to_work( $bob, $room_id );
    })->then( sub {
       Future->done( $room_id );
    })
@@ -262,7 +265,7 @@ sub setup_push
 
 =head2 wait_for_pusher_to_work
 
-   wait_for_pusher_to_work( $sending_user, $room_id, $push_location )
+   wait_for_pusher_to_work( $sending_user, $room_id )
 
 This is mostly a helper function for setup_push, but it might also help in some
 other situations when configuring a pusher.
@@ -275,10 +278,10 @@ =head2 wait_for_pusher_to_work
 
 sub wait_for_pusher_to_work
 {
-   my ( $sending_user, $room_id, $push_loc ) = @_;
+   my ( $sending_user, $room_id ) = @_;
 
    # a future which waits for a push to arrive
-   my $push_future = await_http_request( $push_loc, sub {
+   my $push_future = await_http_request( $PUSH_LOCATION, sub {
        my ( $request ) = @_;
        my $body = $request->body_from_json;
 
@@ -305,10 +308,10 @@ sub wait_for_pusher_to_work
 
 sub check_received_push_with_name
 {
-   my ( $bob, $room_id, $loc, $room_name ) = @_;
+   my ( $bob, $room_id, $room_name ) = @_;
 
    Future->needs_all(
-      await_http_request( $loc, sub {
+      await_http_request( $PUSH_LOCATION, sub {
          my ( $request ) = @_;
          my $body = $request->body_from_json;
 
@@ -352,7 +355,7 @@ sub check_received_push_with_name
 
       my $name = "Test Name";
 
-      setup_push( $alice, $bob, $test_server_info, "/alice_push" )
+      setup_push( $alice, $bob, $test_server_info )
       ->then( sub {
          ( $room_id ) = @_;
 
@@ -361,7 +364,7 @@ sub check_received_push_with_name
             content => { name => $name },
          );
       })->then( sub {
-         check_received_push_with_name( $bob, $room_id, "/alice_push", $name )
+         check_received_push_with_name( $bob, $room_id, $name )
       });
    };
 
@@ -375,7 +378,7 @@ sub check_received_push_with_name
       my ( $alice, $bob, $room_alias, $test_server_info ) = @_;
       my $room_id;
 
-      setup_push( $alice, $bob, $test_server_info, "/alice_push" )
+      setup_push( $alice, $bob, $test_server_info )
       ->then( sub {
          ( $room_id ) = @_;
 
@@ -391,7 +394,7 @@ sub check_received_push_with_name
             content => { alias => $room_alias },
          );
       })->then( sub {
-         check_received_push_with_name( $bob, $room_id, "/alice_push", $room_alias )
+         check_received_push_with_name( $bob, $room_id, $room_alias )
       });
    };
 
@@ -407,7 +410,7 @@ sub check_received_push_with_name
 
       my $name = "Test Name";
 
-      setup_push( $alice, $bob, $test_server_info, "/alice_push" )
+      setup_push( $alice, $bob, $test_server_info )
       ->then( sub {
          ($room_id) = @_;
 
@@ -425,7 +428,7 @@ sub check_received_push_with_name
             content => { room_id => $room_id },
          )
       })->then( sub {
-         check_received_push_with_name( $bob, $room_id, "/alice_push", $name )
+         check_received_push_with_name( $bob, $room_id, $name )
       });
    };
 
@@ -443,14 +446,14 @@ sub check_received_push_with_name
       # are received by push. This is because its "impossible" to test for the
       # absence of second push without doing a third.
 
-      setup_push( $alice, $bob, $test_server_info, "/alice_push" )
+      setup_push( $alice, $bob, $test_server_info )
       ->then( sub {
          ( $room_id ) = @_;
 
          log_if_fail "room_id", $room_id;
 
          Future->needs_all(
-            await_http_request( "/alice_push", sub {
+            await_http_request( $PUSH_LOCATION, sub {
                my ( $request ) = @_;
 
                log_if_fail "Got /alice_push request";
@@ -501,7 +504,7 @@ sub check_received_push_with_name
             my $push_count = 0;  # Counts the number of pushes we've seen in this loop
 
             Future->needs_all(
-               await_http_request( "/alice_push", sub {
+               await_http_request( $PUSH_LOCATION, sub {
                   my ( $request ) = @_;
                   my $body = $request->body_from_json;
 
@@ -591,13 +594,13 @@ sub check_received_push_with_name
       log_if_fail "Regular event " . $room->id_for_event( $regular_event );
 
       matrix_set_pusher(
-         $alice, $test_server_info->client_location . "/alice_push",
+         $alice, $test_server_info->client_location . $PUSH_LOCATION,
       )->then( sub {
          # we need a second local user in the room, so that we can test if
          # alice's pusher is active.
          matrix_join_room( $bob, $room->room_id );
       })->then( sub {
-         wait_for_pusher_to_work( $bob, $room->room_id, "/alice_push" );
+         wait_for_pusher_to_work( $bob, $room->room_id );
       })->then( sub {
          Future->needs_all(
             # we send the rejected event first, and then the regular event, and
@@ -613,7 +616,7 @@ sub check_received_push_with_name
                );
             }),
 
-            await_http_request( "/alice_push" )->then( sub {
+            await_http_request( $PUSH_LOCATION )->then( sub {
                my ( $request ) = @_;
                my $body = $request->body_from_json;
                $request->respond_json( {} );

diff --git a/tests/61push/08_rejected_pushers.pl b/tests/61push/08_rejected_pushers.pl
@@ -1,3 +1,6 @@
+# Per the specification HTTP pushers must point to the following location.
+my $PUSH_LOCATION = "/_matrix/push/v1/notify";
+
 sub create_pusher
 {
    my ( $user, $app_id, $push_key, $url ) = @_;
@@ -20,17 +23,22 @@ sub create_pusher
 
 sub wait_for_push
 {
-   my ( $path, $response ) = @_;
+   my ( $pushkey, $response ) = @_;
 
-   await_http_request( $path, sub {
+   await_http_request( $PUSH_LOCATION, sub {
       my ( $request ) = @_;
       my $body = $request->body_from_json;
 
-      # Respond to all requests, even if we filter them out
-      $request->respond_json( $response // {} );
-
       return unless $body->{notification}{type};
       return unless $body->{notification}{type} eq "m.room.message";
+
+      # Ensure this is the expected pusher.
+      return unless $body->{notification}{devices};
+      return unless $body->{notification}{devices}[0]{pushkey} eq $pushkey;
+
+      # Respond to expected request.
+      $request->respond_json( $response // {} );
+
       return 1;
    });
 }
@@ -46,7 +54,7 @@ sub wait_for_push
 
       my $room_id;
 
-      my $url = $test_server_info->client_location . "/alice_push";
+      my $url = $test_server_info->client_location . $PUSH_LOCATION;
 
       matrix_create_room( $alice, visibility => "private" )->then( sub {
          ( $room_id ) = @_;
@@ -66,10 +74,10 @@ sub wait_for_push
             "m.read" => $event_id
          );
       })->then( sub {
-         create_pusher( $alice, "sytest", "key_1", "$url/1" )
+         create_pusher( $alice, "sytest", "key_1", "$url" )
             ->SyTest::pass_on_done( "Alice's pusher 1 created" );
       })->then( sub {
-         create_pusher( $alice, "sytest", "key_2", "$url/2" )
+         create_pusher( $alice, "sytest", "key_2", "$url" )
             ->SyTest::pass_on_done( "Alice's pusher 2 created" );
       })->then( sub {
          retry_until_success {
@@ -89,16 +97,16 @@ sub wait_for_push
          # It can take a while before we start receiving push on new pushers.
          retry_until_success {
             Future->needs_all(
-               wait_for_push( "/alice_push/1" ),
-               wait_for_push( "/alice_push/2" ),
+               wait_for_push( "key_1" ),
+               wait_for_push( "key_2" ),
                matrix_send_room_text_message( $bob, $room_id, body => "message" )
             )
          }->SyTest::pass_on_done( "Message 1 Pushed" );
       })->then( sub {
          # Now we go and reject a push
          Future->needs_all(
-            wait_for_push( "/alice_push/1", { rejected => [ "key_1" ] } ),
-            wait_for_push( "/alice_push/2" ),
+            wait_for_push( "key_1", { rejected => [ "key_1" ] } ),
+            wait_for_push( "key_2" ),
             matrix_send_room_text_message( $bob, $room_id, body => "message" )
          )->SyTest::pass_on_done( "Message 2 Pushed" );
       })->then( sub {