Merge pull request #6011 from matrix-org/anoa/fix_3pid_validation

Use account_threepid_delegate for 3pid validation

4 files changed, 46 insertions, 42 deletions

diff --git a/changelog.d/6011.feature b/changelog.d/6011.feature
new file mode 100644
index 0000000000..ad16acb12b
--- /dev/null
+++ b/changelog.d/6011.feature
@@ -0,0 +1 @@
+Use account_threepid_delegate.email and account_threepid_delegate.msisdn for validating threepid sessions.
\ No newline at end of file
diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py
index d0c0142740..374372b69e 100644
--- a/synapse/handlers/auth.py
+++ b/synapse/handlers/auth.py
@@ -444,7 +444,16 @@ class AuthHandler(BaseHandler):
 
         logger.info("Getting validated threepid. threepidcreds: %r", (threepid_creds,))
         if self.hs.config.threepid_behaviour_email == ThreepidBehaviour.REMOTE:
-            threepid = yield identity_handler.threepid_from_creds(threepid_creds)
+            if medium == "email":
+                threepid = yield identity_handler.threepid_from_creds(
+                    self.hs.config.account_threepid_delegate_email, threepid_creds
+                )
+            elif medium == "msisdn":
+                threepid = yield identity_handler.threepid_from_creds(
+                    self.hs.config.account_threepid_delegate_msisdn, threepid_creds
+                )
+            else:
+                raise SynapseError(400, "Unrecognized threepid medium: %s" % (medium,))
         elif self.hs.config.threepid_behaviour_email == ThreepidBehaviour.LOCAL:
             row = yield self.store.get_threepid_validation_session(
                 medium,
diff --git a/synapse/handlers/identity.py b/synapse/handlers/identity.py
index 45db1c1c06..f0549666c3 100644
--- a/synapse/handlers/identity.py
+++ b/synapse/handlers/identity.py
@@ -75,59 +75,52 @@ class IdentityHandler(BaseHandler):
         return client_secret, id_server, id_access_token
 
     @defer.inlineCallbacks
-    def threepid_from_creds(self, creds, use_v2=True):
+    def threepid_from_creds(self, id_server, creds):
         """
-        Retrieve and validate a threepid identitier from a "credentials" dictionary
+        Retrieve and validate a threepid identifier from a "credentials" dictionary against a
+        given identity server
 
         Args:
-            creds (dict[str, str]): Dictionary of credentials that contain the following keys:
+            id_server (str|None): The identity server to validate 3PIDs against. If None,
+                we will attempt to extract id_server creds
+
+            creds (dict[str, str]): Dictionary containing the following keys:
+                * id_server|idServer: An optional domain name of an identity server
                 * client_secret|clientSecret: A unique secret str provided by the client
-                * id_server|idServer: the domain of the identity server to query
-                * id_access_token: The access token to authenticate to the identity
-                    server with. Required if use_v2 is true
-            use_v2 (bool): Whether to use v2 Identity Service API endpoints
+                * sid: The ID of the validation session
 
         Returns:
             Deferred[dict[str,str|int]|None]: A dictionary consisting of response params to
                 the /getValidated3pid endpoint of the Identity Service API, or None if the
                 threepid was not found
         """
-        client_secret, id_server, id_access_token = self._extract_items_from_creds_dict(
-            creds
-        )
-
-        # If an id_access_token is not supplied, force usage of v1
-        if id_access_token is None:
-            use_v2 = False
-
-        query_params = {"sid": creds["sid"], "client_secret": client_secret}
-
-        # Decide which API endpoint URLs and query parameters to use
-        if use_v2:
-            url = "https://%s%s" % (
-                id_server,
-                "/_matrix/identity/v2/3pid/getValidated3pid",
+        client_secret = creds.get("client_secret") or creds.get("clientSecret")
+        if not client_secret:
+            raise SynapseError(
+                400, "Missing param client_secret in creds", errcode=Codes.MISSING_PARAM
             )
-            query_params["id_access_token"] = id_access_token
-        else:
-            url = "https://%s%s" % (
-                id_server,
-                "/_matrix/identity/api/v1/3pid/getValidated3pid",
+        session_id = creds.get("sid")
+        if not session_id:
+            raise SynapseError(
+                400, "Missing param session_id in creds", errcode=Codes.MISSING_PARAM
             )
+        if not id_server:
+            # Attempt to get the id_server from the creds dict
+            id_server = creds.get("id_server") or creds.get("idServer")
+            if not id_server:
+                raise SynapseError(
+                    400, "Missing param id_server in creds", errcode=Codes.MISSING_PARAM
+                )
+
+        query_params = {"sid": session_id, "client_secret": client_secret}
+
+        url = "https://%s%s" % (
+            id_server,
+            "/_matrix/identity/api/v1/3pid/getValidated3pid",
+        )
 
-        try:
-            data = yield self.http_client.get_json(url, query_params)
-            return data if "medium" in data else None
-        except HttpResponseException as e:
-            if e.code != 404 or not use_v2:
-                # Generic failure
-                logger.info("getValidated3pid failed with Matrix error: %r", e)
-                raise e.to_synapse_error()
-
-        # This identity server is too old to understand Identity Service API v2
-        # Attempt v1 endpoint
-        logger.info("Got 404 when POSTing JSON %s, falling back to v1 URL", url)
-        return (yield self.threepid_from_creds(creds, use_v2=False))
+        data = yield self.http_client.get_json(url, query_params)
+        return data if "medium" in data else None
 
     @defer.inlineCallbacks
     def bind_threepid(self, creds, mxid, use_v2=True):
diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py
index 785d01ea52..2ea515d2f6 100644
--- a/synapse/rest/client/v2_alpha/account.py
+++ b/synapse/rest/client/v2_alpha/account.py
@@ -523,7 +523,8 @@ class ThreepidRestServlet(RestServlet):
         requester = yield self.auth.get_user_by_req(request)
         user_id = requester.user.to_string()
 
-        threepid = yield self.identity_handler.threepid_from_creds(threepid_creds)
+        # Specify None as the identity server to retrieve it from the request body instead
+        threepid = yield self.identity_handler.threepid_from_creds(None, threepid_creds)
 
         if not threepid:
             raise SynapseError(400, "Failed to auth 3pid", Codes.THREEPID_AUTH_FAILED)