-
Notifications
You must be signed in to change notification settings - Fork 211
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
Prepare for authenticated media freeze #17433
Changes from all commits
99840b3
7569c77
f5ec081
87ae2c2
0c787b4
11cfc0a
bfee314
f532149
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 @@ | ||
Prepare for authenticated media freeze. |
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
|
@@ -1863,6 +1863,18 @@ federation_rr_transactions_per_room_per_second: 40 | |||||||
## Media Store | ||||||||
Config options related to Synapse's media store. | ||||||||
|
||||||||
--- | ||||||||
### `enable_authenticated_media` | ||||||||
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 this would be clearer as Thanks for writing this up though, this is way clearer about what we're hoping this feature will do, it's worth it just for that alone. 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. mrh, actually, it looks like the two options were merged and this option is named properly, but it'd probably be worth pointing out that disabling this option means all media, including those flagged as 'authenticated media', will be available over the unauthenticated endpoint? Since we disable the auth checks when this is turned off. |
||||||||
|
||||||||
When set to true, all subsequent media uploads will be marked as authenticated, and will not be available over legacy | ||||||||
unauthenticated media endpoints (`/_matrix/media/(r0|v3|v1)/download` and `/_matrix/media/(r0|v3|v1)/thumbnail`) - requests for authenticated media over these endpoints will result in a 404. All media, including authenticated media, will be available over the authenticated media endpoints `_matrix/client/v1/media/download` and `_matrix/client/v1/media/thumbnail`. Media uploaded prior to setting this option to true will still be available over the legacy endpoints. Note if the setting is switched to false | ||||||||
after enabling, media marked as authenticated will be available over legacy endpoints. Defaults to false, but | ||||||||
this will change to true in a future Synapse release. | ||||||||
|
||||||||
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.
Suggested change
|
||||||||
Example configuration: | ||||||||
```yaml | ||||||||
enable_authenticated_media: true | ||||||||
``` | ||||||||
--- | ||||||||
### `enable_media_repo` | ||||||||
|
||||||||
|
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.
(on the fence; do you think we ought to add an upgrade notes entry about this change, so people can get ahead of the curve and set the option to false explicitly if they want to make sure their special client won't be bitten by the upcoming freeze? It'd also be a good place to just lay out the plan a bit, since I think for most who don't read TWIM etc, the upcoming changes may not be obvious?)
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.
I'd like to get @turt2live's opinion on this - perhaps it would make sense to link to the blog post announcing the freeze? I think for now though we can merge this (I'd like to get it into the next rc so people can test against it on beta.matrix.org) and figure out how to handle the upgrade notes, etc soon.
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.
Server operators are going to be increasingly incentivized to ensure this option is actually set to
true
rather thanfalse
, regardless of client breakage. When we change the default totrue
we should definitely call out matrix.org's plan as reference, but I wouldn't include language about setting it tofalse
.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.
From brief discussion in backend internal: we should provide users with instructions on how to set it to
false
, but we can have the changelog read as eventually removing the option (making the feature inevitable)