move threepid checker to config, add missing yields

6 files changed, 41 insertions, 21 deletions

diff --git a/synapse/api/auth.py b/synapse/api/auth.py
index a36fb6b3bd..a89687f420 100644
--- a/synapse/api/auth.py
+++ b/synapse/api/auth.py
@@ -26,6 +26,7 @@ import synapse.types
 from synapse import event_auth
 from synapse.api.constants import EventTypes, JoinRules, Membership
 from synapse.api.errors import AuthError, Codes, ResourceLimitError
+from synapse.config.server import is_threepid_reserved
 from synapse.types import UserID
 from synapse.util.caches import CACHE_SIZE_FACTOR, register_cache
 from synapse.util.caches.lrucache import LruCache
@@ -782,6 +783,11 @@ class Auth(object):
         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.
         """
 
         # Never fail an auth check for the server notices users
@@ -797,6 +803,10 @@ class Auth(object):
                 limit_type=self.hs.config.hs_disabled_limit_type
             )
         if self.hs.config.limit_usage_by_mau is True:
+
+            if user_id and threepid:
+                logger.warn("Called with both user_id and threepid, this shoudn't happen")
+
             # 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)
@@ -809,14 +819,13 @@ class Auth(object):
             elif threepid:
                 # If the user does not exist yet, but is signing up with a
                 # reserved threepid then pass auth check
-                if self.store.is_threepid_reserved(threepid):
+                if is_threepid_reserved(self.hs.config, threepid):
                     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="monthly_active_user"
diff --git a/synapse/config/server.py b/synapse/config/server.py
index 2faf472189..c1c7c0105e 100644
--- a/synapse/config/server.py
+++ b/synapse/config/server.py
@@ -404,6 +404,23 @@ class ServerConfig(Config):
                                   " service on the given port.")
 
 
+def is_threepid_reserved(config, threepid):
+    """Check the threepid against the reserved threepid config
+    Args:
+        config(ServerConfig) - to access server config attributes
+        threepid(dict) - The threepid to test for
+
+    Returns:
+        boolean Is the threepid undertest reserved_user
+    """
+
+    for tp in config.mau_limits_reserved_threepids:
+        if (threepid['medium'] == tp['medium']
+                and threepid['address'] == tp['address']):
+            return True
+    return False
+
+
 def read_gc_thresholds(thresholds):
     """Reads the three integer thresholds for garbage collection. Ensures that
     the thresholds are integers if thresholds are supplied.
diff --git a/synapse/rest/client/v1_only/register.py b/synapse/rest/client/v1_only/register.py
index 95873e03d5..dadb376b02 100644
--- a/synapse/rest/client/v1_only/register.py
+++ b/synapse/rest/client/v1_only/register.py
@@ -23,6 +23,7 @@ from twisted.internet import defer
 import synapse.util.stringutils as stringutils
 from synapse.api.constants import LoginType
 from synapse.api.errors import Codes, SynapseError
+from synapse.config.server import is_threepid_reserved
 from synapse.http.servlet import assert_params_in_dict, parse_json_object_from_request
 from synapse.rest.client.v1.base import ClientV1RestServlet
 from synapse.types import create_requester
@@ -282,7 +283,7 @@ class RegisterRestServlet(ClientV1RestServlet):
             if "user" in register_json else None
         )
         threepid = None
-        if session[LoginType.EMAIL_IDENTITY]:
+        if session.get(LoginType.EMAIL_IDENTITY):
             threepid = session["threepidCreds"]
 
         handler = self.handlers.registration_handler
@@ -293,8 +294,8 @@ class RegisterRestServlet(ClientV1RestServlet):
         )
         # Necessary due to auth checks prior to the threepid being
         # written to the db
-        if self.store.is_threepid_reserved(threepid):
-            self.store.upsert_monthly_active_user(registered_user_id)
+        if is_threepid_reserved(self.hs.config, threepid):
+            yield self.store.upsert_monthly_active_user(user_id)
 
         if session[LoginType.EMAIL_IDENTITY]:
             logger.debug("Binding emails %s to %s" % (
diff --git a/synapse/rest/client/v2_alpha/register.py b/synapse/rest/client/v2_alpha/register.py
index f22b7577ea..2fb4d43ccb 100644
--- a/synapse/rest/client/v2_alpha/register.py
+++ b/synapse/rest/client/v2_alpha/register.py
@@ -26,6 +26,7 @@ import synapse
 import synapse.types
 from synapse.api.constants import LoginType
 from synapse.api.errors import Codes, SynapseError, UnrecognizedRequestError
+from synapse.config.server import is_threepid_reserved
 from synapse.http.servlet import (
     RestServlet,
     assert_params_in_dict,
@@ -408,8 +409,8 @@ class RegisterRestServlet(RestServlet):
             )
             # Necessary due to auth checks prior to the threepid being
             # written to the db
-            if self.store.is_threepid_reserved(threepid):
-                self.store.upsert_monthly_active_user(registered_user_id)
+            if is_threepid_reserved(self.hs.config, threepid):
+                yield self.store.upsert_monthly_active_user(registered_user_id)
 
             # remember that we've now registered that user account, and with
             #  what user ID (since the user may not have specified)
diff --git a/synapse/storage/monthly_active_users.py b/synapse/storage/monthly_active_users.py
index 173867c4b1..c7899d7fd2 100644
--- a/synapse/storage/monthly_active_users.py
+++ b/synapse/storage/monthly_active_users.py
@@ -219,17 +219,3 @@ class MonthlyActiveUsersStore(SQLBaseStore):
                     yield self.upsert_monthly_active_user(user_id)
             elif now - last_seen_timestamp > LAST_SEEN_GRANULARITY:
                 yield self.upsert_monthly_active_user(user_id)
-
-    def is_threepid_reserved(self, threepid):
-        """Check the threepid against the reserved threepid config
-        Args:
-            threepid(dict) - The threepid to test for
-        Returns:
-            boolean Is the threepid undertest reserved_user
-        """
-        for tp in self.hs.config.mau_limits_reserved_threepids:
-            if (threepid['medium'] == tp['medium']
-                    and threepid['address'] == tp['address']):
-                return True
-            else:
-                return False
diff --git a/tests/utils.py b/tests/utils.py
index 179b592501..63e30dc6c0 100644
--- a/tests/utils.py
+++ b/tests/utils.py
@@ -26,6 +26,7 @@ from twisted.internet import defer, reactor
 
 from synapse.api.constants import EventTypes
 from synapse.api.errors import CodeMessageException, cs_error
+from synapse.config.server import ServerConfig
 from synapse.federation.transport import server
 from synapse.http.server import HttpServer
 from synapse.server import HomeServer
@@ -158,6 +159,11 @@ def setup_test_homeserver(
         # background, which upsets the test runner.
         config.update_user_directory = False
 
+        def is_threepid_reserved(threepid):
+            return ServerConfig.is_threepid_reserved(config, threepid)
+
+        config.is_threepid_reserved.side_effect = is_threepid_reserved
+
     config.use_frozen_dicts = True
     config.ldap_enabled = False