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

Commit

Permalink
Prevent M_USER_IN_USE from being raised by registration methods until…
Browse files Browse the repository at this point in the history
… after email has been verified (#48)

* Just ignore the `username` parameter on registration as it's not used by DINUM
* Have `/register/available` always return `true`
  • Loading branch information
anoadragon453 authored Jun 22, 2020
1 parent 53949a9 commit 7da71b7
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 42 deletions.
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:
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")

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

0 comments on commit 7da71b7

Please sign in to comment.