Skip to content
This repository was archived by the owner on Apr 26, 2024. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/8962.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix bug where application services couldn't register new ghost users if the server had reached its MAU limit.
7 changes: 7 additions & 0 deletions synapse/api/auth_blocking.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ def __init__(self, hs):
self._limit_usage_by_mau = hs.config.limit_usage_by_mau
self._mau_limits_reserved_threepids = hs.config.mau_limits_reserved_threepids
self._server_name = hs.hostname
self._track_appservice_user_ips = hs.config.appservice.track_appservice_user_ips

async def check_auth_blocking(
self,
Expand Down Expand Up @@ -76,6 +77,12 @@ async def check_auth_blocking(
# We never block the server from doing actions on behalf of
# users.
return
elif requester.app_service and not self._track_appservice_user_ips:
# If we're authenticated as an appservice then we only block
# auth if `track_appservice_user_ips` is set, as that option
# implicitly means that application services are part of MAU
# limits.
return

# Never fail an auth check for the server notices users or support user
# This can be a problem where event creation is prohibited due to blocking
Expand Down
8 changes: 7 additions & 1 deletion synapse/handlers/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -709,6 +709,7 @@ async def get_access_token_for_user_id(
device_id: Optional[str],
valid_until_ms: Optional[int],
puppets_user_id: Optional[str] = None,
is_appservice_ghost: bool = False,
) -> str:
"""
Creates a new access token for the user with the given user ID.
Expand All @@ -725,6 +726,7 @@ async def get_access_token_for_user_id(
we should always have a device ID)
valid_until_ms: when the token is valid until. None for
no expiry.
is_appservice_ghost: Whether the user is an application ghost user
Returns:
The access token for the user's session.
Raises:
Expand All @@ -745,7 +747,11 @@ async def get_access_token_for_user_id(
"Logging in user %s on device %s%s", user_id, device_id, fmt_expiry
)

await self.auth.check_auth_blocking(user_id)
if (
not is_appservice_ghost
or self.hs.config.appservice.track_appservice_user_ips
):
await self.auth.check_auth_blocking(user_id)

access_token = self.macaroon_gen.generate_access_token(user_id)
await self.store.add_access_token_to_user(
Expand Down
7 changes: 6 additions & 1 deletion synapse/handlers/register.py
Original file line number Diff line number Diff line change
Expand Up @@ -630,6 +630,7 @@ async def register_device(
device_id: Optional[str],
initial_display_name: Optional[str],
is_guest: bool = False,
is_appservice_ghost: bool = False,
) -> Tuple[str, str]:
"""Register a device for a user and generate an access token.

Expand All @@ -651,6 +652,7 @@ async def register_device(
device_id=device_id,
initial_display_name=initial_display_name,
is_guest=is_guest,
is_appservice_ghost=is_appservice_ghost,
)
return r["device_id"], r["access_token"]

Expand All @@ -672,7 +674,10 @@ async def register_device(
)
else:
access_token = await self._auth_handler.get_access_token_for_user_id(
user_id, device_id=registered_device_id, valid_until_ms=valid_until_ms
user_id,
device_id=registered_device_id,
valid_until_ms=valid_until_ms,
is_appservice_ghost=is_appservice_ghost,
)

return (registered_device_id, access_token)
Expand Down
12 changes: 10 additions & 2 deletions synapse/replication/http/login.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@ def __init__(self, hs):
self.registration_handler = hs.get_registration_handler()

@staticmethod
async def _serialize_payload(user_id, device_id, initial_display_name, is_guest):
async def _serialize_payload(
user_id, device_id, initial_display_name, is_guest, is_appservice_ghost
):
"""
Args:
device_id (str|None): Device ID to use, if None a new one is
Expand All @@ -48,6 +50,7 @@ async def _serialize_payload(user_id, device_id, initial_display_name, is_guest)
"device_id": device_id,
"initial_display_name": initial_display_name,
"is_guest": is_guest,
"is_appservice_ghost": is_appservice_ghost,
}

async def _handle_request(self, request, user_id):
Expand All @@ -56,9 +59,14 @@ async def _handle_request(self, request, user_id):
device_id = content["device_id"]
initial_display_name = content["initial_display_name"]
is_guest = content["is_guest"]
is_appservice_ghost = content["is_appservice_ghost"]

device_id, access_token = await self.registration_handler.register_device(
user_id, device_id, initial_display_name, is_guest
user_id,
device_id,
initial_display_name,
is_guest,
is_appservice_ghost=is_appservice_ghost,
)

return 200, {"device_id": device_id, "access_token": access_token}
Expand Down
14 changes: 11 additions & 3 deletions synapse/rest/client/v2_alpha/register.py
Original file line number Diff line number Diff line change
Expand Up @@ -655,9 +655,13 @@ async def _do_appservice_registration(self, username, as_token, body):
user_id = await self.registration_handler.appservice_register(
username, as_token
)
return await self._create_registration_details(user_id, body)
return await self._create_registration_details(
user_id, body, is_appservice_ghost=True,
)

async def _create_registration_details(self, user_id, params):
async def _create_registration_details(
self, user_id, params, is_appservice_ghost=False
):
"""Complete registration of newly-registered user

Allocates device_id if one was not given; also creates access_token.
Expand All @@ -674,7 +678,11 @@ async def _create_registration_details(self, user_id, params):
device_id = params.get("device_id")
initial_display_name = params.get("initial_device_display_name")
device_id, access_token = await self.registration_handler.register_device(
user_id, device_id, initial_display_name, is_guest=False
user_id,
device_id,
initial_display_name,
is_guest=False,
is_appservice_ghost=is_appservice_ghost,
)

result.update({"access_token": access_token, "device_id": device_id})
Expand Down
46 changes: 44 additions & 2 deletions tests/test_mau.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

from synapse.api.constants import LoginType
from synapse.api.errors import Codes, HttpResponseException, SynapseError
from synapse.appservice import ApplicationService
from synapse.rest.client.v2_alpha import register, sync

from tests import unittest
Expand Down Expand Up @@ -75,6 +76,45 @@ def test_simple_deny_mau(self):
self.assertEqual(e.code, 403)
self.assertEqual(e.errcode, Codes.RESOURCE_LIMIT_EXCEEDED)

def test_as_ignores_mau(self):
"""Test that application services can still create users when the MAU
limit has been reached. This only works when application service
user ip tracking is disabled.
"""

# Create and sync so that the MAU counts get updated
token1 = self.create_user("kermit1")
self.do_sync_for_user(token1)
token2 = self.create_user("kermit2")
self.do_sync_for_user(token2)

# check we're testing what we think we are: there should be two active users
self.assertEqual(self.get_success(self.store.get_monthly_active_count()), 2)

# We've created and activated two users, we shouldn't be able to
# register new users
with self.assertRaises(SynapseError) as cm:
self.create_user("kermit3")

e = cm.exception
self.assertEqual(e.code, 403)
self.assertEqual(e.errcode, Codes.RESOURCE_LIMIT_EXCEEDED)

# Cheekily add an application service that we use to register a new user
# with.
as_token = "foobartoken"
self.store.services_cache.append(
ApplicationService(
token=as_token,
hostname=self.hs.hostname,
id="SomeASID",
sender="@as_sender:test",
namespaces={"users": [{"regex": "@as_*", "exclusive": True}]},
)
)

self.create_user("as_kermit4", token=as_token)

def test_allowed_after_a_month_mau(self):
# Create and sync so that the MAU counts get updated
token1 = self.create_user("kermit1")
Expand Down Expand Up @@ -192,7 +232,7 @@ def test_tracked_but_not_limited(self):
self.reactor.advance(100)
self.assertEqual(2, self.successResultOf(count))

def create_user(self, localpart):
def create_user(self, localpart, token=None):
request_data = json.dumps(
{
"username": localpart,
Expand All @@ -201,7 +241,9 @@ def create_user(self, localpart):
}
)

request, channel = self.make_request("POST", "/register", request_data)
request, channel = self.make_request(
"POST", "/register", request_data, access_token=token
)

if channel.code != 200:
raise HttpResponseException(
Expand Down