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
183 changes: 168 additions & 15 deletions tests/rest/admin/test_user.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,12 @@

import synapse.rest.admin
from synapse.api.constants import UserTypes
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):
Expand Down Expand Up @@ -320,6 +323,52 @@ 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):
"""
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()

# Set monthly active users to the limit
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(
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 Down Expand Up @@ -368,8 +417,14 @@ class UserRestTestCase(unittest.HomeserverTestCase):
servlets = [
synapse.rest.admin.register_servlets,
login.register_servlets,
sync.register_servlets,
]

def default_config(self):
config = super().default_config()
config.update({"registration_shared_secret": None})
return config

richvdh marked this conversation as resolved.
Show resolved Hide resolved
def prepare(self, reactor, clock, hs):
self.store = hs.get_datastore()

Expand All @@ -386,7 +441,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(
Expand All @@ -409,7 +463,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",
Expand All @@ -425,7 +478,6 @@ 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)
Expand Down Expand Up @@ -473,7 +525,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
Expand Down Expand Up @@ -516,14 +567,114 @@ 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 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.
"""

handler = self.hs.get_registration_handler()

# 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"]
)

# Set monthly active users to the limit
self.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(
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"])

@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):
"""
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.
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.

"""

handler = self.hs.get_registration_handler()

# Set monthly active users to the limit
self.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(
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"])

@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
Expand Down Expand Up @@ -554,14 +705,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
Expand Down Expand Up @@ -595,7 +753,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"})
Expand All @@ -614,7 +771,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"})
Expand Down Expand Up @@ -645,7 +801,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(
Expand Down Expand Up @@ -711,7 +866,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})
Expand Down Expand Up @@ -743,7 +897,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
Expand Down