Update proposal

Notes on problems, workaround, and another alternative

proposals/1730-cs-api-in-login-response.md

@@ -29,6 +29,35 @@ A representative sequence diagram is shown below.
 
 ![Sequence diagram](images/1730-seq-diagram.svg)
 
+### Potential issues
+
+A significant problem with the proposed architecture is that the portal server
+has to proxy the `/login` request, so that it can update the response. This
+leads to the following concerns:
+
+* The target homeserver sees the request coming from the portal server rather
+  than the client, so that the wrong IP address will be recorded against the
+  user's session. (This might be a problem for, for example, IP locking the
+  session, and might affect the `last_seen_ip` field returned by `GET
+  /_matrix/client/r0/devices`.)
+
+  This can be mitigated to some extent via the use of an `X-Forwarded-For`
+  header, but that then requires the portal server to authenticate itself with
+  the target homeserver in some way.
+
+* It causes additional complexity in the portal server, which must now be
+  responsible for making outbound HTTP requests.
+
+* It potentially leads to a privacy leak, since the portal server could snoop
+  on the returned access token. (Given that the portal server must be trusted
+  to some extent in this architecture, it is unclear how much of a concern this
+  really is.)
+
+An alternative implementation of the portal server would be for the portal
+server to redirect the `/login` request with a 307 response. This solves the
+above problems, but may reduce flexibility, or require more state to be managed
+on the portal server [1].
+
 ## Tradeoffs
 
 Alternative solutions might include:
@@ -64,3 +93,23 @@ problems:
 * Since the portal already has knowledge of the location of the C-S API for the
   target homeserver, and has mapped the login request onto the correct HS, it
   feels redundant to have a separate mechanism which repeats that mapping.
+
+### Add an alternative redirection mechanism in the login flow
+
+We could specify that the `/login` response could contain a `redirect` field
+property instead of the existing `user_id`/`access_token`/`device_id`
+properties. The `redirect` property would give the C-S API of the target
+HS. The client would then repeat its `/login` request, and use the specified
+endpoint for all future C-S interaction.
+
+This approach would complicate client implementations.
+
+
+[1] The reason more state is needed is as follows: because the portal is now
+redirecting the login rather than proxying it, it cannot modify the login
+dictionary. This is a problem for the single-sign-on flow, which culminates in
+an `m.login.token` login. The only way that the portal can identify a given
+user session - and thus know where to redirect to - is via the login token, and
+of course, it cannot modify that token without making it invalid for the target
+HS. It therefore has to use the login token as a session identifier, and store
+session state..