Rework UI Auth session validation for registration (#7455)

Be less strict about validation of UI authentication sessions during
registration to match client expecations.

1 files changed, 40 insertions, 14 deletions

diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py
index 7613e5b6ab..9c71702371 100644
--- a/synapse/handlers/auth.py
+++ b/synapse/handlers/auth.py
@@ -252,6 +252,7 @@ class AuthHandler(BaseHandler):
         clientdict: Dict[str, Any],
         clientip: str,
         description: str,
+        validate_clientdict: bool = True,
     ) -> Tuple[dict, dict, str]:
         """
         Takes a dictionary sent by the client in the login / registration
@@ -277,6 +278,10 @@ class AuthHandler(BaseHandler):
             description: A human readable string to be displayed to the user that
                          describes the operation happening on their account.
 
+            validate_clientdict: Whether to validate that the operation happening
+                                 on the account has not changed. If this is false,
+                                 the client dict is persisted instead of validated.
+
         Returns:
             A tuple of (creds, params, session_id).
 
@@ -317,30 +322,51 @@ class AuthHandler(BaseHandler):
             except StoreError:
                 raise SynapseError(400, "Unknown session ID: %s" % (sid,))
 
+            # If the client provides parameters, update what is persisted,
+            # otherwise use whatever was last provided.
+            #
+            # This was designed to allow the client to omit the parameters
+            # and just supply the session in subsequent calls so it split
+            # auth between devices by just sharing the session, (eg. so you
+            # could continue registration from your phone having clicked the
+            # email auth link on there). It's probably too open to abuse
+            # because it lets unauthenticated clients store arbitrary objects
+            # on a homeserver.
+            #
+            # Revisit: Assuming the REST APIs do sensible validation, the data
+            # isn't arbitrary.
+            #
+            # Note that the registration endpoint explicitly removes the
+            # "initial_device_display_name" parameter if it is provided
+            # without a "password" parameter. See the changes to
+            # synapse.rest.client.v2_alpha.register.RegisterRestServlet.on_POST
+            # in commit 544722bad23fc31056b9240189c3cbbbf0ffd3f9.
             if not clientdict:
-                # This was designed to allow the client to omit the parameters
-                # and just supply the session in subsequent calls so it split
-                # auth between devices by just sharing the session, (eg. so you
-                # could continue registration from your phone having clicked the
-                # email auth link on there). It's probably too open to abuse
-                # because it lets unauthenticated clients store arbitrary objects
-                # on a homeserver.
-                # Revisit: Assuming the REST APIs do sensible validation, the data
-                # isn't arbitrary.
                 clientdict = session.clientdict
 
             # Ensure that the queried operation does not vary between stages of
             # the UI authentication session. This is done by generating a stable
-            # comparator based on the URI, method, and body (minus the auth dict)
-            # and storing it during the initial query. Subsequent queries ensure
-            # that this comparator has not changed.
-            comparator = (uri, method, clientdict)
-            if (session.uri, session.method, session.clientdict) != comparator:
+            # comparator based on the URI, method, and client dict (minus the
+            # auth dict) and storing it during the initial query. Subsequent
+            # queries ensure that this comparator has not changed.
+            if validate_clientdict:
+                session_comparator = (session.uri, session.method, session.clientdict)
+                comparator = (uri, method, clientdict)
+            else:
+                session_comparator = (session.uri, session.method)  # type: ignore
+                comparator = (uri, method)  # type: ignore
+
+            if session_comparator != comparator:
                 raise SynapseError(
                     403,
                     "Requested operation has changed during the UI authentication session.",
                 )
 
+            # For backwards compatibility the registration endpoint persists
+            # changes to the client dict instead of validating them.
+            if not validate_clientdict:
+                await self.store.set_ui_auth_clientdict(sid, clientdict)
+
         if not authdict:
             raise InteractiveAuthIncompleteError(
                 self._auth_dict_for_flows(flows, session.session_id)