From 2a98ba0ed31bdd51ea43c0867bee2a5256f2a289 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 8 Nov 2017 10:35:30 +0000 Subject: [PATCH 1/5] Rename redact_content option to include_content The redact_content option never worked because it read the wrong config section. The PR introducing it (https://github.com/matrix-org/synapse/pull/2301) had feedback suggesting the name be changed to not re-use the term 'redact' but this wasn't incorporated. This reanmes the option to give it a less confusing name, and also means that people who've set the redact_content option won't suddenly see a behaviour change when upgrading synapse, but instead can set include_content if they want to. This PR also updates the wording of the config comment to clarify that this has no effect on event_id_only push. Includes https://github.com/matrix-org/synapse/pull/2422 --- synapse/config/push.py | 28 +++++++++++++--------------- synapse/push/httppusher.py | 3 ++- 2 files changed, 15 insertions(+), 16 deletions(-) diff --git a/synapse/config/push.py b/synapse/config/push.py index 9c68318b40..01d4a49784 100644 --- a/synapse/config/push.py +++ b/synapse/config/push.py @@ -1,5 +1,6 @@ # -*- coding: utf-8 -*- # Copyright 2015, 2016 OpenMarket Ltd +# Copyright 2017 New Vector Ltd # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -18,28 +19,25 @@ from ._base import Config class PushConfig(Config): def read_config(self, config): - self.push_redact_content = False + self.push_include_content = True - push_config = config.get("email", {}) - self.push_redact_content = push_config.get("redact_content", False) + push_config = config.get("push", {}) + self.push_include_content = push_config.get("include_content", True) def default_config(self, config_dir_path, server_name, **kwargs): return """ - # Control how push messages are sent to google/apple to notifications. - # Normally every message said in a room with one or more people using - # mobile devices will be posted to a push server hosted by matrix.org - # which is registered with google and apple in order to allow push - # notifications to be sent to these mobile devices. - # - # Setting redact_content to true will make the push messages contain no - # message content which will provide increased privacy. This is a - # temporary solution pending improvements to Android and iPhone apps - # to get content from the app rather than the notification. - # + # Clients requesting push notifications can either have the body of + # the message sent in the notification poke along with other details + # like the sender, or just the event ID and room ID (`event_id_only`). + # If clients choose the former, this option controls whether the + # notification request includes the content of the event (other details + # like the sender are still included). For `event_id_only` push, it + # has no effect. + # For modern android devices the notification content will still appear # because it is loaded by the app. iPhone, however will send a # notification saying only that a message arrived and who it came from. # #push: - # redact_content: false + # include_content: false """ diff --git a/synapse/push/httppusher.py b/synapse/push/httppusher.py index 74c0bc462c..c16f61452c 100644 --- a/synapse/push/httppusher.py +++ b/synapse/push/httppusher.py @@ -1,5 +1,6 @@ # -*- coding: utf-8 -*- # Copyright 2015, 2016 OpenMarket Ltd +# Copyright 2017 New Vector Ltd # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -295,7 +296,7 @@ class HttpPusher(object): if event.type == 'm.room.member': d['notification']['membership'] = event.content['membership'] d['notification']['user_is_target'] = event.state_key == self.user_id - if not self.hs.config.push_redact_content and 'content' in event: + if self.hs.config.push_include_content and 'content' in event: d['notification']['content'] = event.content # We no longer send aliases separately, instead, we send the human From 1b870937ae2de0ba510f0e1db40ae0e9a316d83f Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 8 Nov 2017 11:46:24 +0000 Subject: [PATCH 2/5] Log if any of the old config flags are set --- synapse/config/push.py | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/synapse/config/push.py b/synapse/config/push.py index 01d4a49784..861f5f31a7 100644 --- a/synapse/config/push.py +++ b/synapse/config/push.py @@ -16,14 +16,36 @@ from ._base import Config +import logging + +from twisted.internet import reactor + + +logger = logging.getLogger(__name__) + class PushConfig(Config): def read_config(self, config): - self.push_include_content = True - push_config = config.get("push", {}) self.push_include_content = push_config.get("include_content", True) + if push_config.get("redact_content") is not None: + reactor.callWhenRunning(lambda: logger.warn( + "The push.redact_content content option has never worked. " + "Please set push.include_content if you want this behaviour" + )) + + # There was a a 'redact_content' setting but mistakenly read from the + # 'email' section: check for it and honour it, with a warning. + push_config = config.get("email", {}) + redact_content = push_config.get("redact_content") + if redact_content is not None: + reactor.callWhenRunning(lambda: logger.warn( + "The 'email.redact_content' option is deprecated: " + "please set push.include_content instead" + )) + self.push_include_content = not redact_content + def default_config(self, config_dir_path, server_name, **kwargs): return """ # Clients requesting push notifications can either have the body of From ad408beb663052bc5700015db4716583f40a4536 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 8 Nov 2017 11:50:08 +0000 Subject: [PATCH 3/5] better comments --- synapse/config/push.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/synapse/config/push.py b/synapse/config/push.py index 861f5f31a7..bbfeb05d50 100644 --- a/synapse/config/push.py +++ b/synapse/config/push.py @@ -29,14 +29,17 @@ class PushConfig(Config): push_config = config.get("push", {}) self.push_include_content = push_config.get("include_content", True) + # There was a a 'redact_content' setting but mistakenly read from the + # 'email'section'. Check for the flag in the 'push' section, and log, + # but do not honour it to avoid nasty surprises when people upgrade. if push_config.get("redact_content") is not None: reactor.callWhenRunning(lambda: logger.warn( "The push.redact_content content option has never worked. " "Please set push.include_content if you want this behaviour" )) - # There was a a 'redact_content' setting but mistakenly read from the - # 'email' section: check for it and honour it, with a warning. + # Now check for the one in the 'email' section and honour it, + # with a warning. push_config = config.get("email", {}) redact_content = push_config.get("redact_content") if redact_content is not None: From b2a788e902c6fb6d3c516177fbb9f7e201e5cf0e Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 9 Nov 2017 10:11:42 +0000 Subject: [PATCH 4/5] Make the commented config have the default --- synapse/config/push.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/config/push.py b/synapse/config/push.py index bbfeb05d50..8fc1b98eba 100644 --- a/synapse/config/push.py +++ b/synapse/config/push.py @@ -64,5 +64,5 @@ class PushConfig(Config): # notification saying only that a message arrived and who it came from. # #push: - # include_content: false + # include_content: true """ From 45ab288e072341447cc6375f37df8bace3d1c525 Mon Sep 17 00:00:00 2001 From: David Baker Date: Mon, 13 Nov 2017 18:32:08 +0000 Subject: [PATCH 5/5] Print instead of logging because we had to wait until the logger was set up --- synapse/config/push.py | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/synapse/config/push.py b/synapse/config/push.py index 8fc1b98eba..b7e0d46afa 100644 --- a/synapse/config/push.py +++ b/synapse/config/push.py @@ -16,13 +16,6 @@ from ._base import Config -import logging - -from twisted.internet import reactor - - -logger = logging.getLogger(__name__) - class PushConfig(Config): def read_config(self, config): @@ -33,20 +26,20 @@ class PushConfig(Config): # 'email'section'. Check for the flag in the 'push' section, and log, # but do not honour it to avoid nasty surprises when people upgrade. if push_config.get("redact_content") is not None: - reactor.callWhenRunning(lambda: logger.warn( + print( "The push.redact_content content option has never worked. " "Please set push.include_content if you want this behaviour" - )) + ) # Now check for the one in the 'email' section and honour it, # with a warning. push_config = config.get("email", {}) redact_content = push_config.get("redact_content") if redact_content is not None: - reactor.callWhenRunning(lambda: logger.warn( + print( "The 'email.redact_content' option is deprecated: " "please set push.include_content instead" - )) + ) self.push_include_content = not redact_content def default_config(self, config_dir_path, server_name, **kwargs):