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

* Disallow unauthenticated access to assets in web application routes
* Maintain an htpasswd file for API keys of users to allow requiring
  authorization for assets in reverse proxies like NGINX
    * Add and remove an entry in htpasswd when an API key is added/removed
    * Store the key as part of the user name as we could otherwise only
      store one key/secret pair per user (as users are unique)
	* TODO: Is this a security problem? The key is also in plaintext in
          the database so I suppose it doesn't make things worse.
    * TODO: Handle existing keys and expiration
* Provide API key/secret in `Location` header when redirecting to reverse
  proxy; probably not ideal but as it for a download key/secret won't show
  in the address bar at least
* TODO: Fix other places where assets are downloaded to include API
  key/secret (e.g. cache service)
* TODO: Submit `perl-Apache-Htpasswd` to Factory
* See https://progress.opensuse.org/issues/170380

cpanfile

@@ -4,6 +4,7 @@
 # from dependencies.yaml
 ##################################################
 
+requires 'Apache::Htpasswd';
 requires 'BSD::Resource';
 requires 'CSS::Minifier::XS', '>= 0.01';
 requires 'Capture::Tiny';

dependencies.yaml

@@ -32,6 +32,7 @@ build_requires:
 
 common_requires:
   perl: '>= 5.20.0'
+  perl(Apache::Htpasswd):
   perl(Carp::Always): '>= 0.14.02'
   perl(Config::IniFiles):
   perl(Cpanel::JSON::XS): '>= 4.09'

dist/rpm/openQA.spec

@@ -59,7 +59,7 @@
 # The following line is generated from dependencies.yaml
 %define assetpack_requires perl(CSS::Minifier::XS) >= 0.01 perl(JavaScript::Minifier::XS) >= 0.11 perl(Mojolicious) perl(Mojolicious::Plugin::AssetPack) >= 1.36 perl(YAML::PP) >= 0.026
 # The following line is generated from dependencies.yaml
-%define common_requires ntp-daemon perl >= 5.20.0 perl(Carp::Always) >= 0.14.02 perl(Config::IniFiles) perl(Config::Tiny) perl(Cpanel::JSON::XS) >= 4.09 perl(Cwd) perl(Data::Dump) perl(Data::Dumper) perl(Digest::MD5) perl(Filesys::Df) perl(Getopt::Long) perl(Minion) >= 10.25 perl(Mojolicious) >= 9.340.0 perl(Regexp::Common) perl(Storable) perl(Text::Glob) perl(Time::Moment) perl(Try::Tiny)
+%define common_requires ntp-daemon perl >= 5.20.0 perl(Apache::Htpasswd) perl(Carp::Always) >= 0.14.02 perl(Config::IniFiles) perl(Config::Tiny) perl(Cpanel::JSON::XS) >= 4.09 perl(Cwd) perl(Data::Dump) perl(Data::Dumper) perl(Digest::MD5) perl(Filesys::Df) perl(Getopt::Long) perl(Minion) >= 10.25 perl(Mojolicious) >= 9.340.0 perl(Regexp::Common) perl(Storable) perl(Text::Glob) perl(Time::Moment) perl(Try::Tiny)
 # runtime requirements for the main package that are not required by other sub-packages
 # The following line is generated from dependencies.yaml
 %define main_requires %assetpack_requires bsdtar git-core hostname perl(BSD::Resource) perl(Carp) perl(CommonMark) perl(Config::Tiny) perl(DBD::Pg) >= 3.7.4 perl(DBI) >= 1.632 perl(DBIx::Class) >= 0.082801 perl(DBIx::Class::DeploymentHandler) perl(DBIx::Class::DynamicDefault) perl(DBIx::Class::OptimisticLocking) perl(DBIx::Class::ResultClass::HashRefInflator) perl(DBIx::Class::Schema::Config) perl(DBIx::Class::Storage::Statistics) perl(Date::Format) perl(DateTime) perl(DateTime::Duration) perl(DateTime::Format::Pg) perl(Exporter) perl(Fcntl) perl(File::Basename) perl(File::Copy) perl(File::Copy::Recursive) perl(File::Path) perl(File::Spec) perl(FindBin) perl(Getopt::Long::Descriptive) perl(IO::Handle) perl(IPC::Run) perl(JSON::Validator) perl(LWP::UserAgent) perl(Module::Load::Conditional) perl(Module::Pluggable) perl(Mojo::Base) perl(Mojo::ByteStream) perl(Mojo::IOLoop) perl(Mojo::JSON) perl(Mojo::Pg) perl(Mojo::RabbitMQ::Client) >= 0.2 perl(Mojo::URL) perl(Mojo::Util) perl(Mojolicious::Commands) perl(Mojolicious::Plugin) perl(Mojolicious::Plugin::OAuth2) perl(Mojolicious::Static) perl(Net::OpenID::Consumer) perl(POSIX) perl(Pod::POM) perl(SQL::Translator) perl(Scalar::Util) perl(Sort::Versions) perl(Text::Diff) perl(Time::HiRes) perl(Time::ParseDate) perl(Time::Piece) perl(Time::Seconds) perl(URI::Escape) perl(YAML::PP) >= 0.026 perl(YAML::XS) perl(aliased) perl(base) perl(constant) perl(diagnostics) perl(strict) perl(warnings)

etc/nginx/vhosts.d/openqa-locations.inc

@@ -13,6 +13,8 @@ if_modified_since before;
 ## Optional faster assets downloads for large deployments
 #location /assets {
 #    alias /var/lib/openqa/share/factory/;
+#    auth_basic "login to download assets";
+#    auth_basic_user_file /var/lib/openqa/webui/auth/assets.htpasswd;
 #    autoindex          on;
 #    tcp_nopush         on;
 #    sendfile           on;

etc/openqa/openqa.ini

@@ -126,6 +126,10 @@
 [auth]
 #method = Fake|OpenID|OAuth2
 
+## whether authentication is required to access assets; does NOT affect reverse proxy (checkout
+## the template file `etc/nginx/vhosts.d/openqa-locations.inc` for an NGINX example)
+#require_for_assets = 0
+
 ## for GitHub, salsa.debian.org and providers listed on https://metacpan.org/pod/Mojolicious::Plugin::OAuth2#Configuration
 ## one can use:
 #[oauth2]

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(

lib/OpenQA/WebAPI/Controller/ApiKey.pm

@@ -4,7 +4,17 @@
 package OpenQA::WebAPI::Controller::ApiKey;
 use Mojo::Base 'Mojolicious::Controller', -signatures;
 
+use Apache::Htpasswd;
 use DateTime::Format::Pg;
+use OpenQA::Utils qw(prjdir);
+
+use constant {ASSETS_HTPASSWD_PATH => '/webui/auth/assets.htpasswd'};
+
+sub _update_assets_htpasswd_file ($self, $api_key, $delete) {
+    my $file = Apache::Htpasswd->new(prjdir() . ASSETS_HTPASSWD_PATH);
+    my $name = join('-', $self->current_user->name, $api_key->key);
+    ($delete ? $file->htDelete($name) : $file->htpasswd($name, $api_key->secret)) or die $file->error;
+}
 
 sub index ($self) {
     my @keys = $self->current_user->api_keys;
@@ -27,7 +37,10 @@ sub create ($self) {
         $error = $@;
     }
     unless ($error) {
-        eval { $self->schema->resultset('ApiKeys')->create({user_id => $user->id, t_expiration => $expiration}) };
+        eval {
+            my $key = $self->schema->resultset('ApiKeys')->create({user_id => $user->id, t_expiration => $expiration});
+            $self->_update_assets_htpasswd_file($key, 0);
+        };
         $error = $@;
     }
     if ($error) {
@@ -43,8 +56,11 @@ sub destroy ($self) {
     my $key = $user->find_related('api_keys', {id => $self->param('apikeyid')});
 
     if ($key) {
-        $key->delete;
-        $self->flash(info => 'API key deleted');
+        eval {
+            $self->_update_assets_htpasswd_file($key, 1);
+            $key->delete;
+        };
+        $self->flash($@ ? (error => $@) : (info => 'API key deleted'));
     }
     else {
         $self->flash(error => 'API key not found');

lib/OpenQA/WebAPI/Controller/File.pm

@@ -133,10 +133,24 @@ sub test_asset ($self) {
 
     # map to URL - mojo will canonicalize
     $path = $self->url_for('download_asset', assetpath => $path);
-    $self->app->log->debug("redirect to $path");
+
+    # add user credentials so the reverse proxy can authorize the request
+    my $require_auth_for_assets = $self->app->config->{auth}->{require_for_assets};
+    my $user = $self->current_user;
+    if ($require_auth_for_assets && $user) {
+        if (my $api_key = $user->api_keys->search({}, {order_by => {-desc => 'id'}, rows => 1})->first) {
+            # FIXME: don't hardcode scheme and host
+            $path->scheme('http');
+            $path->host('localhost');
+            $path->userinfo(sprintf('%s-%s:%s', $user->name, $api_key->key, $api_key->secret));
+        }
+    }
+
     # pass the redirect to the reverse proxy - might come back to use
     # in case there is no proxy (e.g. in tests)
-    return $self->redirect_to($path);
+    $self->app->log->debug("redirect to $path");
+    $self->res->code(302)->headers->add(Location => $path->to_unsafe_string);
+    $self->render(text => 'redirection');
 }
 
 sub _serve_static ($self, $asset) {

t/config.t

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