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.
7 changes: 6 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,18 @@ 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): True if this registration is being made via the
admin api, otherwise False.
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)

if localpart is not None:
yield self.check_username(localpart, guest_access_token=guest_access_token)
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 @@ -270,6 +270,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 @@ -432,6 +433,7 @@ async def on_POST(self, request):
password_hash=password_hash,
admin=bool(admin),
user_type=user_type,
by_admin=True,
)

result = await register._create_registration_details(user_id, body)
Expand Down
155 changes: 153 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,55 @@ 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_mau_limit_reached(self):
"""
Register user with Admin API if MAU limit is reached.
dklimpel marked this conversation as resolved.
Show resolved Hide resolved
"""
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
dklimpel marked this conversation as resolved.
Show resolved Hide resolved

# Set monthly active users to the limit
store.get_monthly_active_count = Mock(
return_value=defer.succeed(auth_blocking._max_mau_value)
)
# The registration of a new user fails due to the limit
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 +397,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 @@ -368,6 +421,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 @@ -419,7 +473,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 @@ -516,6 +570,103 @@ def test_create_user(self):
self.assertEqual(False, channel.json_body["is_guest"])
self.assertEqual(False, channel.json_body["deactivated"])

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

# Sync to set admin user to active
# before limit of monthly active users is reached
request, channel = self.make_request(
"GET", "/sync", access_token=self.admin_user_tok
)
self.render(request)

if channel.code != 200:
raise HttpResponseException(
channel.code, channel.result["reason"], channel.result["body"]
).to_synapse_error()
dklimpel marked this conversation as resolved.
Show resolved Hide resolved

# Set monthly active users to the limit
self.store.get_monthly_active_count = Mock(
return_value=defer.succeed(auth_blocking._max_mau_value)
)
# The registration of a new user fails due to the limit
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(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_mau_limit_reached_passiv_admin(self):
dklimpel marked this conversation as resolved.
Show resolved Hide resolved
"""
Try to create a new regular user if MAU limit is reached.
dklimpel marked this conversation as resolved.
Show resolved Hide resolved
Admin user was not active before creating user.
Copy link
Member

Choose a reason for hiding this comment

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

I'm still a bit baffled by why we need this as well as the previous test. Is it really likely that an inactive admin could be blocked from creating a new user?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The test is presented for completeness. It can be left out. We deal with mau. And it would be possible that an active and passive admin/user get different results.

"""
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)
)
# The registration of a new user fails due to the limit
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)

# 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_create_user_email_notif_for_new_users(self):
"""
Check that a new regular user is created successfully and
Expand Down