From 31dc1a0d67cf9320d31943e94c3de33ac4a42652 Mon Sep 17 00:00:00 2001 From: Neil Johnson Date: Tue, 22 Oct 2019 17:19:34 +0100 Subject: [PATCH] ensure that suppresion affects only mau case --- docs/sample_config.yaml | 1 - synapse/api/auth.py | 12 +++++-- synapse/api/constants.py | 7 ++++ synapse/config/server.py | 2 -- .../resource_limits_server_notices.py | 33 +++++++++++-------- .../test_resource_limits_server_notices.py | 25 ++++++++++++-- tests/utils.py | 1 - 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,