Make OAuth2 parameters customizable

* Allow selecting between pre-defined providers 'github' and 'debian_salsa' * Allow configuring custom parameters * Do *not* use attributes within `OAuth2.pm` because we never actually instantiate such an object (`$self` is either the server or a controller object) * Based on WIP state from @phil-hands * See https://progress.opensuse.org/issues/90929
os-autoinst · Apr 12, 2021 · 2877eeb · 2877eeb
1 parent ac0d215
commit 2877eeb
Show file tree

Hide file tree

Showing 4 changed files with 85 additions and 78 deletions.
diff --git a/lib/OpenQA/Setup.pm b/lib/OpenQA/Setup.pm
@@ -77,9 +77,15 @@ sub read_config {
             httpsonly => 1,
         },
         oauth2 => {
-            provider => '',
-            key      => '',
-            secret   => '',
+            provider      => '',
+            key           => '',
+            secret        => '',
+            authorize_url => '',
+            token_url     => '',
+            user_url      => '',
+            token_scope   => '',
+            token_label   => '',
+            nickname_from => '',
         },
         hypnotoad => {
             listen => ['http://localhost:9526/'],

diff --git a/lib/OpenQA/WebAPI/Auth/OAuth2.pm b/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,85 +14,88 @@
 # 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 {
-    my ($self) = @_;
-    $self->config(my $config = $self->app->config->{oauth2});
+sub auth_setup ($server) {
+    my $app    = $server->app;
+    my $config = $app->config->{oauth2};
     croak 'No OAuth2 provider selected' unless my $provider = $config->{provider};
-    my $prov_args = {
-        key    => $self->config->{key},
-        secret => $self->config->{secret},
-    };
-    # I'm afraid I don't quite get where I should tuck this away so that I get
-    # to use it in auth_login, so I'm doing this FIXME_oauth2_ nastiness for
-    # now. I had hoped to tack this onto $prov_args somehow, but I don't know
-    # how to then access that later.
-    if ($provider eq 'github') {
-        $self->config->{FIXME_oauth2_user_url} = 'https://api.github.com/user';
-        # Note: user:email is GitHub-specific, email may be empty
-        $self->config->{FIXME_oauth2_token_scope} = 'user:email';
-        $self->config->{FIXME_oauth2_token_label} = 'token';
-        $self->config->{FIXME_oauth2_nickname_from} = 'login';
-    }
-    elsif ('debian_salsa' eq $provider) {
-        $prov_args->{authorize_url} = 'https://salsa.debian.org/oauth/authorize?response_type=code';
-        $prov_args->{token_url} = 'https://salsa.debian.org/oauth/token';
-        $self->config->{FIXME_oauth2_user_url} = 'https://salsa.debian.org/api/v4/user';
-        $self->config->{FIXME_oauth2_token_scope} = 'read_user';
-        $self->config->{FIXME_oauth2_token_label} = 'Bearer';
-        $self->config->{FIXME_oauth2_nickname_from} = 'username';
-    }
-    elsif ('custom' eq $provider) {
-        $prov_args->{authorize_url} = $self->config->{authorize_url};
-        $prov_args->{token_url} = $self->config->{token_url};
-        $self->config->{FIXME_oauth2_user_url} = $self->config->{user_url};
-        $self->config->{FIXME_oauth2_token_scope} = $self->config->{token_scope};
-        $self->config->{FIXME_oauth2_token_label} = $self->config->{token_label};
-        $self->config->{FIXME_oauth2_nickname_from} = $self->config->{nickname_from};
-    }
-    else {
-        croak "Provider $provider not supported";
-    }
 
-    $self->app->plugin(
-        OAuth2 => {
-            $provider => $prov_args
-        });
+    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},
+            },
+        },
+    );
+    my $params = $parameters_by_provider{$provider};
+    croak "OAuth2 provider '$provider' not supported" unless $params;
+
+    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 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};
 
-    my $get_token_args = {redirect_uri => $self->url_for('login')->userinfo(undef)->to_abs};
-    $get_token_args->{scope} = $self->config->{FIXME_oauth2_token_scope};
-    $self->oauth2->get_token_p($self->config->{provider} => $get_token_args)->then(
+    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 {
-            return unless my $data = shift;  # redirect to ID provider
+            return undef unless my $data = shift;    # redirect to ID provider
 
             # Get or update user details
             my $ua    = Mojo::UserAgent->new;
             my $token = $data->{access_token};
-            my $res   = $ua->get($self->config->{FIXME_oauth2_user_url}, {Authorization => $self->config->{FIXME_oauth2_token_label} . " $token"})->result;
+            my $res
+              = $ua->get($provider_config->{user_url}, {Authorization => "$provider_config->{token_label} $token"})
+              ->result;
             if (my $err = $res->error) {
                 # Note: Using 403 for consistency
-                return $self->render(text => "$err->{code}: $err->{message}", status => 403);
+                return $controller->render(text => "$err->{code}: $err->{message}", status => 403);
             }
             my $details = $res->json;
-            my $user    = $self->schema->resultset('Users')->create_user(
-                "$details->{id}\@$self->config->{provider}",
-                nickname => $details->{$self->config->{FIXME_oauth2_nickname_from}},
+            my $user    = $controller->schema->resultset('Users')->create_user(
+                $details->{id},
+                provider => "oauth2\@$main_config->{provider}",
+                nickname => $details->{$provider_config->{nickname_from}},
                 fullname => $details->{name},
                 email    => $details->{email});
 
-            $self->session->{user} = $user->username;
-            $self->redirect_to('index');
-        })->catch(sub { $self->render(text => shift, status => 403) });
+            $controller->session->{user} = $user->username;
+            $controller->redirect_to('index');
+        })->catch(sub { $controller->render(text => shift, status => 403) });
     return (manual => 1);
 }
 

diff --git a/t/03-auth.t b/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
@@ -51,18 +51,10 @@ 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';
 };

diff --git a/t/config.t b/t/config.t
@@ -76,9 +76,15 @@ 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 => '',
         },
         hypnotoad => {
             listen => ['http://localhost:9526/'],