Merge pull request #3769 from phil-hands/master

Make OAuth2 parameters customizable, get OAuth2 to work with salsa.debian.org (gitlab)

docs/Installing.asciidoc

@@ -303,9 +303,10 @@ according to
 [[authentication]]
 === User authentication
 
-OpenQA supports three different authentication methods - OpenID (default),
-OAuth2 (currently limited to GitHub) and Fake (for development).
-See `auth` section in `/etc/openqa/openqa.ini`.
+OpenQA supports three different authentication methods: OpenID (default),
+OAuth2 and Fake (for development).
+
+Use the `auth` section in `/etc/openqa/openqa.ini` to configure the method:
 
 [source,ini]
 --------------------------------------------------------------------------------
@@ -314,8 +315,15 @@ See `auth` section in `/etc/openqa/openqa.ini`.
 method = OpenID
 --------------------------------------------------------------------------------
 
-Independently of method used, the first user that logs in (if there is no admin yet)
-will automatically get administrator rights!
+Independently of method used, the first user that logs in (if there is no
+admin yet) will automatically get administrator rights!
+
+Note that only one authentication method and only one OpenID/OAuth2 provider
+can be configured at a time. When changing the method/provider no
+users/permissions are lost. However, a new and distinct user (with default
+permissions) will be created when logging in via a different method/provider
+because there is no automatic mapping of identities across different
+methods/providers.
 
 ==== OpenID
 
@@ -339,7 +347,15 @@ This method supports OpenID version up to 2.0.
 
 ==== OAuth2
 
-Login via OAuth 2.0 is currently limited to GitHub.
+An additional Mojolicious plugin is required to use this feature:
+
+[source,sh]
+-------------------------------------------------------------------------------
+# openSUSE
+zypper in 'perl(Mojolicious::Plugin::OAuth2)'
+-------------------------------------------------------------------------------
+
+Example for configuring OAuth2 with GitHub:
 
 [source,ini]
 --------------------------------------------------------------------------------
@@ -353,17 +369,13 @@ key = mykey
 secret = mysecret
 --------------------------------------------------------------------------------
 
-In order to use GitHub for authorization, the instance needs to be
-https://github.com/settings/applications/new[registered on GitHub]. Afterwards
-the key and secret will be visible to the application owner(s).
-
-Note: An additional Mojolicious plugin is required to use this feature:
+In order to use GitHub for authorization, an "OAuth App" needs to be
+https://github.com/settings/applications/new[registered on GitHub]. Use `…/login`
+as callback URL. Afterwards the key and secret will be visible to the application
+owner(s).
 
-[source,sh]
--------------------------------------------------------------------------------
-# openSUSE
-zypper in 'perl(Mojolicious::Plugin::OAuth2)'
--------------------------------------------------------------------------------
+As shown in the comments of the default configuration file, it is also possible
+to use different providers.
 
 ==== Fake
 

etc/openqa/openqa.ini

@@ -85,7 +85,27 @@
 
 ## Authentication method to use for user management
 [auth]
-# method = Fake|OpenID
+# method = Fake|OpenID|OAuth2
+
+# for GitHub, salsa.debian.org and providers listed on https://metacpan.org/pod/Mojolicious::Plugin::OAuth2#Configuration
+# one can use:
+#[oauth2]
+#provider = github|debian_salsa
+#key = ...
+#secret = ...
+
+# alternatively, one can specify parameters manually without relying on magic a provider name:
+#[oauth2]
+#provider = custom
+#unique_name = debian_salsa
+#key = ...
+#secret = ...
+#authorize_url = https://salsa.debian.org/oauth/authorize?response_type=code
+#token_url = https://salsa.debian.org/oauth/token
+#user_url  = https://salsa.debian.org/api/v4/user
+#token_scope = read_user
+#token_label = Bearer
+#nickname_from = username
 
 [logging]
 #logging is to stderr (so systemd journal) by default

lib/OpenQA/Setup.pm

@@ -77,9 +77,16 @@ sub read_config {
             httpsonly => 1,
         },
         oauth2 => {
-            provider => '',
-            key      => '',
-            secret   => '',
+            provider      => '',
+            key           => '',
+            secret        => '',
+            authorize_url => '',
+            token_url     => '',
+            user_url      => '',
+            token_scope   => '',
+            token_label   => '',
+            nickname_from => '',
+            unique_name   => '',
         },
         hypnotoad => {
             listen => ['http://localhost:9526/'],

lib/OpenQA/WebAPI/Auth/OAuth2.pm

@@ -1,4 +1,4 @@
-# Copyright (C) 2020 SUSE LLC
+# Copyright (C) 2020-2021 SUSE LLC
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -14,55 +14,95 @@
 # with this program; if not, see <http://www.gnu.org/licenses/>.
 
 package OpenQA::WebAPI::Auth::OAuth2;
-use Mojo::Base -base;
-
+use Mojo::Base -base, -signatures;
 use Carp 'croak';
 
-has config => undef;
+sub auth_setup ($server) {
+    my $app    = $server->app;
+    my $config = $app->config->{oauth2};
+    croak 'No OAuth2 provider selected' unless my $provider = $config->{provider};
 
-sub auth_setup {
-    my ($self) = @_;
-    $self->config($self->app->config->{oauth2});
-    croak 'No OAuth2 provider selected' unless my $provider = $self->config->{provider};
-    croak "Provider $provider not supported" unless $provider eq 'github';
+    my %parameters_by_provider = (
+        github => {
+            args   => [],
+            config => {
+                user_url      => 'https://api.github.com/user',
+                token_scope   => 'user:email',
+                token_label   => 'token',
+                nickname_from => 'login',
+            },
+        },
+        debian_salsa => {
+            args => [
+                authorize_url => 'https://salsa.debian.org/oauth/authorize?response_type=code',
+                token_url     => 'https://salsa.debian.org/oauth/token',
+            ],
+            config => {
+                user_url      => 'https://salsa.debian.org/api/v4/user',
+                token_scope   => 'read_user',
+                token_label   => 'Bearer',
+                nickname_from => 'username',
+            },
+        },
+        custom => {
+            args => [
+                authorize_url => $config->{authorize_url},
+                token_url     => $config->{token_url},
+            ],
+            config => {
+                user_url      => $config->{user_url},
+                token_scope   => $config->{token_scope},
+                token_label   => $config->{token_label},
+                nickname_from => $config->{nickname_from},
+                unique_name   => $config->{unique_name},
+            },
+        },
+    );
+    my $params = $parameters_by_provider{$provider};
+    croak "OAuth2 provider '$provider' not supported" unless $params;
 
-    $self->app->plugin(
-        OAuth2 => {
-            $provider => {
-                key    => $self->config->{key},
-                secret => $self->config->{secret},
-            }});
+    my %provider_args = (key => $config->{key}, secret => $config->{secret}, @{$params->{args}});
+    $config->{provider_config} = $params->{config};
+    $app->plugin(OAuth2 => {$provider => \%provider_args});
 }
 
-sub auth_login {
-    my ($self) = @_;
-    croak 'Setup was not called' unless $self->config;
+sub update_user ($controller, $main_config, $provider_config, $data) {
+    return undef unless $data;    # redirect to ID provider
+
+    # get or update user details
+    my $ua    = Mojo::UserAgent->new;
+    my $token = $data->{access_token};
+    my $tx    = $ua->get($provider_config->{user_url}, {Authorization => "$provider_config->{token_label} $token"});
+    if (my $err = $tx->error) {
+        my $msg = $err->{code} ? "$err->{code} response: $err->{message}" : "Connection error: $err->{message}";
+        return $controller->render(text => $msg, status => 403);    # return always 403 for consistency
+    }
+    my $details = $tx->res->json;
+    if (ref $details ne 'HASH' || !$details->{id} || !$details->{$provider_config->{nickname_from}}) {
+        return $controller->render(text => 'User data returned by OAuth2 provider is insufficient', status => 403);
+    }
+    my $provider_name = $main_config->{provider};
+    $provider_name = $provider_config->{unique_name} || $provider_name if $provider_name eq 'custom';
+    my $user = $controller->schema->resultset('Users')->create_user(
+        $details->{id},
+        provider => "oauth2\@$provider_name",
+        nickname => $details->{$provider_config->{nickname_from}},
+        fullname => $details->{name},
+        email    => $details->{email});
 
-    my $get_token_args = {redirect_uri => $self->url_for('login')->userinfo(undef)->to_abs};
-    # Note: user:email is GitHub-specific, email may be empty
-    $get_token_args->{scope} = 'user:email';
-    $self->oauth2->get_token_p($self->config->{provider} => $get_token_args)->then(
-        sub {
-            return unless my $data = shift;
+    $controller->session->{user} = $user->username;
+    $controller->redirect_to('index');
+}
 
-            # Get or update user details via GitHub-specific API
-            my $ua    = Mojo::UserAgent->new;
-            my $token = $data->{access_token};
-            my $res   = $ua->get('https://api.github.com/user', {Authorization => "token $token"})->result;
-            if (my $err = $res->error) {
-                # Note: Using 403 for consistency
-                return $self->render(text => "$err->{code}: $err->{message}", status => 403);
-            }
-            my $details = $res->json;
-            my $user    = $self->schema->resultset('Users')->create_user(
-                $details->{id},
-                nickname => $details->{login},
-                fullname => $details->{name},
-                email    => $details->{email});
+sub auth_login ($controller) {
+    croak 'Config was not parsed' unless my $main_config     = $controller->app->config->{oauth2};
+    croak 'Setup was not called'  unless my $provider_config = $main_config->{provider_config};
 
-            $self->session->{user} = $user->username;
-            $self->redirect_to('index');
-        })->catch(sub { $self->render(text => shift, status => 403) });
+    my $get_token_args = {redirect_uri => $controller->url_for('login')->userinfo(undef)->to_abs};
+    $get_token_args->{scope} = $provider_config->{token_scope};
+    $controller->oauth2->get_token_p($main_config->{provider} => $get_token_args)
+      ->then(sub { update_user($controller, $main_config, $provider_config, shift) })
+      ->catch(sub { $controller->render(text => shift, status => 403) });
     return (manual => 1);
 }
 

t/03-auth.t

@@ -1,4 +1,4 @@
-# Copyright (C) 2020 SUSE LLC
+# Copyright (C) 2020-2021 SUSE LLC
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -23,7 +23,9 @@ use Test::Output 'combined_like';
 use Test::Warnings;
 use OpenQA::Test::Database;
 use OpenQA::Test::TimeLimit '10';
+use OpenQA::WebAPI::Auth::OAuth2;
 use Mojo::File qw(tempdir path);
+use Mojo::Transaction;
 
 my $t;
 my $tempdir = tempdir("/tmp/$FindBin::Script-XXXX")->make_path;
@@ -51,20 +53,52 @@ combined_like { test_auth_method_startup('OpenID')->status_is(403) } qr/Claiming
   'Plugin loaded, identity denied';
 
 subtest OAuth2 => sub {
-    lives_ok {
-        $t->app->plugin(
-            OAuth2 => {
-                mocked => {
-                    key => 'deadbeef',
-                }})
-    }
-    'auth mocked';
-
+    lives_ok { $t->app->plugin(OAuth2 => {mocked => {key => 'deadbeef'}}) } 'auth mocked';
     throws_ok { test_auth_method_startup 'OAuth2' } qr/No OAuth2 provider selected/, 'Error with no provider selected';
     throws_ok { test_auth_method_startup('OAuth2', ("[oauth2]\n", "provider = foo\n")) }
-    qr/Provider foo not supported/, 'Error with unsupported provider';
+    qr/OAuth2 provider 'foo' not supported/, 'Error with unsupported provider';
     combined_like { test_auth_method_startup('OAuth2', ("[oauth2]\n", "provider = github\n")) } qr/302 Found/,
       'Plugin loaded';
+
+    my $ua_mock  = Test::MockModule->new('Mojo::UserAgent');
+    my $msg_mock = Test::MockModule->new('Mojo::Message');
+    my @get_args;
+    my $get_tx = Mojo::Transaction->new;
+    $ua_mock->redefine(get => sub { shift; push @get_args, [@_]; $get_tx });
+
+    my $c             = $t->app->build_controller;
+    my %main_cfg      = (provider     => 'custom');
+    my %provider_cfg  = (user_url     => 'http://does-not-exist', token_label => 'bar', nickname_from => 'login');
+    my %data          = (access_token => 'some-token');
+    my %expected_user = (username     => 42, provider => 'oauth2@custom', nickname => 'Demo');
+    my $users         = $t->app->schema->resultset('Users');
+
+    subtest 'failure when requesting user details' => sub {
+        $get_tx->res->error({code => 500, message => 'Internal server error'});
+        OpenQA::WebAPI::Auth::OAuth2::update_user($c, \%main_cfg, \%provider_cfg, \%data);
+        is $c->res->code, 403,                                   'status code';
+        is $c->res->body, '500 response: Internal server error', 'error message';
+        is $c->session->{user}, undef, 'user not set';
+        is_deeply \@get_args, [['http://does-not-exist', {Authorization => 'bar some-token'}]], 'args for get request'
+          or diag explain \@get_args;
+    };
+
+    subtest 'OAuth provider does not provide all mandatory user details' => sub {
+        $get_tx->res->error(undef)->body('{}');
+        OpenQA::WebAPI::Auth::OAuth2::update_user($c, \%main_cfg, \%provider_cfg, \%data);
+        is $c->res->code, 403,                                                     'status code';
+        is $c->res->body, 'User data returned by OAuth2 provider is insufficient', 'error message';
+        is $c->session->{user}, undef, 'user not set';
+    };
+
+    subtest 'requesting user details succeeds' => sub {
+        $get_tx->res->error(undef);
+        $msg_mock->redefine(json => {id => 42, login => 'Demo'});
+        OpenQA::WebAPI::Auth::OAuth2::update_user($c, \%main_cfg, \%provider_cfg, \%data);
+        is $c->res->code, 302, 'status code (redirection)';
+        is $c->session->{user}, '42', 'user set';
+        is $users->search(\%expected_user)->count, 1, 'user created';
+    };
 };
 
 throws_ok { test_auth_method_startup('nonexistant') } qr/Unable to load auth module/,

t/config.t

@@ -76,9 +76,16 @@ subtest 'Test configuration default modes' => sub {
             httpsonly => 1,
         },
         oauth2 => {
-            provider => '',
-            key      => '',
-            secret   => '',
+            provider      => '',
+            key           => '',
+            secret        => '',
+            authorize_url => '',
+            token_url     => '',
+            user_url      => '',
+            token_scope   => '',
+            token_label   => '',
+            nickname_from => '',
+            unique_name   => '',
         },
         hypnotoad => {
             listen => ['http://localhost:9526/'],