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

MSC2233: Unauthenticated Capabilities API #2233

Closed
wants to merge 6 commits into from
Closed

Conversation

anoadragon453
Copy link
Member

@anoadragon453 anoadragon453 commented Aug 15, 2019

@anoadragon453 anoadragon453 self-assigned this Aug 15, 2019
@turt2live turt2live added the proposal A matrix spec change proposal label Aug 15, 2019
@turt2live
Copy link
Member

(labelled for WIP)

@jryans jryans added phase:1 privacy-sprint Temporary label: privacy-related stuff labels Aug 15, 2019
dbkr added a commit to matrix-org/matrix-react-sdk that referenced this pull request Aug 16, 2019
If the server advertises that it supports doing so

This version uses a random me.dbkr prefix until the MSC is
written.

Requires matrix-org/matrix-js-sdk#1017
Implements matrix-org/matrix-spec-proposals#2233
@anoadragon453 anoadragon453 changed the title MSC2233: Expose Homeserver Email Configuration in Registration Parameters MSC2233: Unauthenticated Capabilities API Aug 16, 2019
@anoadragon453 anoadragon453 marked this pull request as ready for review August 16, 2019 11:29
Co-Authored-By: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
@@ -0,0 +1,58 @@
# Unauthenticated Capabilities API
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit concerned about the proliferation of 'features' endpoints (/versions, /_matrix/client/r0/capabilities/server, /_matrix/client/r0/capabilities/user, /_matrix/media/r0/config): the main concern being that it's not obvious what goes where.

Indeed, ISTR the main driver for /_matrix/client/r0/capabilities as opposed to /versions was that it required authentication...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was going to say that putting feature capabilities on /versions doesn't make sense given the name, but then I realized we're already doing that :|

image

These things could be added to /versions then if we think that that's not too weird.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @richvdh that we have far too many endpoints for similar things, although I was never really onboard with putting features inside /versions.

To be honest I wish we had just named them something like /_matrix/${apiname}/config/ so it was at least consistent.

If we could go back in time, I wish we hadn't polluted /versions.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/versions features an unstable_features key which is supposed to be used for additional features that the server supported which are not included in the spec. The examples described in this MSC do not fit these terms.

We could add another key, like config, which would be general enough to hold random public configuration values the server has decided to set up.

Or we can move it to its own API. With regards to this MSC, I think it'd be easy enough to say whether information should go in /versions, /_matrix/client/r0/capabilities/*, or /_matrix/media/r0/config.

And then deciding between capabilities/server and capabilities/user would just be "does this information need to either rely on a user, or is potentially sensitive"? Then it goes in /user, otherwise /server.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we actually have a strong use case for this MSC at all? If this MSC also had a capability it wanted to add then we could discuss the merits of that capability in the context of a new API vs some other solution. Without an example, the distinction between the endpoints is unclear and seems to be building a framework for a feature we don't know if we even want.

If we don't have an example for this API, this MSC should be put on the backburner or closed until we need it imo. I also have general concerns with an unauthenticated version of the endpoint because the cases presented thus far are better solved in other ways.

useful, there are use cases that could benefit from this API being called
from an unauthenticated user as well. For instance, information that a user
would like to know about a server before they sign up. Such as whether the
server sends message content through a media-scanner, or whether the server
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sends message content through a media-scanner

is this really a thing that is better exposed over an API than described in a policy document for a human to read?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can still see a usecase for values that sites may want to pull and present their own filtering for.

from an unauthenticated user as well. For instance, information that a user
would like to know about a server before they sign up. Such as whether the
server sends message content through a media-scanner, or whether the server
allows E2EE session key backup. A client could query this information before
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

allows E2EE session key backup

surely this should be under /versions ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussed in #2233 (comment)

@turt2live turt2live self-requested a review August 16, 2019 13:30
@jryans jryans removed phase:1 privacy-sprint Temporary label: privacy-related stuff labels Aug 16, 2019
@turt2live turt2live removed their request for review August 16, 2019 15:04

While the ability to scope the capabilities to the requesting user is very
useful, there are use cases that could benefit from this API being called
from an unauthenticated user as well. For instance, information that a user
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not convinced that any of the examples that follow need a capabilities endpoint.

  • "We process your data" -> Terms of service acceptance. Could have additional flows for informational things, but this would likely be better served at an advertising page before registration.
  • "We disallow key backup" -> Advertising for the server before the registration page (it'd be a feature of the service and not something we need to care about in the spec, imo)
  • Other information at registration time -> UIA while using params to send data along.
  • Server listing sites -> MSC2063, MSC1929, and/or https://talk.feneas.org/t/serverinfo-specification-for-server-metadata/99

information about the server's capabilities that would be pertinent to
non-authenticated users.

The existing `/capabilities` API would be replaced by `/capabilities/user`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Due to the backwards compatibility section below, it should be deprecated with intent to replace, not replaced.

@@ -0,0 +1,58 @@
# Unauthenticated Capabilities API
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we actually have a strong use case for this MSC at all? If this MSC also had a capability it wanted to add then we could discuss the merits of that capability in the context of a new API vs some other solution. Without an example, the distinction between the endpoints is unclear and seems to be building a framework for a feature we don't know if we even want.

If we don't have an example for this API, this MSC should be put on the backburner or closed until we need it imo. I also have general concerns with an unauthenticated version of the endpoint because the cases presented thus far are better solved in other ways.

@richvdh
Copy link
Member

richvdh commented Aug 19, 2019

given that this seems to be looking for a problem to solve, I'm going to close this for now.

@richvdh richvdh closed this Aug 19, 2019
@turt2live turt2live added abandoned A proposal where the author/shepherd is not responsive and removed proposal-in-review labels Aug 19, 2019
@turt2live turt2live added the kind:feature MSC for not-core and not-maintenance stuff label Apr 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
abandoned A proposal where the author/shepherd is not responsive kind:feature MSC for not-core and not-maintenance stuff proposal A matrix spec change proposal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants