Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Allow new users to be registered via the admin API even if the monthly active user limit has been reached #7263

Merged
merged 13 commits into from
Jun 5, 2020
1 change: 1 addition & 0 deletions changelog.d/7263.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Allow new users to be registered via the admin API even if the monthly active user limit has been reached.
6 changes: 5 additions & 1 deletion synapse/handlers/register.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand All @@ -156,14 +157,17 @@ 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.
dklimpel marked this conversation as resolved.
Show resolved Hide resolved
Returns:
Deferred[str]: user_id
Raises:
SynapseError if there was a problem registering.
"""
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 not by_admin:
yield self.auth.check_auth_blocking(threepid=threepid)
password_hash = None
if password:
password_hash = yield self._auth_handler.hash(password)
Expand Down
2 changes: 2 additions & 0 deletions synapse/rest/admin/users.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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)
Expand Down
165 changes: 163 additions & 2 deletions tests/rest/admin/test_user.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -320,6 +324,59 @@ 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(
handler.register_user(localpart="local_part"), ResourceLimitError
)
dklimpel marked this conversation as resolved.
Show resolved Hide resolved

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
)

dklimpel marked this conversation as resolved.
Show resolved Hide resolved
# 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):

Expand All @@ -344,7 +401,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"])
dklimpel marked this conversation as resolved.
Show resolved Hide resolved

def test_all_users(self):
"""
Expand All @@ -367,6 +424,7 @@ class UserRestTestCase(unittest.HomeserverTestCase):
servlets = [
synapse.rest.admin.register_servlets,
login.register_servlets,
sync.register_servlets,
]

def prepare(self, reactor, clock, hs):
Expand Down Expand Up @@ -418,7 +476,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"])
dklimpel marked this conversation as resolved.
Show resolved Hide resolved

def test_create_server_admin(self):
"""
Expand Down Expand Up @@ -514,6 +572,109 @@ 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.
dklimpel marked this conversation as resolved.
Show resolved Hide resolved
Admin user was active before creating user.
"""
self.hs.config.registration_shared_secret = None
dklimpel marked this conversation as resolved.
Show resolved Hide resolved

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)
dklimpel marked this conversation as resolved.
Show resolved Hide resolved

self.store.get_monthly_active_count = Mock(
return_value=defer.succeed(self.hs.config.max_mau_value + 1)
)

# Set MAU limit
self.get_failure(
handler.register_user(localpart="local_part"), ResourceLimitError
)
dklimpel marked this conversation as resolved.
Show resolved Hide resolved

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
)
dklimpel marked this conversation as resolved.
Show resolved Hide resolved

# 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(
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"), 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"])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

help me out here: what is the difference between these tests and the ones further up at line 327 etc? what is the purpose of calling sync etc?

Perhaps some more comments to explain the test strategy and whwhat we're trying to do would be helpful.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added more comments.
The difference is the sync.
If admin did a sync before mau limit is reached, the admin can send POST requests and create new users.
If admin did not a sync before limit is reached, the admin can not create new users, because the admin can not send POST requests. The admin is limited like a normal user.


def test_set_password(self):
"""
Test setting a new password for another user.
Expand Down