From 8f75ac687f77f21cb8a04b656824a63861f8304b Mon Sep 17 00:00:00 2001 From: dklimpel <5740567+dklimpel@users.noreply.github.com> Date: Sat, 11 Apr 2020 22:00:25 +0200 Subject: [PATCH 1/9] Create user with admin API if MAU is reached --- changelog.d/7263.bugfix | 1 + synapse/handlers/register.py | 4 +- tests/handlers/test_register.py | 9 +- tests/rest/admin/test_user.py | 174 +++++++++++++++++++++++++++++++- 4 files changed, 183 insertions(+), 5 deletions(-) create mode 100644 changelog.d/7263.bugfix diff --git a/changelog.d/7263.bugfix b/changelog.d/7263.bugfix new file mode 100644 index 000000000000..a70378d3781c --- /dev/null +++ b/changelog.d/7263.bugfix @@ -0,0 +1 @@ +Create user with admin API if MAU limit is reached. \ No newline at end of file diff --git a/synapse/handlers/register.py b/synapse/handlers/register.py index 7ffc194f0c67..8e7e692dcbff 100644 --- a/synapse/handlers/register.py +++ b/synapse/handlers/register.py @@ -163,7 +163,9 @@ def register_user( """ yield self.check_registration_ratelimit(address) - yield self.auth.check_auth_blocking(threepid=threepid) + # do not check_auth_blocking if the call is coming through the Admin API + if address: + yield self.auth.check_auth_blocking(threepid=threepid) password_hash = None if password: password_hash = yield self._auth_handler.hash(password) diff --git a/tests/handlers/test_register.py b/tests/handlers/test_register.py index e7b638dbfe49..bd9b13515266 100644 --- a/tests/handlers/test_register.py +++ b/tests/handlers/test_register.py @@ -125,14 +125,17 @@ def test_register_mau_blocked(self): return_value=defer.succeed(self.lots_of_users) ) self.get_failure( - self.handler.register_user(localpart="local_part"), ResourceLimitError + # for MAU it must have an IP address used to perform the registration + self.handler.register_user(localpart="local_part", address="127.0.0.1"), + ResourceLimitError, ) self.store.get_monthly_active_count = Mock( return_value=defer.succeed(self.hs.config.max_mau_value) ) self.get_failure( - self.handler.register_user(localpart="local_part"), ResourceLimitError + self.handler.register_user(localpart="local_part", address="127.0.0.1"), + ResourceLimitError, ) def test_auto_create_auto_join_rooms(self): @@ -288,10 +291,12 @@ def get_or_create_user(self, requester, localpart, displayname, password_hash=No token = self.macaroon_generator.generate_access_token(user_id) if need_register: + # for MAU it must have an IP address used to perform the registration yield self.handler.register_with_store( user_id=user_id, password_hash=password_hash, create_profile_with_displayname=user.localpart, + address="127.0.0.1", ) else: yield self.hs.get_auth_handler().delete_access_tokens_for_user(user_id) diff --git a/tests/rest/admin/test_user.py b/tests/rest/admin/test_user.py index 6416fb5d2a3e..5287584ce27b 100644 --- a/tests/rest/admin/test_user.py +++ b/tests/rest/admin/test_user.py @@ -20,9 +20,13 @@ from mock import Mock +from twisted.internet import defer + import synapse.rest.admin from synapse.api.constants import UserTypes +from synapse.api.errors import Codes, HttpResponseException, ResourceLimitError from synapse.rest.client.v1 import login +from synapse.rest.client.v2_alpha import sync from tests import unittest @@ -320,6 +324,62 @@ def nonce(): self.assertEqual(400, int(channel.result["code"]), msg=channel.result["body"]) self.assertEqual("Invalid user type", channel.json_body["error"]) + def test_register_limit_mau(self): + """ + Register user if MAU limit is reached. + """ + handler = self.hs.get_registration_handler() + store = self.hs.get_datastore() + + self.hs.config.limit_usage_by_mau = True + self.hs.config.max_mau_value = 2 + self.hs.config.mau_trial_days = 0 + + # Set MAU limit + store.get_monthly_active_count = Mock( + return_value=defer.succeed(self.hs.config.max_mau_value + 1) + ) + self.get_failure( + # for MAU it must have an IP address used to perform the registration + handler.register_user(localpart="local_part", address="127.0.0.1"), + ResourceLimitError, + ) + + store.get_monthly_active_count = Mock( + return_value=defer.succeed(self.hs.config.max_mau_value + 1) + ) + self.get_failure( + handler.register_user(localpart="local_part", address="127.0.0.1"), + ResourceLimitError, + ) + + # Register new user with admin API + request, channel = self.make_request("GET", self.url) + self.render(request) + nonce = channel.json_body["nonce"] + + want_mac = hmac.new(key=b"shared", digestmod=hashlib.sha1) + want_mac.update( + nonce.encode("ascii") + b"\x00bob\x00abc123\x00admin\x00support" + ) + want_mac = want_mac.hexdigest() + + body = json.dumps( + { + "nonce": nonce, + "username": "bob", + "password": "abc123", + "admin": True, + "user_type": UserTypes.SUPPORT, + "mac": want_mac, + } + ) + request, channel = self.make_request("POST", self.url, body.encode("utf8")) + self.render(request) + + self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual("@bob:test", channel.json_body["user_id"]) + class UsersListTestCase(unittest.HomeserverTestCase): @@ -344,7 +404,7 @@ def test_no_auth(self): self.render(request) self.assertEqual(401, int(channel.result["code"]), msg=channel.result["body"]) - self.assertEqual("M_MISSING_TOKEN", channel.json_body["errcode"]) + self.assertEqual(Codes.MISSING_TOKEN, channel.json_body["errcode"]) def test_all_users(self): """ @@ -367,6 +427,7 @@ class UserRestTestCase(unittest.HomeserverTestCase): servlets = [ synapse.rest.admin.register_servlets, login.register_servlets, + sync.register_servlets, ] def prepare(self, reactor, clock, hs): @@ -418,7 +479,7 @@ def test_user_does_not_exist(self): self.render(request) self.assertEqual(404, channel.code, msg=channel.json_body) - self.assertEqual("M_NOT_FOUND", channel.json_body["errcode"]) + self.assertEqual(Codes.NOT_FOUND, channel.json_body["errcode"]) def test_create_server_admin(self): """ @@ -514,6 +575,115 @@ def test_create_user(self): self.assertEqual(False, channel.json_body["is_guest"]) self.assertEqual(False, channel.json_body["deactivated"]) + def test_create_user_limit_mau_active_admin(self): + """ + Check that a new regular user is created successfully if MAU limit is reached. + Admin user was active before creating user. + """ + self.hs.config.registration_shared_secret = None + + handler = self.hs.get_registration_handler() + + self.hs.config.limit_usage_by_mau = True + self.hs.config.max_mau_value = 2 + self.hs.config.mau_trial_days = 0 + + def _do_sync_for_user(token): + request, channel = self.make_request("GET", "/sync", access_token=token) + self.render(request) + + if channel.code != 200: + raise HttpResponseException( + channel.code, channel.result["reason"], channel.result["body"] + ).to_synapse_error() + + # Sync to set admin user to active + _do_sync_for_user(self.admin_user_tok) + + self.store.get_monthly_active_count = Mock( + return_value=defer.succeed(self.hs.config.max_mau_value + 1) + ) + + # Set MAU limit + self.get_failure( + # for MAU it must have an IP address used to perform the registration + handler.register_user(localpart="local_part", address="127.0.0.1"), + ResourceLimitError, + ) + + self.store.get_monthly_active_count = Mock( + return_value=defer.succeed(self.hs.config.max_mau_value + 1) + ) + self.get_failure( + handler.register_user(localpart="local_part", address="127.0.0.1"), + ResourceLimitError, + ) + + # Register new user with admin API + url = "/_synapse/admin/v2/users/@bob:test" + + # Create user + body = json.dumps({"password": "abc123", "admin": False}) + + request, channel = self.make_request( + "PUT", + url, + access_token=self.admin_user_tok, + content=body.encode(encoding="utf_8"), + ) + self.render(request) + + self.assertEqual(201, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual("@bob:test", channel.json_body["name"]) + self.assertEqual(False, channel.json_body["admin"]) + + def test_create_user_limit_mau_passiv_admin(self): + """ + Check that a new regular user is created successfully if MAU limit is reached. + Admin user was not active before creating user and creation fails. + """ + self.hs.config.registration_shared_secret = None + + handler = self.hs.get_registration_handler() + + self.hs.config.limit_usage_by_mau = True + self.hs.config.max_mau_value = 2 + self.hs.config.mau_trial_days = 0 + + # Set MAU limit + self.store.get_monthly_active_count = Mock( + return_value=defer.succeed(self.hs.config.max_mau_value + 1) + ) + self.get_failure( + # for MAU it must have an IP address used to perform the registration + handler.register_user(localpart="local_part", address="127.0.0.1"), + ResourceLimitError, + ) + + self.store.get_monthly_active_count = Mock( + return_value=defer.succeed(self.hs.config.max_mau_value + 1) + ) + self.get_failure( + handler.register_user(localpart="local_part", address="127.0.0.1"), + ResourceLimitError, + ) + + # Register new user with admin API + url = "/_synapse/admin/v2/users/@bob:test" + + # Create user + body = json.dumps({"password": "abc123", "admin": False}) + + request, channel = self.make_request( + "PUT", + url, + access_token=self.admin_user_tok, + content=body.encode(encoding="utf_8"), + ) + self.render(request) + + self.assertEqual(500, int(channel.result["code"]), msg=channel.result["body"]) + def test_set_password(self): """ Test setting a new password for another user. From b1ad0f0c03fd5dbf86b1f0ccda78f94aab3253cf Mon Sep 17 00:00:00 2001 From: dklimpel <5740567+dklimpel@users.noreply.github.com> Date: Wed, 29 Apr 2020 19:27:52 +0200 Subject: [PATCH 2/9] Add parameter 'by_admin' to register_user --- changelog.d/7263.bugfix | 2 +- synapse/handlers/register.py | 4 +++- synapse/rest/admin/users.py | 2 ++ 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/changelog.d/7263.bugfix b/changelog.d/7263.bugfix index a70378d3781c..3c2c95d19e9a 100644 --- a/changelog.d/7263.bugfix +++ b/changelog.d/7263.bugfix @@ -1 +1 @@ -Create user with admin API if MAU limit is reached. \ No newline at end of file +Allow new users to be registered via the admin API even if the monthly active user limit has been reached. \ No newline at end of file diff --git a/synapse/handlers/register.py b/synapse/handlers/register.py index 8e7e692dcbff..6edffb533686 100644 --- a/synapse/handlers/register.py +++ b/synapse/handlers/register.py @@ -141,6 +141,7 @@ def register_user( default_display_name=None, address=None, bind_emails=[], + by_admin=False, ): """Registers a new client on the server. @@ -156,6 +157,7 @@ def register_user( will be set to this. Defaults to 'localpart'. address (str|None): the IP address used to perform the registration. bind_emails (List[str]): list of emails to bind to this account. + by_admin (bool): Whether this registration was made by an administrator. Returns: Deferred[str]: user_id Raises: @@ -164,7 +166,7 @@ def register_user( yield self.check_registration_ratelimit(address) # do not check_auth_blocking if the call is coming through the Admin API - if address: + if not by_admin: yield self.auth.check_auth_blocking(threepid=threepid) password_hash = None if password: diff --git a/synapse/rest/admin/users.py b/synapse/rest/admin/users.py index 8551ac19b832..9a405b1ee962 100644 --- a/synapse/rest/admin/users.py +++ b/synapse/rest/admin/users.py @@ -263,6 +263,7 @@ async def on_PUT(self, request, user_id): admin=bool(admin), default_display_name=displayname, user_type=user_type, + by_admin=True, ) if "threepids" in body: @@ -410,6 +411,7 @@ async def on_POST(self, request): password=body["password"], admin=bool(admin), user_type=user_type, + by_admin=True, ) result = await register._create_registration_details(user_id, body) From 4c2d3a7296176d01bbf36b6d55b1aa3dfe8a89f0 Mon Sep 17 00:00:00 2001 From: dklimpel <5740567+dklimpel@users.noreply.github.com> Date: Wed, 29 Apr 2020 20:06:31 +0200 Subject: [PATCH 3/9] Update tests --- tests/handlers/test_register.py | 9 ++------- tests/rest/admin/test_user.py | 21 ++++++--------------- 2 files changed, 8 insertions(+), 22 deletions(-) diff --git a/tests/handlers/test_register.py b/tests/handlers/test_register.py index bd9b13515266..e7b638dbfe49 100644 --- a/tests/handlers/test_register.py +++ b/tests/handlers/test_register.py @@ -125,17 +125,14 @@ def test_register_mau_blocked(self): return_value=defer.succeed(self.lots_of_users) ) self.get_failure( - # for MAU it must have an IP address used to perform the registration - self.handler.register_user(localpart="local_part", address="127.0.0.1"), - ResourceLimitError, + self.handler.register_user(localpart="local_part"), ResourceLimitError ) self.store.get_monthly_active_count = Mock( return_value=defer.succeed(self.hs.config.max_mau_value) ) self.get_failure( - self.handler.register_user(localpart="local_part", address="127.0.0.1"), - ResourceLimitError, + self.handler.register_user(localpart="local_part"), ResourceLimitError ) def test_auto_create_auto_join_rooms(self): @@ -291,12 +288,10 @@ def get_or_create_user(self, requester, localpart, displayname, password_hash=No token = self.macaroon_generator.generate_access_token(user_id) if need_register: - # for MAU it must have an IP address used to perform the registration yield self.handler.register_with_store( user_id=user_id, password_hash=password_hash, create_profile_with_displayname=user.localpart, - address="127.0.0.1", ) else: yield self.hs.get_auth_handler().delete_access_tokens_for_user(user_id) diff --git a/tests/rest/admin/test_user.py b/tests/rest/admin/test_user.py index 5287584ce27b..4ceec55eac92 100644 --- a/tests/rest/admin/test_user.py +++ b/tests/rest/admin/test_user.py @@ -340,17 +340,14 @@ def test_register_limit_mau(self): return_value=defer.succeed(self.hs.config.max_mau_value + 1) ) self.get_failure( - # for MAU it must have an IP address used to perform the registration - handler.register_user(localpart="local_part", address="127.0.0.1"), - ResourceLimitError, + handler.register_user(localpart="local_part"), ResourceLimitError ) store.get_monthly_active_count = Mock( return_value=defer.succeed(self.hs.config.max_mau_value + 1) ) self.get_failure( - handler.register_user(localpart="local_part", address="127.0.0.1"), - ResourceLimitError, + handler.register_user(localpart="local_part"), ResourceLimitError ) # Register new user with admin API @@ -606,17 +603,14 @@ def _do_sync_for_user(token): # Set MAU limit self.get_failure( - # for MAU it must have an IP address used to perform the registration - handler.register_user(localpart="local_part", address="127.0.0.1"), - ResourceLimitError, + handler.register_user(localpart="local_part"), ResourceLimitError ) self.store.get_monthly_active_count = Mock( return_value=defer.succeed(self.hs.config.max_mau_value + 1) ) self.get_failure( - handler.register_user(localpart="local_part", address="127.0.0.1"), - ResourceLimitError, + handler.register_user(localpart="local_part"), ResourceLimitError ) # Register new user with admin API @@ -655,17 +649,14 @@ def test_create_user_limit_mau_passiv_admin(self): return_value=defer.succeed(self.hs.config.max_mau_value + 1) ) self.get_failure( - # for MAU it must have an IP address used to perform the registration - handler.register_user(localpart="local_part", address="127.0.0.1"), - ResourceLimitError, + handler.register_user(localpart="local_part"), ResourceLimitError ) self.store.get_monthly_active_count = Mock( return_value=defer.succeed(self.hs.config.max_mau_value + 1) ) self.get_failure( - handler.register_user(localpart="local_part", address="127.0.0.1"), - ResourceLimitError, + handler.register_user(localpart="local_part"), ResourceLimitError ) # Register new user with admin API From ae0eff0a808fb4ecd7b74598c641a846ac989db8 Mon Sep 17 00:00:00 2001 From: dklimpel <5740567+dklimpel@users.noreply.github.com> Date: Fri, 1 May 2020 22:28:17 +0200 Subject: [PATCH 4/9] Update tests after review --- synapse/handlers/register.py | 3 +- tests/rest/admin/test_user.py | 66 +++++++++++++---------------------- 2 files changed, 27 insertions(+), 42 deletions(-) diff --git a/synapse/handlers/register.py b/synapse/handlers/register.py index 6edffb533686..0b04408a4f16 100644 --- a/synapse/handlers/register.py +++ b/synapse/handlers/register.py @@ -157,7 +157,8 @@ def register_user( will be set to this. Defaults to 'localpart'. address (str|None): the IP address used to perform the registration. bind_emails (List[str]): list of emails to bind to this account. - by_admin (bool): Whether this registration was made by an administrator. + by_admin (bool): True if this registration is being made via the + admin api, otherwise False. Returns: Deferred[str]: user_id Raises: diff --git a/tests/rest/admin/test_user.py b/tests/rest/admin/test_user.py index 4ceec55eac92..19f97f12b0bc 100644 --- a/tests/rest/admin/test_user.py +++ b/tests/rest/admin/test_user.py @@ -324,28 +324,23 @@ def nonce(): self.assertEqual(400, int(channel.result["code"]), msg=channel.result["body"]) self.assertEqual("Invalid user type", channel.json_body["error"]) - def test_register_limit_mau(self): + def test_register_mau_limit_reached(self): """ - Register user if MAU limit is reached. + Register user with Admin API if MAU limit is reached. """ handler = self.hs.get_registration_handler() store = self.hs.get_datastore() + # Configure MAU limit self.hs.config.limit_usage_by_mau = True self.hs.config.max_mau_value = 2 self.hs.config.mau_trial_days = 0 - # Set MAU limit + # Set monthly active users to the limit store.get_monthly_active_count = Mock( - return_value=defer.succeed(self.hs.config.max_mau_value + 1) - ) - self.get_failure( - handler.register_user(localpart="local_part"), ResourceLimitError - ) - - store.get_monthly_active_count = Mock( - return_value=defer.succeed(self.hs.config.max_mau_value + 1) + return_value=defer.succeed(self.hs.config.max_mau_value) ) + # The registration of a new user fails due to the limit self.get_failure( handler.register_user(localpart="local_part"), ResourceLimitError ) @@ -572,7 +567,7 @@ def test_create_user(self): self.assertEqual(False, channel.json_body["is_guest"]) self.assertEqual(False, channel.json_body["deactivated"]) - def test_create_user_limit_mau_active_admin(self): + def test_create_user_mau_limit_reached_active_admin(self): """ Check that a new regular user is created successfully if MAU limit is reached. Admin user was active before creating user. @@ -581,34 +576,28 @@ def test_create_user_limit_mau_active_admin(self): handler = self.hs.get_registration_handler() + # Configure MAU limit self.hs.config.limit_usage_by_mau = True self.hs.config.max_mau_value = 2 self.hs.config.mau_trial_days = 0 - def _do_sync_for_user(token): - request, channel = self.make_request("GET", "/sync", access_token=token) - self.render(request) - - if channel.code != 200: - raise HttpResponseException( - channel.code, channel.result["reason"], channel.result["body"] - ).to_synapse_error() - # Sync to set admin user to active - _do_sync_for_user(self.admin_user_tok) - - self.store.get_monthly_active_count = Mock( - return_value=defer.succeed(self.hs.config.max_mau_value + 1) + # before limit of monthly active users is reached + request, channel = self.make_request( + "GET", "/sync", access_token=self.admin_user_tok ) + self.render(request) - # Set MAU limit - self.get_failure( - handler.register_user(localpart="local_part"), ResourceLimitError - ) + if channel.code != 200: + raise HttpResponseException( + channel.code, channel.result["reason"], channel.result["body"] + ).to_synapse_error() + # Set monthly active users to the limit self.store.get_monthly_active_count = Mock( - return_value=defer.succeed(self.hs.config.max_mau_value + 1) + return_value=defer.succeed(self.hs.config.max_mau_value) ) + # The registration of a new user fails due to the limit self.get_failure( handler.register_user(localpart="local_part"), ResourceLimitError ) @@ -631,30 +620,25 @@ def _do_sync_for_user(token): self.assertEqual("@bob:test", channel.json_body["name"]) self.assertEqual(False, channel.json_body["admin"]) - def test_create_user_limit_mau_passiv_admin(self): + def test_create_user_mau_limit_reached_passiv_admin(self): """ - Check that a new regular user is created successfully if MAU limit is reached. + Try to create a new regular user if MAU limit is reached. Admin user was not active before creating user and creation fails. """ self.hs.config.registration_shared_secret = None handler = self.hs.get_registration_handler() + # Configure MAU limit self.hs.config.limit_usage_by_mau = True self.hs.config.max_mau_value = 2 self.hs.config.mau_trial_days = 0 - # Set MAU limit - self.store.get_monthly_active_count = Mock( - return_value=defer.succeed(self.hs.config.max_mau_value + 1) - ) - self.get_failure( - handler.register_user(localpart="local_part"), ResourceLimitError - ) - + # Set monthly active users to the limit self.store.get_monthly_active_count = Mock( - return_value=defer.succeed(self.hs.config.max_mau_value + 1) + return_value=defer.succeed(self.hs.config.max_mau_value) ) + # The registration of a new user fails due to the limit self.get_failure( handler.register_user(localpart="local_part"), ResourceLimitError ) From b3b347bcd2e3574f0202ea5c249d35674f8fa7f4 Mon Sep 17 00:00:00 2001 From: dklimpel <5740567+dklimpel@users.noreply.github.com> Date: Tue, 12 May 2020 22:04:27 +0200 Subject: [PATCH 5/9] Update MAU limit tests Update because of changes in "Stop Auth methods from polling the config on every req. (#7420)" --- tests/rest/admin/test_user.py | 34 ++++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/tests/rest/admin/test_user.py b/tests/rest/admin/test_user.py index 457635fb3213..782e7e644470 100644 --- a/tests/rest/admin/test_user.py +++ b/tests/rest/admin/test_user.py @@ -330,15 +330,16 @@ def test_register_mau_limit_reached(self): """ handler = self.hs.get_registration_handler() store = self.hs.get_datastore() + auth_blocking = self.hs.get_auth()._auth_blocking # Configure MAU limit - self.hs.config.limit_usage_by_mau = True - self.hs.config.max_mau_value = 2 - self.hs.config.mau_trial_days = 0 + auth_blocking._limit_usage_by_mau = True + auth_blocking._max_mau_value = 2 + auth_blocking._mau_trial_days = 0 # Set monthly active users to the limit store.get_monthly_active_count = Mock( - return_value=defer.succeed(self.hs.config.max_mau_value) + return_value=defer.succeed(auth_blocking._max_mau_value) ) # The registration of a new user fails due to the limit self.get_failure( @@ -577,11 +578,12 @@ def test_create_user_mau_limit_reached_active_admin(self): self.hs.config.registration_shared_secret = None handler = self.hs.get_registration_handler() + auth_blocking = self.hs.get_auth()._auth_blocking # Configure MAU limit - self.hs.config.limit_usage_by_mau = True - self.hs.config.max_mau_value = 2 - self.hs.config.mau_trial_days = 0 + auth_blocking._limit_usage_by_mau = True + auth_blocking._max_mau_value = 2 + auth_blocking._mau_trial_days = 0 # Sync to set admin user to active # before limit of monthly active users is reached @@ -597,7 +599,7 @@ def test_create_user_mau_limit_reached_active_admin(self): # Set monthly active users to the limit self.store.get_monthly_active_count = Mock( - return_value=defer.succeed(self.hs.config.max_mau_value) + return_value=defer.succeed(auth_blocking._max_mau_value) ) # The registration of a new user fails due to the limit self.get_failure( @@ -625,20 +627,21 @@ def test_create_user_mau_limit_reached_active_admin(self): def test_create_user_mau_limit_reached_passiv_admin(self): """ Try to create a new regular user if MAU limit is reached. - Admin user was not active before creating user and creation fails. + Admin user was not active before creating user. """ self.hs.config.registration_shared_secret = None handler = self.hs.get_registration_handler() + auth_blocking = self.hs.get_auth()._auth_blocking # Configure MAU limit - self.hs.config.limit_usage_by_mau = True - self.hs.config.max_mau_value = 2 - self.hs.config.mau_trial_days = 0 + auth_blocking._limit_usage_by_mau = True + auth_blocking._max_mau_value = 2 + auth_blocking._mau_trial_days = 0 # Set monthly active users to the limit self.store.get_monthly_active_count = Mock( - return_value=defer.succeed(self.hs.config.max_mau_value) + return_value=defer.succeed(auth_blocking._max_mau_value) ) # The registration of a new user fails due to the limit self.get_failure( @@ -659,7 +662,10 @@ def test_create_user_mau_limit_reached_passiv_admin(self): ) self.render(request) - self.assertEqual(500, int(channel.result["code"]), msg=channel.result["body"]) + # Admin user is not blocked by mau anymore + self.assertEqual(201, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual("@bob:test", channel.json_body["name"]) + self.assertEqual(False, channel.json_body["admin"]) def test_set_password(self): """ From cf1c9eab6d36219f9209b5576ea3943cfde5c641 Mon Sep 17 00:00:00 2001 From: dklimpel <5740567+dklimpel@users.noreply.github.com> Date: Tue, 2 Jun 2020 23:00:56 +0200 Subject: [PATCH 6/9] Apply suggestions from code review --- tests/rest/admin/test_user.py | 100 ++++++++++++++++++---------------- 1 file changed, 52 insertions(+), 48 deletions(-) diff --git a/tests/rest/admin/test_user.py b/tests/rest/admin/test_user.py index 210b0e520167..1caec10c9163 100644 --- a/tests/rest/admin/test_user.py +++ b/tests/rest/admin/test_user.py @@ -20,15 +20,14 @@ from mock import Mock -from twisted.internet import defer - import synapse.rest.admin from synapse.api.constants import UserTypes -from synapse.api.errors import Codes, HttpResponseException, ResourceLimitError +from synapse.api.errors import HttpResponseException, ResourceLimitError from synapse.rest.client.v1 import login from synapse.rest.client.v2_alpha import sync from tests import unittest +from tests.unittest import override_config class UserRegisterTestCase(unittest.HomeserverTestCase): @@ -324,23 +323,22 @@ def nonce(): self.assertEqual(400, int(channel.result["code"]), msg=channel.result["body"]) self.assertEqual("Invalid user type", channel.json_body["error"]) + @override_config( + {"limit_usage_by_mau": True, "max_mau_value": 2, "mau_trial_days": 0} + ) def test_register_mau_limit_reached(self): """ - Register user with Admin API if MAU limit is reached. + Check we can register a user via the shared secret registration API + even if the MAU limit is reached. """ handler = self.hs.get_registration_handler() store = self.hs.get_datastore() - auth_blocking = self.hs.get_auth()._auth_blocking - - # Configure MAU limit - auth_blocking._limit_usage_by_mau = True - auth_blocking._max_mau_value = 2 - auth_blocking._mau_trial_days = 0 # Set monthly active users to the limit store.get_monthly_active_count = Mock( - return_value=defer.succeed(auth_blocking._max_mau_value) + return_value=self.hs.config.max_mau_value ) + # Check that the blocking of monthly active users is working as expected # The registration of a new user fails due to the limit self.get_failure( handler.register_user(localpart="local_part"), ResourceLimitError @@ -397,7 +395,7 @@ def test_no_auth(self): self.render(request) self.assertEqual(401, int(channel.result["code"]), msg=channel.result["body"]) - self.assertEqual(Codes.MISSING_TOKEN, channel.json_body["errcode"]) + self.assertEqual("M_MISSING_TOKEN", channel.json_body["errcode"]) def test_all_users(self): """ @@ -424,6 +422,11 @@ class UserRestTestCase(unittest.HomeserverTestCase): sync.register_servlets, ] + def default_config(self): + config = super().default_config() + config.update({"registration_shared_secret": None}) + return config + def prepare(self, reactor, clock, hs): self.store = hs.get_datastore() @@ -440,7 +443,6 @@ def test_requester_is_no_admin(self): """ If the user is not a server admin, an error is returned. """ - self.hs.config.registration_shared_secret = None url = "/_synapse/admin/v2/users/@bob:test" request, channel = self.make_request( @@ -463,7 +465,6 @@ def test_user_does_not_exist(self): """ Tests that a lookup for a user that does not exist returns a 404 """ - self.hs.config.registration_shared_secret = None request, channel = self.make_request( "GET", @@ -473,13 +474,12 @@ def test_user_does_not_exist(self): self.render(request) self.assertEqual(404, channel.code, msg=channel.json_body) - self.assertEqual(Codes.NOT_FOUND, channel.json_body["errcode"]) + self.assertEqual("M_NOT_FOUND", channel.json_body["errcode"]) def test_create_server_admin(self): """ Check that a new admin user is created successfully. """ - self.hs.config.registration_shared_secret = None url = "/_synapse/admin/v2/users/@bob:test" # Create user (server admin) @@ -527,7 +527,6 @@ def test_create_user(self): """ Check that a new regular user is created successfully. """ - self.hs.config.registration_shared_secret = None url = "/_synapse/admin/v2/users/@bob:test" # Create user @@ -570,20 +569,17 @@ def test_create_user(self): self.assertEqual(False, channel.json_body["is_guest"]) self.assertEqual(False, channel.json_body["deactivated"]) + @override_config( + {"limit_usage_by_mau": True, "max_mau_value": 2, "mau_trial_days": 0} + ) def test_create_user_mau_limit_reached_active_admin(self): """ - Check that a new regular user is created successfully if MAU limit is reached. + Check that an admin can register a new user via the admin API + even if the MAU limit is reached. Admin user was active before creating user. """ - self.hs.config.registration_shared_secret = None handler = self.hs.get_registration_handler() - auth_blocking = self.hs.get_auth()._auth_blocking - - # Configure MAU limit - auth_blocking._limit_usage_by_mau = True - auth_blocking._max_mau_value = 2 - auth_blocking._mau_trial_days = 0 # Sync to set admin user to active # before limit of monthly active users is reached @@ -595,12 +591,13 @@ def test_create_user_mau_limit_reached_active_admin(self): if channel.code != 200: raise HttpResponseException( channel.code, channel.result["reason"], channel.result["body"] - ).to_synapse_error() + ) # Set monthly active users to the limit self.store.get_monthly_active_count = Mock( - return_value=defer.succeed(auth_blocking._max_mau_value) + return_value=self.hs.config.max_mau_value ) + # Check that the blocking of monthly active users is working as expected # The registration of a new user fails due to the limit self.get_failure( handler.register_user(localpart="local_part"), ResourceLimitError @@ -624,25 +621,23 @@ def test_create_user_mau_limit_reached_active_admin(self): self.assertEqual("@bob:test", channel.json_body["name"]) self.assertEqual(False, channel.json_body["admin"]) - def test_create_user_mau_limit_reached_passiv_admin(self): + @override_config( + {"limit_usage_by_mau": True, "max_mau_value": 2, "mau_trial_days": 0} + ) + def test_create_user_mau_limit_reached_passive_admin(self): """ - Try to create a new regular user if MAU limit is reached. + Check that an admin can register a new user via the admin API + even if the MAU limit is reached. Admin user was not active before creating user. """ - self.hs.config.registration_shared_secret = None handler = self.hs.get_registration_handler() - auth_blocking = self.hs.get_auth()._auth_blocking - - # Configure MAU limit - auth_blocking._limit_usage_by_mau = True - auth_blocking._max_mau_value = 2 - auth_blocking._mau_trial_days = 0 # Set monthly active users to the limit self.store.get_monthly_active_count = Mock( - return_value=defer.succeed(auth_blocking._max_mau_value) + return_value=self.hs.config.max_mau_value ) + # Check that the blocking of monthly active users is working as expected # The registration of a new user fails due to the limit self.get_failure( handler.register_user(localpart="local_part"), ResourceLimitError @@ -667,14 +662,21 @@ def test_create_user_mau_limit_reached_passiv_admin(self): self.assertEqual("@bob:test", channel.json_body["name"]) self.assertEqual(False, channel.json_body["admin"]) + @override_config( + { + "email": { + "enable_notifs": True, + "notif_for_new_users": True, + "notif_from": "test@example.com", + }, + "public_baseurl": "https://example.com", + } + ) def test_create_user_email_notif_for_new_users(self): """ Check that a new regular user is created successfully and got an email pusher. """ - self.hs.config.registration_shared_secret = None - self.hs.config.email_enable_notifs = True - self.hs.config.email_notif_for_new_users = True url = "/_synapse/admin/v2/users/@bob:test" # Create user @@ -705,14 +707,21 @@ def test_create_user_email_notif_for_new_users(self): self.assertEqual(len(pushers), 1) self.assertEqual("@bob:test", pushers[0]["user_name"]) + @override_config( + { + "email": { + "enable_notifs": False, + "notif_for_new_users": False, + "notif_from": "test@example.com", + }, + "public_baseurl": "https://example.com", + } + ) def test_create_user_email_no_notif_for_new_users(self): """ Check that a new regular user is created successfully and got not an email pusher. """ - self.hs.config.registration_shared_secret = None - self.hs.config.email_enable_notifs = False - self.hs.config.email_notif_for_new_users = False url = "/_synapse/admin/v2/users/@bob:test" # Create user @@ -746,7 +755,6 @@ def test_set_password(self): """ Test setting a new password for another user. """ - self.hs.config.registration_shared_secret = None # Change password body = json.dumps({"password": "hahaha"}) @@ -765,7 +773,6 @@ def test_set_displayname(self): """ Test setting the displayname of another user. """ - self.hs.config.registration_shared_secret = None # Modify user body = json.dumps({"displayname": "foobar"}) @@ -796,7 +803,6 @@ def test_set_threepid(self): """ Test setting threepid for an other user. """ - self.hs.config.registration_shared_secret = None # Delete old and add new threepid to user body = json.dumps( @@ -862,7 +868,6 @@ def test_set_user_as_admin(self): """ Test setting the admin flag on a user. """ - self.hs.config.registration_shared_secret = None # Set a user as an admin body = json.dumps({"admin": True}) @@ -894,7 +899,6 @@ def test_accidental_deactivation_prevention(self): Ensure an account can't accidentally be deactivated by using a str value for the deactivated body parameter """ - self.hs.config.registration_shared_secret = None url = "/_synapse/admin/v2/users/@bob:test" # Create user From b9d9bec0effa281de78c871f98f447047fa571f5 Mon Sep 17 00:00:00 2001 From: dklimpel <5740567+dklimpel@users.noreply.github.com> Date: Tue, 2 Jun 2020 23:11:29 +0200 Subject: [PATCH 7/9] code style --- tests/rest/admin/test_user.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/rest/admin/test_user.py b/tests/rest/admin/test_user.py index 1caec10c9163..46e99f77261c 100644 --- a/tests/rest/admin/test_user.py +++ b/tests/rest/admin/test_user.py @@ -335,9 +335,7 @@ def test_register_mau_limit_reached(self): store = self.hs.get_datastore() # Set monthly active users to the limit - store.get_monthly_active_count = Mock( - return_value=self.hs.config.max_mau_value - ) + store.get_monthly_active_count = Mock(return_value=self.hs.config.max_mau_value) # Check that the blocking of monthly active users is working as expected # The registration of a new user fails due to the limit self.get_failure( From d3b0a827a62d79a898f3c32d4b14cfedd4dccf02 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Fri, 5 Jun 2020 11:50:44 +0100 Subject: [PATCH 8/9] Update tests/rest/admin/test_user.py --- tests/rest/admin/test_user.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/tests/rest/admin/test_user.py b/tests/rest/admin/test_user.py index 46e99f77261c..cca5f548e6ad 100644 --- a/tests/rest/admin/test_user.py +++ b/tests/rest/admin/test_user.py @@ -420,11 +420,6 @@ class UserRestTestCase(unittest.HomeserverTestCase): sync.register_servlets, ] - def default_config(self): - config = super().default_config() - config.update({"registration_shared_secret": None}) - return config - def prepare(self, reactor, clock, hs): self.store = hs.get_datastore() From d47b564ad34cc22635d2382eb49b1aed93b3f4bb Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Fri, 5 Jun 2020 12:05:45 +0100 Subject: [PATCH 9/9] Update 7263.bugfix --- changelog.d/7263.bugfix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/7263.bugfix b/changelog.d/7263.bugfix index 3c2c95d19e9a..0b4739261ca1 100644 --- a/changelog.d/7263.bugfix +++ b/changelog.d/7263.bugfix @@ -1 +1 @@ -Allow new users to be registered via the admin API even if the monthly active user limit has been reached. \ No newline at end of file +Allow new users to be registered via the admin API even if the monthly active user limit has been reached. Contributed by @dkimpel.