matrix-org / synapse

Synapse: Matrix homeserver written in Python/Twisted.
https://matrix-org.github.io/synapse
Apache License 2.0
11.82k stars 2.13k forks source link

Remove `account_threepid_delegates` option #5881

Open anoadragon453 opened 5 years ago

anoadragon453 commented 5 years ago

With https://github.com/matrix-org/synapse/pull/5835, we're allowing homeservers to be able to send their own registration emails. Earlier, we allowed homeservers to be able to send password reset emails themselves. Both of these options were just considered temporary to migrate people over from relying on vector.im or another identity server to handle SMTP, to handling SMTP themselves if they required the functionality.

Once a sufficient amount of time has passed, we should remove these options and yell from the mountains that people need to manage SMTP from their Synapse install or disable it altogether.

erikjohnston commented 3 years ago

Removing trust_identity_server_for_password_resets seems fine, but I'm less sure about account_threepid_delegate. We don't seem to mention in the sample config that using it is delegated (its just in the UPGRADE notes), and is also currently the only way of support validating phone numbers?

richvdh commented 3 years ago

sigh. That's annoying, as it makes #9677 harder.

erikjohnston commented 3 years ago

We should figure out if MSISDN is still a thing that is a) used and b) we want to continue to support. It would be nice to strip all this out if its not being used. We should pull the numbers out for how often msisdn is used in Sydent.

Otherwise, we need to port the code to use the new v2 sydent API (or make this work with v2 API)

anoadragon453 commented 3 years ago

We've identified one Element customer that still makes use of this option, though has not had a msisdn registration on their identity server since March 2021. So this may no longer be a concern for any known entity.

However, the context for why some wish to make use of this feature is to improve UX when creating mappings on an identity server for every user is a necessity - rather than something that can happen lazily and at the user's request.

Essentially, if you need 3PID<->MXID mappings to appear on your identity server during user registration (without asking the user to validate their 3PID twice - once for the hs and another time for the is), this feature - plus some Sydent modifications - is one way of going about it.

DINUM were using account_threepid_delegates (for delegation of email verification), but are no longer. Their need was to have the identity server be aware of user's emails - but this has been sidestepped by a little hack to instead use Sydent's internal bind API to register 3PID mappings. They also do not validate msisdns.

Of course it's also nice to allow users to add their phone numbers to their homeserver account. And in Element's UI today, you can't add a phone number to an identity server without first adding it to your homeserver. TravisR has also helpfully noted that Element Web does still support msisdn verification, but matrix.org does not present the UIAA stage as an option to trigger it.

Removing trust_identity_server_for_password_resets seems fine

I'm not aware of anyone else using trust_identity_server_for_password_resets either.

richvdh commented 3 years ago

I've created a separate issue (#10545) to handle removing trust_identity_server_for_password_reset, since that's rather easier.

richvdh commented 3 years ago

However, the context for why some wish to make use of this feature is to improve UX when creating mappings on an identity server for every user is a necessity - rather than something that can happen lazily and at the user's request.

Essentially, if you need 3PID<->MXID mappings to appear on your identity server during user registration (without asking the user to validate their 3PID twice - once for the hs and another time for the is), this feature - plus some Sydent modifications - is one way of going about it.

Having investigated a bit, I'm reasonably sure that this is a red herring. In particular, even if you enable account_threepid_delegates, the IS still isn't told about the MXID for the user that is registering, so can't store a 3PID<->MXID mapping.

In situations where a single organisation controls both the HS and the IS, and wants registration on one to automatically cause a bind on the other, I think the correct approach is Sydent's internal bind API. We could look into implementing support for this in Synapse; alternatively I think it's pretty easily done with a module.

richvdh commented 3 years ago

Let me try to summarize where we stand.

Background

account_threepid_delegates controls whether Synapse performs a threepid validation itself, or delegates that operation to an identity server. In the case of email, Synapse has supported doing its own validation since v1.4.0, but it still has no support for doing phone number validation itself.

Synapse performs threepid validation in the following scenarios:

Sidebar: what are the reasons that a user might want to add an email address or phone number to their homeserver account?

  1. Element-web makes it a prerequisite for adding those addresses to an identity server, which is required for that user to be discoverable via the email address or phone number.
  2. It allows an alternative mechanism for logging in (you can provide an email address or msisdn instead of username.)
  3. In the case of email, but /not/ phone number, it provides a password recovery mechanism.
  4. In the case of email, but /not/ phone number, it means that you can receive notifications of missed messages, etc, via email.

We announced that we would turn off support for threepid validation delegation in Sydent in December 2019, so we're 18 months overdue. Unfortunately, we can't do that right now, because there remains a valid usecase for it:

Adding phone numbers to HS accounts

As above, Synapse does not support doing its own validation of phone numbers, so currently whenever you add a phone number to your HS Account, it calls out to an identity server to vaidate it.

I think first we need to decide if supporting MSISDNs on HS accounts is actually a useful feature - how many people actually use it for logging in? (Or rather, how many new users would miss the ability to do so, since we're not talking about breaking existing MSISDN<->user associations).

Likewise, how many people use MSISDN as a way to look up other users?

I think we have four options here:

  1. Completely drop support for adding new MSISDNs to homeserver accounts and Identity Servers.
  2. Support adding MSISDNs to Identity Servers, for user discovery, but not Homeservers (for login, presumably). This would require development on the Element "account settings" page but is otherwise supported by the protocol and Synapse today.
  3. Full support as today - two suboptions: 3a. Add support for MSISDN validation (via an SMS aggregator) to Synapse. 3b. Roll back on our stated intention to turn off support for threepid validation delegation and reintroduce the API endpoints that it requires back into the spec (or design and use new ones).
babolivier commented 3 years ago

Likewise, how many people use MSISDN as a way to look up other users?

I think mobile clients (at least Element) use this to try and discover Matrix users from your phone's contact? Which sounds like something we probably want to keep supporting.

callahad commented 3 years ago

@callahad to discuss this with Element product folks to make sure we're not missing anything

DMRobertson commented 2 years ago

@callahad to discuss this with Element product folks to make sure we're not missing anything

Bump. Any news?

callahad commented 2 years ago

Let's have @neilisfragile + @nadonomy + @richvdh chat about this one and find a conclusion...

callahad commented 2 years ago

Neil to own scheduling that chat... 🔥 🥔 💨

richvdh commented 2 years ago

13192 drops support for delegation of email validation, which means we now only need to worry about phone number validation.

https://github.com/matrix-org/synapse/issues/5881#issuecomment-894291317 sets out the situation there, but to summarize, we need to do one of:

  1. Completely drop support for adding new MSISDNs to homeserver accounts and Identity Servers.
  2. Support adding MSISDNs to Identity Servers, for user discovery, but not Homeservers (for login, presumably). This would require development on the Element "account settings" page but is otherwise supported by the protocol and Synapse today.
  3. Full support as today - two suboptions: 3a. Add support for MSISDN validation (via an SMS aggregator) to Synapse. 3b. Roll back on our stated intention to turn off support for threepid validation delegation and reintroduce the API endpoints that it requires back into the spec (or design and use new ones).

It is worth noting that this overlaps with the OIDC work to some extent, in that in future it will be up to the OIDC authentication server to manage login. Anyway, this is all covered by https://github.com/vector-im/element-meta/issues/478.

reivilibre commented 2 years ago

It appears that ma1sd might support SMS validation; I don't know if it uses the affected APIs :/ https://github.com/ma1uta/ma1sd

richvdh commented 2 years ago

It appears that ma1sd might support SMS validation; I don't know if it uses the affected APIs :/ ma1uta/ma1sd

I think this is https://github.com/ma1uta/ma1sd/issues/114, but also somewhat orthogonal to whether Synapse uses the deprecated APIs (noting that this is a Synapse issue)

reivilibre commented 2 years ago

I meant more along the lines of: ma1sd might provide the affected APIs and expect Synapse to use them in order to provide its behaviour. If you've already opened an issue then that's fine with me

DMRobertson commented 2 years ago

Coming back over this during triage and not sure what the next steps are. Looks like we need to decide which of Rich's options here we take?

richvdh commented 2 years ago

Yes. Still.

Given that matrix.org's/vector.im's sydents haven't supported MSISDN validation for several months (and afaik nobody else supports it because it requires a commercial agreement with openmarket), I think there's a strong argument that we could now rip out synapse's support for delegation with no loss of functionality.

MadLittleMods commented 1 year ago

Trying to summarize some notes from the triage meeting.

richvdh commented 1 year ago

This isn't a simple case of removing X because we would also want to remove the v1 API's and that's a lot of work.

I'm not sure what X refers to, but presumably "v1 APIs" refers to sydent's support for the V1 ID server api (https://github.com/matrix-org/sydent/issues/338), of which this synapse feature is one, but not the only, blocker.

richvdh commented 1 year ago
  • As @richvdh mentioned, this feature requires an OpenMarket contract which is a very specific distinction. Are people really likely to get a contract just to do this?

In theory it's possible someone could implement another service which implements the V1 IS API and verifies phone numbers a different way, and hence use this synapse feature with it. I find it very unlikely, though.