From 5fa239d6edd6a50aaab7d608178f54ee77fbe5e2 Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Tue, 5 Sep 2023 13:15:08 +0100 Subject: [PATCH 1/7] Drop single filter for user_id --- synapse/storage/databases/main/deviceinbox.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/storage/databases/main/deviceinbox.py b/synapse/storage/databases/main/deviceinbox.py index b471fcb064a2..271cdf923cf5 100644 --- a/synapse/storage/databases/main/deviceinbox.py +++ b/synapse/storage/databases/main/deviceinbox.py @@ -349,7 +349,7 @@ def get_device_messages_txn( table="devices", column="user_id", iterable=user_ids_to_query, - keyvalues={"user_id": user_id, "hidden": False}, + keyvalues={"hidden": False}, retcols=("device_id",), ) From 867c9b0bf348c1ba9d0e28d447d6e7a01d8e8e81 Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Tue, 5 Sep 2023 13:19:17 +0100 Subject: [PATCH 2/7] changelog --- changelog.d/16251.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/16251.bugfix diff --git a/changelog.d/16251.bugfix b/changelog.d/16251.bugfix new file mode 100644 index 000000000000..d52ef5584c3c --- /dev/null +++ b/changelog.d/16251.bugfix @@ -0,0 +1 @@ +Fix a longstanding bug where appservices using MSC2049 to receive to_device messages, would only get messages for one user. \ No newline at end of file From 4ba58ab5dc0b6c7f566bad3d760c07c8c74a9ec6 Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Tue, 5 Sep 2023 14:04:25 +0100 Subject: [PATCH 3/7] fix changelog --- changelog.d/16251.bugfix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/16251.bugfix b/changelog.d/16251.bugfix index d52ef5584c3c..6d3157c7aa31 100644 --- a/changelog.d/16251.bugfix +++ b/changelog.d/16251.bugfix @@ -1 +1 @@ -Fix a longstanding bug where appservices using MSC2049 to receive to_device messages, would only get messages for one user. \ No newline at end of file +Fix a long-standing bug where appservices using MSC2409 to receive to_device messages, would only get messages for one user. \ No newline at end of file From 08ab344cf65b2dd400cd3f71f07314edfe929666 Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Tue, 5 Sep 2023 16:38:30 +0100 Subject: [PATCH 4/7] Add test_application_services_receive_local_to_device_for_many_users --- tests/handlers/test_appservice.py | 131 ++++++++++++++++++++++++++++++ 1 file changed, 131 insertions(+) diff --git a/tests/handlers/test_appservice.py b/tests/handlers/test_appservice.py index 46d022092e82..c1916f17fd0c 100644 --- a/tests/handlers/test_appservice.py +++ b/tests/handlers/test_appservice.py @@ -422,6 +422,18 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: "exclusive_as_user", "password", self.exclusive_as_user_device_id ) + self.exclusive_as_user_2_device_id = "exclusive_as_device_2" + self.exclusive_as_user_2 = self.register_user("exclusive_as_user_2", "password") + self.exclusive_as_user_2_token = self.login( + "exclusive_as_user_2", "password", self.exclusive_as_user_2_device_id + ) + + self.exclusive_as_user_3_device_id = "exclusive_as_device_3" + self.exclusive_as_user_3 = self.register_user("exclusive_as_user_3", "password") + self.exclusive_as_user_3_token = self.login( + "exclusive_as_user_3", "password", self.exclusive_as_user_3_device_id + ) + def _notify_interested_services(self) -> None: # This is normally set in `notify_interested_services` but we need to call the # internal async version so the reactor gets pushed to completion. @@ -849,6 +861,125 @@ def test_application_services_receive_bursts_of_to_device(self) -> None: for count in service_id_to_message_count.values(): self.assertEqual(count, number_of_messages) + @unittest.override_config( + {"experimental_features": {"msc2409_to_device_messages_enabled": True}} + ) + def test_application_services_receive_local_to_device_for_many_users(self) -> None: + """ + Test that when a user sends a to-device message to another user + that is an application service's user namespace, the + application service will receive it. + """ + interested_appservice = self._register_application_service( + namespaces={ + ApplicationService.NS_USERS: [ + { + "regex": "@exclusive_as_user:.+", + "exclusive": True, + }, + { + "regex": "@exclusive_as_user_2:.+", + "exclusive": True, + }, + { + "regex": "@exclusive_as_user_3:.+", + "exclusive": True, + }, + ], + }, + ) + + # Have local_user send a to-device message to exclusive_as_users + message_content = {"some_key": "some really interesting value"} + chan = self.make_request( + "PUT", + "/_matrix/client/r0/sendToDevice/m.room_key_request/3", + content={ + "messages": { + self.exclusive_as_user: { + self.exclusive_as_user_device_id: message_content + }, + self.exclusive_as_user_2: { + self.exclusive_as_user_2_device_id: message_content + }, + self.exclusive_as_user_3: { + self.exclusive_as_user_3_device_id: message_content + }, + } + }, + access_token=self.local_user_token, + ) + self.assertEqual(chan.code, 200, chan.result) + + # Have exclusive_as_user send a to-device message to local_user + for user_token in [ + self.exclusive_as_user_token, + self.exclusive_as_user_2_token, + self.exclusive_as_user_3_token, + ]: + chan = self.make_request( + "PUT", + "/_matrix/client/r0/sendToDevice/m.room_key_request/4", + content={ + "messages": { + self.local_user: {self.local_user_device_id: message_content} + } + }, + access_token=user_token, + ) + self.assertEqual(chan.code, 200, chan.result) + + # Check if our application service - that is interested in exclusive_as_user - received + # the to-device message as part of an AS transaction. + # Only the local_user -> exclusive_as_user to-device message should have been forwarded to the AS. + # + # The uninterested application service should not have been notified at all. + self.send_mock.assert_called_once() + ( + service, + _events, + _ephemeral, + to_device_messages, + _otks, + _fbks, + _device_list_summary, + ) = self.send_mock.call_args[0] + + # Assert that this was the same to-device message that local_user sent + self.assertEqual(service, interested_appservice) + + # Assert expected number of messages + self.assertEqual(len(to_device_messages), 3) + + device_msg_1 = { + "msg": to_device_messages[0], + "expected_user": self.exclusive_as_user, + "expected_device": self.exclusive_as_user_device_id, + } + + device_msg_2 = { + "msg": to_device_messages[1], + "expected_user": self.exclusive_as_user_2, + "expected_device": self.exclusive_as_user_2_device_id, + } + + device_msg_3 = { + "msg": to_device_messages[2], + "expected_user": self.exclusive_as_user_3, + "expected_device": self.exclusive_as_user_3_device_id, + } + + for device_msg in [device_msg_1, device_msg_2, device_msg_3]: + self.assertEqual(device_msg["msg"]["type"], "m.room_key_request") + self.assertEqual(device_msg["msg"]["sender"], self.local_user) + self.assertEqual( + device_msg["msg"]["to_user_id"], device_msg["expected_user"] + ) + self.assertEqual( + device_msg["msg"]["to_device_id"], device_msg["expected_device"] + ) + self.assertEqual(device_msg["msg"]["content"], message_content) + def _register_application_service( self, namespaces: Optional[Dict[str, Iterable[Dict]]] = None, From b408a22dbd58ec143a184b406694605fce95acb1 Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Tue, 5 Sep 2023 16:39:25 +0100 Subject: [PATCH 5/7] Fix docs --- tests/handlers/test_appservice.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/handlers/test_appservice.py b/tests/handlers/test_appservice.py index c1916f17fd0c..41fce0ab9654 100644 --- a/tests/handlers/test_appservice.py +++ b/tests/handlers/test_appservice.py @@ -866,9 +866,9 @@ def test_application_services_receive_bursts_of_to_device(self) -> None: ) def test_application_services_receive_local_to_device_for_many_users(self) -> None: """ - Test that when a user sends a to-device message to another user - that is an application service's user namespace, the - application service will receive it. + Test that when a user sends a to-device message to many users + in an application service's user namespace, the + application service will receive all of them. """ interested_appservice = self._register_application_service( namespaces={ From fbefeba3eee783a2b25fbeea2c977a765ac14de5 Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Tue, 5 Sep 2023 18:36:51 +0100 Subject: [PATCH 6/7] Update tests/handlers/test_appservice.py Co-authored-by: Patrick Cloke --- tests/handlers/test_appservice.py | 47 +++++++++++++++---------------- 1 file changed, 22 insertions(+), 25 deletions(-) diff --git a/tests/handlers/test_appservice.py b/tests/handlers/test_appservice.py index 41fce0ab9654..610e083e5701 100644 --- a/tests/handlers/test_appservice.py +++ b/tests/handlers/test_appservice.py @@ -951,34 +951,31 @@ def test_application_services_receive_local_to_device_for_many_users(self) -> No # Assert expected number of messages self.assertEqual(len(to_device_messages), 3) - device_msg_1 = { - "msg": to_device_messages[0], - "expected_user": self.exclusive_as_user, - "expected_device": self.exclusive_as_user_device_id, - } + for device_msg in to_device_messages: + self.assertEqual(device_msg["type"], "m.room_key_request") + self.assertEqual(device_msg["sender"], self.local_user) + self.assertEqual(device_msg["content"], message_content) - device_msg_2 = { - "msg": to_device_messages[1], - "expected_user": self.exclusive_as_user_2, - "expected_device": self.exclusive_as_user_2_device_id, - } + self.assertEqual( + to_device_messages[0]["to_user_id"], self.exclusive_as_user + ) + self.assertEqual( + to_device_messages[0]["to_device_id"], self.exclusive_as_user_device_id, + ) - device_msg_3 = { - "msg": to_device_messages[2], - "expected_user": self.exclusive_as_user_3, - "expected_device": self.exclusive_as_user_3_device_id, - } + self.assertEqual( + to_device_messages[1]["to_user_id"], self.exclusive_as_user_2 + ) + self.assertEqual( + to_device_messages[1]["to_device_id"], self.exclusive_as_user_2_device_id, + ) - for device_msg in [device_msg_1, device_msg_2, device_msg_3]: - self.assertEqual(device_msg["msg"]["type"], "m.room_key_request") - self.assertEqual(device_msg["msg"]["sender"], self.local_user) - self.assertEqual( - device_msg["msg"]["to_user_id"], device_msg["expected_user"] - ) - self.assertEqual( - device_msg["msg"]["to_device_id"], device_msg["expected_device"] - ) - self.assertEqual(device_msg["msg"]["content"], message_content) + self.assertEqual( + to_device_messages[2]["to_user_id"], self.exclusive_as_user_3 + ) + self.assertEqual( + to_device_messages[2]["to_device_id"], self.exclusive_as_user_3_device_id, + ) def _register_application_service( self, From 6f1865755ade66aa4d64adda7dad1e15f082ab76 Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Tue, 5 Sep 2023 19:45:51 +0100 Subject: [PATCH 7/7] Lint --- tests/handlers/test_appservice.py | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/tests/handlers/test_appservice.py b/tests/handlers/test_appservice.py index 610e083e5701..a7e6cdd66a35 100644 --- a/tests/handlers/test_appservice.py +++ b/tests/handlers/test_appservice.py @@ -956,25 +956,22 @@ def test_application_services_receive_local_to_device_for_many_users(self) -> No self.assertEqual(device_msg["sender"], self.local_user) self.assertEqual(device_msg["content"], message_content) + self.assertEqual(to_device_messages[0]["to_user_id"], self.exclusive_as_user) self.assertEqual( - to_device_messages[0]["to_user_id"], self.exclusive_as_user - ) - self.assertEqual( - to_device_messages[0]["to_device_id"], self.exclusive_as_user_device_id, + to_device_messages[0]["to_device_id"], + self.exclusive_as_user_device_id, ) + self.assertEqual(to_device_messages[1]["to_user_id"], self.exclusive_as_user_2) self.assertEqual( - to_device_messages[1]["to_user_id"], self.exclusive_as_user_2 - ) - self.assertEqual( - to_device_messages[1]["to_device_id"], self.exclusive_as_user_2_device_id, + to_device_messages[1]["to_device_id"], + self.exclusive_as_user_2_device_id, ) + self.assertEqual(to_device_messages[2]["to_user_id"], self.exclusive_as_user_3) self.assertEqual( - to_device_messages[2]["to_user_id"], self.exclusive_as_user_3 - ) - self.assertEqual( - to_device_messages[2]["to_device_id"], self.exclusive_as_user_3_device_id, + to_device_messages[2]["to_device_id"], + self.exclusive_as_user_3_device_id, ) def _register_application_service(