From 74c56f794cc33adb52746bf76c49ec5ca1edebed Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 1 Nov 2017 10:23:21 +0000 Subject: [PATCH] Break dependency of auth_handler on device_handler I'm going to need to make the device_handler depend on the auth_handler, so I need to break this dependency to avoid a cycle. It turns out that the auth_handler was only using the device_handler in one place which was an edge case which we can more elegantly handle by throwing an error rather than fixing it up. --- synapse/handlers/auth.py | 15 ++++++--------- synapse/rest/client/v1/login.py | 3 --- synapse/rest/client/v2_alpha/register.py | 1 - 3 files changed, 6 insertions(+), 13 deletions(-) diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py index 93d8ac0e04..051995bcce 100644 --- a/synapse/handlers/auth.py +++ b/synapse/handlers/auth.py @@ -75,7 +75,6 @@ class AuthHandler(BaseHandler): logger.info("Extra password_providers: %r", self.password_providers) self.hs = hs # FIXME better possibility to access registrationHandler later? - self.device_handler = hs.get_device_handler() self.macaroon_gen = hs.get_macaroon_generator() self._password_enabled = hs.config.password_enabled @@ -406,8 +405,7 @@ class AuthHandler(BaseHandler): return self.sessions[session_id] @defer.inlineCallbacks - def get_access_token_for_user_id(self, user_id, device_id=None, - initial_display_name=None): + def get_access_token_for_user_id(self, user_id, device_id=None): """ Creates a new access token for the user with the given user ID. @@ -421,13 +419,10 @@ class AuthHandler(BaseHandler): device_id (str|None): the device ID to associate with the tokens. None to leave the tokens unassociated with a device (deprecated: we should always have a device ID) - initial_display_name (str): display name to associate with the - device if it needs re-registering Returns: The access token for the user's session. Raises: StoreError if there was a problem storing the token. - LoginError if there was an authentication problem. """ logger.info("Logging in user %s on device %s", user_id, device_id) access_token = yield self.issue_access_token(user_id, device_id) @@ -437,9 +432,11 @@ class AuthHandler(BaseHandler): # really don't want is active access_tokens without a record of the # device, so we double-check it here. if device_id is not None: - yield self.device_handler.check_device_registered( - user_id, device_id, initial_display_name - ) + try: + yield self.store.get_device(user_id, device_id) + except StoreError: + yield self.store.delete_access_token(access_token) + raise StoreError(400, "Login raced against device deletion") defer.returnValue(access_token) diff --git a/synapse/rest/client/v1/login.py b/synapse/rest/client/v1/login.py index d24590011b..55d2fb056e 100644 --- a/synapse/rest/client/v1/login.py +++ b/synapse/rest/client/v1/login.py @@ -219,7 +219,6 @@ class LoginRestServlet(ClientV1RestServlet): ) access_token = yield auth_handler.get_access_token_for_user_id( canonical_user_id, device_id, - login_submission.get("initial_device_display_name"), ) result = { @@ -241,7 +240,6 @@ class LoginRestServlet(ClientV1RestServlet): device_id = yield self._register_device(user_id, login_submission) access_token = yield auth_handler.get_access_token_for_user_id( user_id, device_id, - login_submission.get("initial_device_display_name"), ) result = { "user_id": user_id, # may have changed @@ -284,7 +282,6 @@ class LoginRestServlet(ClientV1RestServlet): ) access_token = yield auth_handler.get_access_token_for_user_id( registered_user_id, device_id, - login_submission.get("initial_device_display_name"), ) result = { diff --git a/synapse/rest/client/v2_alpha/register.py b/synapse/rest/client/v2_alpha/register.py index d9a8cdbbb5..a077146c89 100644 --- a/synapse/rest/client/v2_alpha/register.py +++ b/synapse/rest/client/v2_alpha/register.py @@ -566,7 +566,6 @@ class RegisterRestServlet(RestServlet): access_token = ( yield self.auth_handler.get_access_token_for_user_id( user_id, device_id=device_id, - initial_display_name=params.get("initial_device_display_name") ) )