Merge pull request #6082 from Martchus/asset-auth-webapp-only

Add config option to restrict asset downloads to logged-in users

etc/openqa/openqa.ini

@@ -125,6 +125,10 @@
 ## Authentication method to use for user management
 [auth]
 #method = Fake|OpenID|OAuth2
+## whether authentication is required to access assets; caveats:
+## - Does NOT affect assets made available outside the scope of the openQA service (e.g. via Apache, NGINX or NFS).
+## - Might not work well with other features like openqa-clone-job and the cache service.
+#require_for_assets = 0
 
 ## for GitHub, salsa.debian.org and providers listed on https://metacpan.org/pod/Mojolicious::Plugin::OAuth2#Configuration
 ## one can use:

lib/OpenQA/Setup.pm

@@ -61,6 +61,7 @@ sub read_config ($app) {
         },
         auth => {
             method => 'OpenID',
+            require_for_assets => 0,
         },
         'scm git' => {
             update_remote => '',

lib/OpenQA/WebAPI.pm

@@ -141,11 +141,15 @@ sub startup ($self) {
     $r->get('/tests/list_scheduled_ajax')->name('tests_ajax')->to('test#list_scheduled_ajax');
 
     # only provide a URL helper - this is overtaken by apache
-    $r->get('/assets/*assetpath')->name('download_asset')->to('file#download_asset');
+    my $config = $app->config;
+    my $require_auth_for_assets = $config->{auth}->{require_for_assets};
+    my $assets_r = $require_auth_for_assets ? $logged_in : $r;
+    $assets_r->get('/assets/*assetpath')->name('download_asset')->to('file#download_asset');
 
     my $test_r = $r->any('/tests/<testid:num>');
     $test_r = $test_r->under('/')->to('test#referer_check');
     my $test_auth = $auth->any('/tests/<testid:num>' => {format => 0});
+    my $test_asset_r = $require_auth_for_assets ? $test_auth : $test_r;
     $test_r->get('/')->name('test')->to('test#show');
     $test_r->get('/ajax')->name('job_next_previous_ajax')->to('test#job_next_previous_ajax');
     $test_r->get('/modules/:moduleid/fails')->name('test_module_fails')->to('test#module_fails');
@@ -171,9 +175,9 @@ sub startup ($self) {
     $test_r->get('/video' => sub ($c) { $c->render_testfile('test/video') })->name('video');
     $test_r->get('/logfile' => sub ($c) { $c->render_testfile('test/logfile') })->name('logfile');
     # adding assetid => qr/\d+/ doesn't work here. wtf?
-    $test_r->get('/asset/<assetid:num>')->name('test_asset_id')->to('file#test_asset');
-    $test_r->get('/asset/#assettype/#assetname')->name('test_asset_name')->to('file#test_asset');
-    $test_r->get('/asset/#assettype/#assetname/*subpath')->name('test_asset_name_path')->to('file#test_asset');
+    $test_asset_r->get('/asset/<assetid:num>')->name('test_asset_id')->to('file#test_asset');
+    $test_asset_r->get('/asset/#assettype/#assetname')->name('test_asset_name')->to('file#test_asset');
+    $test_asset_r->get('/asset/#assettype/#assetname/*subpath')->name('test_asset_name_path')->to('file#test_asset');
 
     my $developer_auth = $test_r->under('/developer')->to('session#ensure_admin');
     $developer_auth->get('/ws-console')->name('developer_ws_console')->to('developer#ws_console');
@@ -413,11 +417,13 @@ sub startup ($self) {
     $api_ro->post('/webhooks/product')->name('apiv1_evaluate_webhook_product')->to('webhook#product');
 
     # api/v1/assets
+
+    my $api_assets_readonly = $require_auth_for_assets ? $api_ru : $api_public_r;
     $api_ro->post('/assets')->name('apiv1_post_asset')->to('asset#register');
-    $api_public_r->get('/assets')->name('apiv1_get_asset')->to('asset#list');
+    $api_assets_readonly->get('/assets')->name('apiv1_get_asset')->to('asset#list');
     $api_ra->post('/assets/cleanup')->name('apiv1_trigger_asset_cleanup')->to('asset#trigger_cleanup');
-    $api_public_r->get('/assets/<id:num>')->name('apiv1_get_asset_id')->to('asset#get');
-    $api_public_r->get('/assets/#type/#name')->name('apiv1_get_asset_name')->to('asset#get');
+    $api_assets_readonly->get('/assets/<id:num>')->name('apiv1_get_asset_id')->to('asset#get');
+    $api_assets_readonly->get('/assets/#type/#name')->name('apiv1_get_asset_name')->to('asset#get');
     $api_ra->delete('/assets/<id:num>')->name('apiv1_delete_asset')->to('asset#delete');
     $api_ra->delete('/assets/#type/#name')->name('apiv1_delete_asset_name')->to('asset#delete');
 
@@ -529,7 +535,7 @@ sub startup ($self) {
         });
 
     # allow configuring Cross-Origin Resource Sharing (CORS)
-    if (my $access_control_allow_origin = $app->config->{global}->{access_control_allow_origin_header}) {
+    if (my $access_control_allow_origin = $config->{global}->{access_control_allow_origin_header}) {
         my %allowed_origins = map { trim($_) => 1 } split ',', $access_control_allow_origin;
         my $fallback_origin = delete $allowed_origins{'*'} ? '*' : undef;
         $app->hook(

t/03-auth.t

@@ -19,13 +19,17 @@ use Mojo::URL;
 use MIME::Base64 qw(encode_base64url decode_base64url);
 use Mojolicious;
 
+my $file_api_mock = Test::MockModule->new('OpenQA::WebAPI::Controller::File');
+$file_api_mock->redefine(download_asset => sub ($self) { $self->render(text => 'asset-ok') });
+$file_api_mock->redefine(test_asset => sub ($self) { $self->render(text => 'test-asset-ok') });
+
 my $tempdir = tempdir("/tmp/$FindBin::Script-XXXX")->make_path;
 $ENV{OPENQA_CONFIG} = $tempdir;
 OpenQA::Test::Database->new->create;
 
 sub test_auth_method_startup ($auth, @options) {
-    my @conf = ("[auth]\n", "method = \t  $auth \t\n", "[openid]\n", "httpsonly = 0\n");
-    $tempdir->child('openqa.ini')->spew(join('', @conf, @options));
+    my @conf = ("[auth]\n", "method = \t  $auth \t\n");
+    $tempdir->child('openqa.ini')->spew(join('', @conf, @options, "[openid]\n", "httpsonly = 0\n"));
     my $t = Test::Mojo->new('OpenQA::WebAPI');
     is $t->app->config->{auth}->{method}, $auth, "started successfully with auth $auth";
     $t->get_ok('/login' => {Referer => 'http://open.qa/tests/42'});
@@ -36,6 +40,21 @@ sub mojo_has_request_debug { $Mojolicious::VERSION <= 9.21 }
 combined_like { test_auth_method_startup('Fake')->status_is(302) } mojo_has_request_debug ? qr/302 Found/ : qr//,
   'Plugin loaded';
 
+subtest 'restricted asset downloads with setting `[auth] require_for_assets = 1`' => sub {
+    my $t = test_auth_method_startup('Fake', "require_for_assets = 1\n");
+    my $expected_redirect = '/login?return_page=%2Fassets%2Fiso%2Ftest.iso';
+    $t->get_ok('/assets/iso/test.iso')->status_is(200)->content_is('asset-ok', 'can access asset when logged in');
+    $t->get_ok('/tests/42/asset/iso/test.iso')->status_is(200);
+    $t->content_is('test-asset-ok', 'can access test asset when logged in');
+    $t->get_ok('/logout')->status_is(302)->get_ok('/assets/iso/test.iso')->status_is(302);
+    $t->content_unlike(qr/asset-ok/, 'asset not accessible when logged out');
+    $t->header_is('Location', $expected_redirect, 'redirect to login when accessing asset');
+    $t->get_ok('/logout')->status_is(302);
+    $t->get_ok('/tests/42/asset/iso/test.iso')->status_is(302);
+    $t->header_like('Location', qr|/login\?return_page=.*test.iso|, 'redirect to login when accessing test asset');
+    $t->content_unlike(qr/asset-ok/, 'test asset not accessible when logged out');
+};
+
 subtest OpenID => sub {
     # OpenID relies on external server which we mock to not rely on external dependencies
     my $openid_mock = Test::MockModule->new('Net::OpenID::Consumer');

t/config.t

@@ -59,6 +59,7 @@ subtest 'Test configuration default modes' => sub {
         },
         auth => {
             method => 'Fake',
+            require_for_assets => 0,
         },
         'scm git' => {
             update_remote => '',