diff --git a/CHANGELOG.md b/CHANGELOG.md index a4271d3f..8f86975f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,7 +14,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), * S3 datastores can now specify a `prefixLength` to improve S3 performance on some providers. See `config.sample.yaml` for details. * Add `multipartUploads` flag for running MMR against unsupported S3 providers. See `config.sample.yaml` for details. * A new "leaky bucket" rate limit algorithm has been applied to downloads. See `rateLimit.buckets` in the config for details. -* Add *unstable* support for [MSC3916: Authentication for media](https://github.com/matrix-org/matrix-spec-proposals/pull/3916). +* Add support for [MSC3916: Authentication for media](https://github.com/matrix-org/matrix-spec-proposals/pull/3916). * To enable full support, use `signingKeyPath` in your config. See sample config for details. ### Changed diff --git a/api/r0/versions.go b/api/r0/versions.go index 023f9a66..47fc5e79 100644 --- a/api/r0/versions.go +++ b/api/r0/versions.go @@ -1,13 +1,14 @@ package r0 import ( + "net/http" + "slices" + "github.com/getsentry/sentry-go" "github.com/t2bot/matrix-media-repo/api/_apimeta" "github.com/t2bot/matrix-media-repo/api/_responses" "github.com/t2bot/matrix-media-repo/matrix" - "net/http" - "github.com/t2bot/matrix-media-repo/common/rcontext" ) @@ -18,9 +19,18 @@ func ClientVersions(r *http.Request, rctx rcontext.RequestContext, user _apimeta sentry.CaptureException(err) return _responses.InternalServerError("unable to get versions") } - if versions.UnstableFeatures == nil { - versions.UnstableFeatures = make(map[string]bool) + + // This is where we'd add our feature/version support as needed + if versions.Versions == nil { + versions.Versions = make([]string, 1) + } + + // We add v1.11 by force, even though we can't reliably say the rest of the server implements it. This + // is because server admins which point `/versions` at us are effectively opting in to whatever features + // we need to advertise support for. In our case, it's at least Authenticated Media (MSC3916). + if !slices.Contains(versions.Versions, "v1.11") { + versions.Versions = append(versions.Versions, "v1.11") } - versions.UnstableFeatures["org.matrix.msc3916"] = true + return versions } diff --git a/api/routes.go b/api/routes.go index 054a2a1b..036a5f51 100644 --- a/api/routes.go +++ b/api/routes.go @@ -46,18 +46,14 @@ func buildRoutes() http.Handler { register([]string{"POST"}, PrefixClient, "logout/all", mxSpecV3TransitionCS, router, makeRoute(_routers.RequireAccessToken(r0.LogoutAll), "logout_all", counter)) register([]string{"POST"}, PrefixMedia, "create", mxV1, router, makeRoute(_routers.RequireAccessToken(v1.CreateMedia), "create", counter)) register([]string{"GET"}, PrefixClient, "versions", mxNoVersion, router, makeRoute(_routers.OptionalAccessToken(r0.ClientVersions), "client_versions", counter)) - - // MSC3916 - Authentication & endpoint API separation - register([]string{"GET"}, PrefixClient, "media/preview_url", msc3916, router, previewUrlRoute) - register([]string{"GET"}, PrefixClient, "media/config", msc3916, router, configRoute) + register([]string{"GET"}, PrefixClient, "media/preview_url", mxV1, router, previewUrlRoute) + register([]string{"GET"}, PrefixClient, "media/config", mxV1, router, configRoute) authedDownloadRoute := makeRoute(_routers.RequireAccessToken(unstable.ClientDownloadMedia), "download", counter) - register([]string{"GET"}, PrefixClient, "media/download/:server/:mediaId/:filename", msc3916, router, authedDownloadRoute) - register([]string{"GET"}, PrefixClient, "media/download/:server/:mediaId", msc3916, router, authedDownloadRoute) - register([]string{"GET"}, PrefixClient, "media/thumbnail/:server/:mediaId", msc3916, router, makeRoute(_routers.RequireAccessToken(unstable.ClientThumbnailMedia), "thumbnail", counter)) - register([]string{"GET"}, PrefixFederation, "media/download/:server/:mediaId", msc3916, router, makeRoute(_routers.RequireServerAuth(unstable.FederationDownloadMedia), "download", counter)) - register([]string{"GET"}, PrefixFederation, "media/download/:mediaId", msc3916v2, router, makeRoute(_routers.RequireServerAuth(unstable.FederationDownloadMedia), "download", counter)) - register([]string{"GET"}, PrefixFederation, "media/thumbnail/:server/:mediaId", msc3916, router, makeRoute(_routers.RequireServerAuth(unstable.FederationThumbnailMedia), "thumbnail", counter)) - register([]string{"GET"}, PrefixFederation, "media/thumbnail/:mediaId", msc3916v2, router, makeRoute(_routers.RequireServerAuth(unstable.FederationThumbnailMedia), "thumbnail", counter)) + register([]string{"GET"}, PrefixClient, "media/download/:server/:mediaId/:filename", mxV1, router, authedDownloadRoute) + register([]string{"GET"}, PrefixClient, "media/download/:server/:mediaId", mxV1, router, authedDownloadRoute) + register([]string{"GET"}, PrefixClient, "media/thumbnail/:server/:mediaId", mxV1, router, makeRoute(_routers.RequireAccessToken(unstable.ClientThumbnailMedia), "thumbnail", counter)) + register([]string{"GET"}, PrefixFederation, "media/download/:mediaId", mxV1, router, makeRoute(_routers.RequireServerAuth(unstable.FederationDownloadMedia), "download", counter)) + register([]string{"GET"}, PrefixFederation, "media/thumbnail/:mediaId", mxV1, router, makeRoute(_routers.RequireServerAuth(unstable.FederationThumbnailMedia), "thumbnail", counter)) // Custom features register([]string{"GET"}, PrefixMedia, "local_copy/:server/:mediaId", mxUnstable, router, makeRoute(_routers.RequireAccessToken(unstable.LocalCopy), "local_copy", counter)) @@ -145,8 +141,6 @@ var ( //mxAllSpec matrixVersions = []string{"r0", "v1", "v3", "unstable", "unstable/io.t2bot.media" /* and MSC routes */} mxUnstable matrixVersions = []string{"unstable", "unstable/io.t2bot.media"} msc4034 matrixVersions = []string{"unstable/org.matrix.msc4034"} - msc3916 matrixVersions = []string{"unstable/org.matrix.msc3916"} - msc3916v2 matrixVersions = []string{"unstable/org.matrix.msc3916.v2"} mxSpecV3Transition matrixVersions = []string{"r0", "v1", "v3"} mxSpecV3TransitionCS matrixVersions = []string{"r0", "v3"} mxR0 matrixVersions = []string{"r0"} diff --git a/pipelines/_steps/download/try_download.go b/pipelines/_steps/download/try_download.go index dea01dbd..3c2e959c 100644 --- a/pipelines/_steps/download/try_download.go +++ b/pipelines/_steps/download/try_download.go @@ -64,7 +64,7 @@ func TryDownload(ctx rcontext.RequestContext, origin string, mediaId string) (*d var downloadUrl string usesMultipartFormat := false if ctx.Config.SigningKeyPath != "" { - downloadUrl = fmt.Sprintf("%s/_matrix/federation/unstable/org.matrix.msc3916.v2/media/download/%s", baseUrl, url.PathEscape(mediaId)) + downloadUrl = fmt.Sprintf("%s/_matrix/federation/v1/media/download/%s", baseUrl, url.PathEscape(mediaId)) resp, err = matrix.FederatedGet(ctx, downloadUrl, realHost, ctx.Config.SigningKeyPath) metrics.MediaDownloaded.With(prometheus.Labels{"origin": origin}).Inc() if err != nil { diff --git a/test/msc3916_downloads_suite_test.go b/test/msc3916_downloads_suite_test.go index d08a8147..7f37505a 100644 --- a/test/msc3916_downloads_suite_test.go +++ b/test/msc3916_downloads_suite_test.go @@ -79,18 +79,18 @@ func (s *MSC3916DownloadsSuite) TestClientDownloads() { assert.Equal(t, client1.ServerName, origin) assert.NotEmpty(t, mediaId) - raw, err := client2.DoRaw("GET", fmt.Sprintf("/_matrix/client/unstable/org.matrix.msc3916/media/download/%s/%s", origin, mediaId), nil, "", nil) + raw, err := client2.DoRaw("GET", fmt.Sprintf("/_matrix/client/v1/media/download/%s/%s", origin, mediaId), nil, "", nil) assert.NoError(t, err) assert.Equal(t, http.StatusUnauthorized, raw.StatusCode) - raw, err = client2.DoRaw("GET", fmt.Sprintf("/_matrix/client/unstable/org.matrix.msc3916/media/download/%s/%s/whatever.png", origin, mediaId), nil, "", nil) + raw, err = client2.DoRaw("GET", fmt.Sprintf("/_matrix/client/v1/media/download/%s/%s/whatever.png", origin, mediaId), nil, "", nil) assert.NoError(t, err) assert.Equal(t, http.StatusUnauthorized, raw.StatusCode) - raw, err = client1.DoRaw("GET", fmt.Sprintf("/_matrix/client/unstable/org.matrix.msc3916/media/download/%s/%s", origin, mediaId), nil, "", nil) + raw, err = client1.DoRaw("GET", fmt.Sprintf("/_matrix/client/v1/media/download/%s/%s", origin, mediaId), nil, "", nil) assert.NoError(t, err) assert.Equal(t, http.StatusOK, raw.StatusCode) test_internals.AssertIsTestImage(t, raw.Body) - raw, err = client1.DoRaw("GET", fmt.Sprintf("/_matrix/client/unstable/org.matrix.msc3916/media/download/%s/%s/whatever.png", origin, mediaId), nil, "", nil) + raw, err = client1.DoRaw("GET", fmt.Sprintf("/_matrix/client/v1/media/download/%s/%s/whatever.png", origin, mediaId), nil, "", nil) assert.NoError(t, err) assert.Equal(t, http.StatusOK, raw.StatusCode) test_internals.AssertIsTestImage(t, raw.Body) @@ -121,7 +121,7 @@ func (s *MSC3916DownloadsSuite) TestFederationDownloads() { assert.NotEmpty(t, mediaId) // Verify the federation download *fails* when lacking auth - uri := fmt.Sprintf("/_matrix/federation/unstable/org.matrix.msc3916.v2/media/download/%s", mediaId) + uri := fmt.Sprintf("/_matrix/federation/v1.v2/media/download/%s", mediaId) raw, err := remoteClient.DoRaw("GET", uri, nil, "", nil) assert.NoError(t, err) assert.Equal(t, http.StatusUnauthorized, raw.StatusCode) @@ -145,7 +145,7 @@ func (s *MSC3916DownloadsSuite) TestFederationMakesAuthedDownloads() { err := matrix.TestsOnlyInjectSigningKey(s.deps.Homeservers[0].ServerName, s.deps.Homeservers[0].ExternalClientServerApiUrl) assert.NoError(t, err) testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - assert.Equal(t, fmt.Sprintf("/_matrix/federation/unstable/org.matrix.msc3916.v2/media/download/%s", mediaId), r.URL.Path) + assert.Equal(t, fmt.Sprintf("/_matrix/federation/v1.v2/media/download/%s", mediaId), r.URL.Path) origin, err := matrix.ValidateXMatrixAuth(r, true) assert.NoError(t, err) assert.Equal(t, client1.ServerName, origin) @@ -158,7 +158,7 @@ func (s *MSC3916DownloadsSuite) TestFederationMakesAuthedDownloads() { origin = fmt.Sprintf("%s:%s", test_internals.DockerHostAddress(), u.Port()) config.AddDomainForTesting(test_internals.DockerHostAddress(), nil) // no port for config lookup - raw, err := client1.DoRaw("GET", fmt.Sprintf("/_matrix/client/unstable/org.matrix.msc3916/media/download/%s/%s", origin, mediaId), nil, "", nil) + raw, err := client1.DoRaw("GET", fmt.Sprintf("/_matrix/client/v1/media/download/%s/%s", origin, mediaId), nil, "", nil) assert.NoError(t, err) assert.Equal(t, http.StatusOK, raw.StatusCode) } @@ -187,7 +187,7 @@ func (s *MSC3916DownloadsSuite) TestFederationFollowsRedirects() { // Mock homeserver (1st hop) testServer1 := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - assert.Equal(t, fmt.Sprintf("/_matrix/federation/unstable/org.matrix.msc3916.v2/media/download/%s", mediaId), r.URL.Path) + assert.Equal(t, fmt.Sprintf("/_matrix/federation/v1.v2/media/download/%s", mediaId), r.URL.Path) origin, err := matrix.ValidateXMatrixAuth(r, true) assert.NoError(t, err) assert.Equal(t, client1.ServerName, origin) @@ -200,7 +200,7 @@ func (s *MSC3916DownloadsSuite) TestFederationFollowsRedirects() { origin = fmt.Sprintf("%s:%s", test_internals.DockerHostAddress(), u.Port()) config.AddDomainForTesting(test_internals.DockerHostAddress(), nil) // no port for config lookup - raw, err := client1.DoRaw("GET", fmt.Sprintf("/_matrix/client/unstable/org.matrix.msc3916/media/download/%s/%s", origin, mediaId), nil, "", nil) + raw, err := client1.DoRaw("GET", fmt.Sprintf("/_matrix/client/v1/media/download/%s/%s", origin, mediaId), nil, "", nil) assert.NoError(t, err) assert.Equal(t, http.StatusOK, raw.StatusCode) @@ -226,7 +226,7 @@ func (s *MSC3916DownloadsSuite) TestFederationMakesAuthedDownloadsAndFallsBack() origin, err := matrix.ValidateXMatrixAuth(r, true) assert.NoError(t, err) assert.Equal(t, client1.ServerName, origin) - assert.Equal(t, fmt.Sprintf("/_matrix/federation/unstable/org.matrix.msc3916.v2/media/download/%s", mediaId), r.URL.Path) + assert.Equal(t, fmt.Sprintf("/_matrix/federation/v1.v2/media/download/%s", mediaId), r.URL.Path) w.WriteHeader(http.StatusNotFound) _, _ = w.Write([]byte("{\"errcode\":\"M_UNRECOGNIZED\"}")) reqNum++ @@ -242,7 +242,7 @@ func (s *MSC3916DownloadsSuite) TestFederationMakesAuthedDownloadsAndFallsBack() origin = fmt.Sprintf("%s:%s", test_internals.DockerHostAddress(), u.Port()) config.AddDomainForTesting(test_internals.DockerHostAddress(), nil) // no port for config lookup - raw, err := client1.DoRaw("GET", fmt.Sprintf("/_matrix/client/unstable/org.matrix.msc3916/media/download/%s/%s", origin, mediaId), nil, "", nil) + raw, err := client1.DoRaw("GET", fmt.Sprintf("/_matrix/client/v1/media/download/%s/%s", origin, mediaId), nil, "", nil) assert.NoError(t, err) assert.Equal(t, http.StatusOK, raw.StatusCode) diff --git a/test/msc3916_misc_client_endpoints_suite_test.go b/test/msc3916_misc_client_endpoints_suite_test.go index 2aaa2cb3..1a0801fd 100644 --- a/test/msc3916_misc_client_endpoints_suite_test.go +++ b/test/msc3916_misc_client_endpoints_suite_test.go @@ -61,11 +61,11 @@ func (s *MSC3916MiscClientEndpointsSuite) TestPreviewUrlRequiresAuth() { qs := url.Values{ "url": []string{s.htmlPage.PublicUrl}, } - raw, err := client2.DoRaw("GET", "/_matrix/client/unstable/org.matrix.msc3916/media/preview_url", qs, "", nil) + raw, err := client2.DoRaw("GET", "/_matrix/client/v1/media/preview_url", qs, "", nil) assert.NoError(t, err) assert.Equal(t, http.StatusUnauthorized, raw.StatusCode) - raw, err = client1.DoRaw("GET", "/_matrix/client/unstable/org.matrix.msc3916/media/preview_url", qs, "", nil) + raw, err = client1.DoRaw("GET", "/_matrix/client/v1/media/preview_url", qs, "", nil) assert.NoError(t, err) assert.Equal(t, http.StatusOK, raw.StatusCode) } @@ -81,11 +81,11 @@ func (s *MSC3916MiscClientEndpointsSuite) TestConfigRequiresAuth() { UserId: "", // no auth on this client } - raw, err := client2.DoRaw("GET", "/_matrix/client/unstable/org.matrix.msc3916/media/config", nil, "", nil) + raw, err := client2.DoRaw("GET", "/_matrix/client/v1/media/config", nil, "", nil) assert.NoError(t, err) assert.Equal(t, http.StatusUnauthorized, raw.StatusCode) - raw, err = client1.DoRaw("GET", "/_matrix/client/unstable/org.matrix.msc3916/media/config", nil, "", nil) + raw, err = client1.DoRaw("GET", "/_matrix/client/v1/media/config", nil, "", nil) assert.NoError(t, err) assert.Equal(t, http.StatusOK, raw.StatusCode) } diff --git a/test/msc3916_thumbnails_suite_test.go b/test/msc3916_thumbnails_suite_test.go index 5ea5bc84..f2f5d0f0 100644 --- a/test/msc3916_thumbnails_suite_test.go +++ b/test/msc3916_thumbnails_suite_test.go @@ -57,8 +57,6 @@ func (s *MSC3916ThumbnailsSuite) TestClientThumbnails() { assert.NoError(t, err) fname := "image" + util.ExtensionForContentType(contentType) - //res := new(test_internals.MatrixUploadResponse) - //err = client1.DoReturnJson("POST", "/_matrix/client/unstable/org.matrix.msc3916/media/upload", url.Values{"filename": []string{fname}}, contentType, img, res) res, err := client1.Upload(fname, contentType, img) assert.NoError(t, err) assert.NotEmpty(t, res.MxcUri) @@ -74,11 +72,11 @@ func (s *MSC3916ThumbnailsSuite) TestClientThumbnails() { "method": []string{"scale"}, } - raw, err := client2.DoRaw("GET", fmt.Sprintf("/_matrix/client/unstable/org.matrix.msc3916/media/thumbnail/%s/%s", origin, mediaId), qs, "", nil) + raw, err := client2.DoRaw("GET", fmt.Sprintf("/_matrix/client/v1/media/thumbnail/%s/%s", origin, mediaId), qs, "", nil) assert.NoError(t, err) assert.Equal(t, http.StatusUnauthorized, raw.StatusCode) - raw, err = client1.DoRaw("GET", fmt.Sprintf("/_matrix/client/unstable/org.matrix.msc3916/media/thumbnail/%s/%s", origin, mediaId), qs, "", nil) + raw, err = client1.DoRaw("GET", fmt.Sprintf("/_matrix/client/v1/media/thumbnail/%s/%s", origin, mediaId), qs, "", nil) assert.NoError(t, err) assert.Equal(t, http.StatusOK, raw.StatusCode) //test_internals.AssertIsTestImage(t, raw.Body) // we can't verify that the resulting image is correct @@ -109,7 +107,7 @@ func (s *MSC3916ThumbnailsSuite) TestFederationThumbnails() { assert.NotEmpty(t, mediaId) // Verify the federation download *fails* when lacking auth - uri := fmt.Sprintf("/_matrix/federation/unstable/org.matrix.msc3916.v2/media/thumbnail/%s", mediaId) + uri := fmt.Sprintf("/_matrix/federation/v1.v2/media/thumbnail/%s", mediaId) qs := url.Values{ "width": []string{"96"}, "height": []string{"96"},