Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Skip tests requiring unauthenticated media on Synapse #747

Merged
merged 4 commits into from
Nov 20, 2024
Merged

Skip tests requiring unauthenticated media on Synapse #747

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
272068b
Skip TestContent on Synapse
reivilibre Nov 19, 2024
634b54a
Skip more unauthed media tests on Synapse
reivilibre Nov 19, 2024
97cbd91
Also MediaWithoutFilename
reivilibre Nov 19, 2024
1704a0a
Add comments explaining the SkipIf(Synapse) statements
reivilibre Nov 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions tests/csapi/apidoc_content_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
31 changes: 22 additions & 9 deletions tests/media_filename_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ 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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like we would want to keep these tests but change them to use authentication?

I think I'd prefer to disable authenticated media when testing with complement rather than not test these paths?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests already exist with authentication, see e.g. L56 for this one

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah right, fair enough

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a comment or something like "Synapse doesn't support unauthenticated media anymore"?

t.Parallel()

mxcUri := alice.UploadContent(t, data.MatrixPng, filename, "image/png")
Expand Down Expand Up @@ -69,6 +71,8 @@ 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()

mxcUri := alice.UploadContent(t, data.MatrixPng, asciiFileName, "image/png")
Expand Down Expand Up @@ -107,6 +111,8 @@ 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()

mxcUri := alice.UploadContent(t, data.MatrixPng, unicodeFileName, "image/png")
Expand Down Expand Up @@ -136,6 +142,8 @@ 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()

mxcUri := alice.UploadContent(t, data.MatrixPng, unicodeFileName, "image/png")
Expand All @@ -161,6 +169,8 @@ 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()

mxcUri := alice.UploadContent(t, data.MatrixPng, unicodeFileName, "image/png")
Expand All @@ -185,11 +195,12 @@ 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")
// 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()

Expand Down Expand Up @@ -221,11 +232,12 @@ 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")
// 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()

Expand Down Expand Up @@ -259,11 +271,12 @@ 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")
// 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()

Expand Down
4 changes: 4 additions & 0 deletions tests/media_nofilename_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,14 @@ 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) {
// 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)

Expand Down
15 changes: 12 additions & 3 deletions tests/media_thumbnail_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,12 @@ 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) {
// Synapse no longer allows downloads over the unauthenticated media endpoints by default
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)
Expand All @@ -57,7 +62,11 @@ 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) {
// Synapse no longer allows downloads over the unauthenticated media endpoints by default
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)
Expand Down Expand Up @@ -94,7 +103,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"},
Expand Down
Loading