From a90a0f5c8a38ff7f99a12dd436b75680d3fee747 Mon Sep 17 00:00:00 2001
From: David Baker <dave@matrix.org>
Date: Fri, 21 Apr 2017 11:32:48 +0100
Subject: [PATCH] Propagate errors sensibly from proxied IS requests

When we're proxying Matrix endpoints, parse out Matrix error
responses and turn them into SynapseErrors so they can be
propagated sensibly upstream.
---
 synapse/handlers/identity.py | 10 +++++-----
 synapse/http/client.py       | 30 ++++++++++++++++++++++++++++++
 synapse/server.py            |  8 +++++++-
 3 files changed, 42 insertions(+), 6 deletions(-)

diff --git a/synapse/handlers/identity.py b/synapse/handlers/identity.py
index 6a53c5eb4..41f978d99 100644
--- a/synapse/handlers/identity.py
+++ b/synapse/handlers/identity.py
@@ -35,7 +35,7 @@ class IdentityHandler(BaseHandler):
     def __init__(self, hs):
         super(IdentityHandler, self).__init__(hs)
 
-        self.http_client = hs.get_simple_http_client()
+        self.proxy_client = hs.get_matrix_proxy_client()
 
         self.trusted_id_servers = set(hs.config.trusted_third_party_id_servers)
         self.trust_any_id_server_just_for_testing_do_not_use = (
@@ -83,7 +83,7 @@ class IdentityHandler(BaseHandler):
 
         data = {}
         try:
-            data = yield self.http_client.get_json(
+            data = yield self.proxy_client.get_json(
                 "https://%s%s" % (
                     id_server,
                     "/_matrix/identity/api/v1/3pid/getValidated3pid"
@@ -118,7 +118,7 @@ class IdentityHandler(BaseHandler):
             raise SynapseError(400, "No client_secret in creds")
 
         try:
-            data = yield self.http_client.post_urlencoded_get_json(
+            data = yield self.proxy_client.post_urlencoded_get_json(
                 "https://%s%s" % (
                     id_server, "/_matrix/identity/api/v1/3pid/bind"
                 ),
@@ -151,7 +151,7 @@ class IdentityHandler(BaseHandler):
         params.update(kwargs)
 
         try:
-            data = yield self.http_client.post_json_get_json(
+            data = yield self.proxy_client.post_json_get_json(
                 "https://%s%s" % (
                     id_server,
                     "/_matrix/identity/api/v1/validate/email/requestToken"
@@ -185,7 +185,7 @@ class IdentityHandler(BaseHandler):
         params.update(kwargs)
 
         try:
-            data = yield self.http_client.post_json_get_json(
+            data = yield self.proxy_client.post_json_get_json(
                 "https://%s%s" % (
                     id_server,
                     "/_matrix/identity/api/v1/validate/msisdn/requestToken"
diff --git a/synapse/http/client.py b/synapse/http/client.py
index ca2f770f5..c8b76b219 100644
--- a/synapse/http/client.py
+++ b/synapse/http/client.py
@@ -145,6 +145,9 @@ class SimpleHttpClient(object):
 
         body = yield preserve_context_over_fn(readBody, response)
 
+        if response.code / 100 != 2:
+            raise CodeMessageException(response.code, body)
+
         defer.returnValue(json.loads(body))
 
     @defer.inlineCallbacks
@@ -306,6 +309,33 @@ class SimpleHttpClient(object):
         defer.returnValue((length, headers, response.request.absoluteURI, response.code))
 
 
+class MatrixProxyClient(object):
+    """
+    An HTTP client that proxies other Matrix endpoints, ie. if the remote endpoint
+    returns Matrix-style error response, this will raise the appropriate SynapseError
+    """
+    def __init__(self, hs):
+        self.simpleHttpClient = SimpleHttpClient(hs)
+
+    @defer.inlineCallbacks
+    def post_json_get_json(self, uri, post_json):
+        try:
+            result = yield self.simpleHttpClient.post_json_get_json(uri, post_json)
+            defer.returnValue(result)
+        except CodeMessageException as cme:
+            ex = None
+            try:
+                errbody = json.loads(cme.msg)
+                errcode = errbody['errcode']
+                errtext = errbody['error']
+                ex = SynapseError(cme.code, errtext, errcode)
+            except:
+                pass
+            if ex is not None:
+                raise ex
+            raise cme
+
+
 # XXX: FIXME: This is horribly copy-pasted from matrixfederationclient.
 # The two should be factored out.
 
diff --git a/synapse/server.py b/synapse/server.py
index 631015256..f73aef19c 100644
--- a/synapse/server.py
+++ b/synapse/server.py
@@ -48,7 +48,9 @@ from synapse.handlers.typing import TypingHandler
 from synapse.handlers.events import EventHandler, EventStreamHandler
 from synapse.handlers.initial_sync import InitialSyncHandler
 from synapse.handlers.receipts import ReceiptsHandler
-from synapse.http.client import SimpleHttpClient, InsecureInterceptableContextFactory
+from synapse.http.client import (
+    SimpleHttpClient, InsecureInterceptableContextFactory, MatrixProxyClient
+)
 from synapse.http.matrixfederationclient import MatrixFederationHttpClient
 from synapse.notifier import Notifier
 from synapse.push.pusherpool import PusherPool
@@ -127,6 +129,7 @@ class HomeServer(object):
         'filtering',
         'http_client_context_factory',
         'simple_http_client',
+        'matrix_proxy_client',
         'media_repository',
         'federation_transport_client',
         'federation_sender',
@@ -188,6 +191,9 @@ class HomeServer(object):
     def build_simple_http_client(self):
         return SimpleHttpClient(self)
 
+    def build_matrix_proxy_client(self):
+        return MatrixProxyClient(self)
+
     def build_v1auth(self):
         orf = Auth(self)
         # Matrix spec makes no reference to what HTTP status code is returned,