Fix fallback auth on Python 3 (#4197)

3 files changed, 121 insertions, 22 deletions

diff --git a/changelog.d/4197.bugfix b/changelog.d/4197.bugfix
new file mode 100644
index 0000000000..c7c01da0f3
--- /dev/null
+++ b/changelog.d/4197.bugfix
@@ -0,0 +1 @@
+Fallback auth now accepts the session parameter on Python 3.
diff --git a/synapse/rest/client/v2_alpha/auth.py b/synapse/rest/client/v2_alpha/auth.py
index c39f53b987..fa73bdf3a1 100644
--- a/synapse/rest/client/v2_alpha/auth.py
+++ b/synapse/rest/client/v2_alpha/auth.py
@@ -21,7 +21,7 @@ from synapse.api.constants import LoginType
 from synapse.api.errors import SynapseError
 from synapse.api.urls import CLIENT_V2_ALPHA_PREFIX
 from synapse.http.server import finish_request
-from synapse.http.servlet import RestServlet
+from synapse.http.servlet import RestServlet, parse_string
 
 from ._base import client_v2_patterns
 
@@ -131,16 +131,12 @@ class AuthRestServlet(RestServlet):
         self.auth_handler = hs.get_auth_handler()
         self.registration_handler = hs.get_handlers().registration_handler
 
-    @defer.inlineCallbacks
     def on_GET(self, request, stagetype):
-        yield
-        if stagetype == LoginType.RECAPTCHA:
-            if ('session' not in request.args or
-                    len(request.args['session']) == 0):
-                raise SynapseError(400, "No session supplied")
-
-            session = request.args["session"][0]
+        session = parse_string(request, "session")
+        if not session:
+            raise SynapseError(400, "No session supplied")
 
+        if stagetype == LoginType.RECAPTCHA:
             html = RECAPTCHA_TEMPLATE % {
                 'session': session,
                 'myurl': "%s/auth/%s/fallback/web" % (
@@ -155,10 +151,8 @@ class AuthRestServlet(RestServlet):
 
             request.write(html_bytes)
             finish_request(request)
-            defer.returnValue(None)
+            return None
         elif stagetype == LoginType.TERMS:
-            session = request.args['session'][0]
-
             html = TERMS_TEMPLATE % {
                 'session': session,
                 'terms_url': "%s_matrix/consent?v=%s" % (
@@ -176,25 +170,25 @@ class AuthRestServlet(RestServlet):
 
             request.write(html_bytes)
             finish_request(request)
-            defer.returnValue(None)
+            return None
         else:
             raise SynapseError(404, "Unknown auth stage type")
 
     @defer.inlineCallbacks
     def on_POST(self, request, stagetype):
-        yield
+
+        session = parse_string(request, "session")
+        if not session:
+            raise SynapseError(400, "No session supplied")
+
         if stagetype == LoginType.RECAPTCHA:
-            if ('g-recaptcha-response' not in request.args or
-                    len(request.args['g-recaptcha-response'])) == 0:
-                raise SynapseError(400, "No captcha response supplied")
-            if ('session' not in request.args or
-                    len(request.args['session'])) == 0:
-                raise SynapseError(400, "No session supplied")
+            response = parse_string(request, "g-recaptcha-response")
 
-            session = request.args['session'][0]
+            if not response:
+                raise SynapseError(400, "No captcha response supplied")
 
             authdict = {
-                'response': request.args['g-recaptcha-response'][0],
+                'response': response,
                 'session': session,
             }
 
diff --git a/tests/rest/client/v2_alpha/test_auth.py b/tests/rest/client/v2_alpha/test_auth.py
new file mode 100644
index 0000000000..7fa120a10f
--- /dev/null
+++ b/tests/rest/client/v2_alpha/test_auth.py
@@ -0,0 +1,104 @@
+# -*- coding: utf-8 -*-
+# Copyright 2018 New Vector
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+
+from twisted.internet.defer import succeed
+
+from synapse.api.constants import LoginType
+from synapse.rest.client.v1 import admin
+from synapse.rest.client.v2_alpha import auth, register
+
+from tests import unittest
+
+
+class FallbackAuthTests(unittest.HomeserverTestCase):
+
+    servlets = [
+        auth.register_servlets,
+        admin.register_servlets,
+        register.register_servlets,
+    ]
+    hijack_auth = False
+
+    def make_homeserver(self, reactor, clock):
+
+        config = self.default_config()
+
+        config.enable_registration_captcha = True
+        config.recaptcha_public_key = "brokencake"
+        config.registrations_require_3pid = []
+
+        hs = self.setup_test_homeserver(config=config)
+        return hs
+
+    def prepare(self, reactor, clock, hs):
+        auth_handler = hs.get_auth_handler()
+
+        self.recaptcha_attempts = []
+
+        def _recaptcha(authdict, clientip):
+            self.recaptcha_attempts.append((authdict, clientip))
+            return succeed(True)
+
+        auth_handler.checkers[LoginType.RECAPTCHA] = _recaptcha
+
+    @unittest.INFO
+    def test_fallback_captcha(self):
+
+        request, channel = self.make_request(
+            "POST",
+            "register",
+            {"username": "user", "type": "m.login.password", "password": "bar"},
+        )
+        self.render(request)
+
+        # Returns a 401 as per the spec
+        self.assertEqual(request.code, 401)
+        # Grab the session
+        session = channel.json_body["session"]
+        # Assert our configured public key is being given
+        self.assertEqual(
+            channel.json_body["params"]["m.login.recaptcha"]["public_key"], "brokencake"
+        )
+
+        request, channel = self.make_request(
+            "GET", "auth/m.login.recaptcha/fallback/web?session=" + session
+        )
+        self.render(request)
+        self.assertEqual(request.code, 200)
+
+        request, channel = self.make_request(
+            "POST",
+            "auth/m.login.recaptcha/fallback/web?session="
+            + session
+            + "&g-recaptcha-response=a",
+        )
+        self.render(request)
+        self.assertEqual(request.code, 200)
+
+        # The recaptcha handler is called with the response given
+        self.assertEqual(len(self.recaptcha_attempts), 1)
+        self.assertEqual(self.recaptcha_attempts[0][0]["response"], "a")
+
+        # Now we have fufilled the recaptcha fallback step, we can then send a
+        # request to the register API with the session in the authdict.
+        request, channel = self.make_request(
+            "POST", "register", {"auth": {"session": session}}
+        )
+        self.render(request)
+        self.assertEqual(channel.code, 200)
+
+        # We're given a registered user.
+        self.assertEqual(channel.json_body["user_id"], "@user:test")