Skip to content
This repository has been archived by the owner on Dec 13, 2023. 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 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/5641.misc
@@ -0,0 +1 @@
Remove access-token support from RegistrationHandler.register.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is outdated as .register no longer exists?

27 changes: 3 additions & 24 deletions synapse/handlers/register.py
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
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
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
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
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
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
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