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

How to communicate per-user experimental features to clients? #1532

Closed
H-Shay opened this issue May 23, 2023 · 6 comments
Closed

How to communicate per-user experimental features to clients? #1532

H-Shay opened this issue May 23, 2023 · 6 comments

Comments

@H-Shay
Copy link
Contributor

H-Shay commented May 23, 2023

In Synapse we are implementing the ability to turn some unstable features on per-user (via the admin API). Once this is implemented, certain experimental features will be available to be toggled on per-person via the Admin API, in addition to retaining the ability to toggle on those features system-wide in the configuration.

However, this poses a problem when considering how to advertise that those features are enabled to the clients. Traditionally, clients checked the /versions endpoint, which in turn checked the Synapse configuration to determine what experimental features were enabled. With the changes being implemented this is no longer possible, as some experimental features may be enabled per-user. As the /versions endpoint is not authed and as such does not have an access token that we can extract user info from to determine which unstable features are currently available, there is no way to correctly communicate to clients which experimental features are enabled.

Some suggestions to remedy this are:

  • make /versions optionally accept authentication and ask the clients to change their request code to use the authed /versions when checking which unstable features are enabled.
  • move advertising the unstable features to the /capabilities, which is authed

Which of these would be most amenable - or is there another more acceptable solution? Would this require a MSC?

@turt2live
Copy link
Member

It might depend on which features in particular are looking to be user-scoped, as advertising unstable features is (usually) a problem for the MSC to solve, however if the feature is meant to be per-user once stable then there's a different conversation to be had about the MSC/feature's merits and options.

@H-Shay
Copy link
Contributor Author

H-Shay commented May 23, 2023

The current features that would be available per-user under this scheme are:
matrix-org/matrix-spec-proposals#3026: busy presence state enabled
matrix-org/matrix-spec-proposals#3881: enable remotely toggling push notifications for another client
matrix-org/matrix-spec-proposals#3967: do not require UIA when first uploading cross-signing keys.

I don't think these are meant to be per-user after approval? The idea is give server admins the ability to either switch these on for all users (as is currently possible) or allow for individual users to opt-in even if the features are not enabled for the whole server - presumably until the features in question are stabilized, in which case they should be available to all users of the server.

@turt2live
Copy link
Member

hmm, that's a bit harder to think about. Generally speaking, the spec tries to not get involved in the exact details of unstable implementation to give the most flexibility possible to the MSC. However, giving MSC authors, and therefore MSC implementors, more tools to prove the feature is equally generally welcome.

In short, I think your suggestion to make /versions optionally require authentication would fit best for the use case here (and does require an MSC, albeit a small one). The MSC might have a fair bit of discussion attached to it, though.

From a spec perspective, I'm not sure it's the best idea to present unstable features conditionally to users which aren't meant to be conditional upon acceptance to the spec. This is an area where the spec largely defers to implementations though, as it's not exactly the spec's place to say how unstable features are handled (except for requiring that they use unstable namespaces, and generally encouraging people to consider how bad the tech debt will be upon the MSC becoming stable).

The rationale for using /versions with auth over something else is a bit long, and optional reading:

Ideally, to keep the spec completely independent from the mechanics of unstable feature development we'd ask that Synapse create a dedicated API surface of some sort to manage this case. Maybe it implements an optionally authed /versions, introduces an all-new endpoint, or uses a namespaced capability, any of which may or may not end up in an MSC for other servers to follow. A challenge with this would be that it creates de-facto spec so long as clients adopt usage of it, which means MSC or not it should be in the spec. There's also risk that clients just won't implement a server-specific API for any number of reasons, making the feature a waste of engineering effort.

We are more opposed to de-facto spec than slightly ugly implementation, which means we could suggest using /capabilities formally for this, but we also made a very clear and conscious decision to say that /capabilities should never be a way to turn features on and off like this. Admittedly, this was to protect the Client-Server API from becoming modular and not for unstable features, however opening the door for unstable features can easily lead to a modular CS API. Currently, servers which implement any part of the CS API are required to implement all of it, unless otherwise noted within the spec. Whether this is a reasonable bar is a conversation for another thread.

If we don't want implementations to create their own API surface and we don't want to use /capabilties, then we're either defining a new API or using something we already have. New endpoints are often harder to get implemented, and /versions has already been abused to advertise unstable features. Adding auth to /versions thus becomes the least resistant path forwards.

A problem for the MSC to solve/consider is how to prevent servers from lying to authed clients about the spec versions they implement, or what the unauthed unstable_features object should say, or if the spec needs to care about these "problems" at all. It may be that the MSC takes the direction of a new endpoint (hopefully deprecating the use of /versions for unstable feature flagging along the way), but I'd personally find that harder to justify.

@H-Shay
Copy link
Contributor Author

H-Shay commented May 24, 2023

Thank you so much for the thorough reply - your illumination of the rationale and context behind your suggestion was very helpful. I will submit a MSC to make /versions optionally require authentication and see where that goes.

@zecakeh
Copy link
Contributor

zecakeh commented Mar 29, 2024

This is solved now that MSC4026 is merged in version 1.10?

@clokep clokep closed this as completed Mar 29, 2024
@clokep
Copy link
Member

clokep commented Mar 29, 2024

Good catch!

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

No branches or pull requests

4 participants