-
Notifications
You must be signed in to change notification settings - Fork 375
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
MSC2461: Proposal for Authenticated Content Repository API #2461
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,79 @@ | ||
# Authenticated Content Repository API | ||
Currently anyone can fetch resources from content repositories. | ||
This can be undesired behaviour for several reasons as discussed | ||
in [synapse#2150](https://github.com/matrix-org/synapse/issues/2150) and [synapse#2133](https://github.com/matrix-org/synapse/issues/2133). | ||
Homeservers might want to be able to restrict access to the content they serve. | ||
|
||
## Proposal | ||
Homeservers may reject unauthenticated access to media endpoints. | ||
|
||
When an unauthenticated client accesses an endpoint, the homeserver | ||
may reject the request like it would with an authenticated endpoint. | ||
|
||
Thus it returns status code 401 and an error | ||
with an errcode of M_MISSING_TOKEN or M_UNKNOWN_TOKEN as apropriate. | ||
|
||
Example response: | ||
```json | ||
{ | ||
"errcode": "M_MISSING_TOKEN", | ||
"error": "Media access is restricted to authenticated clients" | ||
} | ||
``` | ||
|
||
### Configuration | ||
To allow clients to predetermine whether authentication is required, | ||
the configuration field m.media.unauthenticated is added. | ||
It specifies what content can be accessed unauthenticated. | ||
|
||
The following behaviours are defined: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Threaded discussions are preferred @helaan
An unpublished version contained "Each entry in the following table is a subset of the preceding ones, with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, I'll switch to threaded replies then.
About What do you think will be the mode that servers in the public federation will use when this MSC is implemented and it is worth it to have it flexible? Personally, I think federated servers will switch to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In my opinion I agree Especially Having Preserving compatibility with older clients is also a choice admins should make. If I were to create the simplest form of this proposal it would just state.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd generally just recommend we have it turned on for the client-server API regardless. There's nothing stopping a server from accepting lack of auth, and realistically the spec should fix the general case of media being locked down. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Such a change should only happen after some server-server API is in place though, right? |
||
|
||
| Enum value | Description | | ||
| ---------- | ----------- | | ||
| null / missing | All content can be accessed unauthenticated | | ||
| m.cached | Only cached content can be accessed unauthenticated | | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this even needed? It sounds odd - someone in the room accesses the content and then suddenly everyone can as it is cached There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Replying to the first part of the question, as the latter is unrelated as clarified elsewhere. |
||
| m.local | Only content with an authority the server is responsible for can be accessed unauthenticated | | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What does this mean? Basically all users in the room can see it, but noone else? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See other comment. |
||
| m.unspecified | Unauthenticated access is possible but not specified | | ||
| m.none | No content can be accessed unauthenticated | | ||
|
||
Example response: | ||
```json | ||
{ | ||
"m.media.unauthenticated": "m.local" | ||
} | ||
``` | ||
|
||
Clients can decide based on this | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why not add the authentification type to {
"url": "blah",
"info": { ... },
"authenticated": "m.local"
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I might have miscommunicated the intent of this proposal by adding synapse#2150 considering it is only tagentally related. I've read the conversation some time ago and added it from memory. Only checking whether I cought the issues I was thinking of. I have clarified the intent in a new revision. |
||
if sharing a download link to a non Matrix user is possible. | ||
|
||
### Server to server | ||
To reduce the amount of server to server communication, | ||
when one homeserver tries to fetch content from another homeserver, | ||
the configuration should first be retrieved and cached. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is no federation API for this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I figured that because servers use the client API for media downloads they could also use it for the media config endpoint There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. They cannot. Also they aren't supposed to be using the client endpoints, they just haven't been defined correctly in the server-side endpoints. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Then this proposal is the time to specify that I'd say. Sharing media will be hard. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe the "federation api" could be an extra query parameter on existing download/thumbnail endpoints, which can be as simple as |
||
When the value is m.none the server should not attempt to fetch the | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I assume my other comment clears the first part up, but For the latter, yes, that is an expected consequence of this proposal. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "restricted" as in "can only be accessed once authenticated"? And that any server over federation can request such media in an authenticated user's stead? |
||
content from the remote server | ||
and return status code 404 and an error with an errcode of M_NOT_FOUND. | ||
|
||
Example response: | ||
```json | ||
{ | ||
"errcode": "M_NOT_FOUND", | ||
"error": "Remote homeserver rejected access" | ||
} | ||
``` | ||
|
||
## Potential issues | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also mentioned in MSC701 (I think, I'm going off memory) is a way to solve the authentication issue without potentially leaking your access token. Clients which have 'download this file' or 'open in new tab' buttons will need to pass along the access token via the query string. In doing so, when someone copies the link and pastes it to someone else they've exposed their account. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I really try to stay away from crypto, so this is just a naive simplification of MSC701: The authentication token is created through HMAC(access_token, media_id). |
||
Once homeservers enable this behaviour with a m.media.unauthenticated | ||
value other than null, older clients will not be able to access some content. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not only older clients, but many desktop clients will have a hard time accessing images now. Some of the toolkits people use don't allow them to add headers before the request, so they'd need to add special code for this to buffer the media ahead of time. This isn't a problem we need to fix in the MSC though, just something to be aware of. |
||
This is desired for the server operator and undesired for the user. | ||
|
||
Additionally older clients and servers might encounter an unexpected error code | ||
which may lead to unknown behaviour. | ||
|
||
## Alternatives | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will need a section comparing it to MSC701 |
||
All media endpoints could always require authentication, | ||
but then the server to server exchange would still need | ||
to be extended to allow access for remote homeservers. | ||
|
||
## Security considerations | ||
None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added where? To which endpoint? Down there you have that as a reply and not as sending, so something is off
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added endpoint in new revision