Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Send IS for adding MSISDNs only when required by HS #10599

Closed
jryans opened this issue Aug 20, 2019 · 9 comments · Fixed by matrix-org/matrix-react-sdk#3368
Closed

Send IS for adding MSISDNs only when required by HS #10599

jryans opened this issue Aug 20, 2019 · 9 comments · Fixed by matrix-org/matrix-react-sdk#3368

Comments

@jryans
Copy link
Collaborator

jryans commented Aug 20, 2019

A few MSISDN items are unclear in the privacy sprint:

  • What is the fate of registration with MSISDN?
  • If we use the IS to add MSISDNs to your account, does it mean the HS must preserve the trusted list of allowed ISes?
@jryans jryans added X-Blocked X-Needs-Product More input needed from the Product team Privacy phase:1 labels Aug 20, 2019
@jryans
Copy link
Collaborator Author

jryans commented Aug 20, 2019

From the privacy sync:

  • Investigate usage metrics
  • Consider fallback options

@ara4n
Copy link
Member

ara4n commented Aug 27, 2019

unsure why this is still blocked - afaik the conclusions from last week were that the fate of registration by MSISDN is a) we support it you fall back to v1, b) we don't implement it on the HS for now, c) in future, we will provide a HS module to let users do it if they want to foot the bill?

@dbkr
Copy link
Member

dbkr commented Aug 28, 2019

Conclusion from IRL with Tom is that we should update synapse similar to how we did email, in that we make it use an IS of its own choosing for sending SMS, then add a corresponding flag in /versions to tell the client it can request sms tokens without supplying an ID server.

@dbkr
Copy link
Member

dbkr commented Aug 28, 2019

Filed matrix-org/synapse#5929 to track synapse changes. On the riot-web side, we just need to honour the same flag for the msisdn adding stuff.

@jryans jryans changed the title Placeholder issue for MSISDN details Send IS for adding MSISDNs only when required by HS Aug 28, 2019
@jryans jryans removed X-Blocked X-Needs-Product More input needed from the Product team labels Aug 28, 2019
@jryans
Copy link
Collaborator Author

jryans commented Aug 28, 2019

Thanks all, looks like we have a clear plan. I morphed this issue to do the Riot Web work of checking the flag. We should coordinate with Synapse side to decide if we're reusing the same flag from email, or something new for MSISDNs.

@jryans
Copy link
Collaborator Author

jryans commented Aug 29, 2019

I believe @turt2live is a writing a (previously missing) MSC around the email and MSISDNs changes, which may incidentally describe whatever flags have been agreed.

@jryans
Copy link
Collaborator Author

jryans commented Aug 30, 2019

MSC2263 records that m.require_identity_server will return a boolean that applies to both email and MSISDN cases.

@turt2live
Copy link
Member

If we use the IS to add MSISDNs to your account, does it mean the HS must preserve the trusted list of allowed ISes?

I think this question is still unanswered, or if it is answered I'm failing to interpret it. Either way, it's a good thing for a different issue, I think?

@jryans
Copy link
Collaborator Author

jryans commented Sep 2, 2019

If we use the IS to add MSISDNs to your account, does it mean the HS must preserve the trusted list of allowed ISes?

I think this question is still unanswered, or if it is answered I'm failing to interpret it. Either way, it's a good thing for a different issue, I think?

I was assuming Tom and Dave worked out that we don't need the trusted list, since they agreed to proceed in the same way we did for email handling. @lampholder, can you confirm?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants