Further registration refactoring

* `RegistrationHandler.appservice_register` no longer issues an access token:
  instead it is left for the caller to do it. (There are two of these, one in
  `synapse/rest/client/v1/register.py`, which now simply calls
  `AuthHandler.issue_access_token`, and the other in
  `synapse/rest/client/v2_alpha/register.py`, which is covered below).

* In `synapse/rest/client/v2_alpha/register.py`, move the generation of
  access_tokens into `_create_registration_details`. This means that the normal
  flow no longer needs to call `AuthHandler.issue_access_token`; the
  shared-secret flow can tell `RegistrationHandler.register` not to generate a
  token; and the appservice flow continues to work despite the above change.

2 files changed, 40 insertions, 14 deletions

diff --git a/synapse/rest/client/v1/register.py b/synapse/rest/client/v1/register.py

index 8e1f1b7845..28b59952c3 100644
--- a/synapse/rest/client/v1/register.py
+++ b/synapse/rest/client/v1/register.py
@@ -60,6 +60,7 @@ class RegisterRestServlet(ClientV1RestServlet):
         # TODO: persistent storage
         self.sessions = {}
         self.enable_registration = hs.config.enable_registration
+        self.auth_handler = hs.get_auth_handler()
 
     def on_GET(self, request):
         if self.hs.config.enable_registration_captcha:
@@ -299,9 +300,10 @@ class RegisterRestServlet(ClientV1RestServlet):
         user_localpart = register_json["user"].encode("utf-8")
 
         handler = self.handlers.registration_handler
-        (user_id, token) = yield handler.appservice_register(
+        user_id = yield handler.appservice_register(
             user_localpart, as_token
         )
+        token = yield self.auth_handler.issue_access_token(user_id)
         self._remove_session(session)
         defer.returnValue({
             "user_id": user_id,
diff --git a/synapse/rest/client/v2_alpha/register.py b/synapse/rest/client/v2_alpha/register.py

index 5db953a1e3..04004cfbbd 100644
--- a/synapse/rest/client/v2_alpha/register.py
+++ b/synapse/rest/client/v2_alpha/register.py
@@ -226,19 +226,17 @@ class RegisterRestServlet(RestServlet):
 
             add_email = True
 
-        access_token = yield self.auth_handler.issue_access_token(
+        result = yield self._create_registration_details(
             registered_user_id
         )
 
         if add_email and result and LoginType.EMAIL_IDENTITY in result:
             threepid = result[LoginType.EMAIL_IDENTITY]
             yield self._register_email_threepid(
-                registered_user_id, threepid, access_token,
+                registered_user_id, threepid, result["access_token"],
                 params.get("bind_email")
             )
 
-        result = yield self._create_registration_details(registered_user_id,
-                                                         access_token)
         defer.returnValue((200, result))
 
     def on_OPTIONS(self, _):
@@ -246,10 +244,10 @@ class RegisterRestServlet(RestServlet):
 
     @defer.inlineCallbacks
     def _do_appservice_registration(self, username, as_token):
-        (user_id, token) = yield self.registration_handler.appservice_register(
+        user_id = yield self.registration_handler.appservice_register(
             username, as_token
         )
-        defer.returnValue((yield self._create_registration_details(user_id, token)))
+        defer.returnValue((yield self._create_registration_details(user_id)))
 
     @defer.inlineCallbacks
     def _do_shared_secret_registration(self, username, password, mac):
@@ -273,10 +271,12 @@ class RegisterRestServlet(RestServlet):
                 403, "HMAC incorrect",
             )
 
-        (user_id, token) = yield self.registration_handler.register(
-            localpart=username, password=password
+        (user_id, _) = yield self.registration_handler.register(
+            localpart=username, password=password, generate_token=False,
         )
-        defer.returnValue((yield self._create_registration_details(user_id, token)))
+
+        result = yield self._create_registration_details(user_id)
+        defer.returnValue(result)
 
     @defer.inlineCallbacks
     def _register_email_threepid(self, user_id, threepid, token, bind_email):
@@ -349,11 +349,31 @@ class RegisterRestServlet(RestServlet):
         defer.returnValue()
 
     @defer.inlineCallbacks
-    def _create_registration_details(self, user_id, token):
-        refresh_token = yield self.auth_handler.issue_refresh_token(user_id)
+    def _create_registration_details(self, user_id):
+        """Complete registration of newly-registered user
+
+        Issues access_token and refresh_token, and builds the success response
+        body.
+
+        Args:
+            (str) user_id: full canonical @user:id
+
+
+        Returns:
+            defer.Deferred: (object) dictionary for response from /register
+        """
+
+        access_token = yield self.auth_handler.issue_access_token(
+            user_id
+        )
+
+        refresh_token = yield self.auth_handler.issue_refresh_token(
+            user_id
+        )
+
         defer.returnValue({
             "user_id": user_id,
-            "access_token": token,
+            "access_token": access_token,
             "home_server": self.hs.hostname,
             "refresh_token": refresh_token,
         })
@@ -366,7 +386,11 @@ class RegisterRestServlet(RestServlet):
             generate_token=False,
             make_guest=True
         )
-        access_token = self.auth_handler.generate_access_token(user_id, ["guest = true"])
+        access_token = self.auth_handler.generate_access_token(
+            user_id, ["guest = true"]
+        )
+        # XXX the "guest" caveat is not copied by /tokenrefresh. That's ok
+        # so long as we don't return a refresh_token here.
         defer.returnValue((200, {
             "user_id": user_id,
             "access_token": access_token,