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

The on_threepid_bind module API is called when a 3PID is *added* (associated locally), not when it is *bound* #14955

Closed
anoadragon453 opened this issue Jan 31, 2023 · 3 comments
Labels
A-3PID 3rd party identifiers: e.g. email, phone number O-Occasional Affects or can be seen by some users regularly or most users rarely S-Minor Blocks non-critical functionality, workarounds exist. T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks.

Comments

@anoadragon453
Copy link
Member

anoadragon453 commented Jan 31, 2023

Confusing terminology strikes again. Generally, if you "add" a third-party ID (3PID) to your account in Matrix, you are making an association between your Matrix ID and a 3PID that is scoped to your homeserver. You can allow your Matrix ID to be discovered by remote users who have your 3PID by "binding" the 3PID to your Matrix ID on an identity server.

This terminology is reflected in the endpoints, and was coined during the privacy sprint.

Associating your 3PID with your Matrix ID on your homeserver

Binding a 3PID to your Matrix ID on an identity server

We have a on_threepid_bind module callback which fires when you add an association between a 3PID and your Matrix ID on your local homeserver. It specifically does not fire when you perform a bind against and identity server.

I think the ideal scenario would be to have 2 module callback methods:

  • async def on_update_user_third_party_identifier(user_id: str, medium: str, address: str) - called before a user adds or removes a 3PID to their account at the homeserver scope.
  • async def on_update_identity_server_binding(id_server: str, user_id: str, medium: str, address: str) - called before a user adds or removes a binding of their 3PID and Matrix to a given identity server.

It may also be advantageous to allow these methods to block the action. When a module decides to block a local association, this could result in a 400/M_UNKNOWN(?) response to the client. If blocking a bind against an identity server, this could instead result in a response of 200, {"id_server_unbind_result": "no-support"} being sent to the client. This would allow you to prevent a user from making a binding of non-company email addresses, or to identity servers other than those that are authorised.

@anoadragon453 anoadragon453 added A-3PID 3rd party identifiers: e.g. email, phone number T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks. labels Jan 31, 2023
@anoadragon453
Copy link
Member Author

#13227 raises another use case for the on_update_identity_server_binding method. If a module wanted to take over the task of unbinding from an identity server that the client was not aware of, then it would need a module callback to fire when a user attempted to perform an unbind - whether or not any binding on any identity servers are actually known to the homeserver.

In this case, the proposed design above wouldn't cut it. on_update_identity_server_binding would not be called if an association on an identity server was not known to the homeserver.

Additionally, if several bindings were known to the homeserver, then on_update_identity_server_binding would be called multiple times. And unless the Synapse module did some book-keeping itself, it would likely attempt to perform an unbind against its configured identity server more than once.

Perhaps instead, on_update_identity_server_binding could look like:

async def on_update_identity_server_binding(user_id: str, medium: str, address: str, id_server: Optional[str]) -> bool

where id_server should be set to an identity server domain string if an attempt to unbind against a specific identity server is about to happen. Additionally, any time a user's action would result in an unbind of an association (if they call POST /_matrix/client/v3/account/3pid/unbind or POST /_matrix/client/v3/account/3pid/delete, on_update_identity_server_binding would be called with id_server set to None.

A module such as synapse-bind-sydent, which performs 3PID binds and unbinds against a configured Sydent instance, could act only on the case that id_server is None, and ignore any other (dis)associations the client tries to do.

@anoadragon453
Copy link
Member Author

anoadragon453 commented Feb 1, 2023

It may also be advantageous to allow these methods to block the action. When a module decides to block a local association, this could result in a 400/M_UNKNOWN(?) response to the client.

Well, unfortunately the spec doesn't specify a 400 error code for either POST /_matrix/client/v3/account/3pid/add nor POST /_matrix/client/v3/account/3pid/bind (opened an issue at matrix-org/matrix-spec#1426) yet. Until that is resolved, we should probably just have each method return None.

It'd be pretty easy to change those methods to allow returning a bool in the future.

@H-Shay H-Shay added O-Occasional Affects or can be seen by some users regularly or most users rarely S-Minor Blocks non-critical functionality, workarounds exist. labels Feb 2, 2023
@anoadragon453
Copy link
Member Author

on_threepid_bind has been deprecated in favour of a new on_add_user_third_party_identifier module API callback, added in #15044.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-3PID 3rd party identifiers: e.g. email, phone number O-Occasional Affects or can be seen by some users regularly or most users rarely S-Minor Blocks non-critical functionality, workarounds exist. T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks.
Projects
None yet
Development

No branches or pull requests

2 participants