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

De-emphasise and reduce the usage of /_synapse endpoints #544

Open
ShadowJonathan opened this issue Oct 29, 2022 · 1 comment
Open

De-emphasise and reduce the usage of /_synapse endpoints #544

ShadowJonathan opened this issue Oct 29, 2022 · 1 comment

Comments

@ShadowJonathan
Copy link
Contributor

This is something I noticed while working on conduit support (#513), where it seems that complement currently requires or emphasizes the usage of /_synapse endpoints.

Specifically, it uses them like so:

  • Under "Image Requirements" in README.md, it asks for /_synapse/admin/v1/register to be supported for "admin" registrations.
    • This is then used in RegisterUser where isAdmin is true, which is used by the following tests:
      • TestCanRegisterAdmin
      • TestServerNotices
  • TestRegistration, under some t.Runs

From a puritan perspective, these should not exist within complement, as they test Synapse-specific behaviour that other servers have to then copy uncritically, and without blessing or formalisation from the spec.

From such a perspective, the concept of "admins" is also dubious, as the spec (explicitly) makes no serious note of them, leaving server administration as an implementation detail. (See MSC3593 for some context reading on an attempt to broach/formalise this as well.)


For all of complement's current usage of this endpoint, there is one problem with a suggestion of removing all of them altogether; Server Notices are defined in the spec, and so the absence of a formal mechanism which empowers servers/users to send server notices is unfortunate, but this means complement has no "closed system" way of implementing this.

To make the rejection of a /_synapse specific API explicit, and to develop a better way of testing server notices, I'm suggesting to introduce a /_complement API space, which servers should enable only in their corresponding complement images, and (for now) be only used for sending server notices (exact API forthcoming), not to elevate individual users as a "server admin", as this concept does not properly exist from a matrix specification standpoint.

This API space should only perform individual elevated actions, such as sending a single server notice via a POST API.

Hopefully, this way, we can avoid biasing to a certain implementation, and help make complement more pure or agnostic when testing conformity.

@ShadowJonathan
Copy link
Contributor Author

One additional note: When I was converting sytests, I had to reject a bunch of them (some of them found in sytest.ignored.list) because they were testing synapse-specific behaviour such as this, would it be possible to still write them, but place them in a subdirectory of matrix-org/synapse?

This way, it is explicit that these tests are for synapse-specific behaviour, but yet they'd be using the complement framework to test them, which could become an interesting development.

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

1 participant