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

Add a config flag to inhibit M_USER_IN_USE during registration #11743

Merged
merged 9 commits into from
Jan 26, 2022
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/11743.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add a config flag to inhibit M_USER_IN_USE during registration.
10 changes: 10 additions & 0 deletions docs/sample_config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1436,6 +1436,16 @@ account_threepid_delegates:
#
#auto_join_rooms_for_guests: false

# Whether to inhibit errors raised when registering a new account if the user ID
# already exists. If turned on, that requests to /register/available will always
# show a user ID as available, and Synapse won't raise an error when starting
# a registration with a user ID that already exists. However, Synapse will still
# raise an error if the registration completes and the username conflicts.
#
# Defaults to false.
#
#inhibit_user_in_use_error: true


## Metrics ###

Expand Down
12 changes: 12 additions & 0 deletions synapse/config/registration.py
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,8 @@ def read_config(self, config, **kwargs):
# The success template used during fallback auth.
self.fallback_success_template = self.read_template("auth_success.html")

self.inhibit_user_in_use_error = config.get("inhibit_user_in_use_error", False)

def generate_config_section(self, generate_secrets=False, **kwargs):
if generate_secrets:
registration_shared_secret = 'registration_shared_secret: "%s"' % (
Expand Down Expand Up @@ -446,6 +448,16 @@ def generate_config_section(self, generate_secrets=False, **kwargs):
# Defaults to true.
#
#auto_join_rooms_for_guests: false

# Whether to inhibit errors raised when registering a new account if the user ID
# already exists. If turned on, that requests to /register/available will always
# show a user ID as available, and Synapse won't raise an error when starting
# a registration with a user ID that already exists. However, Synapse will still
# raise an error if the registration completes and the username conflicts.
#
# Defaults to false.
#
#inhibit_user_in_use_error: true
"""
% locals()
)
Expand Down
26 changes: 14 additions & 12 deletions synapse/handlers/register.py
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ async def check_username(
localpart: str,
guest_access_token: Optional[str] = None,
assigned_user_id: Optional[str] = None,
inhibit_user_in_use_error: bool = False,
) -> None:
if types.contains_invalid_mxid_characters(localpart):
raise SynapseError(
Expand Down Expand Up @@ -171,21 +172,22 @@ async def check_username(

users = await self.store.get_users_by_id_case_insensitive(user_id)
if users:
if not guest_access_token:
if not inhibit_user_in_use_error and not guest_access_token:
raise SynapseError(
400, "User ID already taken.", errcode=Codes.USER_IN_USE
)
user_data = await self.auth.get_user_by_access_token(guest_access_token)
if (
not user_data.is_guest
or UserID.from_string(user_data.user_id).localpart != localpart
):
raise AuthError(
403,
"Cannot register taken user ID without valid guest "
"credentials for that user.",
errcode=Codes.FORBIDDEN,
)
if guest_access_token:
user_data = await self.auth.get_user_by_access_token(guest_access_token)
if (
not user_data.is_guest
or UserID.from_string(user_data.user_id).localpart != localpart
):
raise AuthError(
403,
"Cannot register taken user ID without valid guest "
"credentials for that user.",
errcode=Codes.FORBIDDEN,
)

if guest_access_token is None:
try:
Expand Down
11 changes: 11 additions & 0 deletions synapse/rest/client/register.py
Original file line number Diff line number Diff line change
Expand Up @@ -339,12 +339,19 @@ def __init__(self, hs: "HomeServer"):
),
)

self.inhibit_user_in_use_error = (
hs.config.registration.inhibit_user_in_use_error
)

async def on_GET(self, request: Request) -> Tuple[int, JsonDict]:
if not self.hs.config.registration.enable_registration:
raise SynapseError(
403, "Registration has been disabled", errcode=Codes.FORBIDDEN
)

if self.inhibit_user_in_use_error:
return 200, {"available": True}
Comment on lines +352 to +353
Copy link
Contributor

Choose a reason for hiding this comment

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

Part of me feels uncomfortable lying here. I wondered about returning 400 or 403 with an appropriate error code. But I can imagine a) that clients would break, and b) that would require a spec change.


ip = request.getClientIP()
with self.ratelimiter.ratelimit(ip) as wait_deferred:
await wait_deferred
Expand Down Expand Up @@ -422,6 +429,9 @@ def __init__(self, hs: "HomeServer"):
self._refresh_tokens_enabled = (
hs.config.registration.refreshable_access_token_lifetime is not None
)
self._inhibit_user_in_use_error = (
hs.config.registration.inhibit_user_in_use_error
)

self._registration_flows = _calculate_registration_flows(
hs.config, self.auth_handler
Expand Down Expand Up @@ -564,6 +574,7 @@ async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]:
desired_username,
guest_access_token=guest_access_token,
assigned_user_id=registered_user_id,
inhibit_user_in_use_error=self._inhibit_user_in_use_error,
Copy link
Contributor Author

@babolivier babolivier Jan 20, 2022

Choose a reason for hiding this comment

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

I'm not adding another check later on (e.g. after we've checked that the UIA flow is complete) because the registration handler's register_user (which is called towards the end of this function) already does that.

)

# Check if the user-interactive authentication flows are complete, if
Expand Down
41 changes: 41 additions & 0 deletions tests/rest/client/test_register.py
Original file line number Diff line number Diff line change
Expand Up @@ -726,6 +726,47 @@ def test_reject_invalid_email(self):
{"errcode": "M_UNKNOWN", "error": "Unable to parse email address"},
)

@override_config(
{
"inhibit_user_in_use_error": True,
}
)
def test_inhibit_user_in_use_error(self):
"""Tests that the 'inhibit_user_in_use_error' configuration flag behaves
correctly.
"""
username = "arthur"

# Manually register the user, so we know the test isn't passing because of a lack
# of clashing.
reg_handler = self.hs.get_registration_handler()
self.get_success(reg_handler.register_user(username))

# Check that /available correctly ignores the username provided despite the
# username being already registered.
channel = self.make_request("GET", "register/available?username=" + username)
self.assertEquals(200, channel.code, channel.result)

# Test that when starting a UIA registration flow the request doesn't fail because
# of a conflicting username
channel = self.make_request(
"POST",
"register",
{"username": username, "type": "m.login.password", "password": "foo"},
)
self.assertEqual(channel.code, 401)
self.assertIn("session", channel.json_body)

# Test that finishing the registration fails because of a conflicting username.
session = channel.json_body["session"]
channel = self.make_request(
"POST",
"register",
{"auth": {"session": session, "type": LoginType.DUMMY}},
)
self.assertEqual(channel.code, 400, channel.json_body)
self.assertEqual(channel.json_body["errcode"], Codes.USER_IN_USE)


class AccountValidityTestCase(unittest.HomeserverTestCase):

Expand Down