Bugzilla – Attachment 152673 Details for
Bug 33778
Move t/Auth_with_shibboleth.t to db_dependent
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 33778: Further polishing
Bug-33778-Further-polishing.patch (text/plain), 13.36 KB, created by
Martin Renvoize (ashimema)
on 2023-06-26 11:02:50 UTC
(
hide
)
Description:
Bug 33778: Further polishing
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2023-06-26 11:02:50 UTC
Size:
13.36 KB
patch
obsolete
>From 94d6464b89c9a4fd0a58475b1ae05f019bb52e4d Mon Sep 17 00:00:00 2001 >From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> >Date: Fri, 19 May 2023 06:45:22 +0000 >Subject: [PATCH] Bug 33778: Further polishing > >Use mock_preference. >Move final tests for BaseURLs into one subtest. >Change reset_config to allow passing parameters replacing some variables. >Copyright line. > >Test plan: >Run t/Auth_with_shibboleth.t > >Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> >Signed-off-by: David Nind <david@davidnind.com> >Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> >--- > t/Auth_with_shibboleth.t | 206 ++++++++++++--------------------------- > 1 file changed, 63 insertions(+), 143 deletions(-) > >diff --git a/t/Auth_with_shibboleth.t b/t/Auth_with_shibboleth.t >index af94463872..8b464da682 100755 >--- a/t/Auth_with_shibboleth.t >+++ b/t/Auth_with_shibboleth.t >@@ -1,5 +1,7 @@ > #!/usr/bin/perl > >+# Copyright 2014, 2023 Koha development team >+# > # This file is part of Koha. > # > # Koha is free software; you can redistribute it and/or modify it >@@ -18,94 +20,61 @@ > use Modern::Perl; > use utf8; > >-use Test::More tests => 18; >+use Test::More tests => 5; > use Test::MockModule; > use Test::Warn; > use CGI qw(-utf8 ); > use File::Temp qw(tempdir); > >+use t::lib::Mocks; > use t::lib::Mocks::Logger; > use t::lib::TestBuilder; > >-use C4::Context; >+use C4::Auth_with_shibboleth qw( shib_ok login_shib_url get_login_shib checkpw_shib ); > use Koha::Database; > > my $schema = Koha::Database->new->schema; > $schema->storage->txn_begin; > my $builder = t::lib::TestBuilder->new; >+my $logger = t::lib::Mocks::Logger->new(); > >-# Mock Variables >-my $matchpoint = 'userid'; >-my $autocreate = 0; >-my $welcome = 0; >-my $sync = 0; >-my %mapping = ( >- 'userid' => { 'is' => 'uid' }, >- 'surname' => { 'is' => 'sn' }, >- 'dateexpiry' => { 'is' => 'exp' }, >- 'categorycode' => { 'is' => 'cat' }, >- 'address' => { 'is' => 'add' }, >- 'city' => { 'is' => 'city' }, >- 'emailpro' => { 'is' => 'emailpro' }, >-); >-$ENV{'uid'} = "test1234"; >-$ENV{'sn'} = undef; >-$ENV{'exp'} = undef; >-$ENV{'cat'} = undef; >-$ENV{'add'} = undef; >-$ENV{'city'} = undef; >-$ENV{'emailpro'} = undef; >- >-# Setup Mocks >-## Mock Context >-my $context = Test::MockModule->new('C4::Context'); >- >-### Mock ->config >-$context->mock( 'config', \&mockedConfig ); >+# Mock variables >+my $shibboleth; >+change_config({}); >+my $interface = 'opac'; > >-### Mock ->preference >-my $OPACBaseURL = "testopac.com"; >-my $staffClientBaseURL = "teststaff.com"; >-$context->mock( 'preference', \&mockedPref ); >+# Mock few preferences >+t::lib::Mocks::mock_preference('OPACBaseURL', 'testopac.com' ); >+t::lib::Mocks::mock_preference('StaffClientBaseURL', 'teststaff.com' ); >+t::lib::Mocks::mock_preference( 'EmailFieldPrimary', 'OFF' ); >+t::lib::Mocks::mock_preference( 'EmailFieldPrecedence', 'emailpro' ); > >-### Mock ->tz >+# Mock Context: config, tz and interface >+my $context = Test::MockModule->new('C4::Context'); >+$context->mock( 'config', sub { return $shibboleth; } ); # easier than caching by Mocks::mock_config > $context->mock( 'timezone', sub { return 'local'; } ); >+$context->mock( 'interface', sub { return $interface; } ); > >-### Mock ->interface >-my $interface = 'opac'; >-$context->mock( 'interface', \&mockedInterface ); >- >-### Mock Letters >+# Mock Letters: GetPreparedLetter, EnqueueLetter and SendQueuedMessages >+# We want to test the params > my $mocked_letters = Test::MockModule->new('C4::Letters'); >-# we want to test the params > $mocked_letters->mock( 'GetPreparedLetter', sub { > warn "GetPreparedLetter called"; > return 1; > }); >-# we don't care about EnqueueLetter for now > $mocked_letters->mock( 'EnqueueLetter', sub { > warn "EnqueueLetter called"; > # return a 'message_id' > return 42; > }); >-# we don't care about EnqueueLetter for now > $mocked_letters->mock( 'SendQueuedMessages', sub { > my $params = shift; > warn "SendQueuedMessages called with message_id: $params->{message_id}"; > return 1; > }); > >-# Tests >-############################################################## >+# Start testing ---------------------------------------------------------------- > >-my $logger = t::lib::Mocks::Logger->new(); >- >-# Can module load >-use C4::Auth_with_shibboleth qw( shib_ok login_shib_url get_login_shib checkpw_shib ); >-require_ok('C4::Auth_with_shibboleth'); >- >-# Subroutine tests >-## shib_ok > subtest "shib_ok tests" => sub { > plan tests => 5; > my $result; >@@ -114,13 +83,13 @@ subtest "shib_ok tests" => sub { > is( shib_ok(), '1', "good config" ); > > # bad config, no debug >- $matchpoint = undef; >+ delete $shibboleth->{matchpoint}; > warnings_are { $result = shib_ok() } > [ { carped => 'shibboleth matchpoint not defined' }, ], > "undefined matchpoint = fatal config, warning given"; > is( $result, '0', "bad config" ); > >- $matchpoint = 'email'; >+ change_config({ matchpoint => 'email' }); > warnings_are { $result = shib_ok() } > [ { carped => 'shibboleth matchpoint not mapped' }, ], > "unmapped matchpoint = fatal config, warning given"; >@@ -128,7 +97,7 @@ subtest "shib_ok tests" => sub { > > # add test for undefined shibboleth block > $logger->clear; >- reset_config(); >+ change_config({}); > }; > > subtest "login_shib_url tests" => sub { >@@ -175,29 +144,19 @@ subtest "login_shib_url tests" => sub { > close $fh_read; > }; > >-## get_login_shib > subtest "get_login_shib tests" => sub { >- > plan tests => 3; > >- my $login; >- >- $login = get_login_shib(); >- >+ my $login = get_login_shib(); > $logger->debug_is("koha borrower field to match: userid", "borrower match field debug info") > ->debug_is("shibboleth attribute to match: uid", "shib match attribute debug info") > ->clear(); >- > is( $login, "test1234", "good config, attribute value returned" ); > }; > >-## checkpw_shib > subtest "checkpw_shib tests" => sub { > plan tests => 33; > >- my $shib_login; >- my ( $retval, $retcard, $retuserid ); >- > # Test borrower data > my $test_borrowers = [ > { cardnumber => 'testcardnumber', userid => 'test1234', surname => 'renvoize', address => 'myaddress', city => 'johnston', email => undef }, >@@ -209,9 +168,8 @@ subtest "checkpw_shib tests" => sub { > my $library = $builder->build_object({ class => 'Koha::Libraries' }); > > # good user >- $shib_login = "test1234"; >- ( $retval, $retcard, $retuserid ) = checkpw_shib($shib_login); >- >+ my $shib_login = "test1234"; >+ my ( $retval, $retcard, $retuserid ) = checkpw_shib($shib_login); > is( $logger->count(), 2, "Two debugging entries"); > is( $retval, "1", "user authenticated" ); > is( $retcard, "testcardnumber", "expected cardnumber returned" ); >@@ -229,8 +187,7 @@ subtest "checkpw_shib tests" => sub { > ->clear(); > > # duplicated matchpoint >- $matchpoint = 'email'; >- $mapping{'email'} = { is => 'email' }; >+ change_config({ matchpoint => 'email', mapping => { email => { is => 'email' }} }); > $shib_login = 'kid@clamp.io'; > ( $retval, $retcard, $retuserid ) = checkpw_shib($shib_login); > is( $retval, "0", "user not authenticated if duplicated matchpoint" ); >@@ -244,11 +201,8 @@ subtest "checkpw_shib tests" => sub { > ->warn_is('There are several users with email of kid@clamp.io, matchpoints must be unique', "duplicated matchpoint warned with debug") > ->clear(); > >- reset_config(); >- > # autocreate user (welcome) >- $autocreate = 1; >- $welcome = 1; >+ change_config({ autocreate => 1, welcome => 1 }); > $shib_login = 'test4321'; > $ENV{'uid'} = 'test4321'; > $ENV{'sn'} = "pika"; >@@ -280,11 +234,9 @@ subtest "checkpw_shib tests" => sub { > is_deeply( [ map { $rec->$_ } qw/surname dateexpiry address city/ ], > [qw/pika 2017-01-01 Address City/], > 'Found $new_user surname' ); >- $autocreate = 0; >- $welcome = 0; > > # sync user >- $sync = 1; >+ $shibboleth->{sync} = 1; > $ENV{'city'} = 'AnotherCity'; > ( $retval, $retcard, $retuserid ) = checkpw_shib($shib_login); > $logger->debug_is("koha borrower field to match: userid", "borrower match field debug info") >@@ -298,7 +250,7 @@ subtest "checkpw_shib tests" => sub { > is_deeply( [ map { $rec->$_ } qw/surname dateexpiry address city/ ], > [qw/pika 2017-01-01 Address AnotherCity/], > 'Found $sync_user synced city' ); >- $sync = 0; >+ change_config({ sync => 0 }); > > # good user > $shib_login = "test1234"; >@@ -317,109 +269,67 @@ subtest "checkpw_shib tests" => sub { > $logger->info_is("There are several users with userid of martin, matchpoints must be unique", "Duplicated matchpoint warned to info"); > }; > >-## _get_uri - opac >-$OPACBaseURL = "testopac.com"; >+subtest 'get_uri' => sub { >+plan tests => 13; >+# Tests for OPAC >+t::lib::Mocks::mock_preference('OPACBaseURL', 'testopac.com' ); > is( C4::Auth_with_shibboleth::_get_uri(), > "https://testopac.com", "https opac uri returned" ); > > $logger->clear; > >-$OPACBaseURL = "http://testopac.com"; >+t::lib::Mocks::mock_preference('OPACBaseURL', 'http://testopac.com' ); > my $result = C4::Auth_with_shibboleth::_get_uri(); > is( $result, "https://testopac.com", "https opac uri returned" ); > $logger->warn_is("Shibboleth requires OPACBaseURL/staffClientBaseURL to use the https protocol!", "Improper protocol logged to warn") > ->clear(); > >-$OPACBaseURL = "https://testopac.com"; >+t::lib::Mocks::mock_preference('OPACBaseURL', 'https://testopac.com' ); > is( C4::Auth_with_shibboleth::_get_uri(), > "https://testopac.com", "https opac uri returned" ); > > $logger->clear(); > >-$OPACBaseURL = undef; >+t::lib::Mocks::mock_preference('OPACBaseURL', undef ); > $result = C4::Auth_with_shibboleth::_get_uri(); > is( $result, "https://", "https $interface uri returned" ); > > $logger->warn_is("Syspref staffClientBaseURL or OPACBaseURL not set!", "undefined OPACBaseURL - received expected warning") > ->clear(); > >-## _get_uri - intranet >+# Tests for staff client > $interface = 'intranet'; >-$staffClientBaseURL = "teststaff.com"; >+t::lib::Mocks::mock_preference('StaffClientBaseURL', 'teststaff.com' ); > is( C4::Auth_with_shibboleth::_get_uri(), > "https://teststaff.com", "https $interface uri returned" ); > >- > $logger->clear; > >-$staffClientBaseURL = "http://teststaff.com"; >+t::lib::Mocks::mock_preference('StaffClientBaseURL', 'http://teststaff.com' ); > $result = C4::Auth_with_shibboleth::_get_uri(); > is( $result, "https://teststaff.com", "https $interface uri returned" ); >-$logger->warn_is("Shibboleth requires OPACBaseURL/staffClientBaseURL to use the https protocol!") >+$logger->warn_is("Shibboleth requires OPACBaseURL/staffClientBaseURL to use the https protocol!", 'check protocol warn') > ->clear; > >-$staffClientBaseURL = "https://teststaff.com"; >+t::lib::Mocks::mock_preference('StaffClientBaseURL', 'https://teststaff.com' ); > is( C4::Auth_with_shibboleth::_get_uri(), > "https://teststaff.com", "https $interface uri returned" ); > is( $logger->count(), 0, 'No logging' ); > >-$staffClientBaseURL = undef; >+t::lib::Mocks::mock_preference('StaffClientBaseURL', undef ); > $result = C4::Auth_with_shibboleth::_get_uri(); > is( $result, "https://", "https $interface uri returned" ); > $logger->warn_is("Syspref staffClientBaseURL or OPACBaseURL not set!", "undefined staffClientBaseURL - received expected warning") > ->clear; >+}; >+$schema->storage->txn_rollback; > >-## _get_shib_config >-# Internal helper function, covered in tests above >- >-sub mockedConfig { >- my $param = shift; >- >- my %shibboleth = ( >- 'autocreate' => $autocreate, >- 'welcome' => $welcome, >- 'sync' => $sync, >- 'matchpoint' => $matchpoint, >- 'mapping' => \%mapping >- ); >- >- return \%shibboleth; >-} >- >-sub mockedPref { >- my $param = $_[1]; >- my $return; >- >- if ( $param eq 'OPACBaseURL' ) { >- $return = $OPACBaseURL; >- } >- >- if ( $param eq 'staffClientBaseURL' ) { >- $return = $staffClientBaseURL; >- } >- >- if ( $param eq 'EmailFieldPrimary' ) { >- $return = 'OFF'; >- } >- >- if ( $param eq 'EmailFieldPrecedence' ) { >- $return = 'emailpro'; >- } >- >- return $return; >-} >+# Internal helper function > >-sub mockedInterface { >- return $interface; >-} >+sub change_config { >+ my $params = shift; > >-## Convenience method to reset config >-sub reset_config { >- $matchpoint = 'userid'; >- $autocreate = 0; >- $welcome = 0; >- $sync = 0; >- %mapping = ( >+ my %mapping = ( > 'userid' => { 'is' => 'uid' }, > 'surname' => { 'is' => 'sn' }, > 'dateexpiry' => { 'is' => 'exp' }, >@@ -429,6 +339,18 @@ sub reset_config { > 'emailpro' => { 'is' => 'emailpro' }, > 'branchcode' => { 'is' => 'branchcode' }, > ); >+ if( exists $params->{mapping} ) { >+ $mapping{$_} = $params->{mapping}->{$_} for keys %{$params->{mapping}}; >+ } >+ $shibboleth = { >+ autocreate => $params->{autocreate} // 0, >+ welcome => $params->{welcome} // 0, >+ sync => $params->{sync} // 0, >+ matchpoint => $params->{matchpoint} // 'userid', >+ mapping => \%mapping, >+ }; >+ >+ # Change environment too > $ENV{'uid'} = "test1234"; > $ENV{'sn'} = undef; > $ENV{'exp'} = undef; >@@ -436,6 +358,4 @@ sub reset_config { > $ENV{'add'} = undef; > $ENV{'city'} = undef; > $ENV{'emailpro'} = undef; >- >- return 1; > } >-- >2.41.0
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 33778
:
151454
|
151455
|
151456
|
151457
|
151494
|
151495
|
151496
|
151497
|
152672
| 152673 |
152674
|
152675
|
152676