From 4c2ed3f20ef5361ea04da9c678d157d8735ca120 Mon Sep 17 00:00:00 2001
From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
Date: Mon, 24 Feb 2020 15:18:38 +0000
Subject: [PATCH] Fix minor issues with email config (#6962)

 * Give `notif_template_html`, `notif_template_text` default values (fixes #6960)
 * Don't complain if `smtp_host` and `smtp_port` are unset, since they have sensible defaults (fixes #6961)
 * Set the example for `enable_notifs` to `True`, for consistency and because it's more useful
 * Raise errors as ConfigError rather than RuntimeError for nicer formatting
---
 changelog.d/6962.bugfix       |  1 +
 docs/sample_config.yaml       |  9 ++---
 synapse/config/emailconfig.py | 66 ++++++++++++++++-------------------
 3 files changed, 36 insertions(+), 40 deletions(-)
 create mode 100644 changelog.d/6962.bugfix

diff --git a/changelog.d/6962.bugfix b/changelog.d/6962.bugfix
new file mode 100644
index 000000000..9f5229d40
--- /dev/null
+++ b/changelog.d/6962.bugfix
@@ -0,0 +1 @@
+Fix a couple of bugs in email configuration handling.
diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml
index 8a036071e..54cbe840d 100644
--- a/docs/sample_config.yaml
+++ b/docs/sample_config.yaml
@@ -1409,10 +1409,6 @@ email:
   #
   #require_transport_security: true
 
-  # Enable sending emails for messages that the user has missed
-  #
-  #enable_notifs: false
-
   # notif_from defines the "From" address to use when sending emails.
   # It must be set if email sending is enabled.
   #
@@ -1430,6 +1426,11 @@ email:
   #
   #app_name: my_branded_matrix_server
 
+  # Uncomment the following to enable sending emails for messages that the user
+  # has missed. Disabled by default.
+  #
+  #enable_notifs: true
+
   # Uncomment the following to disable automatic subscription to email
   # notifications for new users. Enabled by default.
   #
diff --git a/synapse/config/emailconfig.py b/synapse/config/emailconfig.py
index 74853f9fa..f31fc85ec 100644
--- a/synapse/config/emailconfig.py
+++ b/synapse/config/emailconfig.py
@@ -27,6 +27,12 @@ import pkg_resources
 
 from ._base import Config, ConfigError
 
+MISSING_PASSWORD_RESET_CONFIG_ERROR = """\
+Password reset emails are enabled on this homeserver due to a partial
+'email' block. However, the following required keys are missing:
+    %s
+"""
+
 
 class EmailConfig(Config):
     section = "email"
@@ -142,24 +148,18 @@ class EmailConfig(Config):
             bleach
 
         if self.threepid_behaviour_email == ThreepidBehaviour.LOCAL:
-            required = ["smtp_host", "smtp_port", "notif_from"]
-
             missing = []
-            for k in required:
-                if k not in email_config:
-                    missing.append("email." + k)
+            if not self.email_notif_from:
+                missing.append("email.notif_from")
 
             # public_baseurl is required to build password reset and validation links that
             # will be emailed to users
             if config.get("public_baseurl") is None:
                 missing.append("public_baseurl")
 
-            if len(missing) > 0:
-                raise RuntimeError(
-                    "Password resets emails are configured to be sent from "
-                    "this homeserver due to a partial 'email' block. "
-                    "However, the following required keys are missing: %s"
-                    % (", ".join(missing),)
+            if missing:
+                raise ConfigError(
+                    MISSING_PASSWORD_RESET_CONFIG_ERROR % (", ".join(missing),)
                 )
 
             # These email templates have placeholders in them, and thus must be
@@ -245,32 +245,25 @@ class EmailConfig(Config):
             )
 
         if self.email_enable_notifs:
-            required = [
-                "smtp_host",
-                "smtp_port",
-                "notif_from",
-                "notif_template_html",
-                "notif_template_text",
-            ]
-
             missing = []
-            for k in required:
-                if k not in email_config:
-                    missing.append(k)
-
-            if len(missing) > 0:
-                raise RuntimeError(
-                    "email.enable_notifs is True but required keys are missing: %s"
-                    % (", ".join(["email." + k for k in missing]),)
-                )
+            if not self.email_notif_from:
+                missing.append("email.notif_from")
 
             if config.get("public_baseurl") is None:
-                raise RuntimeError(
-                    "email.enable_notifs is True but no public_baseurl is set"
+                missing.append("public_baseurl")
+
+            if missing:
+                raise ConfigError(
+                    "email.enable_notifs is True but required keys are missing: %s"
+                    % (", ".join(missing),)
                 )
 
-            self.email_notif_template_html = email_config["notif_template_html"]
-            self.email_notif_template_text = email_config["notif_template_text"]
+            self.email_notif_template_html = email_config.get(
+                "notif_template_html", "notif_mail.html"
+            )
+            self.email_notif_template_text = email_config.get(
+                "notif_template_text", "notif_mail.txt"
+            )
 
             for f in self.email_notif_template_text, self.email_notif_template_html:
                 p = os.path.join(self.email_template_dir, f)
@@ -323,10 +316,6 @@ class EmailConfig(Config):
           #
           #require_transport_security: true
 
-          # Enable sending emails for messages that the user has missed
-          #
-          #enable_notifs: false
-
           # notif_from defines the "From" address to use when sending emails.
           # It must be set if email sending is enabled.
           #
@@ -344,6 +333,11 @@ class EmailConfig(Config):
           #
           #app_name: my_branded_matrix_server
 
+          # Uncomment the following to enable sending emails for messages that the user
+          # has missed. Disabled by default.
+          #
+          #enable_notifs: true
+
           # Uncomment the following to disable automatic subscription to email
           # notifications for new users. Enabled by default.
           #