Bugzilla – Attachment 32373 Details for
Bug 8446
Shibboleth authentication
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
BUG8446, QA Followup: Use DBIx::Class
BUG8446-QA-Followup-Use-DBIxClass.patch (text/plain), 7.19 KB, created by
Martin Renvoize (ashimema)
on 2014-10-15 16:23:36 UTC
(
hide
)
Description:
BUG8446, QA Followup: Use DBIx::Class
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2014-10-15 16:23:36 UTC
Size:
7.19 KB
patch
obsolete
>From 8242e289b1d99c6b12c46414f208278a0f6e4c6c Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@ptfs-europe.com> >Date: Mon, 29 Sep 2014 07:51:42 +0000 >Subject: [PATCH] BUG8446, QA Followup: Use DBIx::Class > >- Convert Auth_with_shibboleth to use dbic stanzas. > >Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> >--- > C4/Auth.pm | 2 +- > C4/Auth_with_shibboleth.pm | 22 ++++++--------- > t/Auth_with_shibboleth.t | 65 ++++++++++++++++++++++---------------------- > 3 files changed, 43 insertions(+), 46 deletions(-) > >diff --git a/C4/Auth.pm b/C4/Auth.pm >index 684faae..7dfe3d2 100644 >--- a/C4/Auth.pm >+++ b/C4/Auth.pm >@@ -1639,7 +1639,7 @@ sub checkpw { > > # Then, we check if it matches a valid koha user > if ($shib_login) { >- my ( $retval, $retcard, $retuserid ) = C4::Auth_with_shibboleth::checkpw_shib( $dbh, $shib_login ); # EXTERNAL AUTH >+ my ( $retval, $retcard, $retuserid ) = C4::Auth_with_shibboleth::checkpw_shib( $shib_login ); # EXTERNAL AUTH > ($retval) and return ( $retval, $retcard, $retuserid ); > return 0; > } >diff --git a/C4/Auth_with_shibboleth.pm b/C4/Auth_with_shibboleth.pm >index d2a4cdf..f9555fb 100644 >--- a/C4/Auth_with_shibboleth.pm >+++ b/C4/Auth_with_shibboleth.pm >@@ -21,6 +21,7 @@ use Modern::Perl; > > use C4::Debug; > use C4::Context; >+use Koha::Database; > use Carp; > use CGI; > >@@ -89,21 +90,16 @@ sub get_login_shib { > sub checkpw_shib { > $debug and warn "checkpw_shib"; > >- my ( $dbh, $match ) = @_; >- my ( $retnumber, $userid ); >+ my ( $match ) = @_; > my $config = _get_shib_config(); > $debug and warn "User Shibboleth-authenticated as: $match"; > >- # Does the given shibboleth attribute value ($match) match a valid koha user ? >- my $sth = $dbh->prepare( >- "select cardnumber, userid from borrowers where $config->{matchpoint}=?" >- ); >- $sth->execute($match); >- if ( $sth->rows ) { >- my @retvals = $sth->fetchrow; >- $retnumber = $retvals[0]; >- $userid = $retvals[1]; >- return ( 1, $retnumber, $userid ); >+ # Does the given shibboleth attribute value ($match) match a valid koha user ? >+ my $borrower = >+ Koha::Database->new()->schema()->resultset('Borrower') >+ ->find( { $config->{matchpoint} => $match } ); >+ if ( defined($borrower) ) { >+ return ( 1, $borrower->get_column('cardnumber'), $borrower->get_column('userid') ); > } > > # If we reach this point, the user is not a valid koha user >@@ -265,7 +261,7 @@ Returns the shibboleth login attribute should it be found present in the http se > > Given a database handle and a shib_login attribute, this routine checks for a matching local user and if found returns true, their cardnumber and their userid. If a match is not found, then this returns false. > >- my ( $retval, $retcard, $retuserid ) = C4::Auth_with_shibboleth::checkpw_shib( $dbh, $shib_login ); >+ my ( $retval, $retcard, $retuserid ) = C4::Auth_with_shibboleth::checkpw_shib( $shib_login ); > > =head1 SEE ALSO > >diff --git a/t/Auth_with_shibboleth.t b/t/Auth_with_shibboleth.t >index bd627e2..0a39b9b 100644 >--- a/t/Auth_with_shibboleth.t >+++ b/t/Auth_with_shibboleth.t >@@ -7,7 +7,7 @@ use Modern::Perl; > use Test::More tests => 6; > use Test::MockModule; > use Test::Warn; >-use DBD::Mock; >+use Test::DBIx::Class {schema_class => 'Koha::Schema', connect_info => ['dbi:SQLite:dbname=:memory:','','']}; > > use CGI; > use C4::Context; >@@ -17,25 +17,10 @@ my $matchpoint = 'userid'; > my %mapping = ( 'userid' => { 'is' => 'uid' }, ); > $ENV{'uid'} = "test1234"; > >-#my %shibboleth = ( >-# 'matchpoint' => $matchpoint, >-# 'mapping' => \%mapping >-#); >- > # Setup Mocks > ## Mock Context > my $context = new Test::MockModule('C4::Context'); > >-### Mock ->dbh >-$context->mock( >- '_new_dbh', >- sub { >- my $dbh = DBI->connect( 'DBI:Mock:', '', '' ) >- || die "Cannot create handle: $DBI::errstr\n"; >- return $dbh; >- } >-); >- > ### Mock ->config > $context->mock( 'config', \&mockedConfig ); > >@@ -64,8 +49,17 @@ sub mockedPref { > return $return; > } > >-# Convenience methods >-## Reset Context >+## Mock Database >+my $database = new Test::MockModule('Koha::Database'); >+ >+### Mock ->schema >+$database->mock( 'schema', \&mockedSchema ); >+ >+sub mockedSchema { >+ return Schema(); >+} >+ >+## Convenience method to reset config > sub reset_config { > $matchpoint = 'userid'; > %mapping = ( 'userid' => { 'is' => 'uid' }, ); >@@ -75,7 +69,7 @@ sub reset_config { > } > > # Tests >-my $dbh = C4::Context->dbh(); >+############################################################## > > # Can module load > use_ok('C4::Auth_with_shibboleth'); >@@ -155,21 +149,27 @@ subtest "get_login_shib tests" => sub { > > ## checkpw_shib > subtest "checkpw_shib tests" => sub { >- plan tests => 12; >- >- my $shib_login = 'test1234'; >- my @borrower_results = >- ( [ 'cardnumber', 'userid' ], [ 'testcardnumber', 'test1234' ], ); >- $dbh->{mock_add_resultset} = \@borrower_results; >+ plan tests => 13; > >+ my $shib_login; > my ( $retval, $retcard, $retuserid ); > >+ # Setup Mock Database Data >+ fixtures_ok [ >+ 'Borrower' => [ >+ [qw/cardnumber userid surname address city/], >+ [qw/testcardnumber test1234 renvoize myaddress johnston/], >+ ], >+ ], >+ 'Installed some custom fixtures via the Populate fixture class'; >+ > # debug off > $C4::Auth_with_shibboleth::debug = '0'; > > # good user >+ $shib_login = "test1234"; > warnings_are { >- ( $retval, $retcard, $retuserid ) = checkpw_shib( $dbh, $shib_login ); >+ ( $retval, $retcard, $retuserid ) = checkpw_shib( $shib_login ); > } > [], "good user with no debug"; > is( $retval, "1", "user authenticated" ); >@@ -177,21 +177,20 @@ subtest "checkpw_shib tests" => sub { > is( $retuserid, "test1234", "expected userid returned" ); > > # bad user >+ $shib_login = 'martin'; > warnings_are { >- ( $retval, $retcard, $retuserid ) = checkpw_shib( $dbh, $shib_login ); >+ ( $retval, $retcard, $retuserid ) = checkpw_shib( $shib_login ); > } > [], "bad user with no debug"; > is( $retval, "0", "user not authenticated" ); > >- # reset db mock >- $dbh->{mock_add_resultset} = \@borrower_results; >- > # debug on > $C4::Auth_with_shibboleth::debug = '1'; > > # good user >+ $shib_login = "test1234"; > warnings_exist { >- ( $retval, $retcard, $retuserid ) = checkpw_shib( $dbh, $shib_login ); >+ ( $retval, $retcard, $retuserid ) = checkpw_shib( $shib_login ); > } > [ qr/checkpw_shib/, qr/User Shibboleth-authenticated as:/ ], > "good user with debug enabled"; >@@ -200,8 +199,9 @@ subtest "checkpw_shib tests" => sub { > is( $retuserid, "test1234", "expected userid returned" ); > > # bad user >+ $shib_login = "martin"; > warnings_exist { >- ( $retval, $retcard, $retuserid ) = checkpw_shib( $dbh, $shib_login ); >+ ( $retval, $retcard, $retuserid ) = checkpw_shib( $shib_login ); > } > [ > qr/checkpw_shib/, >@@ -218,3 +218,4 @@ is( C4::Auth_with_shibboleth::_get_uri(), > "https://testopac.com", "https opac uri returned" ); > > ## _get_shib_config >+# Internal helper function, covered in tests above >-- >1.7.10.4
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 8446
:
10841
|
22098
|
22107
|
22860
|
23224
|
23225
|
23226
|
25765
|
25766
|
26176
|
26177
|
26178
|
26187
|
26749
|
26750
|
26751
|
26752
|
26760
|
26761
|
26762
|
28892
|
28893
|
28894
|
28895
|
28896
|
28897
|
28898
|
28899
|
28900
|
28901
|
28902
|
28903
|
30017
|
30018
|
30019
|
30020
|
30021
|
30022
|
30023
|
30030
|
30031
|
30033
|
30283
|
30284
|
30285
|
30286
|
30335
|
30353
|
30354
|
30355
|
30356
|
30384
|
30594
|
30604
|
30605
|
30616
|
30617
|
30618
|
30619
|
30620
|
30621
|
30684
|
31407
|
31408
|
31836
|
31905
|
31906
|
31907
|
32125
|
32158
|
32162
|
32163
|
32164
|
32165
|
32166
|
32167
|
32168
|
32169
|
32170
|
32365
|
32366
|
32367
|
32368
|
32369
|
32370
|
32371
|
32372
| 32373 |
32374
|
32426
|
32427