ensure that suppresion affects only mau case

7 files changed, 57 insertions, 24 deletions

diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml
index 952b880cbd..abdc062b68 100644
--- a/docs/sample_config.yaml
+++ b/docs/sample_config.yaml
@@ -241,7 +241,6 @@ listeners:
 #
 #hs_disabled: False
 #hs_disabled_message: 'Human readable reason for why the HS is blocked'
-#hs_disabled_limit_type: 'error code(str), to help clients decode reason'
 
 # Monthly Active User Blocking
 #
diff --git a/synapse/api/auth.py b/synapse/api/auth.py
index cb50579fd2..c6294ff491 100644
--- a/synapse/api/auth.py
+++ b/synapse/api/auth.py
@@ -25,7 +25,13 @@ from twisted.internet import defer
 import synapse.logging.opentracing as opentracing
 import synapse.types
 from synapse import event_auth
-from synapse.api.constants import EventTypes, JoinRules, Membership, UserTypes
+from synapse.api.constants import (
+    EventTypes,
+    JoinRules,
+    LimitBlockingTypes,
+    Membership,
+    UserTypes,
+)
 from synapse.api.errors import (
     AuthError,
     Codes,
@@ -743,7 +749,7 @@ class Auth(object):
                 self.hs.config.hs_disabled_message,
                 errcode=Codes.RESOURCE_LIMIT_EXCEEDED,
                 admin_contact=self.hs.config.admin_contact,
-                limit_type=self.hs.config.hs_disabled_limit_type,
+                limit_type=LimitBlockingTypes.HS_DISABLED,
             )
         if self.hs.config.limit_usage_by_mau is True:
             assert not (user_id and threepid)
@@ -776,5 +782,5 @@ class Auth(object):
                     "Monthly Active User Limit Exceeded",
                     admin_contact=self.hs.config.admin_contact,
                     errcode=Codes.RESOURCE_LIMIT_EXCEEDED,
-                    limit_type="monthly_active_user",
+                    limit_type=LimitBlockingTypes.MONTHLY_ACTIVE_USER,
                 )
diff --git a/synapse/api/constants.py b/synapse/api/constants.py
index f29bce560c..996ef48e24 100644
--- a/synapse/api/constants.py
+++ b/synapse/api/constants.py
@@ -133,3 +133,10 @@ class RelationTypes(object):
     ANNOTATION = "m.annotation"
     REPLACE = "m.replace"
     REFERENCE = "m.reference"
+
+
+class LimitBlockingTypes(object):
+    """Reasons that a server may be blocked"""
+
+    MONTHLY_ACTIVE_USER = "monthly_active_user"
+    HS_DISABLED = "hs_disabled"
diff --git a/synapse/config/server.py b/synapse/config/server.py
index 0a795bfb06..b36c0417ac 100644
--- a/synapse/config/server.py
+++ b/synapse/config/server.py
@@ -193,7 +193,6 @@ class ServerConfig(Config):
         # Options to disable HS
         self.hs_disabled = config.get("hs_disabled", False)
         self.hs_disabled_message = config.get("hs_disabled_message", "")
-        self.hs_disabled_limit_type = config.get("hs_disabled_limit_type", "")
 
         # Admin uri to direct users at should their instance become blocked
         # due to resource constraints
@@ -676,7 +675,6 @@ class ServerConfig(Config):
         #
         #hs_disabled: False
         #hs_disabled_message: 'Human readable reason for why the HS is blocked'
-        #hs_disabled_limit_type: 'error code(str), to help clients decode reason'
 
         # Monthly Active User Blocking
         #
diff --git a/synapse/server_notices/resource_limits_server_notices.py b/synapse/server_notices/resource_limits_server_notices.py
index c62a9301f3..72015c4c03 100644
--- a/synapse/server_notices/resource_limits_server_notices.py
+++ b/synapse/server_notices/resource_limits_server_notices.py
@@ -22,6 +22,7 @@ from synapse.api.constants import (
     EventTypes,
     ServerNoticeLimitReached,
     ServerNoticeMsgType,
+    LimitBlockingTypes,
 )
 from synapse.api.errors import AuthError, ResourceLimitError, SynapseError
 from synapse.server_notices.server_notices_manager import SERVER_NOTICE_ROOM_TAG
@@ -91,24 +92,28 @@ class ResourceLimitsServerNotices(object):
         currently_blocked, ref_events = yield self._is_room_currently_blocked(room_id)
 
         try:
-            if not self._config.mau_limit_alerting:
-                # Alerting disabled, reset room if necessary and return
+            is_auth_blocking = False
+            event_limit_type = ""
+
+            try:
+                # Normally should always pass in user_id to check_auth_blocking
+                # if you have it, but in this case are checking what would happen
+                # to other users if they were to arrive.
+                yield self._auth.check_auth_blocking()
+            except ResourceLimitError as e:
+                is_auth_blocking = True
+                event_body = e.msg
+                event_limit_type = e.limit_type
+
+            if (
+                not self._config.mau_limit_alerting
+                and event_limit_type is LimitBlockingTypes.MONTHLY_ACTIVE_USER
+            ):
+                # MAU alerting disabled, reset room if necessary and return
                 if currently_blocked:
                     self._remove_limit_block_notification(ref_events, user_id)
                 return
 
-            is_auth_blocking = False
-            if self._config.mau_limit_alerting:
-                try:
-                    # Normally should always pass in user_id to check_auth_blocking
-                    # if you have it, but in this case are checking what would happen
-                    # to other users if they were to arrive.
-                    yield self._auth.check_auth_blocking()
-                except ResourceLimitError as e:
-                    is_auth_blocking = True
-                    event_body = e.msg
-                    event_limit_type = e.limit_type
-
             if currently_blocked and not is_auth_blocking:
                 # Room is notifying of a block, when it ought not to be.
                 yield self._remove_limit_block_notification(user_id, ref_events)
diff --git a/tests/server_notices/test_resource_limits_server_notices.py b/tests/server_notices/test_resource_limits_server_notices.py
index 705fb0d376..b5085944e5 100644
--- a/tests/server_notices/test_resource_limits_server_notices.py
+++ b/tests/server_notices/test_resource_limits_server_notices.py
@@ -17,7 +17,7 @@ from mock import Mock
 
 from twisted.internet import defer
 
-from synapse.api.constants import EventTypes, ServerNoticeMsgType
+from synapse.api.constants import EventTypes, LimitBlockingTypes, ServerNoticeMsgType
 from synapse.api.errors import ResourceLimitError
 from synapse.server_notices.resource_limits_server_notices import (
     ResourceLimitsServerNotices,
@@ -165,12 +165,29 @@ class TestResourceLimitsServerNotices(unittest.HomeserverTestCase):
         """
         self.hs.config.mau_limit_alerting = False
         self._rlsn._auth.check_auth_blocking = Mock(
-            side_effect=ResourceLimitError(403, "foo")
+            side_effect=ResourceLimitError(
+                403, "foo", limit_type=LimitBlockingTypes.MONTHLY_ACTIVE_USER
+            )
         )
         self.get_success(self._rlsn.maybe_send_server_notice_to_user(self.user_id))
 
         self.assertTrue(self._send_notice.call_count == 0)
 
+    def test_check_hs_disabled_unaffected_by_mau_alert_suppression(self):
+        """
+        Test that when a server is disabled, that MAU limit alerting is ignored.
+        """
+        self.hs.config.mau_limit_alerting = False
+        self._rlsn._auth.check_auth_blocking = Mock(
+            side_effect=ResourceLimitError(
+                403, "foo", limit_type=LimitBlockingTypes.HS_DISABLED
+            )
+        )
+        self.get_success(self._rlsn.maybe_send_server_notice_to_user(self.user_id))
+
+        # Would be better to check contents, but 2 calls == set blocking event
+        self.assertEqual(self._send_notice.call_count, 2)
+
     def test_maybe_send_server_notice_when_alerting_suppressed_room_blocked(self):
         """
         When the room is already in a blocked state, test that when alerting
@@ -178,7 +195,9 @@ class TestResourceLimitsServerNotices(unittest.HomeserverTestCase):
         """
         self.hs.config.mau_limit_alerting = False
         self._rlsn._auth.check_auth_blocking = Mock(
-            side_effect=ResourceLimitError(403, "foo")
+            side_effect=ResourceLimitError(
+                403, "foo", limit_type=LimitBlockingTypes.MONTHLY_ACTIVE_USER
+            )
         )
         self._rlsn._server_notices_manager.__is_room_currently_blocked = Mock(
             return_value=defer.succeed((True, []))
diff --git a/tests/utils.py b/tests/utils.py
index 46ef2959f2..28f31ec43c 100644
--- a/tests/utils.py
+++ b/tests/utils.py
@@ -145,7 +145,6 @@ def default_config(name, parse=False):
         "limit_usage_by_mau": False,
         "hs_disabled": False,
         "hs_disabled_message": "",
-        "hs_disabled_limit_type": "",
         "max_mau_value": 50,
         "mau_trial_days": 0,
         "mau_stats_only": False,