From 5e835240bcfb8732d60bc1acf66a817026ba8d07 Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Fri, 23 Aug 2024 11:17:14 -0300 Subject: [PATCH] Bug 37711: Add support for staff interface auto-register on IdP Before this patches, only OPAC auto-registration at login using OAuth2/OIDC IdPs was allowed. This patch makes the staff interface to be able to auto-register new users. On doing it, I had to track down the possible values for the `interface` parameter in the case of the staff interface. Koha::Template::Plugin::AuthClient uses 'staff' internally so I sticked with it. The whole area needs more param validation and exceptions to avoid mistakes for not knowing the valid values. I added some safeguards on the Koha::REST::Plugin::Auth::IdP helpers. They get tested on the added tests. To test: 1. Apply this patch 2. Run: $ prove t/db_dependent/Koha/REST/Plugin/Auth/IdP.t => SUCCESS: Tests pass! 3. Have a valid OAuth... 4. Enable Staff auto-register 5. Login with an external IdP on the staff interface => SUCCESS: It gets registered! Note: it will probably lack permissions to access the staff interface, but you can check on the DB it got registered. 6. Sign off :-D Sponsored-by: ByWater Solutions --- Koha/REST/Plugin/Auth/IdP.pm | 28 +++-- t/db_dependent/Koha/REST/Plugin/Auth/IdP.t | 114 +++++++++++++++------ 2 files changed, 105 insertions(+), 37 deletions(-) diff --git a/Koha/REST/Plugin/Auth/IdP.pm b/Koha/REST/Plugin/Auth/IdP.pm index 8c3b010f3dd..97a2322de1d 100644 --- a/Koha/REST/Plugin/Auth/IdP.pm +++ b/Koha/REST/Plugin/Auth/IdP.pm @@ -28,6 +28,7 @@ use Koha::Patrons; use C4::Auth qw(create_basic_session); use CGI; +use List::MoreUtils qw(any); =head1 NAME @@ -45,25 +46,40 @@ sub register { =head3 auth.register my $patron = $c->auth->register( - { data => $patron_data, + { + data => $patron_data, domain => $domain, interface => $interface } ); -If no patron passed, creates a new I if the provider is configured -to do so for the domain. +This helper creates a new I using the (already) mapped data +provided in the I attribute. + +A check is done on the passed I and I to validate +the provider is configured to allow auto registration. + +Valid values for B are I and I. An exception will be thrown +if other values or none are passed. =cut $app->helper( 'auth.register' => sub { my ( $c, $params ) = @_; - my $data = $params->{data}; - my $domain = $params->{domain}; + my $data = $params->{data}; + my $domain = $params->{domain}; my $interface = $params->{interface}; - unless ( $interface eq 'opac' && $domain->auto_register ) { + Koha::Exceptions::MissingParameter->throw( parameter => 'interface' ) + unless $interface; + + Koha::Exceptions::BadParameter->throw( parameter => 'interface' ) + unless any { $interface eq $_ } qw{ opac staff }; + + if ( $interface eq 'opac' && !$domain->auto_register_opac + || $interface eq 'staff' && !$domain->auto_register_staff ) + { Koha::Exceptions::Auth::Unauthorized->throw( code => 401 ); } diff --git a/t/db_dependent/Koha/REST/Plugin/Auth/IdP.t b/t/db_dependent/Koha/REST/Plugin/Auth/IdP.t index 48325367ade..8b7d3465905 100755 --- a/t/db_dependent/Koha/REST/Plugin/Auth/IdP.t +++ b/t/db_dependent/Koha/REST/Plugin/Auth/IdP.t @@ -35,7 +35,8 @@ post '/register_user' => sub { try { my $domain = Koha::Auth::Identity::Provider::Domains->find( $params->{domain_id} ); my $patron = $c->auth->register( - { data => $params->{data}, + { + data => $params->{data}, domain => $domain, interface => $params->{interface} } @@ -44,6 +45,10 @@ post '/register_user' => sub { } catch { if ( ref($_) eq 'Koha::Exceptions::Auth::Unauthorized' ) { $c->render( status => 401, json => { message => 'unauthorized' } ); + } elsif ( ref($_) eq 'Koha::Exceptions::BadParameter' ) { + $c->render( status => 400, json => { message => 'bad parameter: ' . $_->parameter } ); + } elsif ( ref($_) eq 'Koha::Exceptions::MissingParameter' ) { + $c->render( status => 400, json => { message => 'missing parameter: ' . $_->parameter } ); } else { $c->render( status => 500, json => { message => 'other error' } ); } @@ -77,53 +82,101 @@ use Koha::Database; my $schema = Koha::Database->new()->schema(); my $builder = t::lib::TestBuilder->new; +my $t = Test::Mojo->new; + # FIXME: sessionStorage defaults to mysql, but it seems to break transaction handling # this affects the other REST api tests t::lib::Mocks::mock_preference( 'SessionStorage', 'tmp' ); subtest 'auth.register helper' => sub { - plan tests => 6; + + plan tests => 18; $schema->storage->txn_begin; # generate a random patron - my $patron_to_delete = $builder->build_object({ class => 'Koha::Patrons' }); - my $userid = $patron_to_delete->userid; - # delete patron - $patron_to_delete->delete; + my $patron_to_delete_1 = $builder->build_object( { class => 'Koha::Patrons' } ); + my $patron_to_delete_2 = $builder->build_object( { class => 'Koha::Patrons' } ); + my $userid_1 = $patron_to_delete_1->userid; + my $userid_2 = $patron_to_delete_2->userid; - my $provider = $builder->build_object( { class => 'Koha::Auth::Identity::Providers', value => { matchpoint => 'email' } } ); - - my $domain_with_register = $builder->build_object( - { class => 'Koha::Auth::Identity::Provider::Domains', - value => { identity_provider_id => $provider->id, domain => 'domain1.com', auto_register => 1 } + # delete patron + $patron_to_delete_1->delete; + $patron_to_delete_2->delete; + + my $provider = + $builder->build_object( { class => 'Koha::Auth::Identity::Providers', value => { matchpoint => 'email' } } ); + + my $domain_1 = $builder->build_object( + { + class => 'Koha::Auth::Identity::Provider::Domains', + value => { + identity_provider_id => $provider->id, + domain => 'domain1.com', + auto_register_opac => 1, + auto_register_staff => 0 + } } ); - my $domain_without_register = $builder->build_object( - { class => 'Koha::Auth::Identity::Provider::Domains', - value => { identity_provider_id => $provider->id, domain => 'domain2.com', auto_register => 0 } + my $domain_2 = $builder->build_object( + { + class => 'Koha::Auth::Identity::Provider::Domains', + value => { + identity_provider_id => $provider->id, + domain => 'domain2.com', + auto_register_opac => 0, + auto_register_staff => 1 + } } ); - my $library = $builder->build_object( { class => 'Koha::Libraries' } ); - my $category = $builder->build_object( { class => 'Koha::Patron::Categories' } ); - - my $user_data = { - firstname => 'test', - surname => 'test', - userid => $userid, - branchcode => $library->branchcode, - categorycode => $category->categorycode - }; - - my $t = Test::Mojo->new; + my $library = $builder->build_object( { class => 'Koha::Libraries' } ); + my $category = $builder->build_object( { class => 'Koha::Patron::Categories' } ); + + $t->post_ok( + '/register_user' => json => { + data => { + firstname => 'test', + surname => 'test', + userid => $userid_1, + branchcode => $library->branchcode, + categorycode => $category->categorycode + }, + domain_id => $domain_1->identity_provider_domain_id, + interface => 'opac' + } + )->status_is(200)->json_is( '/userid', $userid_1 ); + + $t->post_ok( '/register_user' => json => + { data => {}, domain_id => $domain_2->identity_provider_domain_id, interface => 'opac' } )->status_is(401) + ->json_is( '/message', 'unauthorized' ); + + $t->post_ok( '/register_user' => json => + { data => {}, domain_id => $domain_1->identity_provider_domain_id, interface => 'staff' } )->status_is(401) + ->json_is( '/message', 'unauthorized' ); + + $t->post_ok( + '/register_user' => json => { + data => { + firstname => 'test', + surname => 'test', + userid => $userid_2, + branchcode => $library->branchcode, + categorycode => $category->categorycode + }, + domain_id => $domain_2->identity_provider_domain_id, + interface => 'staff' + } + )->status_is(200)->json_is( '/userid', $userid_2 ); - $t->post_ok( '/register_user' => json => { data => $user_data, domain_id => $domain_with_register->identity_provider_domain_id, interface => 'opac' } )->status_is(200) - ->json_has( '/firstname', 'test' ); + $t->post_ok( '/register_user' => json => + { data => {}, domain_id => $domain_1->identity_provider_domain_id } )->status_is(400) + ->json_is( '/message', 'missing parameter: interface' ); - $t->post_ok( '/register_user' => json => { data => $user_data, domain_id => $domain_without_register->identity_provider_domain_id, interface => 'opac' } )->status_is(401) - ->json_has( '/message', 'unauthorized' ); + $t->post_ok( '/register_user' => json => + { data => {}, domain_id => $domain_1->identity_provider_domain_id, interface => 'invalid' } )->status_is(400) + ->json_is( '/message', 'bad parameter: interface' ); $schema->storage->txn_rollback; }; @@ -135,7 +188,6 @@ subtest 'auth.session helper' => sub { my $patron = $builder->build_object( { class => 'Koha::Patrons' } ); - my $t = Test::Mojo->new; $t->post_ok( '/start_session' => json => { userid => $patron->userid } )->status_is(200)->json_has( '/status', 'ok' ); $schema->storage->txn_rollback; -- 2.46.0