From 58534dd53dba8670af501c110aaf16a60798f1e5 Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Wed, 14 Oct 2020 14:21:24 -0400
Subject: [PATCH 1/6] Include a simple message in notification emails for
 encrypted messages.

---
 synapse/push/mailer.py           | 16 +++++++++++-----
 synapse/res/templates/notif.html | 26 ++++++++++++++++----------
 synapse/res/templates/notif.txt  |  7 +++++++
 tests/push/test_email.py         | 17 ++++++++++++++++-
 4 files changed, 50 insertions(+), 16 deletions(-)

diff --git a/synapse/push/mailer.py b/synapse/push/mailer.py
index 455a1acb46a8..155791b75495 100644
--- a/synapse/push/mailer.py
+++ b/synapse/push/mailer.py
@@ -387,8 +387,8 @@ async def get_notif_vars(self, notif, user_id, notif_event, room_state_ids):
         return ret
 
     async def get_message_vars(self, notif, event, room_state_ids):
-        if event.type != EventTypes.Message:
-            return
+        if event.type != EventTypes.Message and event.type != EventTypes.Encrypted:
+            return None
 
         sender_state_event_id = room_state_ids[("m.room.member", event.sender)]
         sender_state_event = await self.store.get_event(sender_state_event_id)
@@ -399,10 +399,8 @@ async def get_message_vars(self, notif, event, room_state_ids):
         # sender_hash % the number of default images to choose from
         sender_hash = string_ordinal_total(event.sender)
 
-        msgtype = event.content.get("msgtype")
-
         ret = {
-            "msgtype": msgtype,
+            "event_type": event.type,
             "is_historical": event.event_id != notif["event_id"],
             "id": event.event_id,
             "ts": event.origin_server_ts,
@@ -411,6 +409,14 @@ async def get_message_vars(self, notif, event, room_state_ids):
             "sender_hash": sender_hash,
         }
 
+        # Encrypted messages don't have any additional useful information.
+        if event.type == EventTypes.Encrypted:
+            return ret
+
+        msgtype = event.content.get("msgtype")
+
+        ret["msgtype"] = msgtype
+
         if msgtype == "m.text":
             self.add_text_message_vars(ret, event)
         elif msgtype == "m.image":
diff --git a/synapse/res/templates/notif.html b/synapse/res/templates/notif.html
index 1a6c70b5624d..4fc593c011c4 100644
--- a/synapse/res/templates/notif.html
+++ b/synapse/res/templates/notif.html
@@ -20,16 +20,22 @@
                 <div class="sender_name">{% if message.msgtype == "m.emote" %}*{% endif %} {{ message.sender_name }}</div>
             {% endif %}
             <div class="message_body">
-                {% if message.msgtype == "m.text" %}
-                    {{ message.body_text_html }}
-                {% elif message.msgtype == "m.emote" %}
-                    {{ message.body_text_html }}
-                {% elif message.msgtype == "m.notice" %}
-                    {{ message.body_text_html }}
-                {% elif message.msgtype == "m.image" %}
-                    <img src="{{ message.image_url|mxc_to_http(640, 480, scale) }}" />
-                {% elif message.msgtype == "m.file" %}
-                    <span class="filename">{{ message.body_text_plain }}</span>
+                {% if message.event_type == "m.room.encrypted" %}
+                    An encrypted message.
+                {% elif message.event_type == "m.room.message" %}
+                    {% if message.msgtype == "m.text" %}
+                        {{ message.body_text_html }}
+                    {% elif message.msgtype == "m.emote" %}
+                        {{ message.body_text_html }}
+                    {% elif message.msgtype == "m.notice" %}
+                        {{ message.body_text_html }}
+                    {% elif message.msgtype == "m.image" %}
+                        <img src="{{ message.image_url|mxc_to_http(640, 480, scale) }}" />
+                    {% elif message.msgtype == "m.file" %}
+                        <span class="filename">{{ message.body_text_plain }}</span>
+                    {% else %}
+                        An unknown event.
+                    {% endif %}
                 {% endif %}
             </div>
         </td>
diff --git a/synapse/res/templates/notif.txt b/synapse/res/templates/notif.txt
index a37bee98332c..63976241f6a5 100644
--- a/synapse/res/templates/notif.txt
+++ b/synapse/res/templates/notif.txt
@@ -1,4 +1,7 @@
 {% for message in notif.messages %}
+{% if message.event_type == "m.room.encrypted" %}
+An encrypted message.
+{% elif message.event_type == "m.room.message" %}
 {% if message.msgtype == "m.emote" %}* {% endif %}{{ message.sender_name }} ({{ message.ts|format_ts("%H:%M") }})
 {% if message.msgtype == "m.text" %}
 {{ message.body_text_plain }}
@@ -10,6 +13,10 @@
 {{ message.body_text_plain }}
 {% elif message.msgtype == "m.file" %}
 {{ message.body_text_plain }}
+{% elif message.msgtype == "m.room.encrypted" %}
+{% else %}
+An unknown event.
+{% endif %}
 {% endif %}
 {% endfor %}
 
diff --git a/tests/push/test_email.py b/tests/push/test_email.py
index 322456864065..1684d2cff25a 100644
--- a/tests/push/test_email.py
+++ b/tests/push/test_email.py
@@ -158,8 +158,23 @@ def test_multiple_members_email(self):
         # We should get emailed about those messages
         self._check_for_mail()
 
+    def test_encrypted_message(self):
+        room = self.helper.create_room_as(self.user_id, tok=self.access_token)
+        self.helper.invite(
+            room=room, src=self.user_id, tok=self.access_token, targ=self.others[0].id
+        )
+        self.helper.join(room=room, user=self.others[0].id, tok=self.others[0].token)
+
+        # The other user sends some messages
+        self.helper.send_event(
+            room, "m.room.encrypted", {}, tok=self.others[0].token
+        )
+
+        # We should get emailed about that message
+        self._check_for_mail()
+
     def _check_for_mail(self):
-        "Check that the user receives an email notification"
+        """Check that the user receives an email notification"""
 
         # Get the stream ordering before it gets sent
         pushers = self.get_success(

From 9ede5ebec2672cbc7bdad92428b1c5a6e6e805d6 Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Wed, 14 Oct 2020 14:25:21 -0400
Subject: [PATCH 2/6] Better whitespace control in templates.

---
 synapse/res/templates/notif.html      | 48 +++++++++++++--------------
 synapse/res/templates/notif.txt       | 28 ++++++++--------
 synapse/res/templates/notif_mail.html | 26 +++++++--------
 synapse/res/templates/notif_mail.txt  |  6 ++--
 synapse/res/templates/room.html       | 26 +++++++--------
 synapse/res/templates/room.txt        | 12 +++----
 6 files changed, 73 insertions(+), 73 deletions(-)

diff --git a/synapse/res/templates/notif.html b/synapse/res/templates/notif.html
index 4fc593c011c4..8f1388cc68e2 100644
--- a/synapse/res/templates/notif.html
+++ b/synapse/res/templates/notif.html
@@ -1,47 +1,47 @@
-{% for message in notif.messages %}
+{%- for message in notif.messages %}
     <tr class="{{ "historical_message" if message.is_historical else "message" }}">
         <td class="sender_avatar">
-            {% if loop.index0 == 0 or notif.messages[loop.index0 - 1].sender_name != notif.messages[loop.index0].sender_name %}
-                {% if message.sender_avatar_url %}
+            {%- if loop.index0 == 0 or notif.messages[loop.index0 - 1].sender_name != notif.messages[loop.index0].sender_name %}
+                {%- if message.sender_avatar_url %}
                     <img alt="" class="sender_avatar" src="{{ message.sender_avatar_url|mxc_to_http(32,32) }}"  />
-                {% else %}
-                    {% if message.sender_hash % 3 == 0 %}
+                {%- else %}
+                    {%- if message.sender_hash % 3 == 0 %}
                         <img class="sender_avatar" src="https://riot.im/img/external/avatar-1.png"  />
-                    {% elif message.sender_hash % 3 == 1 %}
+                    {%- elif message.sender_hash % 3 == 1 %}
                         <img class="sender_avatar" src="https://riot.im/img/external/avatar-2.png"  />
-                    {% else %}
+                    {%- else %}
                         <img class="sender_avatar" src="https://riot.im/img/external/avatar-3.png"  />
-                    {% endif %}
-                {% endif %}
-            {% endif %}
+                    {%- endif %}
+                {%- endif %}
+            {%- endif %}
         </td>
         <td class="message_contents">
-            {% if loop.index0 == 0 or notif.messages[loop.index0 - 1].sender_name != notif.messages[loop.index0].sender_name %}
-                <div class="sender_name">{% if message.msgtype == "m.emote" %}*{% endif %} {{ message.sender_name }}</div>
-            {% endif %}
+            {%- if loop.index0 == 0 or notif.messages[loop.index0 - 1].sender_name != notif.messages[loop.index0].sender_name %}
+                <div class="sender_name">{%- if message.msgtype == "m.emote" %}*{%- endif %} {{ message.sender_name }}</div>
+            {%- endif %}
             <div class="message_body">
-                {% if message.event_type == "m.room.encrypted" %}
+                {%- if message.event_type == "m.room.encrypted" %}
                     An encrypted message.
-                {% elif message.event_type == "m.room.message" %}
-                    {% if message.msgtype == "m.text" %}
+                {%- elif message.event_type == "m.room.message" %}
+                    {%- if message.msgtype == "m.text" %}
                         {{ message.body_text_html }}
-                    {% elif message.msgtype == "m.emote" %}
+                    {%- elif message.msgtype == "m.emote" %}
                         {{ message.body_text_html }}
-                    {% elif message.msgtype == "m.notice" %}
+                    {%- elif message.msgtype == "m.notice" %}
                         {{ message.body_text_html }}
-                    {% elif message.msgtype == "m.image" %}
+                    {%- elif message.msgtype == "m.image" %}
                         <img src="{{ message.image_url|mxc_to_http(640, 480, scale) }}" />
-                    {% elif message.msgtype == "m.file" %}
+                    {%- elif message.msgtype == "m.file" %}
                         <span class="filename">{{ message.body_text_plain }}</span>
-                    {% else %}
+                    {%- else %}
                         An unknown event.
-                    {% endif %}
-                {% endif %}
+                    {%- endif %}
+                {%- endif %}
             </div>
         </td>
         <td class="message_time">{{ message.ts|format_ts("%H:%M") }}</td>
     </tr>
-{% endfor %}
+{%- endfor %}
 <tr class="notif_link">
     <td></td>
     <td>
diff --git a/synapse/res/templates/notif.txt b/synapse/res/templates/notif.txt
index 63976241f6a5..09f344ca5b8d 100644
--- a/synapse/res/templates/notif.txt
+++ b/synapse/res/templates/notif.txt
@@ -1,23 +1,23 @@
-{% for message in notif.messages %}
-{% if message.event_type == "m.room.encrypted" %}
+{%- for message in notif.messages %}
+{%- if message.event_type == "m.room.encrypted" %}
 An encrypted message.
-{% elif message.event_type == "m.room.message" %}
-{% if message.msgtype == "m.emote" %}* {% endif %}{{ message.sender_name }} ({{ message.ts|format_ts("%H:%M") }})
-{% if message.msgtype == "m.text" %}
+{%- elif message.event_type == "m.room.message" %}
+{%- if message.msgtype == "m.emote" %}* {%- endif %}{{ message.sender_name }} ({{ message.ts|format_ts("%H:%M") }})
+{%- if message.msgtype == "m.text" %}
 {{ message.body_text_plain }}
-{% elif message.msgtype == "m.emote" %}
+{%- elif message.msgtype == "m.emote" %}
 {{ message.body_text_plain }}
-{% elif message.msgtype == "m.notice" %}
+{%- elif message.msgtype == "m.notice" %}
 {{ message.body_text_plain }}
-{% elif message.msgtype == "m.image" %}
+{%- elif message.msgtype == "m.image" %}
 {{ message.body_text_plain }}
-{% elif message.msgtype == "m.file" %}
+{%- elif message.msgtype == "m.file" %}
 {{ message.body_text_plain }}
-{% elif message.msgtype == "m.room.encrypted" %}
-{% else %}
+{%- elif message.msgtype == "m.room.encrypted" %}
+{%- else %}
 An unknown event.
-{% endif %}
-{% endif %}
-{% endfor %}
+{%- endif %}
+{%- endif %}
+{%- endfor %}
 
 View {{ room.title }} at {{ notif.link }}
diff --git a/synapse/res/templates/notif_mail.html b/synapse/res/templates/notif_mail.html
index a2dfeb9e9f78..27d41827907f 100644
--- a/synapse/res/templates/notif_mail.html
+++ b/synapse/res/templates/notif_mail.html
@@ -2,8 +2,8 @@
 <html lang="en">
     <head>
         <style type="text/css">
-            {% include 'mail.css' without context %}
-            {% include "mail-%s.css" % app_name ignore missing without context %}
+            {%- include 'mail.css' without context %}
+            {%- include "mail-%s.css" % app_name ignore missing without context %}
         </style>
     </head>
     <body>
@@ -18,21 +18,21 @@
                                 <div class="summarytext">{{ summary_text }}</div>
                             </td>
                             <td class="logo">
-                                {% if app_name == "Riot" %}
+                                {%- if app_name == "Riot" %}
                                     <img src="http://riot.im/img/external/riot-logo-email.png" width="83" height="83" alt="[Riot]"/>
-                                {% elif app_name == "Vector" %}
+                                {%- elif app_name == "Vector" %}
                                     <img src="http://matrix.org/img/vector-logo-email.png" width="64" height="83" alt="[Vector]"/>
-                                {% elif app_name == "Element" %}
+                                {%- elif app_name == "Element" %}
                                     <img src="https://static.element.io/images/email-logo.png" width="83" height="83" alt="[Element]"/>
-                                {% else %}
+                                {%- else %}
                                     <img src="http://matrix.org/img/matrix-120x51.png" width="120" height="51" alt="[matrix]"/>
-                                {% endif %}
+                                {%- endif %}
                             </td>
                         </tr>
                     </table>
-                    {% for room in rooms %}
-                        {% include 'room.html' with context %}
-                    {% endfor %}
+                    {%- for room in rooms %}
+                        {%- include 'room.html' with context %}
+                    {%- endfor %}
                     <div class="footer">
                         <a href="{{ unsubscribe_link }}">Unsubscribe</a>
                         <br/>
@@ -41,12 +41,12 @@
                             Sending email at {{ reason.now|format_ts("%c") }} due to activity in room {{ reason.room_name }} because
                             an event was received at {{ reason.received_at|format_ts("%c") }}
                             which is more than {{ "%.1f"|format(reason.delay_before_mail_ms / (60*1000)) }} ({{ reason.delay_before_mail_ms }}) mins ago,
-                            {% if reason.last_sent_ts %}
+                            {%- if reason.last_sent_ts %}
                                 and the last time we sent a mail for this room was {{ reason.last_sent_ts|format_ts("%c") }},
                                 which is more than {{ "%.1f"|format(reason.throttle_ms / (60*1000)) }} (current throttle_ms) mins ago.
-                            {% else %}
+                            {%- else %}
                                 and we don't have a last time we sent a mail for this room.
-                            {% endif %}
+                            {%- endif %}
                         </div>
                     </div>
                 </td>
diff --git a/synapse/res/templates/notif_mail.txt b/synapse/res/templates/notif_mail.txt
index 24843042a540..df3c253979ca 100644
--- a/synapse/res/templates/notif_mail.txt
+++ b/synapse/res/templates/notif_mail.txt
@@ -2,9 +2,9 @@ Hi {{ user_display_name }},
 
 {{ summary_text }}
 
-{% for room in rooms %}
-{% include 'room.txt' with context %}
-{% endfor %}
+{%- for room in rooms %}
+{%- include 'room.txt' with context %}
+{%- endfor %}
 
 You can disable these notifications at {{ unsubscribe_link }}
 
diff --git a/synapse/res/templates/room.html b/synapse/res/templates/room.html
index b8525fef888c..4fc6f6ac9b31 100644
--- a/synapse/res/templates/room.html
+++ b/synapse/res/templates/room.html
@@ -1,23 +1,23 @@
 <table class="room">
     <tr class="room_header">
         <td class="room_avatar">
-            {% if room.avatar_url %}
+            {%- if room.avatar_url %}
                 <img alt="" src="{{ room.avatar_url|mxc_to_http(48,48) }}" />
-            {% else %}
-                {% if room.hash % 3 == 0 %}
+            {%- else %}
+                {%- if room.hash % 3 == 0 %}
                     <img alt="" src="https://riot.im/img/external/avatar-1.png"  />
-                {% elif room.hash % 3 == 1 %}
+                {%- elif room.hash % 3 == 1 %}
                     <img alt="" src="https://riot.im/img/external/avatar-2.png"  />
-                {% else %}
+                {%- else %}
                     <img alt="" src="https://riot.im/img/external/avatar-3.png"  />
-                {% endif %}
-            {% endif %}
+                {%- endif %}
+            {%- endif %}
         </td>
         <td class="room_name" colspan="2">
             {{ room.title }}
         </td>
     </tr>
-    {% if room.invite %}
+    {%- if room.invite %}
         <tr>
             <td></td>
             <td>
@@ -25,9 +25,9 @@
             </td>
             <td></td>
         </tr>
-    {% else %}
-        {% for notif in room.notifs %}
-            {% include 'notif.html' with context %}
-        {% endfor %}
-    {% endif %}
+    {%- else %}
+        {%- for notif in room.notifs %}
+            {%- include 'notif.html' with context %}
+        {%- endfor %}
+    {%- endif %}
 </table>
diff --git a/synapse/res/templates/room.txt b/synapse/res/templates/room.txt
index 84648c710ece..df841e9e6f00 100644
--- a/synapse/res/templates/room.txt
+++ b/synapse/res/templates/room.txt
@@ -1,9 +1,9 @@
 {{ room.title }}
 
-{% if room.invite %}
+{%- if room.invite %}
     You've been invited, join at {{ room.link }}
-{% else %}
-    {% for notif in room.notifs %}
-        {% include 'notif.txt' with context %}
-    {% endfor %}
-{% endif %}
+{%- else %}
+    {%- for notif in room.notifs %}
+        {%- include 'notif.txt' with context %}
+    {%- endfor %}
+{%- endif %}

From d0b2bfaa3c4c98eceaa5e2604023416a5c5a10f2 Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Wed, 14 Oct 2020 14:29:47 -0400
Subject: [PATCH 3/6] Changelog.

---
 changelog.d/8545.bugfix | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 changelog.d/8545.bugfix

diff --git a/changelog.d/8545.bugfix b/changelog.d/8545.bugfix
new file mode 100644
index 000000000000..64ba307df069
--- /dev/null
+++ b/changelog.d/8545.bugfix
@@ -0,0 +1 @@
+Fix a long standing bug where email notifications for encrypted messages were blank.

From a2fa409fd7d6a74fd90d5b3c712121d8e8aac831 Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Wed, 14 Oct 2020 14:38:05 -0400
Subject: [PATCH 4/6] Lint.

---
 tests/push/test_email.py | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/tests/push/test_email.py b/tests/push/test_email.py
index 1684d2cff25a..55545d93410d 100644
--- a/tests/push/test_email.py
+++ b/tests/push/test_email.py
@@ -166,9 +166,7 @@ def test_encrypted_message(self):
         self.helper.join(room=room, user=self.others[0].id, tok=self.others[0].token)
 
         # The other user sends some messages
-        self.helper.send_event(
-            room, "m.room.encrypted", {}, tok=self.others[0].token
-        )
+        self.helper.send_event(room, "m.room.encrypted", {}, tok=self.others[0].token)
 
         # We should get emailed about that message
         self._check_for_mail()

From f3c936fa9ea0cf3d72f793fc15197bfc3504e488 Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Thu, 15 Oct 2020 07:13:23 -0400
Subject: [PATCH 5/6] Remove bogus elif clause.

---
 synapse/res/templates/notif.txt | 1 -
 1 file changed, 1 deletion(-)

diff --git a/synapse/res/templates/notif.txt b/synapse/res/templates/notif.txt
index 09f344ca5b8d..0ce9f9e52ab5 100644
--- a/synapse/res/templates/notif.txt
+++ b/synapse/res/templates/notif.txt
@@ -13,7 +13,6 @@ An encrypted message.
 {{ message.body_text_plain }}
 {%- elif message.msgtype == "m.file" %}
 {{ message.body_text_plain }}
-{%- elif message.msgtype == "m.room.encrypted" %}
 {%- else %}
 An unknown event.
 {%- endif %}

From 7219ec7c7aec963a3b208449f22e456273b683b2 Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Thu, 15 Oct 2020 12:50:23 -0400
Subject: [PATCH 6/6] Update message for invalid events.

---
 synapse/res/templates/notif.html | 2 +-
 synapse/res/templates/notif.txt  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/synapse/res/templates/notif.html b/synapse/res/templates/notif.html
index 8f1388cc68e2..6d76064d132f 100644
--- a/synapse/res/templates/notif.html
+++ b/synapse/res/templates/notif.html
@@ -34,7 +34,7 @@
                     {%- elif message.msgtype == "m.file" %}
                         <span class="filename">{{ message.body_text_plain }}</span>
                     {%- else %}
-                        An unknown event.
+                        A message with unrecognised content.
                     {%- endif %}
                 {%- endif %}
             </div>
diff --git a/synapse/res/templates/notif.txt b/synapse/res/templates/notif.txt
index 0ce9f9e52ab5..1ee7da3c50ef 100644
--- a/synapse/res/templates/notif.txt
+++ b/synapse/res/templates/notif.txt
@@ -14,7 +14,7 @@ An encrypted message.
 {%- elif message.msgtype == "m.file" %}
 {{ message.body_text_plain }}
 {%- else %}
-An unknown event.
+A message with unrecognised content.
 {%- endif %}
 {%- endif %}
 {%- endfor %}