search

VILLASframework / web-backend

A go implementation of the backend for VILLASweb
https://fein-aachen.org/en/projects/villas-web/
GNU General Public License v3.0
1 stars 2 forks source link

Provide Content-disposition header for files delivered via S3 object-store #73

Closed stv0g closed 1 year ago

stv0g commented 3 years ago

We need to figure out why this was commented out: https://git.rwth-aachen.de/acs/public/villas/web-backend-go/-/blob/master/routes/file/file_s3.go#L122

stv0g commented 3 years ago

Most likely this was commented out as it broke the signed S3 requests.

The backend generates a short-lived cryptographically signed S3 URL which will be provided to the user. I remember that our S3 server (minio) hat some issues with these signed URLs when we added other headers. We should test this again.

stv0g commented 3 years ago

mentioned in commit b3ba58647c9972a1df3908e90d779e05f81c5613

stv0g commented 3 years ago

In GitLab by @skolen on Oct 28, 2021, 15:45

Hi @stvogel

There seems to be an incompatibility between your fix in b3ba5864 and our current s3 setup in k8s. As we need a functional version of the s3 object storage urgently for @jan.dinkelbach , I removed the content disposition header and the url update in the getS3Url file method again (see 5d3b5be9).

We need to properly test your fix and also check if all helm chart configs are compatible before deploying the content disposition header fix again.

stv0g commented 3 years ago

Hi Sonja,

in which cluster did the error appear?

stv0g commented 3 years ago

In GitLab by @skolen on Oct 28, 2021, 15:51

in our os cluster in the "usual" villas depoyment

stv0g commented 3 years ago

Okay, I checked the helm chart config there which is fine.

stv0g commented 3 years ago

In GitLab by @skolen on Oct 28, 2021, 16:49

mentioned in commit 0ed8e2b3b196bafa5c596da4f4419d55ae79b806

stv0g commented 3 years ago

In GitLab by @skolen on Oct 28, 2021, 16:58

I have investigated a bit further.

Here the Parse method of the url package is used to create a url type in go. This method expects the scheme to be specified in the parameter string, otherwise its behavior is not properly defined. In our helm charts, we do not specify the scheme of the s3 endpoints. I think this could be a reason why an overall wrong URL is returned to the fontend upon file download GET request - which makes the request fail.

I am testing right now if this is getting us closer to solving the issue.

stv0g commented 3 years ago

In GitLab by @skolen on Oct 28, 2021, 17:10

Another update: specifying the external s3 endpoint including "http://" results in correct presigned urls. :-)

However we have a problem with mixed http and https contents. Our s3 object storage serves http while VILLASweb uses https. Most browsers block such combinations because they are a potential security risk, see for example here.

@stvogel Can we configure our s3 object storage in k8s to serve https instead of http?

stv0g commented 3 years ago

Okay the missing scheme in the public s3 endpoint is now fixed here https://git.rwth-aachen.de/acs/public/catalogue/-/commit/c60b242a5704918c4d82c1492409223ef34062de in chart version v0.7.1

stv0g commented 3 years ago

@stvogel Can we configure our s3 object storage in k8s to serve https instead of http?

In fact it is hosted via https for the public endpoint (depending on whether the user decicdes to enable TLS in the ingress). Only the internal endpoint is using http. But we should keep it like that.

stv0g commented 2 years ago

This should be fixed now :)