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

Remove access-token support from RegistrationHandler.register #5641

Merged
merged 2 commits into from
Jul 8, 2019
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/5641.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Remove access-token support from RegistrationHandler.register, and rename it.
27 changes: 3 additions & 24 deletions synapse/handlers/register.py
Original file line number Diff line number Diff line change
Expand Up @@ -138,11 +138,10 @@ def check_username(self, localpart, guest_access_token=None, assigned_user_id=No
)

@defer.inlineCallbacks
def register(
def register_user(
self,
localpart=None,
password=None,
generate_token=True,
guest_access_token=None,
make_guest=False,
admin=False,
Expand All @@ -160,19 +159,14 @@ def register(
password (unicode) : The password to assign to this user so they can
login again. This can be None which means they cannot login again
via a password (e.g. the user is an application service user).
generate_token (bool): Whether a new access token should be
generated. Having this be True should be considered deprecated,
since it offers no means of associating a device_id with the
access_token. Instead you should call auth_handler.issue_access_token
after registration.
user_type (str|None): type of user. One of the values from
api.constants.UserTypes, or None for a normal user.
default_display_name (unicode|None): if set, the new user's displayname
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.
Returns:
A tuple of (user_id, access_token).
Deferred[str]: user_id
Raises:
RegistrationError if there was a problem registering.
"""
Expand Down Expand Up @@ -206,12 +200,8 @@ def register(
elif default_display_name is None:
default_display_name = localpart

token = None
if generate_token:
token = self.macaroon_gen.generate_access_token(user_id)
yield self.register_with_store(
user_id=user_id,
token=token,
password_hash=password_hash,
was_guest=was_guest,
make_guest=make_guest,
Expand All @@ -230,21 +220,17 @@ def register(
else:
# autogen a sequential user ID
attempts = 0
token = None
user = None
while not user:
localpart = yield self._generate_user_id(attempts > 0)
user = UserID(localpart, self.hs.hostname)
user_id = user.to_string()
yield self.check_user_id_not_appservice_exclusive(user_id)
if generate_token:
token = self.macaroon_gen.generate_access_token(user_id)
if default_display_name is None:
default_display_name = localpart
try:
yield self.register_with_store(
user_id=user_id,
token=token,
password_hash=password_hash,
make_guest=make_guest,
create_profile_with_displayname=default_display_name,
Expand All @@ -254,7 +240,6 @@ def register(
# if user id is taken, just generate another
user = None
user_id = None
token = None
attempts += 1
if not self.hs.config.user_consent_at_registration:
yield self._auto_join_rooms(user_id)
Expand All @@ -272,7 +257,7 @@ def register(
# Bind email to new account
yield self._register_email_threepid(user_id, threepid_dict, None, False)

defer.returnValue((user_id, token))
defer.returnValue(user_id)

@defer.inlineCallbacks
def _auto_join_rooms(self, user_id):
Expand Down Expand Up @@ -534,7 +519,6 @@ def _join_user_to_room(self, requester, room_identifier):
def register_with_store(
self,
user_id,
token=None,
password_hash=None,
was_guest=False,
make_guest=False,
Expand All @@ -548,9 +532,6 @@ def register_with_store(

Args:
user_id (str): The desired user ID to register.
token (str): The desired access token to use for this user. If this
is not None, the given access token is associated with the user
id.
password_hash (str|None): Optional. The password hash for this user.
was_guest (bool): Optional. Whether this is a guest account being
upgraded to a non-guest account.
Expand Down Expand Up @@ -586,7 +567,6 @@ def register_with_store(
if self.hs.config.worker_app:
return self._register_client(
user_id=user_id,
token=token,
password_hash=password_hash,
was_guest=was_guest,
make_guest=make_guest,
Expand All @@ -599,7 +579,6 @@ def register_with_store(
else:
return self.store.register(
user_id=user_id,
token=token,
password_hash=password_hash,
was_guest=was_guest,
make_guest=make_guest,
Expand Down
10 changes: 2 additions & 8 deletions synapse/module_api/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,6 @@ def register(self, localpart, displayname=None, emails=[]):
_, access_token = yield self.register_device(user_id)
defer.returnValue((user_id, access_token))

@defer.inlineCallbacks
def register_user(self, localpart, displayname=None, emails=[]):
"""Registers a new user with given localpart and optional displayname, emails.

Expand All @@ -115,15 +114,10 @@ def register_user(self, localpart, displayname=None, emails=[]):
Returns:
Deferred[str]: user_id
"""
user_id, _ = yield self.hs.get_registration_handler().register(
localpart=localpart,
default_display_name=displayname,
bind_emails=emails,
generate_token=False,
return self.hs.get_registration_handler().register_user(
localpart=localpart, default_display_name=displayname, bind_emails=emails
)

defer.returnValue(user_id)

def register_device(self, user_id, device_id=None, initial_display_name=None):
"""Register a device for a user and generate an access token.

Expand Down
6 changes: 0 additions & 6 deletions synapse/replication/http/register.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ def __init__(self, hs):
@staticmethod
def _serialize_payload(
user_id,
token,
password_hash,
was_guest,
make_guest,
Expand All @@ -51,9 +50,6 @@ def _serialize_payload(
"""
Args:
user_id (str): The desired user ID to register.
token (str): The desired access token to use for this user. If this
is not None, the given access token is associated with the user
id.
password_hash (str|None): Optional. The password hash for this user.
was_guest (bool): Optional. Whether this is a guest account being
upgraded to a non-guest account.
Expand All @@ -68,7 +64,6 @@ def _serialize_payload(
address (str|None): the IP address used to perform the regitration.
"""
return {
"token": token,
"password_hash": password_hash,
"was_guest": was_guest,
"make_guest": make_guest,
Expand All @@ -85,7 +80,6 @@ def _handle_request(self, request, user_id):

yield self.registration_handler.register_with_store(
user_id=user_id,
token=content["token"],
password_hash=content["password_hash"],
was_guest=content["was_guest"],
make_guest=content["make_guest"],
Expand Down
3 changes: 1 addition & 2 deletions synapse/rest/admin/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -219,11 +219,10 @@ def on_POST(self, request):

register = RegisterRestServlet(self.hs)

(user_id, _) = yield register.registration_handler.register(
user_id = yield register.registration_handler.register_user(
localpart=body["username"].lower(),
password=body["password"],
admin=bool(admin),
generate_token=False,
user_type=user_type,
)

Expand Down
14 changes: 4 additions & 10 deletions synapse/rest/client/v1/login.py
Original file line number Diff line number Diff line change
Expand Up @@ -314,10 +314,8 @@ def do_jwt_login(self, login_submission):

registered_user_id = yield self.auth_handler.check_user_exists(user_id)
if not registered_user_id:
registered_user_id, _ = (
yield self.registration_handler.register(
localpart=user, generate_token=False
)
registered_user_id = yield self.registration_handler.register_user(
localpart=user
)

result = yield self._register_device_with_callback(
Expand Down Expand Up @@ -505,12 +503,8 @@ def on_successful_auth(
user_id = UserID(localpart, self._hostname).to_string()
registered_user_id = yield self._auth_handler.check_user_exists(user_id)
if not registered_user_id:
registered_user_id, _ = (
yield self._registration_handler.register(
localpart=localpart,
generate_token=False,
default_display_name=user_display_name,
)
registered_user_id = yield self._registration_handler.register_user(
localpart=localpart, default_display_name=user_display_name
)

login_token = self._macaroon_gen.generate_short_term_login_token(
Expand Down
11 changes: 5 additions & 6 deletions synapse/rest/client/v2_alpha/register.py
Original file line number Diff line number Diff line change
Expand Up @@ -464,11 +464,10 @@ def on_POST(self, request):
Codes.THREEPID_IN_USE,
)

(registered_user_id, _) = yield self.registration_handler.register(
registered_user_id = yield self.registration_handler.register_user(
localpart=desired_username,
password=new_password,
guest_access_token=guest_access_token,
generate_token=False,
threepid=threepid,
address=client_addr,
)
Expand Down Expand Up @@ -542,8 +541,8 @@ def _do_shared_secret_registration(self, username, password, body):
if not compare_digest(want_mac, got_mac):
raise SynapseError(403, "HMAC incorrect")

(user_id, _) = yield self.registration_handler.register(
localpart=username, password=password, generate_token=False
user_id = yield self.registration_handler.register_user(
localpart=username, password=password
)

result = yield self._create_registration_details(user_id, body)
Expand Down Expand Up @@ -577,8 +576,8 @@ def _create_registration_details(self, user_id, params):
def _do_guest_registration(self, params, address=None):
if not self.hs.config.allow_guest_access:
raise SynapseError(403, "Guest access is disabled")
user_id, _ = yield self.registration_handler.register(
generate_token=False, make_guest=True, address=address
user_id = yield self.registration_handler.register_user(
make_guest=True, address=address
)

# we don't allow guests to specify their own device_id, because
Expand Down
53 changes: 28 additions & 25 deletions tests/handlers/test_register.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,21 +129,21 @@ def test_register_mau_blocked(self):
return_value=defer.succeed(self.lots_of_users)
)
self.get_failure(
self.handler.register(localpart="local_part"), ResourceLimitError
self.handler.register_user(localpart="local_part"), ResourceLimitError
)

self.store.get_monthly_active_count = Mock(
return_value=defer.succeed(self.hs.config.max_mau_value)
)
self.get_failure(
self.handler.register(localpart="local_part"), ResourceLimitError
self.handler.register_user(localpart="local_part"), ResourceLimitError
)

def test_auto_create_auto_join_rooms(self):
room_alias_str = "#room:test"
self.hs.config.auto_join_rooms = [room_alias_str]
res = self.get_success(self.handler.register(localpart="jeff"))
rooms = self.get_success(self.store.get_rooms_for_user(res[0]))
user_id = self.get_success(self.handler.register_user(localpart="jeff"))
rooms = self.get_success(self.store.get_rooms_for_user(user_id))
directory_handler = self.hs.get_handlers().directory_handler
room_alias = RoomAlias.from_string(room_alias_str)
room_id = self.get_success(directory_handler.get_association(room_alias))
Expand All @@ -154,34 +154,34 @@ def test_auto_create_auto_join_rooms(self):
def test_auto_create_auto_join_rooms_with_no_rooms(self):
self.hs.config.auto_join_rooms = []
frank = UserID.from_string("@frank:test")
res = self.get_success(self.handler.register(frank.localpart))
self.assertEqual(res[0], frank.to_string())
rooms = self.get_success(self.store.get_rooms_for_user(res[0]))
user_id = self.get_success(self.handler.register_user(frank.localpart))
self.assertEqual(user_id, frank.to_string())
rooms = self.get_success(self.store.get_rooms_for_user(user_id))
self.assertEqual(len(rooms), 0)

def test_auto_create_auto_join_where_room_is_another_domain(self):
self.hs.config.auto_join_rooms = ["#room:another"]
frank = UserID.from_string("@frank:test")
res = self.get_success(self.handler.register(frank.localpart))
self.assertEqual(res[0], frank.to_string())
rooms = self.get_success(self.store.get_rooms_for_user(res[0]))
user_id = self.get_success(self.handler.register_user(frank.localpart))
self.assertEqual(user_id, frank.to_string())
rooms = self.get_success(self.store.get_rooms_for_user(user_id))
self.assertEqual(len(rooms), 0)

def test_auto_create_auto_join_where_auto_create_is_false(self):
self.hs.config.autocreate_auto_join_rooms = False
room_alias_str = "#room:test"
self.hs.config.auto_join_rooms = [room_alias_str]
res = self.get_success(self.handler.register(localpart="jeff"))
rooms = self.get_success(self.store.get_rooms_for_user(res[0]))
user_id = self.get_success(self.handler.register_user(localpart="jeff"))
rooms = self.get_success(self.store.get_rooms_for_user(user_id))
self.assertEqual(len(rooms), 0)

def test_auto_create_auto_join_rooms_when_support_user_exists(self):
room_alias_str = "#room:test"
self.hs.config.auto_join_rooms = [room_alias_str]

self.store.is_support_user = Mock(return_value=True)
res = self.get_success(self.handler.register(localpart="support"))
rooms = self.get_success(self.store.get_rooms_for_user(res[0]))
user_id = self.get_success(self.handler.register_user(localpart="support"))
rooms = self.get_success(self.store.get_rooms_for_user(user_id))
self.assertEqual(len(rooms), 0)
directory_handler = self.hs.get_handlers().directory_handler
room_alias = RoomAlias.from_string(room_alias_str)
Expand Down Expand Up @@ -209,27 +209,31 @@ def test_auto_create_auto_join_where_no_consent(self):

# When:-
# * the user is registered and post consent actions are called
res = self.get_success(self.handler.register(localpart="jeff"))
self.get_success(self.handler.post_consent_actions(res[0]))
user_id = self.get_success(self.handler.register_user(localpart="jeff"))
self.get_success(self.handler.post_consent_actions(user_id))

# Then:-
# * Ensure that they have not been joined to the room
rooms = self.get_success(self.store.get_rooms_for_user(res[0]))
rooms = self.get_success(self.store.get_rooms_for_user(user_id))
self.assertEqual(len(rooms), 0)

def test_register_support_user(self):
res = self.get_success(
self.handler.register(localpart="user", user_type=UserTypes.SUPPORT)
user_id = self.get_success(
self.handler.register_user(localpart="user", user_type=UserTypes.SUPPORT)
)
self.assertTrue(self.store.is_support_user(res[0]))
d = self.store.is_support_user(user_id)
self.assertTrue(self.get_success(d))

def test_register_not_support_user(self):
res = self.get_success(self.handler.register(localpart="user"))
self.assertFalse(self.store.is_support_user(res[0]))
user_id = self.get_success(self.handler.register_user(localpart="user"))
d = self.store.is_support_user(user_id)
self.assertFalse(self.get_success(d))

def test_invalid_user_id_length(self):
invalid_user_id = "x" * 256
self.get_failure(self.handler.register(localpart=invalid_user_id), SynapseError)
self.get_failure(
self.handler.register_user(localpart=invalid_user_id), SynapseError
)

@defer.inlineCallbacks
def get_or_create_user(self, requester, localpart, displayname, password_hash=None):
Expand Down Expand Up @@ -267,13 +271,12 @@ def get_or_create_user(self, requester, localpart, displayname, password_hash=No
if need_register:
yield self.handler.register_with_store(
user_id=user_id,
token=token,
password_hash=password_hash,
create_profile_with_displayname=user.localpart,
)
else:
yield self.hs.get_auth_handler().delete_access_tokens_for_user(user_id)
yield self.store.add_access_token_to_user(user_id=user_id, token=token)
yield self.store.add_access_token_to_user(user_id=user_id, token=token)

if displayname is not None:
# logger.info("setting user display name: %s -> %s", user_id, displayname)
Expand Down