Switch to stable endpoints

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

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
 }

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"}

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 {

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)
 

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)
 }

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"},