From 0dbfae03f96ae14672bbdc5cf5c3aecd93636803 Mon Sep 17 00:00:00 2001
From: Richard van der Hoff <richard@matrix.org>
Date: Tue, 19 Mar 2019 11:04:12 +0000
Subject: [PATCH] Enforce hs_disabled_message correctly

Fixes a bug where hs_disabled_message was not enforced for 3pid-based requests
if there was no server_notices_mxid configured.
---
 changelog.d/4888.bugfix |  2 ++
 synapse/api/auth.py     |  8 +++++---
 tests/api/test_auth.py  | 17 +++++++++++++++++
 3 files changed, 24 insertions(+), 3 deletions(-)
 create mode 100644 changelog.d/4888.bugfix

diff --git a/changelog.d/4888.bugfix b/changelog.d/4888.bugfix
new file mode 100644
index 0000000000..0e193709e5
--- /dev/null
+++ b/changelog.d/4888.bugfix
@@ -0,0 +1,2 @@
+Fix a bug where hs_disabled_message was sometimes not correctly enforced.
+
diff --git a/synapse/api/auth.py b/synapse/api/auth.py
index 5992d30623..ee646a97e8 100644
--- a/synapse/api/auth.py
+++ b/synapse/api/auth.py
@@ -788,9 +788,11 @@ class Auth(object):
 
         # Never fail an auth check for the server notices users or support user
         # This can be a problem where event creation is prohibited due to blocking
-        is_support = yield self.store.is_support_user(user_id)
-        if user_id == self.hs.config.server_notices_mxid or is_support:
-            return
+        if user_id is not None:
+            if user_id == self.hs.config.server_notices_mxid:
+                return
+            if (yield self.store.is_support_user(user_id)):
+                return
 
         if self.hs.config.hs_disabled:
             raise ResourceLimitError(
diff --git a/tests/api/test_auth.py b/tests/api/test_auth.py
index d77f20e876..d0d36f96fa 100644
--- a/tests/api/test_auth.py
+++ b/tests/api/test_auth.py
@@ -344,6 +344,23 @@ class AuthTestCase(unittest.TestCase):
         self.assertEquals(e.exception.errcode, Codes.RESOURCE_LIMIT_EXCEEDED)
         self.assertEquals(e.exception.code, 403)
 
+    @defer.inlineCallbacks
+    def test_hs_disabled_no_server_notices_user(self):
+        """Check that 'hs_disabled_message' works correctly when there is no
+        server_notices user.
+        """
+        # this should be the default, but we had a bug where the test was doing the wrong
+        # thing, so let's make it explicit
+        self.hs.config.server_notices_mxid = None
+
+        self.hs.config.hs_disabled = True
+        self.hs.config.hs_disabled_message = "Reason for being disabled"
+        with self.assertRaises(ResourceLimitError) as e:
+            yield self.auth.check_auth_blocking()
+        self.assertEquals(e.exception.admin_contact, self.hs.config.admin_contact)
+        self.assertEquals(e.exception.errcode, Codes.RESOURCE_LIMIT_EXCEEDED)
+        self.assertEquals(e.exception.code, 403)
+
     @defer.inlineCallbacks
     def test_server_notices_mxid_special_cased(self):
         self.hs.config.hs_disabled = True