Merge pull request #2728 from matrix-org/rav/validate_user_via_ui_auth

Factor out a validate_user_via_ui_auth method

synapse/handlers/auth.py

@@ -88,6 +88,49 @@ class AuthHandler(BaseHandler):
                 )
         self._supported_login_types = frozenset(login_types)
 
+    @defer.inlineCallbacks
+    def validate_user_via_ui_auth(self, requester, request_body, clientip):
+        """
+        Checks that the user is who they claim to be, via a UI auth.
+
+        This is used for things like device deletion and password reset where
+        the user already has a valid access token, but we want to double-check
+        that it isn't stolen by re-authenticating them.
+
+        Args:
+            requester (Requester): The user, as given by the access token
+
+            request_body (dict): The body of the request sent by the client
+
+            clientip (str): The IP address of the client.
+
+        Returns:
+            defer.Deferred[dict]: the parameters for this request (which may
+                have been given only in a previous call).
+
+        Raises:
+            InteractiveAuthIncompleteError if the client has not yet completed
+                any of the permitted login flows
+
+            AuthError if the client has completed a login flow, and it gives
+                a different user to `requester`
+        """
+
+        # we only support password login here
+        flows = [[LoginType.PASSWORD]]
+
+        result, params, _ = yield self.check_auth(
+            flows, request_body, clientip,
+        )
+
+        user_id = result[LoginType.PASSWORD]
+
+        # check that the UI auth matched the access token
+        if user_id != requester.user.to_string():
+            raise AuthError(403, "Invalid auth")
+
+        defer.returnValue(params)
+
     @defer.inlineCallbacks
     def check_auth(self, flows, clientdict, clientip):
         """

synapse/rest/client/v2_alpha/account.py

@@ -19,7 +19,7 @@ from twisted.internet import defer
 
 from synapse.api.auth import has_access_token
 from synapse.api.constants import LoginType
-from synapse.api.errors import Codes, LoginError, SynapseError
+from synapse.api.errors import Codes, SynapseError
 from synapse.http.servlet import (
     RestServlet, assert_params_in_request,
     parse_json_object_from_request,
@@ -103,26 +103,32 @@ class PasswordRestServlet(RestServlet):
     @interactive_auth_handler
     @defer.inlineCallbacks
     def on_POST(self, request):
-        yield run_on_reactor()
-
         body = parse_json_object_from_request(request)
 
-        result, params, _ = yield self.auth_handler.check_auth([
+        # there are two possibilities here. Either the user does not have an
-            [LoginType.PASSWORD],
+        # access token, and needs to do a password reset; or they have one and
-            [LoginType.EMAIL_IDENTITY],
+        # need to validate their identity.
-            [LoginType.MSISDN],
+        #
-        ], body, self.hs.get_ip_from_request(request))
+        # In the first case, we offer a couple of means of identifying
+        # themselves (email and msisdn, though it's unclear if msisdn actually
+        # works).
+        #
+        # In the second case, we require a password to confirm their identity.
 
-        user_id = None
+        if has_access_token(request):
-        requester = None
-
-        if LoginType.PASSWORD in result:
-            # if using password, they should also be logged in
             requester = yield self.auth.get_user_by_req(request)
+            params = yield self.auth_handler.validate_user_via_ui_auth(
+                requester, body, self.hs.get_ip_from_request(request),
+            )
             user_id = requester.user.to_string()
-            if user_id != result[LoginType.PASSWORD]:
+        else:
-                raise LoginError(400, "", Codes.UNKNOWN)
+            requester = None
-        elif LoginType.EMAIL_IDENTITY in result:
+            result, params, _ = yield self.auth_handler.check_auth(
+                [[LoginType.EMAIL_IDENTITY], [LoginType.MSISDN]],
+                body, self.hs.get_ip_from_request(request),
+            )
+
+            if LoginType.EMAIL_IDENTITY in result:
                 threepid = result[LoginType.EMAIL_IDENTITY]
                 if 'medium' not in threepid or 'address' not in threepid:
                     raise SynapseError(500, "Malformed threepid")
@@ -171,40 +177,21 @@ class DeactivateAccountRestServlet(RestServlet):
     def on_POST(self, request):
         body = parse_json_object_from_request(request)
 
-        # if the caller provides an access token, it ought to be valid.
+        requester = yield self.auth.get_user_by_req(request)
-        requester = None
-        if has_access_token(request):
-            requester = yield self.auth.get_user_by_req(
-                request,
-            )  # type: synapse.types.Requester
 
         # allow ASes to dectivate their own users
-        if requester and requester.app_service:
+        if requester.app_service:
             yield self._deactivate_account_handler.deactivate_account(
                 requester.user.to_string()
             )
             defer.returnValue((200, {}))
 
-        result, params, _ = yield self.auth_handler.check_auth([
+        yield self.auth_handler.validate_user_via_ui_auth(
-            [LoginType.PASSWORD],
+            requester, body, self.hs.get_ip_from_request(request),
-        ], body, self.hs.get_ip_from_request(request))
+        )
-
+        yield self._deactivate_account_handler.deactivate_account(
-        if LoginType.PASSWORD in result:
+            requester.user.to_string(),
-            user_id = result[LoginType.PASSWORD]
-            # if using password, they should also be logged in
-            if requester is None:
-                raise SynapseError(
-                    400,
-                    "Deactivate account requires an access_token",
-                    errcode=Codes.MISSING_TOKEN
         )
-            if requester.user.to_string() != user_id:
-                raise LoginError(400, "", Codes.UNKNOWN)
-        else:
-            logger.error("Auth succeeded but no known type!", result.keys())
-            raise SynapseError(500, "", Codes.UNKNOWN)
-
-        yield self._deactivate_account_handler.deactivate_account(user_id)
         defer.returnValue((200, {}))
 
 

synapse/rest/client/v2_alpha/devices.py

@@ -17,7 +17,7 @@ import logging
 
 from twisted.internet import defer
 
-from synapse.api import constants, errors
+from synapse.api import errors
 from synapse.http import servlet
 from ._base import client_v2_patterns, interactive_auth_handler
 
@@ -63,6 +63,8 @@ class DeleteDevicesRestServlet(servlet.RestServlet):
     @interactive_auth_handler
     @defer.inlineCallbacks
     def on_POST(self, request):
+        requester = yield self.auth.get_user_by_req(request)
+
         try:
             body = servlet.parse_json_object_from_request(request)
         except errors.SynapseError as e:
@@ -78,11 +80,10 @@ class DeleteDevicesRestServlet(servlet.RestServlet):
                 400, "No devices supplied", errcode=errors.Codes.MISSING_PARAM
             )
 
-        result, params, _ = yield self.auth_handler.check_auth([
+        result, params, _ = yield self.auth_handler.validate_user_via_ui_auth(
-            [constants.LoginType.PASSWORD],
+            requester, body, self.hs.get_ip_from_request(request),
-        ], body, self.hs.get_ip_from_request(request))
+        )
 
-        requester = yield self.auth.get_user_by_req(request)
         yield self.device_handler.delete_devices(
             requester.user.to_string(),
             body['devices'],
@@ -129,16 +130,13 @@ class DeviceRestServlet(servlet.RestServlet):
             else:
                 raise
 
-        result, params, _ = yield self.auth_handler.check_auth([
+        yield self.auth_handler.validate_user_via_ui_auth(
-            [constants.LoginType.PASSWORD],
+            requester, body, self.hs.get_ip_from_request(request),
-        ], body, self.hs.get_ip_from_request(request))
+        )
 
-        # check that the UI auth matched the access token
+        yield self.device_handler.delete_device(
-        user_id = result[constants.LoginType.PASSWORD]
+            requester.user.to_string(), device_id,
-        if user_id != requester.user.to_string():
+        )
-            raise errors.AuthError(403, "Invalid auth")
-
-        yield self.device_handler.delete_device(user_id, device_id)
         defer.returnValue((200, {}))
 
     @defer.inlineCallbacks