Attachment 170647 Details for Bug 37711 – Bug 37711: Add support for staff interface auto-register on IdP

[patch] Bug 37711: Add support for staff interface auto-register on IdP

Bug-37711-Add-support-for-staff-interface-auto-reg.patch (text/plain), 10.37 KB, created by Tomás Cohen Arazi (tcohen) on 2024-08-23 15:29:28 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Tomás Cohen Arazi (tcohen)

Created: 2024-08-23 15:29:28 UTC

Size: 10.37 KB

patch

obsolete

>From 5e835240bcfb8732d60bc1acf66a817026ba8d07 Mon Sep 17 00:00:00 2001
>From: Tomas Cohen Arazi <tomascohen@theke.io>
>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<Koha::Patron> if the provider is configured
>-to do so for the domain.
>+This helper creates a new I<Koha::Patron> using the (already) mapped data
>+provided in the I<data> attribute.
>+
>+A check is done on the passed I<interface> and I<domain> to validate
>+the provider is configured to allow auto registration.
>+
>+Valid values for B<interface> are I<opac> and I<staff>. 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

Actions: View | Diff | Splinter Review

Attachments on bug 37711: 170643 | 170644 | 170645 | 170646 | 170647 | 174090 | 174091 | 174092 | 174093 | 174094