From 886be75ad1bc60e016611b453b9644e8db17a9f1 Mon Sep 17 00:00:00 2001
From: Neil Johnson <neil@fragile.org.uk>
Date: Fri, 3 Aug 2018 22:29:03 +0100
Subject: bug fixes

---
 synapse/handlers/auth.py                | 15 ++-------------
 synapse/handlers/register.py            |  8 ++++----
 synapse/storage/monthly_active_users.py |  3 +--
 tests/api/test_auth.py                  | 10 +++-------
 tests/handlers/test_register.py         |  1 -
 5 files changed, 10 insertions(+), 27 deletions(-)

diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py
index 8f9cff92e8..7ea8ce9f94 100644
--- a/synapse/handlers/auth.py
+++ b/synapse/handlers/auth.py
@@ -520,7 +520,7 @@ class AuthHandler(BaseHandler):
         """
         logger.info("Logging in user %s on device %s", user_id, device_id)
         access_token = yield self.issue_access_token(user_id, device_id)
-        yield self._check_mau_limits()
+        yield self.auth.check_auth_blocking()
 
         # the device *should* have been registered before we got here; however,
         # it's possible we raced against a DELETE operation. The thing we
@@ -734,7 +734,7 @@ class AuthHandler(BaseHandler):
 
     @defer.inlineCallbacks
     def validate_short_term_login_token_and_get_user_id(self, login_token):
-        yield self._check_mau_limits()
+        yield self.auth.check_auth_blocking()
         auth_api = self.hs.get_auth()
         user_id = None
         try:
@@ -907,17 +907,6 @@ class AuthHandler(BaseHandler):
         else:
             return defer.succeed(False)
 
-    @defer.inlineCallbacks
-    def _check_mau_limits(self):
-        """
-        Ensure that if mau blocking is enabled that invalid users cannot
-        log in.
-        """
-        error = AuthError(
-            403, "Monthly Active User limits exceeded", errcode=Codes.MAU_LIMIT_EXCEEDED
-        )
-        yield self.auth.check_auth_blocking(error)
-
 
 @attr.s
 class MacaroonGenerator(object):
diff --git a/synapse/handlers/register.py b/synapse/handlers/register.py
index 706ed8c292..8cf0a36a8f 100644
--- a/synapse/handlers/register.py
+++ b/synapse/handlers/register.py
@@ -540,7 +540,7 @@ class RegistrationHandler(BaseHandler):
         Do not accept registrations if monthly active user limits exceeded
          and limiting is enabled
         """
-        error = RegistrationError(
-            403, "Monthly Active User limits exceeded", errcode=Codes.MAU_LIMIT_EXCEEDED
-        )
-        yield self.auth.check_auth_blocking(error)
+        try:
+            yield self.auth.check_auth_blocking()
+        except AuthError as e:
+            raise RegistrationError(e.code, e.message, e.errcode)
diff --git a/synapse/storage/monthly_active_users.py b/synapse/storage/monthly_active_users.py
index 6def6830d0..135837507a 100644
--- a/synapse/storage/monthly_active_users.py
+++ b/synapse/storage/monthly_active_users.py
@@ -54,7 +54,7 @@ class MonthlyActiveUsersStore(SQLBaseStore):
                 """
             txn.execute(sql, (self.hs.config.max_mau_value,))
 
-        res = yield self.runInteraction("reap_monthly_active_users", _reap_users)
+        yield self.runInteraction("reap_monthly_active_users", _reap_users)
         # It seems poor to invalidate the whole cache, Postgres supports
         # 'Returning' which would allow me to invalidate only the
         # specific users, but sqlite has no way to do this and instead
@@ -64,7 +64,6 @@ class MonthlyActiveUsersStore(SQLBaseStore):
         # something about it if and when the perf becomes significant
         self._user_last_seen_monthly_active.invalidate_all()
         self.get_monthly_active_count.invalidate_all()
-        return res
 
     @cached(num_args=0)
     def get_monthly_active_count(self):
diff --git a/tests/api/test_auth.py b/tests/api/test_auth.py
index 54bdf28663..e963963c73 100644
--- a/tests/api/test_auth.py
+++ b/tests/api/test_auth.py
@@ -452,12 +452,8 @@ class AuthTestCase(unittest.TestCase):
         lots_of_users = 100
         small_number_of_users = 1
 
-        error = AuthError(
-            403, "MAU Limit Exceeded", errcode=Codes.MAU_LIMIT_EXCEEDED
-        )
-
         # Ensure no error thrown
-        yield self.auth.check_auth_blocking(error)
+        yield self.auth.check_auth_blocking()
 
         self.hs.config.limit_usage_by_mau = True
 
@@ -466,10 +462,10 @@ class AuthTestCase(unittest.TestCase):
         )
 
         with self.assertRaises(AuthError):
-            yield self.auth.check_auth_blocking(error)
+            yield self.auth.check_auth_blocking()
 
         # Ensure does not throw an error
         self.store.get_monthly_active_count = Mock(
             return_value=defer.succeed(small_number_of_users)
         )
-        yield self.auth.check_auth_blocking(error)
+        yield self.auth.check_auth_blocking()
diff --git a/tests/handlers/test_register.py b/tests/handlers/test_register.py
index 6b5b8b3772..4ea59a58de 100644
--- a/tests/handlers/test_register.py
+++ b/tests/handlers/test_register.py
@@ -104,7 +104,6 @@ class RegistrationTestCase(unittest.TestCase):
         self.store.get_monthly_active_count = Mock(
             return_value=defer.succeed(self.lots_of_users)
         )
-
         with self.assertRaises(RegistrationError):
             yield self.handler.get_or_create_user("requester", 'b', "display_name")
 
-- 
cgit 1.4.1