Remove access-token support from RegistrationHandler.register (#5641)

Nothing uses this now, so we can remove the dead code, and clean up the
API.

Since we're changing the shape of the return value anyway, we take the
opportunity to give the method a better name.

changelog.d/5641.misc

@@ -0,0 +1 @@
+Remove access-token support from RegistrationHandler.register, and rename it.

synapse/handlers/register.py

@@ -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,
@@ -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.
  """
@@ -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,
@@ -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,
@@ -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:
@@ -278,7 +263,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):
@@ -541,7 +526,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,
@@ -555,9 +539,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.
@@ -593,7 +574,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,
@@ -606,7 +586,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,

synapse/module_api/__init__.py

@@ -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.
 
@@ -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.
 

synapse/replication/http/register.py

@@ -38,7 +38,6 @@ def __init__(self, hs):
  @staticmethod
  def _serialize_payload(
  user_id,
- token,
  password_hash,
  was_guest,
  make_guest,
@@ -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.
@@ -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,
@@ -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"],

synapse/rest/admin/__init__.py

@@ -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,
  )
 

synapse/rest/client/v1/login.py

@@ -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(
@@ -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(

synapse/rest/client/v2_alpha/register.py

@@ -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,
  )
@@ -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)
@@ -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

tests/handlers/test_register.py

@@ -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))
@@ -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)
@@ -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):
@@ -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)