From cddfa82dc2842b7e168b4b4f1cad74370e401085 Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Mon, 22 Jun 2020 14:39:51 +0100 Subject: [PATCH 1/8] Remove obsolete comment about ancient temporary code Signed-off-by: Olivier Wilkinson (reivilibre) --- synapse/push/httppusher.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/synapse/push/httppusher.py b/synapse/push/httppusher.py index ed60dbc1bf99..a5357b0096a2 100644 --- a/synapse/push/httppusher.py +++ b/synapse/push/httppusher.py @@ -334,9 +334,7 @@ def _build_notification_dict(self, event, tweaks, badge): "room_id": event.room_id, "type": event.type, "sender": event.user_id, - "counts": { # -- we don't mark messages as read yet so - # we have no way of knowing - # Just set the badge to 1 until we have read receipts + "counts": { "unread": badge, # 'missed_calls': 2 }, From a2b3b8875cb79eb946b9d04becf7d3ac4557000d Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Wed, 1 Jul 2020 09:24:46 +0100 Subject: [PATCH 2/8] Implement hack to set push priority based on whether the tweaks indicate the event might cause effects. --- synapse/push/httppusher.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/synapse/push/httppusher.py b/synapse/push/httppusher.py index a5357b0096a2..af3d545b38a3 100644 --- a/synapse/push/httppusher.py +++ b/synapse/push/httppusher.py @@ -25,6 +25,7 @@ from synapse.push import PusherConfigException from . import push_rule_evaluator, push_tools +from ..api.constants import EventTypes logger = logging.getLogger(__name__) @@ -305,12 +306,23 @@ def _process_one(self, push_action): @defer.inlineCallbacks def _build_notification_dict(self, event, tweaks, badge): + priority = "low" + if ( + event.type == EventTypes.Encrypted + or tweaks.get("highlight") + or tweaks.get("sound") + ): + # HACK send our push as high priority only if it generates a sound, highlight + # or may do so (i.e. is encrypted so has unknown effects). + priority = "high" + if self.data.get("format") == "event_id_only": d = { "notification": { "event_id": event.event_id, "room_id": event.room_id, "counts": {"unread": badge}, + "prio": priority, "devices": [ { "app_id": self.app_id, @@ -334,6 +346,7 @@ def _build_notification_dict(self, event, tweaks, badge): "room_id": event.room_id, "type": event.type, "sender": event.user_id, + "prio": priority, "counts": { "unread": badge, # 'missed_calls': 2 From 09656919927e961a67ff45e6c4a03638897e2cbb Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Wed, 1 Jul 2020 09:34:22 +0100 Subject: [PATCH 3/8] Changelog for 7765 Signed-off-by: Olivier Wilkinson (reivilibre) --- changelog.d/7765.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/7765.misc diff --git a/changelog.d/7765.misc b/changelog.d/7765.misc new file mode 100644 index 000000000000..fa9cfd24cbdc --- /dev/null +++ b/changelog.d/7765.misc @@ -0,0 +1 @@ +Send push notifications with a high or low priority depending upon whether they may generate user-observable effects. From 72cced24ec540fdabcee8c2a96ea7ca6fdb69ba0 Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Wed, 1 Jul 2020 09:45:36 +0100 Subject: [PATCH 4/8] Antilint --- synapse/push/httppusher.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/push/httppusher.py b/synapse/push/httppusher.py index af3d545b38a3..b87782e11ce8 100644 --- a/synapse/push/httppusher.py +++ b/synapse/push/httppusher.py @@ -24,8 +24,8 @@ from synapse.metrics.background_process_metrics import run_as_background_process from synapse.push import PusherConfigException -from . import push_rule_evaluator, push_tools from ..api.constants import EventTypes +from . import push_rule_evaluator, push_tools logger = logging.getLogger(__name__) From 62f3373f0f9921e95059519e98fc3c98d754a3ba Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Wed, 1 Jul 2020 14:36:36 +0100 Subject: [PATCH 5/8] Add tests for push priority Signed-off-by: Olivier Wilkinson (reivilibre) --- tests/push/test_http.py | 382 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 380 insertions(+), 2 deletions(-) diff --git a/tests/push/test_http.py b/tests/push/test_http.py index baf9c785f48c..e8ff970b5c2d 100644 --- a/tests/push/test_http.py +++ b/tests/push/test_http.py @@ -25,7 +25,6 @@ class HTTPPusherTests(HomeserverTestCase): - servlets = [ synapse.rest.admin.register_servlets_for_client_rest_resource, room.register_servlets, @@ -35,7 +34,6 @@ class HTTPPusherTests(HomeserverTestCase): hijack_auth = False def make_homeserver(self, reactor, clock): - self.push_attempts = [] m = Mock() @@ -157,3 +155,383 @@ def test_sends_http(self): pushers = list(pushers) self.assertEqual(len(pushers), 1) self.assertTrue(pushers[0]["last_stream_ordering"] > last_stream_ordering) + + def test_sends_high_priority_for_encrypted(self): + """ + The HTTP pusher will send pushes at high priority if they correspond + to an encrypted message. + This will happen both in 1:1 rooms and larger rooms. + """ + # Register the user who gets notified + user_id = self.register_user("user", "pass") + access_token = self.login("user", "pass") + + # Register the user who sends the message + other_user_id = self.register_user("otheruser", "pass") + other_access_token = self.login("otheruser", "pass") + + # Register a third user + yet_another_user_id = self.register_user("yetanotheruser", "pass") + yet_another_access_token = self.login("yetanotheruser", "pass") + + # Create a room + room = self.helper.create_room_as(user_id, tok=access_token) + + # Invite the other person + self.helper.invite(room=room, src=user_id, tok=access_token, targ=other_user_id) + + # The other user joins + self.helper.join(room=room, user=other_user_id, tok=other_access_token) + + # Register the pusher + user_tuple = self.get_success( + self.hs.get_datastore().get_user_by_access_token(access_token) + ) + token_id = user_tuple["token_id"] + + self.get_success( + self.hs.get_pusherpool().add_pusher( + user_id=user_id, + access_token=token_id, + kind="http", + app_id="m.http", + app_display_name="HTTP Push Notifications", + device_display_name="pushy push", + pushkey="a@example.com", + lang=None, + data={"url": "example.com"}, + ) + ) + + # Send an encrypted event + # I know there'd normally be set-up of an encrypted room first + # but this will do for our purposes + self.helper.send_event( + room, + "m.room.encrypted", + content={ + "algorithm": "m.megolm.v1.aes-sha2", + "sender_key": "6lImKbzK51MzWLwHh8tUM3UBBSBrLlgup/OOCGTvumM", + "ciphertext": "AwgAErABoRxwpMipdgiwXgu46rHiWQ0DmRj0qUlPrMraBUDk" + "leTnJRljpuc7IOhsYbLY3uo2WI0ab/ob41sV+3JEIhODJPqH" + "TK7cEZaIL+/up9e+dT9VGF5kRTWinzjkeqO8FU5kfdRjm+3w" + "0sy3o1OCpXXCfO+faPhbV/0HuK4ndx1G+myNfK1Nk/CxfMcT" + "BT+zDS/Df/QePAHVbrr9uuGB7fW8ogW/ulnydgZPRluusFGv" + "J3+cg9LoPpZPAmv5Me3ec7NtdlfN0oDZ0gk3TiNkkhsxDG9Y" + "YcNzl78USI0q8+kOV26Bu5dOBpU4WOuojXZHJlP5lMgdzLLl" + "EQ0", + "session_id": "IigqfNWLL+ez/Is+Duwp2s4HuCZhFG9b9CZKTYHtQ4A", + "device_id": "AHQDUSTAAA", + }, + tok=other_access_token, + ) + + # Advance time a bit, so the pusher will register something has happened + self.pump() + + # Make the push succeed + self.push_attempts[0][0].callback({}) + self.pump() + + # Check our push made it with high priority + self.assertEqual(len(self.push_attempts), 1) + self.assertEqual(self.push_attempts[0][1], "example.com") + self.assertEqual(self.push_attempts[0][2]["notification"]["prio"], "high") + + # Invite yet another person — we want to make this room not a 1:1 + # (as encrypted messages in a 1:1 currently have tweaks applied + # so it doesn't properly exercise the condition of all encrypted + # messages need to be high). + self.helper.invite( + room=room, src=user_id, tok=access_token, targ=yet_another_user_id + ) + + # Yet another user joins + self.helper.join( + room=room, user=yet_another_user_id, tok=yet_another_access_token + ) + + # Check no push notifications are sent regarding the membership changes + # (that would confuse the test) + self.pump() + self.assertEqual(len(self.push_attempts), 1) + + # Send another encrypted event + self.helper.send_event( + room, + "m.room.encrypted", + content={ + "ciphertext": "AwgAEoABtEuic/2DF6oIpNH+q/PonzlhXOVho8dTv0tzFr5m" + "9vTo50yabx3nxsRlP2WxSqa8I07YftP+EKWCWJvTkg6o7zXq" + "6CK+GVvLQOVgK50SfvjHqJXN+z1VEqj+5mkZVN/cAgJzoxcH" + "zFHkwDPJC8kQs47IHd8EO9KBUK4v6+NQ1uE/BIak4qAf9aS/" + "kI+f0gjn9IY9K6LXlah82A/iRyrIrxkCkE/n0VfvLhaWFecC" + "sAWTcMLoF6fh1Jpke95mljbmFSpsSd/eEQw", + "device_id": "SRCFTWTHXO", + "session_id": "eMA+bhGczuTz1C5cJR1YbmrnnC6Goni4lbvS5vJ1nG4", + "algorithm": "m.megolm.v1.aes-sha2", + "sender_key": "rC/XSIAiYrVGSuaHMop8/pTZbku4sQKBZwRwukgnN1c", + }, + tok=other_access_token, + ) + + # Advance time a bit, so the pusher will register something has happened + self.pump() + self.assertEqual(len(self.push_attempts), 2) + self.assertEqual(self.push_attempts[1][1], "example.com") + self.assertEqual(self.push_attempts[1][2]["notification"]["prio"], "high") + + def test_sends_high_priority_for_one_to_one_only(self): + """ + The HTTP pusher will send pushes at high priority if they correspond + to a message in a one-to-one room. + """ + # Register the user who gets notified + user_id = self.register_user("user", "pass") + access_token = self.login("user", "pass") + + # Register the user who sends the message + other_user_id = self.register_user("otheruser", "pass") + other_access_token = self.login("otheruser", "pass") + + # Register a third user + yet_another_user_id = self.register_user("yetanotheruser", "pass") + yet_another_access_token = self.login("yetanotheruser", "pass") + + # Create a room + room = self.helper.create_room_as(user_id, tok=access_token) + + # Invite the other person + self.helper.invite(room=room, src=user_id, tok=access_token, targ=other_user_id) + + # The other user joins + self.helper.join(room=room, user=other_user_id, tok=other_access_token) + + # Register the pusher + user_tuple = self.get_success( + self.hs.get_datastore().get_user_by_access_token(access_token) + ) + token_id = user_tuple["token_id"] + + self.get_success( + self.hs.get_pusherpool().add_pusher( + user_id=user_id, + access_token=token_id, + kind="http", + app_id="m.http", + app_display_name="HTTP Push Notifications", + device_display_name="pushy push", + pushkey="a@example.com", + lang=None, + data={"url": "example.com"}, + ) + ) + + # Send a message + self.helper.send(room, body="Hi!", tok=other_access_token) + + # Advance time a bit, so the pusher will register something has happened + self.pump() + + # Make the push succeed + self.push_attempts[0][0].callback({}) + self.pump() + + # Check our push made it with high priority — this is a one-to-one room + self.assertEqual(len(self.push_attempts), 1) + self.assertEqual(self.push_attempts[0][1], "example.com") + self.assertEqual(self.push_attempts[0][2]["notification"]["prio"], "high") + + # Invite yet another person — we want to make this room not a 1:1. + self.helper.invite( + room=room, src=user_id, tok=access_token, targ=yet_another_user_id + ) + + # Yet another user joins + self.helper.join( + room=room, user=yet_another_user_id, tok=yet_another_access_token + ) + + # Check no push notifications are sent regarding the membership changes + # (that would confuse the test) + self.pump() + self.assertEqual(len(self.push_attempts), 1) + + # Send another event + self.helper.send(room, body="Welcome!", tok=other_access_token) + + # Advance time a bit, so the pusher will register something has happened + self.pump() + self.assertEqual(len(self.push_attempts), 2) + self.assertEqual(self.push_attempts[1][1], "example.com") + + # check that this is low-priority + self.assertEqual(self.push_attempts[1][2]["notification"]["prio"], "low") + + def test_sends_high_priority_for_mention(self): + """ + The HTTP pusher will send pushes at high priority if they correspond + to a message containing the user's display name. + """ + # Register the user who gets notified + user_id = self.register_user("user", "pass") + access_token = self.login("user", "pass") + + # Register the user who sends the message + other_user_id = self.register_user("otheruser", "pass") + other_access_token = self.login("otheruser", "pass") + + # Register a third user + yet_another_user_id = self.register_user("yetanotheruser", "pass") + yet_another_access_token = self.login("yetanotheruser", "pass") + + # Create a room + room = self.helper.create_room_as(user_id, tok=access_token) + + # Invite the other people + self.helper.invite(room=room, src=user_id, tok=access_token, targ=other_user_id) + self.helper.invite( + room=room, src=user_id, tok=access_token, targ=yet_another_user_id + ) + + # The other users join + self.helper.join(room=room, user=other_user_id, tok=other_access_token) + self.helper.join( + room=room, user=yet_another_user_id, tok=yet_another_access_token + ) + + # Register the pusher + user_tuple = self.get_success( + self.hs.get_datastore().get_user_by_access_token(access_token) + ) + token_id = user_tuple["token_id"] + + self.get_success( + self.hs.get_pusherpool().add_pusher( + user_id=user_id, + access_token=token_id, + kind="http", + app_id="m.http", + app_display_name="HTTP Push Notifications", + device_display_name="pushy push", + pushkey="a@example.com", + lang=None, + data={"url": "example.com"}, + ) + ) + + # Send a message + self.helper.send(room, body="Oh, user, hello!", tok=other_access_token) + + # Advance time a bit, so the pusher will register something has happened + self.pump() + + # Make the push succeed + self.push_attempts[0][0].callback({}) + self.pump() + + # Check our push made it with high priority + self.assertEqual(len(self.push_attempts), 1) + self.assertEqual(self.push_attempts[0][1], "example.com") + self.assertEqual(self.push_attempts[0][2]["notification"]["prio"], "high") + + # Send another event, this time with no mention + self.helper.send(room, body="Are you there?", tok=other_access_token) + + # Advance time a bit, so the pusher will register something has happened + self.pump() + self.assertEqual(len(self.push_attempts), 2) + self.assertEqual(self.push_attempts[1][1], "example.com") + + # check that this is low-priority + self.assertEqual(self.push_attempts[1][2]["notification"]["prio"], "low") + + def test_sends_high_priority_for_atroom(self): + """ + The HTTP pusher will send pushes at high priority if they correspond + to a message that contains @room. + """ + # Register the user who gets notified + user_id = self.register_user("user", "pass") + access_token = self.login("user", "pass") + + # Register the user who sends the message + other_user_id = self.register_user("otheruser", "pass") + other_access_token = self.login("otheruser", "pass") + + # Register a third user + yet_another_user_id = self.register_user("yetanotheruser", "pass") + yet_another_access_token = self.login("yetanotheruser", "pass") + + # Create a room (as other_user so the power levels are compatible with + # other_user sending @room). + room = self.helper.create_room_as(other_user_id, tok=other_access_token) + + # Invite the other people + self.helper.invite( + room=room, src=other_user_id, tok=other_access_token, targ=user_id + ) + self.helper.invite( + room=room, + src=other_user_id, + tok=other_access_token, + targ=yet_another_user_id, + ) + + # The other users join + self.helper.join(room=room, user=user_id, tok=access_token) + self.helper.join( + room=room, user=yet_another_user_id, tok=yet_another_access_token + ) + + # Register the pusher + user_tuple = self.get_success( + self.hs.get_datastore().get_user_by_access_token(access_token) + ) + token_id = user_tuple["token_id"] + + self.get_success( + self.hs.get_pusherpool().add_pusher( + user_id=user_id, + access_token=token_id, + kind="http", + app_id="m.http", + app_display_name="HTTP Push Notifications", + device_display_name="pushy push", + pushkey="a@example.com", + lang=None, + data={"url": "example.com"}, + ) + ) + + # Send a message + self.helper.send( + room, + body="@room eeek! There's a spider on the table!", + tok=other_access_token, + ) + + # Advance time a bit, so the pusher will register something has happened + self.pump() + + # Make the push succeed + self.push_attempts[0][0].callback({}) + self.pump() + + # Check our push made it with high priority + self.assertEqual(len(self.push_attempts), 1) + self.assertEqual(self.push_attempts[0][1], "example.com") + self.assertEqual(self.push_attempts[0][2]["notification"]["prio"], "high") + + # Send another event, this time as someone without the power of @room + self.helper.send( + room, body="@room the spider is gone", tok=yet_another_access_token + ) + + # Advance time a bit, so the pusher will register something has happened + self.pump() + self.assertEqual(len(self.push_attempts), 2) + self.assertEqual(self.push_attempts[1][1], "example.com") + + # check that this is low-priority + self.assertEqual(self.push_attempts[1][2]["notification"]["prio"], "low") From 833e69d259b343d31267e7d9038aca5897f1de9e Mon Sep 17 00:00:00 2001 From: reivilibre <38398653+reivilibre@users.noreply.github.com> Date: Wed, 1 Jul 2020 14:38:00 +0100 Subject: [PATCH 6/8] Update synapse/push/httppusher.py Co-authored-by: Brendan Abolivier --- synapse/push/httppusher.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/push/httppusher.py b/synapse/push/httppusher.py index b87782e11ce8..f6a3e486b46d 100644 --- a/synapse/push/httppusher.py +++ b/synapse/push/httppusher.py @@ -24,7 +24,7 @@ from synapse.metrics.background_process_metrics import run_as_background_process from synapse.push import PusherConfigException -from ..api.constants import EventTypes +from synapse.api.constants import EventTypes from . import push_rule_evaluator, push_tools logger = logging.getLogger(__name__) From ad082ff4ef1866fc5006b13d4cbb57c45eafa67c Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Wed, 1 Jul 2020 14:39:37 +0100 Subject: [PATCH 7/8] Antilint --- synapse/push/httppusher.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/push/httppusher.py b/synapse/push/httppusher.py index f6a3e486b46d..2fac07593bad 100644 --- a/synapse/push/httppusher.py +++ b/synapse/push/httppusher.py @@ -20,11 +20,11 @@ from twisted.internet import defer from twisted.internet.error import AlreadyCalled, AlreadyCancelled +from synapse.api.constants import EventTypes from synapse.logging import opentracing from synapse.metrics.background_process_metrics import run_as_background_process from synapse.push import PusherConfigException -from synapse.api.constants import EventTypes from . import push_rule_evaluator, push_tools logger = logging.getLogger(__name__) From 8f1b4607ca76bac352c1f599141aec11ae93c04e Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Wed, 1 Jul 2020 16:09:58 +0100 Subject: [PATCH 8/8] Remove needless invites from tests. --- tests/push/test_http.py | 38 +------------------------------------- 1 file changed, 1 insertion(+), 37 deletions(-) diff --git a/tests/push/test_http.py b/tests/push/test_http.py index e8ff970b5c2d..b567868b02da 100644 --- a/tests/push/test_http.py +++ b/tests/push/test_http.py @@ -88,9 +88,6 @@ def test_sends_http(self): # Create a room room = self.helper.create_room_as(user_id, tok=access_token) - # Invite the other person - self.helper.invite(room=room, src=user_id, tok=access_token, targ=other_user_id) - # The other user joins self.helper.join(room=room, user=other_user_id, tok=other_access_token) @@ -177,9 +174,6 @@ def test_sends_high_priority_for_encrypted(self): # Create a room room = self.helper.create_room_as(user_id, tok=access_token) - # Invite the other person - self.helper.invite(room=room, src=user_id, tok=access_token, targ=other_user_id) - # The other user joins self.helper.join(room=room, user=other_user_id, tok=other_access_token) @@ -238,15 +232,10 @@ def test_sends_high_priority_for_encrypted(self): self.assertEqual(self.push_attempts[0][1], "example.com") self.assertEqual(self.push_attempts[0][2]["notification"]["prio"], "high") - # Invite yet another person — we want to make this room not a 1:1 + # Add yet another person — we want to make this room not a 1:1 # (as encrypted messages in a 1:1 currently have tweaks applied # so it doesn't properly exercise the condition of all encrypted # messages need to be high). - self.helper.invite( - room=room, src=user_id, tok=access_token, targ=yet_another_user_id - ) - - # Yet another user joins self.helper.join( room=room, user=yet_another_user_id, tok=yet_another_access_token ) @@ -301,9 +290,6 @@ def test_sends_high_priority_for_one_to_one_only(self): # Create a room room = self.helper.create_room_as(user_id, tok=access_token) - # Invite the other person - self.helper.invite(room=room, src=user_id, tok=access_token, targ=other_user_id) - # The other user joins self.helper.join(room=room, user=other_user_id, tok=other_access_token) @@ -342,11 +328,6 @@ def test_sends_high_priority_for_one_to_one_only(self): self.assertEqual(self.push_attempts[0][1], "example.com") self.assertEqual(self.push_attempts[0][2]["notification"]["prio"], "high") - # Invite yet another person — we want to make this room not a 1:1. - self.helper.invite( - room=room, src=user_id, tok=access_token, targ=yet_another_user_id - ) - # Yet another user joins self.helper.join( room=room, user=yet_another_user_id, tok=yet_another_access_token @@ -388,12 +369,6 @@ def test_sends_high_priority_for_mention(self): # Create a room room = self.helper.create_room_as(user_id, tok=access_token) - # Invite the other people - self.helper.invite(room=room, src=user_id, tok=access_token, targ=other_user_id) - self.helper.invite( - room=room, src=user_id, tok=access_token, targ=yet_another_user_id - ) - # The other users join self.helper.join(room=room, user=other_user_id, tok=other_access_token) self.helper.join( @@ -467,17 +442,6 @@ def test_sends_high_priority_for_atroom(self): # other_user sending @room). room = self.helper.create_room_as(other_user_id, tok=other_access_token) - # Invite the other people - self.helper.invite( - room=room, src=other_user_id, tok=other_access_token, targ=user_id - ) - self.helper.invite( - room=room, - src=other_user_id, - tok=other_access_token, - targ=yet_another_user_id, - ) - # The other users join self.helper.join(room=room, user=user_id, tok=access_token) self.helper.join(