From 272068b0da8080a8fb19cce251859343d3add87d Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Tue, 19 Nov 2024 10:34:54 +0000 Subject: [PATCH 1/4] Skip TestContent on Synapse This will be expected to fail when Synapse enforces authenticated media by default in https://github.com/element-hq/synapse/pull/17889 --- tests/csapi/apidoc_content_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/csapi/apidoc_content_test.go b/tests/csapi/apidoc_content_test.go index be8724c1..522f6e2e 100644 --- a/tests/csapi/apidoc_content_test.go +++ b/tests/csapi/apidoc_content_test.go @@ -8,9 +8,13 @@ import ( "github.com/matrix-org/complement" "github.com/matrix-org/complement/helpers" "github.com/matrix-org/complement/internal/data" + "github.com/matrix-org/complement/runtime" ) func TestContent(t *testing.T) { + // Synapse no longer allows downloads over the unauthenticated media endpoints by default + runtime.SkipIf(t, runtime.Synapse) + deployment := complement.Deploy(t, 1) defer deployment.Destroy(t) From 634b54aa56b44313c2f52e1cf7eecefa4fb3ee2b Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Tue, 19 Nov 2024 13:11:50 +0000 Subject: [PATCH 2/4] Skip more unauthed media tests on Synapse --- tests/media_filename_test.go | 23 ++++++++++++++--------- tests/media_thumbnail_test.go | 13 ++++++++++--- 2 files changed, 24 insertions(+), 12 deletions(-) diff --git a/tests/media_filename_test.go b/tests/media_filename_test.go index 709174f2..c24ba234 100644 --- a/tests/media_filename_test.go +++ b/tests/media_filename_test.go @@ -41,6 +41,7 @@ func TestMediaFilenames(t *testing.T) { var filename = filename t.Run(fmt.Sprintf("Can download file '%s'", filename), func(t *testing.T) { + runtime.SkipIf(t, runtime.Synapse) t.Parallel() mxcUri := alice.UploadContent(t, data.MatrixPng, filename, "image/png") @@ -69,6 +70,7 @@ func TestMediaFilenames(t *testing.T) { // sytest: Can download specifying a different ASCII file name t.Run("Can download specifying a different ASCII file name", func(t *testing.T) { + runtime.SkipIf(t, runtime.Synapse) t.Parallel() mxcUri := alice.UploadContent(t, data.MatrixPng, asciiFileName, "image/png") @@ -107,6 +109,7 @@ func TestMediaFilenames(t *testing.T) { // sytest: Can download specifying a different Unicode file name t.Run("Can download specifying a different Unicode file name", func(t *testing.T) { + runtime.SkipIf(t, runtime.Synapse) t.Parallel() mxcUri := alice.UploadContent(t, data.MatrixPng, unicodeFileName, "image/png") @@ -136,6 +139,7 @@ func TestMediaFilenames(t *testing.T) { // sytest: Can download with Unicode file name locally t.Run("Can download with Unicode file name locally", func(t *testing.T) { + runtime.SkipIf(t, runtime.Synapse) t.Parallel() mxcUri := alice.UploadContent(t, data.MatrixPng, unicodeFileName, "image/png") @@ -161,6 +165,7 @@ func TestMediaFilenames(t *testing.T) { // sytest: Can download with Unicode file name over federation t.Run("Can download with Unicode file name over federation", func(t *testing.T) { + runtime.SkipIf(t, runtime.Synapse) t.Parallel() mxcUri := alice.UploadContent(t, data.MatrixPng, unicodeFileName, "image/png") @@ -185,11 +190,11 @@ func TestMediaFilenames(t *testing.T) { }) t.Run("Will serve safe media types as inline", func(t *testing.T) { - if runtime.Homeserver != runtime.Synapse && runtime.Homeserver != runtime.Conduwuit { + if runtime.Homeserver != runtime.Conduwuit { // We need to check that this security behaviour is being correctly run in - // Synapse or conduwuit, but since this is not part of the Matrix spec we do not assume + // conduwuit, but since this is not part of the Matrix spec we do not assume // other homeservers are doing so. - t.Skip("Skipping test of Content-Disposition header requirements on non-Synapse and non-conduwuit homeserver") + t.Skip("Skipping test of Content-Disposition header requirements on non-conduwuit homeserver") } t.Parallel() @@ -221,11 +226,11 @@ func TestMediaFilenames(t *testing.T) { }) t.Run("Will serve safe media types with parameters as inline", func(t *testing.T) { - if runtime.Homeserver != runtime.Synapse && runtime.Homeserver != runtime.Conduwuit { + if runtime.Homeserver != runtime.Conduwuit { // We need to check that this security behaviour is being correctly run in - // Synapse or conduwuit, but since this is not part of the Matrix spec we do not assume + // conduwuit, but since this is not part of the Matrix spec we do not assume // other homeservers are doing so. - t.Skip("Skipping test of Content-Disposition header requirements on non-Synapse and non-conduwuit homeserver") + t.Skip("Skipping test of Content-Disposition header requirements on non-conduwuit homeserver") } t.Parallel() @@ -259,11 +264,11 @@ func TestMediaFilenames(t *testing.T) { }) t.Run("Will serve unsafe media types as attachments", func(t *testing.T) { - if runtime.Homeserver != runtime.Synapse && runtime.Homeserver != runtime.Conduwuit { + if runtime.Homeserver != runtime.Conduwuit { // We need to check that this security behaviour is being correctly run in - // Synapse or conduwuit, but since this is not part of the Matrix spec we do not assume + // conduwuit, but since this is not part of the Matrix spec we do not assume // other homeservers are doing so. - t.Skip("Skipping test of Content-Disposition header requirements on non-Synapse and non-conduwuit homeserver") + t.Skip("Skipping test of Content-Disposition header requirements on non-conduwuit homeserver") } t.Parallel() diff --git a/tests/media_thumbnail_test.go b/tests/media_thumbnail_test.go index 19a8cb5b..b0de784c 100644 --- a/tests/media_thumbnail_test.go +++ b/tests/media_thumbnail_test.go @@ -36,7 +36,11 @@ func TestLocalPngThumbnail(t *testing.T) { uri := alice.UploadContent(t, data.LargePng, fileName, contentType) - fetchAndValidateThumbnail(t, alice, uri, false) + t.Run("test /_matrix/media/v3 endpoint", func(t *testing.T) { + runtime.SkipIf(t, runtime.Synapse) + fetchAndValidateThumbnail(t, alice, uri, false) + }) + t.Run("test /_matrix/client/v1/media endpoint", func(t *testing.T) { runtime.SkipIf(t, runtime.Dendrite) fetchAndValidateThumbnail(t, alice, uri, true) @@ -57,7 +61,10 @@ func TestRemotePngThumbnail(t *testing.T) { uri := alice.UploadContent(t, data.LargePng, fileName, contentType) - fetchAndValidateThumbnail(t, bob, uri, false) + t.Run("test /_matrix/media/v3 endpoint", func(t *testing.T) { + runtime.SkipIf(t, runtime.Synapse) + fetchAndValidateThumbnail(t, bob, uri, false) + }) t.Run("test /_matrix/client/v1/media endpoint", func(t *testing.T) { runtime.SkipIf(t, runtime.Dendrite) @@ -94,7 +101,7 @@ func TestFederationThumbnail(t *testing.T) { uri := alice.UploadContent(t, data.LargePng, fileName, contentType) mediaOrigin, mediaId := client.SplitMxc(uri) - path := []string{"_matrix", "media", "v3", "thumbnail", mediaOrigin, mediaId} + path := []string{"_matrix", "client", "v1", "media", "thumbnail", mediaOrigin, mediaId} res := alice.MustDo(t, "GET", path, client.WithQueries(url.Values{ "width": []string{"32"}, "height": []string{"32"}, From 97cbd917c8dd81f464457da4fbbaa2063bfeb56d Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Tue, 19 Nov 2024 17:24:43 +0000 Subject: [PATCH 3/4] Also MediaWithoutFilename --- tests/media_nofilename_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/media_nofilename_test.go b/tests/media_nofilename_test.go index e7042bae..f97480ae 100644 --- a/tests/media_nofilename_test.go +++ b/tests/media_nofilename_test.go @@ -10,10 +10,13 @@ import ( "github.com/matrix-org/complement/federation" "github.com/matrix-org/complement/helpers" "github.com/matrix-org/complement/must" + "github.com/matrix-org/complement/runtime" ) // Can handle uploads and remote/local downloads without a file name func TestMediaWithoutFileName(t *testing.T) { + runtime.SkipIf(t, runtime.Synapse) + deployment := complement.Deploy(t, 1) defer deployment.Destroy(t) From 1704a0a4e33b1da7fdf569a48a7cf1216358bed6 Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Wed, 20 Nov 2024 14:35:29 +0000 Subject: [PATCH 4/4] Add comments explaining the SkipIf(Synapse) statements --- tests/media_filename_test.go | 8 ++++++++ tests/media_nofilename_test.go | 1 + tests/media_thumbnail_test.go | 2 ++ 3 files changed, 11 insertions(+) diff --git a/tests/media_filename_test.go b/tests/media_filename_test.go index c24ba234..d5a857d5 100644 --- a/tests/media_filename_test.go +++ b/tests/media_filename_test.go @@ -41,6 +41,7 @@ func TestMediaFilenames(t *testing.T) { var filename = filename t.Run(fmt.Sprintf("Can download file '%s'", filename), func(t *testing.T) { + // Synapse no longer allows downloads over the unauthenticated media endpoints by default runtime.SkipIf(t, runtime.Synapse) t.Parallel() @@ -70,6 +71,7 @@ func TestMediaFilenames(t *testing.T) { // sytest: Can download specifying a different ASCII file name t.Run("Can download specifying a different ASCII file name", func(t *testing.T) { + // Synapse no longer allows downloads over the unauthenticated media endpoints by default runtime.SkipIf(t, runtime.Synapse) t.Parallel() @@ -109,6 +111,7 @@ func TestMediaFilenames(t *testing.T) { // sytest: Can download specifying a different Unicode file name t.Run("Can download specifying a different Unicode file name", func(t *testing.T) { + // Synapse no longer allows downloads over the unauthenticated media endpoints by default runtime.SkipIf(t, runtime.Synapse) t.Parallel() @@ -139,6 +142,7 @@ func TestMediaFilenames(t *testing.T) { // sytest: Can download with Unicode file name locally t.Run("Can download with Unicode file name locally", func(t *testing.T) { + // Synapse no longer allows downloads over the unauthenticated media endpoints by default runtime.SkipIf(t, runtime.Synapse) t.Parallel() @@ -165,6 +169,7 @@ func TestMediaFilenames(t *testing.T) { // sytest: Can download with Unicode file name over federation t.Run("Can download with Unicode file name over federation", func(t *testing.T) { + // Synapse no longer allows downloads over the unauthenticated media endpoints by default runtime.SkipIf(t, runtime.Synapse) t.Parallel() @@ -194,6 +199,7 @@ func TestMediaFilenames(t *testing.T) { // We need to check that this security behaviour is being correctly run in // conduwuit, but since this is not part of the Matrix spec we do not assume // other homeservers are doing so. + // Skip Synapse because it no longer allows downloads over the unauthenticated media endpoints by default t.Skip("Skipping test of Content-Disposition header requirements on non-conduwuit homeserver") } t.Parallel() @@ -230,6 +236,7 @@ func TestMediaFilenames(t *testing.T) { // We need to check that this security behaviour is being correctly run in // conduwuit, but since this is not part of the Matrix spec we do not assume // other homeservers are doing so. + // Skip Synapse because it no longer allows downloads over the unauthenticated media endpoints by default t.Skip("Skipping test of Content-Disposition header requirements on non-conduwuit homeserver") } t.Parallel() @@ -268,6 +275,7 @@ func TestMediaFilenames(t *testing.T) { // We need to check that this security behaviour is being correctly run in // conduwuit, but since this is not part of the Matrix spec we do not assume // other homeservers are doing so. + // Skip Synapse because it no longer allows downloads over the unauthenticated media endpoints by default t.Skip("Skipping test of Content-Disposition header requirements on non-conduwuit homeserver") } t.Parallel() diff --git a/tests/media_nofilename_test.go b/tests/media_nofilename_test.go index f97480ae..d72791af 100644 --- a/tests/media_nofilename_test.go +++ b/tests/media_nofilename_test.go @@ -15,6 +15,7 @@ import ( // Can handle uploads and remote/local downloads without a file name func TestMediaWithoutFileName(t *testing.T) { + // Synapse no longer allows downloads over the unauthenticated media endpoints by default runtime.SkipIf(t, runtime.Synapse) deployment := complement.Deploy(t, 1) diff --git a/tests/media_thumbnail_test.go b/tests/media_thumbnail_test.go index b0de784c..03f9cc8d 100644 --- a/tests/media_thumbnail_test.go +++ b/tests/media_thumbnail_test.go @@ -37,6 +37,7 @@ func TestLocalPngThumbnail(t *testing.T) { uri := alice.UploadContent(t, data.LargePng, fileName, contentType) t.Run("test /_matrix/media/v3 endpoint", func(t *testing.T) { + // Synapse no longer allows downloads over the unauthenticated media endpoints by default runtime.SkipIf(t, runtime.Synapse) fetchAndValidateThumbnail(t, alice, uri, false) }) @@ -62,6 +63,7 @@ func TestRemotePngThumbnail(t *testing.T) { uri := alice.UploadContent(t, data.LargePng, fileName, contentType) t.Run("test /_matrix/media/v3 endpoint", func(t *testing.T) { + // Synapse no longer allows downloads over the unauthenticated media endpoints by default runtime.SkipIf(t, runtime.Synapse) fetchAndValidateThumbnail(t, bob, uri, false) })