diff --git a/lib/OpenQA/WebAPI/Auth/OpenID.pm b/lib/OpenQA/WebAPI/Auth/OpenID.pm index 9d4a09ac12e..39e5be7165f 100644 --- a/lib/OpenQA/WebAPI/Auth/OpenID.pm +++ b/lib/OpenQA/WebAPI/Auth/OpenID.pm @@ -9,19 +9,19 @@ use LWP::UserAgent; use Net::OpenID::Consumer; use MIME::Base64 qw(encode_base64url decode_base64url); -sub auth_login ($self) { - my $url = $self->app->config->{global}->{base_url} || $self->req->url->base->to_string; +sub auth_login ($c) { + my $url = $c->app->config->{global}->{base_url} || $c->req->url->base->to_string; # force secure connection after login - $url =~ s,^http://,https://, if $self->app->config->{openid}->{httpsonly}; + $url =~ s,^http://,https://, if $c->app->config->{openid}->{httpsonly}; my $csr = Net::OpenID::Consumer->new( ua => LWP::UserAgent->new, required_root => $url, - consumer_secret => $self->app->config->{_openid_secret}, + consumer_secret => $c->app->config->{_openid_secret}, ); - my $claimed_id = $csr->claimed_identity($self->config->{openid}->{provider}); + my $claimed_id = $csr->claimed_identity($c->config->{openid}->{provider}); if (!defined $claimed_id) { log_error("Claiming OpenID identity for URL '$url' failed: " . $csr->err); return; @@ -47,7 +47,7 @@ sub auth_login ($self) { ); my $return_url = Mojo::URL->new(qq{$url/response}); - if (my $return_page = $self->param('return_page') || $self->req->headers->referrer) { + if (my $return_page = $c->param('return_page') || $c->req->headers->referrer) { $return_page = Mojo::URL->new($return_page)->path_query; # return_page is encoded using base64 (in a version that avoids / and + symbol) # as any special characters like / or ? when urlencoded via % symbols, @@ -63,77 +63,63 @@ sub auth_login ($self) { return (error => $csr->err); } -sub auth_response ($self) { - my %params = @{$self->req->params->pairs}; - my $url = $self->app->config->{global}->{base_url} || $self->req->url->base; +sub _first_last_name ($ax) { join(' ', $ax->{'value.firstname'} // '', $ax->{'value.lastname'} // '') } + +sub _create_user ($c, $id, $email, $nickname, $fullname) { + $c->schema->resultset('Users')->create_user($id, email => $email, nickname => $nickname, fullname => $fullname); +} + +sub _handle_verified ($c, $vident) { + my $sreg = $vident->signed_extension_fields('http://openid.net/extensions/sreg/1.1'); + my $ax = $vident->signed_extension_fields('http://openid.net/srv/ax/1.0'); + + my $email = $sreg->{email} || $ax->{'value.email'} || 'nobody@example.com'; + my $nickname = $sreg->{nickname} || $ax->{'value.nickname'} || $ax->{'value.firstname'}; + unless ($nickname) { + my @a = split(/\/([^\/]+)$/, $vident->{identity}); + $nickname = $a[1]; + } + + my $fullname = $sreg->{fullname} || $ax->{'value.fullname'} || _first_last_name($ax) || $nickname; + + _create_user($c, $vident->{identity}, $email, $nickname, $fullname); + $c->session->{user} = $vident->{identity}; +} + +sub auth_response ($c) { + my %params = @{$c->req->params->pairs}; + my $url = $c->app->config->{global}->{base_url} || $c->req->url->base; return (error => 'Got response on http but https is forced. MOJO_REVERSE_PROXY not set?') - if ($self->app->config->{openid}->{httpsonly} && $url !~ /^https:\/\//); + if ($c->app->config->{openid}->{httpsonly} && $url !~ /^https:\/\//); %params = map { $_ => URI::Escape::uri_unescape($params{$_}) } keys %params; my $csr = Net::OpenID::Consumer->new( - debug => sub { $self->app->log->debug('Net::OpenID::Consumer: ' . join(' ', @_)); }, + debug => sub (@args) { $c->app->log->debug('Net::OpenID::Consumer: ' . join(' ', @args)) }, ua => LWP::UserAgent->new, required_root => $url, - consumer_secret => $self->app->config->{_openid_secret}, + consumer_secret => $c->app->config->{_openid_secret}, args => \%params, ); - my $err_handler = sub { - my ($err, $txt) = @_; - $self->app->log->error("OpenID: $err: $txt"); - $self->flash(error => "$err: $txt"); + my $err_handler = sub ($err, $txt) { + $c->app->log->error("OpenID: $err: $txt"); + $c->flash(error => "$err: $txt"); return (error => 0); }; $csr->handle_server_response( - not_openid => sub { - return $err_handler->('Failed to login', 'OpenID provider returned invalid data. Please retry again'); - }, - setup_needed => sub { - my $setup_url = shift; - + not_openid => + sub () { $err_handler->('Failed to login', 'OpenID provider returned invalid data. Please retry again') }, + setup_needed => sub ($setup_url) { # Redirect the user to $setup_url $setup_url = URI::Escape::uri_unescape($setup_url); - $self->app->log->debug(qq{setup_url[$setup_url]}); + $c->app->log->debug(qq{setup_url[$setup_url]}); return (redirect => $setup_url, error => 0); }, - cancelled => sub { }, # Do something appropriate when the user hits "cancel" at the OP - verified => sub { - my $vident = shift; - my $sreg = $vident->signed_extension_fields('http://openid.net/extensions/sreg/1.1'); - my $ax = $vident->signed_extension_fields('http://openid.net/srv/ax/1.0'); - - my $email = $sreg->{email} || $ax->{'value.email'} || 'nobody@example.com'; - my $nickname = $sreg->{nickname} || $ax->{'value.nickname'} || $ax->{'value.firstname'}; - unless ($nickname) { - my @a = split(/\/([^\/]+)$/, $vident->{identity}); - $nickname = $a[1]; - } - my $fullname = $sreg->{fullname} || $ax->{'value.fullname'}; - unless ($fullname) { - if ($ax->{'value.firstname'}) { - $fullname = $ax->{'value.firstname'}; - if ($ax->{'value.lastname'}) { - $fullname .= ' ' . $ax->{'value.lastname'}; - } - } - else { - $fullname = $nickname; - } - } - - my $user = $self->schema->resultset('Users')->create_user( - $vident->{identity}, - email => $email, - nickname => $nickname, - fullname => $fullname - ); - $self->session->{user} = $vident->{identity}; - }, - error => sub { - return $err_handler->(@_); - }, + cancelled => sub () { }, # Do something appropriate when the user hits "cancel" at the OP + verified => sub ($vident) { _handle_verified($c, $vident) }, + error => sub (@args) { $err_handler->(@args) }, ); return (redirect => decode_base64url($csr->args('return_page'), error => 0)) if $csr->args('return_page'); diff --git a/t/03-auth-openid.t b/t/03-auth-openid.t new file mode 100644 index 00000000000..10bfdd42fd1 --- /dev/null +++ b/t/03-auth-openid.t @@ -0,0 +1,36 @@ +# Copyright SUSE LLC +# SPDX-License-Identifier: GPL-2.0-or-later + +use Test::Most; +use Test::Warnings ':report_warnings'; +use Test::MockModule; +use Test::MockObject; +use FindBin; +use lib "$FindBin::Bin/lib", "$FindBin::Bin/../external/os-autoinst-common/lib"; +use OpenQA::Test::TimeLimit '10'; +use OpenQA::WebAPI::Auth::OpenID; + + +is OpenQA::WebAPI::Auth::OpenID::_first_last_name({'value.firstname' => 'Mordred'}), 'Mordred ', + '_first_last_name concats also with empty fields'; +my (%openid_res, %openid_res2); +my $vident = Test::MockObject->new->set_series('signed_extension_fields', \%openid_res, \%openid_res2); +my $user = $vident->{identity} = 'mordred'; +my $users = Test::MockObject->new->set_always('create_user', 1); +my $schema = Test::MockObject->new->set_always(resultset => $users); +my %session; +my $c = Test::MockObject->new->set_always(schema => $schema); +ok OpenQA::WebAPI::Auth::OpenID::_create_user($c, $user, 'nobody\@example.com', $user, $user), 'can call _create_user'; +$c->set_always(session => \%session); +ok OpenQA::WebAPI::Auth::OpenID::_handle_verified($c, $vident), 'can call _handle_verified'; +$users->called_ok('create_user', 'new user is created for initial login'); +is(($users->call_args(2))[1], 'mordred', 'new user created with details'); +$c->set_always( + req => Test::MockObject->new->set_always(params => Test::MockObject->new->set_always(pairs => [1, 2])) + ->set_always(url => Test::MockObject->new->set_always(base => 'openqa'))) + ->set_always(app => Test::MockObject->new->set_always(config => {}) + ->set_always(log => Test::MockObject->new->set_true('error', 'debug')))->set_true('flash'); +is OpenQA::WebAPI::Auth::OpenID::auth_response($c), 0, 'can call auth_response'; +$c->app->log->called_ok('error', 'an error was logged for call without proper config'); + +done_testing; diff --git a/t/03-auth.t b/t/03-auth.t index e44d67d789a..bfa3cd45410 100644 --- a/t/03-auth.t +++ b/t/03-auth.t @@ -59,7 +59,7 @@ subtest OpenID => sub { handle_server_response => sub ($self, %args) { }, args => sub ($self, $query) { {return_page => encode_base64url('/tests/42')}->{$query} }); $t->get_ok('/response')->status_is(302); - $t->header_is('Location', '/tests/42', 'redirect to original papge after login'); + $t->header_is('Location', '/tests/42', 'redirect to original page after login'); $t->get_ok('/api_keys')->status_is(302); $t->header_is('Location', '/login?return_page=%2Fapi_keys', 'remember return_page for ensure_operator');