Skip to content
This repository was archived by the owner on Apr 12, 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/48.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Prevent `/register` from raising `M_USER_IN_USE` until UI Auth has been completed. Have `/register/available` always return true.
16 changes: 15 additions & 1 deletion synapse/handlers/register.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,19 @@ def __init__(self, hs):
self.session_lifetime = hs.config.session_lifetime

@defer.inlineCallbacks
def check_username(self, localpart, guest_access_token=None, assigned_user_id=None):
def check_username(
self, localpart, guest_access_token=None, assigned_user_id=None,
):
"""

Args:
localpart (str|None): The user's localpart
guest_access_token (str|None): A guest's access token
assigned_user_id (str|None): An existing User ID for this user if pre-calculated

Returns:
Deferred
"""
if types.contains_invalid_mxid_characters(localpart):
raise SynapseError(
400,
Expand Down Expand Up @@ -122,6 +134,8 @@ def check_username(self, localpart, guest_access_token=None, assigned_user_id=No
raise SynapseError(
400, "User ID already taken.", errcode=Codes.USER_IN_USE
)

# Retrieve guest user information from provided access token
user_data = yield self.auth.get_user_by_access_token(guest_access_token)
if not user_data["is_guest"] or user_data["user"].localpart != localpart:
raise AuthError(
Expand Down
45 changes: 12 additions & 33 deletions synapse/rest/client/v2_alpha/register.py
Original file line number Diff line number Diff line change
Expand Up @@ -357,15 +357,9 @@ async def on_GET(self, request):
403, "Registration has been disabled", errcode=Codes.FORBIDDEN
)

ip = self.hs.get_ip_from_request(request)
with self.ratelimiter.ratelimit(ip) as wait_deferred:
Copy link
Member

Choose a reason for hiding this comment

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

why do you remove the ratelimiter check here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Now that this endpoint just always returns {"available": True} I don't think there's much point in ratelimiting it.

Copy link
Member

Choose a reason for hiding this comment

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

you're right, I agree
I forgot it was change about /register/available

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, hard to notice in the diff :)

await wait_deferred

username = parse_string(request, "username", required=True)

await self.registration_handler.check_username(username)

return 200, {"available": True}
# We are not interested in logging in via a username in this deployment.
# Simply allow anything here as it won't be used later.
return 200, {"available": True}


class RegisterRestServlet(RestServlet):
Expand Down Expand Up @@ -443,14 +437,15 @@ async def on_POST(self, request):
body["password_hash"] = await self.auth_handler.hash(password)
desired_password_hash = body["password_hash"]

# We don't care about usernames for this deployment. In fact, the act
# of checking whether they exist already can leak metadata about
# which users are already registered.
#
# Usernames are already derived via the provided email.
# So, if they're not necessary, just ignore them.
#
# (we do still allow appservices to set them below)
desired_username = None
if "username" in body:
if (
not isinstance(body["username"], string_types)
or len(body["username"]) > 512
):
raise SynapseError(400, "Invalid username")
desired_username = body["username"]

desired_display_name = body.get("display_name")

Expand All @@ -466,7 +461,7 @@ async def on_POST(self, request):
# Set the desired user according to the AS API (which uses the
# 'user' key not 'username'). Since this is a new addition, we'll
# fallback to 'username' if they gave one.
desired_username = body.get("user", desired_username)
desired_username = body.get("user", body.get("username"))

# XXX we should check that desired_username is valid. Currently
# we give appservices carte blanche for any insanity in mxids,
Expand All @@ -485,15 +480,6 @@ async def on_POST(self, request):
)
return 200, result # we throw for non 200 responses

# for regular registration, downcase the provided username before
# attempting to register it. This should mean
# that people who try to register with upper-case in their usernames
# don't get a nasty surprise. (Note that we treat username
# case-insenstively in login, so they are free to carry on imagining
# that their username is CrAzYh4cKeR if that keeps them happy)
if desired_username is not None:
desired_username = desired_username.lower()

# == Normal User Registration == (everyone else)
if not self.hs.config.enable_registration:
raise SynapseError(403, "Registration has been disabled")
Expand All @@ -519,13 +505,6 @@ async def on_POST(self, request):
session_id, "registered_user_id", None
)

if desired_username is not None:
await self.registration_handler.check_username(
desired_username,
guest_access_token=guest_access_token,
assigned_user_id=registered_user_id,
)

auth_result, params, session_id = await self.auth_handler.check_auth(
self._registration_flows,
request,
Expand Down
8 changes: 0 additions & 8 deletions tests/rest/client/v2_alpha/test_register.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,14 +90,6 @@ def test_POST_bad_password(self):
self.assertEquals(channel.result["code"], b"400", channel.result)
self.assertEquals(channel.json_body["error"], "Invalid password")

def test_POST_bad_username(self):
request_data = json.dumps({"username": 777, "password": "monkey"})
request, channel = self.make_request(b"POST", self.url, request_data)
self.render(request)

self.assertEquals(channel.result["code"], b"400", channel.result)
self.assertEquals(channel.json_body["error"], "Invalid username")

Copy link
Member Author

Choose a reason for hiding this comment

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

This test is no longer relevant.

def test_POST_user_valid(self):
user_id = "@kermit:test"
device_id = "frogfone"
Expand Down