Stop Auth methods from polling the config on every req. (matrix-org#7420

)

changelog.d/7420.misc

@@ -0,0 +1 @@
+Prevent methods in `synapse.handlers.auth` from polling the homeserver config every request.

synapse/api/auth.py

@@ -26,16 +26,15 @@
 import synapse.logging.opentracing as opentracing
 import synapse.types
 from synapse import event_auth
-from synapse.api.constants import EventTypes, LimitBlockingTypes, Membership, UserTypes
+from synapse.api.auth_blocking import AuthBlocking
+from synapse.api.constants import EventTypes, Membership
 from synapse.api.errors import (
  AuthError,
  Codes,
  InvalidClientTokenError,
  MissingClientTokenError,
- ResourceLimitError,
 )
 from synapse.api.room_versions import KNOWN_ROOM_VERSIONS
-from synapse.config.server import is_threepid_reserved
 from synapse.events import EventBase
 from synapse.types import StateMap, UserID
 from synapse.util.caches import CACHE_SIZE_FACTOR, register_cache
@@ -77,7 +76,11 @@ def __init__(self, hs):
  self.token_cache = LruCache(CACHE_SIZE_FACTOR * 10000)
  register_cache("cache", "token_cache", self.token_cache)
 
+ self._auth_blocking = AuthBlocking(self.hs)
+
  self._account_validity = hs.config.account_validity
+ self._track_appservice_user_ips = hs.config.track_appservice_user_ips
+ self._macaroon_secret_key = hs.config.macaroon_secret_key
 
  @defer.inlineCallbacks
  def check_from_context(self, room_version: str, event, context, do_sig_check=True):
@@ -191,7 +194,7 @@ def get_user_by_req(
  opentracing.set_tag("authenticated_entity", user_id)
  opentracing.set_tag("appservice_id", app_service.id)
 
- if ip_addr and self.hs.config.track_appservice_user_ips:
+ if ip_addr and self._track_appservice_user_ips:
  yield self.store.insert_client_ip(
  user_id=user_id,
  access_token=access_token,
@@ -454,7 +457,7 @@ def validate_macaroon(self, macaroon, type_string, user_id):
  # access_tokens include a nonce for uniqueness: any value is acceptable
  v.satisfy_general(lambda c: c.startswith("nonce = "))
 
- v.verify(macaroon, self.hs.config.macaroon_secret_key)
+ v.verify(macaroon, self._macaroon_secret_key)
 
  def _verify_expiry(self, caveat):
  prefix = "time < "
@@ -663,71 +666,5 @@ def check_user_in_room_or_world_readable(
  % (user_id, room_id),
  )
 
- @defer.inlineCallbacks
- def check_auth_blocking(self, user_id=None, threepid=None, user_type=None):
- """Checks if the user should be rejected for some external reason,
- such as monthly active user limiting or global disable flag
-
- Args:
- user_id(str|None): If present, checks for presence against existing
- MAU cohort
-
- threepid(dict|None): If present, checks for presence against configured
- reserved threepid. Used in cases where the user is trying register
- with a MAU blocked server, normally they would be rejected but their
- threepid is on the reserved list. user_id and
- threepid should never be set at the same time.
-
- user_type(str|None): If present, is used to decide whether to check against
- certain blocking reasons like MAU.
- """
-
- # 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
- 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(
- 403,
- self.hs.config.hs_disabled_message,
- errcode=Codes.RESOURCE_LIMIT_EXCEEDED,
- admin_contact=self.hs.config.admin_contact,
- limit_type=LimitBlockingTypes.HS_DISABLED,
- )
- if self.hs.config.limit_usage_by_mau is True:
- assert not (user_id and threepid)
-
- # If the user is already part of the MAU cohort or a trial user
- if user_id:
- timestamp = yield self.store.user_last_seen_monthly_active(user_id)
- if timestamp:
- return
-
- is_trial = yield self.store.is_trial_user(user_id)
- if is_trial:
- return
- elif threepid:
- # If the user does not exist yet, but is signing up with a
- # reserved threepid then pass auth check
- if is_threepid_reserved(
- self.hs.config.mau_limits_reserved_threepids, threepid
- ):
- return
- elif user_type == UserTypes.SUPPORT:
- # If the user does not exist yet and is of type "support",
- # allow registration. Support users are excluded from MAU checks.
- return
- # Else if there is no room in the MAU bucket, bail
- current_mau = yield self.store.get_monthly_active_count()
- if current_mau >= self.hs.config.max_mau_value:
- raise ResourceLimitError(
- 403,
- "Monthly Active User Limit Exceeded",
- admin_contact=self.hs.config.admin_contact,
- errcode=Codes.RESOURCE_LIMIT_EXCEEDED,
- limit_type=LimitBlockingTypes.MONTHLY_ACTIVE_USER,
- )
+ def check_auth_blocking(self, *args, **kwargs):
+ return self._auth_blocking.check_auth_blocking(*args, **kwargs)

synapse/api/auth_blocking.py

@@ -0,0 +1,104 @@
+# -*- coding: utf-8 -*-
+# Copyright 2020 The Matrix.org Foundation C.I.C.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import logging
+
+from twisted.internet import defer
+
+from synapse.api.constants import LimitBlockingTypes, UserTypes
+from synapse.api.errors import Codes, ResourceLimitError
+from synapse.config.server import is_threepid_reserved
+
+logger = logging.getLogger(__name__)
+
+
+class AuthBlocking(object):
+ def __init__(self, hs):
+ self.store = hs.get_datastore()
+
+ self._server_notices_mxid = hs.config.server_notices_mxid
+ self._hs_disabled = hs.config.hs_disabled
+ self._hs_disabled_message = hs.config.hs_disabled_message
+ self._admin_contact = hs.config.admin_contact
+ self._max_mau_value = hs.config.max_mau_value
+ self._limit_usage_by_mau = hs.config.limit_usage_by_mau
+ self._mau_limits_reserved_threepids = hs.config.mau_limits_reserved_threepids
+
+ @defer.inlineCallbacks
+ def check_auth_blocking(self, user_id=None, threepid=None, user_type=None):
+ """Checks if the user should be rejected for some external reason,
+ such as monthly active user limiting or global disable flag
+
+ Args:
+ user_id(str|None): If present, checks for presence against existing
+ MAU cohort
+
+ threepid(dict|None): If present, checks for presence against configured
+ reserved threepid. Used in cases where the user is trying register
+ with a MAU blocked server, normally they would be rejected but their
+ threepid is on the reserved list. user_id and
+ threepid should never be set at the same time.
+
+ user_type(str|None): If present, is used to decide whether to check against
+ certain blocking reasons like MAU.
+ """
+
+ # 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
+ if user_id is not None:
+ if user_id == self._server_notices_mxid:
+ return
+ if (yield self.store.is_support_user(user_id)):
+ return
+
+ if self._hs_disabled:
+ raise ResourceLimitError(
+ 403,
+ self._hs_disabled_message,
+ errcode=Codes.RESOURCE_LIMIT_EXCEEDED,
+ admin_contact=self._admin_contact,
+ limit_type=LimitBlockingTypes.HS_DISABLED,
+ )
+ if self._limit_usage_by_mau is True:
+ assert not (user_id and threepid)
+
+ # If the user is already part of the MAU cohort or a trial user
+ if user_id:
+ timestamp = yield self.store.user_last_seen_monthly_active(user_id)
+ if timestamp:
+ return
+
+ is_trial = yield self.store.is_trial_user(user_id)
+ if is_trial:
+ return
+ elif threepid:
+ # If the user does not exist yet, but is signing up with a
+ # reserved threepid then pass auth check
+ if is_threepid_reserved(self._mau_limits_reserved_threepids, threepid):
+ return
+ elif user_type == UserTypes.SUPPORT:
+ # If the user does not exist yet and is of type "support",
+ # allow registration. Support users are excluded from MAU checks.
+ return
+ # Else if there is no room in the MAU bucket, bail
+ current_mau = yield self.store.get_monthly_active_count()
+ if current_mau >= self._max_mau_value:
+ raise ResourceLimitError(
+ 403,
+ "Monthly Active User Limit Exceeded",
+ admin_contact=self._admin_contact,
+ errcode=Codes.RESOURCE_LIMIT_EXCEEDED,
+ limit_type=LimitBlockingTypes.MONTHLY_ACTIVE_USER,
+ )

tests/api/test_auth.py

@@ -52,6 +52,10 @@ def setUp(self):
  self.hs.handlers = TestHandlers(self.hs)
  self.auth = Auth(self.hs)
 
+ # AuthBlocking reads from the hs' config on initialization. We need to
+ # modify its config instead of the hs'
+ self.auth_blocking = self.auth._auth_blocking
+
  self.test_user = "@foo:bar"
  self.test_token = b"_test_token_"
 
@@ -321,15 +325,15 @@ def get_user(tok):
 
  @defer.inlineCallbacks
  def test_blocking_mau(self):
- self.hs.config.limit_usage_by_mau = False
- self.hs.config.max_mau_value = 50
+ self.auth_blocking._limit_usage_by_mau = False
+ self.auth_blocking._max_mau_value = 50
  lots_of_users = 100
  small_number_of_users = 1
 
  # Ensure no error thrown
  yield defer.ensureDeferred(self.auth.check_auth_blocking())
 
- self.hs.config.limit_usage_by_mau = True
+ self.auth_blocking._limit_usage_by_mau = True
 
  self.store.get_monthly_active_count = Mock(
  return_value=defer.succeed(lots_of_users)
@@ -349,8 +353,8 @@ def test_blocking_mau(self):
 
  @defer.inlineCallbacks
  def test_blocking_mau__depending_on_user_type(self):
- self.hs.config.max_mau_value = 50
- self.hs.config.limit_usage_by_mau = True
+ self.auth_blocking._max_mau_value = 50
+ self.auth_blocking._limit_usage_by_mau = True
 
  self.store.get_monthly_active_count = Mock(return_value=defer.succeed(100))
  # Support users allowed
@@ -370,12 +374,12 @@ def test_blocking_mau__depending_on_user_type(self):
 
  @defer.inlineCallbacks
  def test_reserved_threepid(self):
- self.hs.config.limit_usage_by_mau = True
- self.hs.config.max_mau_value = 1
+ self.auth_blocking._limit_usage_by_mau = True
+ self.auth_blocking._max_mau_value = 1
  self.store.get_monthly_active_count = lambda: defer.succeed(2)
  threepid = {"medium": "email", "address": "reserved@server.com"}
  unknown_threepid = {"medium": "email", "address": "unreserved@server.com"}
- self.hs.config.mau_limits_reserved_threepids = [threepid]
+ self.auth_blocking._mau_limits_reserved_threepids = [threepid]
 
  with self.assertRaises(ResourceLimitError):
  yield defer.ensureDeferred(self.auth.check_auth_blocking())
@@ -389,8 +393,8 @@ def test_reserved_threepid(self):
 
  @defer.inlineCallbacks
  def test_hs_disabled(self):
- self.hs.config.hs_disabled = True
- self.hs.config.hs_disabled_message = "Reason for being disabled"
+ self.auth_blocking._hs_disabled = True
+ self.auth_blocking._hs_disabled_message = "Reason for being disabled"
  with self.assertRaises(ResourceLimitError) as e:
  yield defer.ensureDeferred(self.auth.check_auth_blocking())
  self.assertEquals(e.exception.admin_contact, self.hs.config.admin_contact)
@@ -404,10 +408,10 @@ def test_hs_disabled_no_server_notices_user(self):
  """
  # 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.auth_blocking._server_notices_mxid = None
 
- self.hs.config.hs_disabled = True
- self.hs.config.hs_disabled_message = "Reason for being disabled"
+ self.auth_blocking._hs_disabled = True
+ self.auth_blocking._hs_disabled_message = "Reason for being disabled"
  with self.assertRaises(ResourceLimitError) as e:
  yield defer.ensureDeferred(self.auth.check_auth_blocking())
  self.assertEquals(e.exception.admin_contact, self.hs.config.admin_contact)
@@ -416,8 +420,8 @@ def test_hs_disabled_no_server_notices_user(self):
 
  @defer.inlineCallbacks
  def test_server_notices_mxid_special_cased(self):
- self.hs.config.hs_disabled = True
+ self.auth_blocking._hs_disabled = True
  user = "@user:server"
- self.hs.config.server_notices_mxid = user
- self.hs.config.hs_disabled_message = "Reason for being disabled"
+ self.auth_blocking._server_notices_mxid = user
+ self.auth_blocking._hs_disabled_message = "Reason for being disabled"
  yield defer.ensureDeferred(self.auth.check_auth_blocking(user))