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

Remove the unspecced, "legacy" account validity endpoints #15271

Open
anoadragon453 opened this issue Mar 15, 2023 · 0 comments
Open

Remove the unspecced, "legacy" account validity endpoints #15271

anoadragon453 opened this issue Mar 15, 2023 · 0 comments
Labels
A-Spec-Compliance places where synapse does not conform to the spec 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. X-Consult-Clients Need to investigate if this change breaks clients

Comments

@anoadragon453
Copy link
Member

anoadragon453 commented Mar 15, 2023

We have an undocumented and unspecced Account Validity feature currently implemented in Synapse. This was originally written for DINUM. Since then, all of the functionality has been moved into the module API (#9884), and can be implemented by modules. https://github.com/matrix-org/synapse-email-account-validity is the only known implementation of this API.

Synapse still has an implementation of account validity however, gated behind an undocumented account_validity config option. Completely removing the account validity feature out of Synapse's main codebase is not what this issue is about though.

In the original implementation of account validity (https://github.com/matrix-org/synapse/pull/5047/files#diff-6fc5658389633ef773a5c9ed669e5273eecff1c01311d7ac2c0721119a9ad21b, https://github.com/matrix-org/synapse/pull/5073/files#diff-6fc5658389633ef773a5c9ed669e5273eecff1c01311d7ac2c0721119a9ad21b), the following endpoints were registered:

plus one Admin API endpoint:

In the effort to move towards a module in April 2021, account validity module API callbacks were created which were tied to these "legacy" endpoints. If one of the legacy endpoints were called, then these callbacks would fire.

# Temporary hooks to allow for a transition from `/_matrix/client` endpoints
# to `/_synapse/client/account_validity`. See `register_account_validity_callbacks`.
ON_LEGACY_SEND_MAIL_CALLBACK = Callable[[str], Awaitable]
ON_LEGACY_RENEW_CALLBACK = Callable[[str], Awaitable[Tuple[bool, bool, int]]]
ON_LEGACY_ADMIN_REQUEST = Callable[[Request], Awaitable]

At the same time, modules were also given the ability to register their own endpoints under /_synapse/client/, which is what synapse-email-account-validity does does. Client implementations were encouraged to move to these /_synapse/client/email_account_validity/... endpoints instead.

https://github.com/matrix-org/synapse-email-account-validity still implements these legacy callbacks though: in case a client still uses them, or a user clicks on an email still containing an old GET /_matrix/client/v3/account_validity/renew link.

However, it has been sufficiently long since #9884 (April 2021), that I think we can remove the legacy endpoints and module API callbacks. The legacy module API callbacks are also not documented, so it's unlikely their usage has spread much.

I will ask DINUM if they are still using the unspecced /_matrix/client/... endpoints in any form. According to @giomfo, DINUM are not currently using the feature at all, but plan to again in the future. While Tchap clients have not yet been updated to point to the new endpoint, they're just going to use a proxy to rewrite the request path to ensure clients switch immediately.

Therefore I suggest we just rip out the "legacy" endpoints and module API callbacks without a deprecation period.

@anoadragon453 anoadragon453 added A-Spec-Compliance places where synapse does not conform to the spec T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks. X-Consult-Clients Need to investigate if this change breaks clients labels Mar 15, 2023
@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 Mar 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Spec-Compliance places where synapse does not conform to the spec 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. X-Consult-Clients Need to investigate if this change breaks clients
Projects
None yet
Development

No branches or pull requests

2 participants