Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Support IS v2 API with authentication for requests proxied to the IS #5786

Closed
jryans opened this issue Jul 29, 2019 · 11 comments
Closed

Support IS v2 API with authentication for requests proxied to the IS #5786

jryans opened this issue Jul 29, 2019 · 11 comments
Assignees
Labels
z-privacy-sprint (Deprecated Label)

Comments

@jryans
Copy link
Contributor

jryans commented Jul 29, 2019

Some homeserver requests (such as /account/3pid/email/requestToken) are proxied to the IS.

We are in the process of upgrading ISes to support MSC2140, which defines an IS v2 API with authentication.

Homeservers that proxy to the IS will need to add support for this new API version and auth scheme. It is anticipated that the IS v1 API will be deprecated and removed soon after v2 goes live.

@jryans
Copy link
Contributor Author

jryans commented Aug 12, 2019

For requests proxied by the HS to the IS, the HS should relay the M_TERMS_NOT_SIGNED error back to the client, so that it's clear how the client should resolve the situation. This will ease some of the complexity involved in element-hq/element-web#10525. (I have mentioned this detail in a comment on the MSC.)

@jryans
Copy link
Contributor Author

jryans commented Aug 12, 2019

After another look at the MSC, the client is meant to send the is_token to the HS for requests that the HS proxies to the IS, so please take that into account when implementing this MSC.

@anoadragon453
Copy link
Member

Are we doing this for Synapse's /_matrix/client/r0/register/email/requestToken, /_matrix/client/r0/account/password/email/requestToken, or are we not bothering since we eventually want to get rid of the homeserver contacting the identity server for these?

Aka, are we only implementing API v2 for /account/3pid/email/requestToken and 3PID invites?

@dbkr
Copy link
Member

dbkr commented Aug 20, 2019

Yeah, I wouldn't bother with the register / password requestTokens if synapse is just deprecating any use of ISes for that.

@jryans
Copy link
Contributor Author

jryans commented Aug 20, 2019

I think you'd also need to cover /account/3pid/msisdn/requestToken, so the full list would be:

  • /account/3pid/email/requestToken
  • /account/3pid/msisdn/requestToken
  • /rooms/{roomId}/invite for 3PID invites

MSC2140 doesn't appear to clearly state that an is_token should be passed for 3PID invites, so I'll add a comment.

@jryans
Copy link
Contributor Author

jryans commented Aug 20, 2019

MSC comment added.

@jryans
Copy link
Contributor Author

jryans commented Aug 21, 2019

(As a small aside, please note that the token key was recently renamed in the MSC to id_access_token.)

I've been testing this out in Riot by passing the access token... Unfortunately (at least for the requestToken endpoints), current Synapse fails with things like TypeError: requestEmailToken() got an unexpected keyword argument 'id_access_token' when passing additional arguments in the request.

I am afraid this means we may need to add a temporary extra flag in /versions to indicate to the client whether the HS is able to accept the access token or not.

@jryans
Copy link
Contributor Author

jryans commented Aug 22, 2019

I will develop the Riot code here to check for m.id_access_token in unstable_features so I am not blocked on this. We can always tweak what Riot checks for later if the backend approach changes.

@anoadragon453
Copy link
Member

So the m.id_access_token flag just needs to be a constant True to tell the client that we will accept it, right?

@jryans
Copy link
Contributor Author

jryans commented Aug 29, 2019

So the m.id_access_token flag just needs to be a constant True to tell the client that we will accept it, right?

Yes, I think so. It only exists as marker to say "this Synapse will not explode if I add this extra data to the request".

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
z-privacy-sprint (Deprecated Label)
Projects
None yet
Development

No branches or pull requests

3 participants