Directly link to file domain without redirect #6810
Conversation
perlpunk
force-pushed
the
secure-user-files
branch
from
2b09c57 to
e469e38
Compare
| my $file_security_policy = $self->app->config->{global}->{file_security_policy}; | ||
| my $allow_insecure = $file_security_policy eq 'insecure-browsing'; | ||
| if ($file_security_policy =~ m/^domain:/ and my $file_domain = $self->app->config->{global}->{file_domain}) { | ||
| $allow_insecure = 1 if $file_domain eq $self->req->url->to_abs->host_port; |
There was a problem hiding this comment.
maybe you can skip some assignments here. And use directly $headers->content_disposition("attachment; filename=$filename;").
To me it looks like
if ($file_security_policy =~ m/^domain:/ and my $file_domain = $self->app->config->{global}->{file_domain}) {
if $file_domain eq $self->req->url->to_abs->host_port || $filetype !~ m/(html|svg)/) && $ext ne 'iso' {
$headers->content_disposition("attachment; filename=$filename;");}}```There was a problem hiding this comment.
I don't see how that helps.
First, the logic seems to be reversed (if the current domain is the file domain, serve as attachment -> no, it should not be served as an attachment in that case).
Second, why do all this and then repeat calling $headers->content_disposition(...) below? Note that this needs to be called even if if (my $filetype = $self->app->types->type($ext)) is false.
Give me a full patch instead so I can better see what you mean. As it is, it doesn't make sense to me.
This reverts commit 6db1f2e.
…oute" This reverts commit a007fe1.
Replace the `url_for` with helper functions which return the domain name whenever this is applicable based on the setting. - Link to domain for download assets like iso and `Uploaded logs` (includes also vars.json) - Link to domain for the video replay - No change in the redirection, assuming that this is out of scope here, and that in general it stays despite the direct link in the href. issue: https://progress.opensuse.org/issues/189888 Signed-off-by: Ioannis Bonatakis <[email protected]> Amended by: [email protected] Remove changes in templates && tests, see following commit
Serve as attachment when: * file_security_policy=domain:... and * file is requested under normal domain * file is html or svg Remove all redirection handling, as we have direct links to the file domain
perlpunk
force-pushed
the
secure-user-files
branch
from
e469e38 to
015470f
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #6810 +/- ##
=======================================
Coverage 99.26% 99.26%
=======================================
Files 402 402
Lines 41373 41394 +21
=======================================
+ Hits 41067 41088 +21
Misses 306 306 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
to avoid too much repetition
|
I added a couple of tests for all three |
perlpunk
force-pushed
the
secure-user-files
branch
from
b005c43 to
f46916c
Compare
3 participants
This reverts #6793, #6759 (exceptions from redirection).
Then I amend #6804 to only keep the
log_urlhelper, and in the next commit I only use it for files generated by the test, as files generated by openQA itself should be safe.In the last commit I remove the redirection and replace it by serving the files as attachments if necessary.
Issue: https://progress.opensuse.org/issues/189888
Probably some tests will fail