Bugzilla – Attachment 51745 Details for
Bug 16492
Checkouts ( and possibly checkins and other actions ) will use the patron home branch as the logged in library
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 16492 - Checkouts ( and possibly checkins and other actions ) will use the patron home branch as the logged in library
Bug-16492---Checkouts--and-possibly-checkins-and-o.patch (text/plain), 5.27 KB, created by
Kyle M Hall (khall)
on 2016-05-24 18:31:50 UTC
(
hide
)
Description:
Bug 16492 - Checkouts ( and possibly checkins and other actions ) will use the patron home branch as the logged in library
Filename:
MIME Type:
Creator:
Kyle M Hall (khall)
Created:
2016-05-24 18:31:50 UTC
Size:
5.27 KB
patch
obsolete
>From 6a516cede942dcc1462faeffdf62883fd735274d Mon Sep 17 00:00:00 2001 >From: Kyle M Hall <kyle@bywatersolutions.com> >Date: Tue, 10 May 2016 16:36:03 +0000 >Subject: [PATCH] Bug 16492 - Checkouts ( and possibly checkins and other > actions ) will use the patron home branch as the logged in library > >Bug 14507 introduced the use of checkpw in C4::SIP::ILS::Patron so that >non-Koha internal authentication processes would be able to function via >SIP ( LDAP et al ). > >The problem is that checkpw changes the userenv to that of the patron! >This is not usually an issue in Koha because most of the time that >patron running through checkpw is the one to be logged in. > >Aside from SIP2 the only other area where this may be an issue is in SCO >when using SelfCheckoutByLogin. > >Test Plan: >1) On master, check out an item to a patron via SIP2 >2) Note the checkout lists the item as having been checked out > from the patron's home library not matter which library is was > supposed to be checked out from. >3) Apply this patch >4) Re-checkout the item >5) The item should now be checked out as if it was checked out from > the library as defined in the SIP configuration file. >--- > C4/Auth.pm | 10 +++++----- > C4/SIP/ILS/Patron.pm | 2 +- > t/db_dependent/Auth.t | 34 +++++++++++++++++++++++++--------- > 3 files changed, 31 insertions(+), 15 deletions(-) > >diff --git a/C4/Auth.pm b/C4/Auth.pm >index c06fc1a..a5cf72c 100644 >--- a/C4/Auth.pm >+++ b/C4/Auth.pm >@@ -1738,7 +1738,7 @@ sub get_session { > } > > sub checkpw { >- my ( $dbh, $userid, $password, $query, $type ) = @_; >+ my ( $dbh, $userid, $password, $query, $type, $no_set_userenv ) = @_; > $type = 'opac' unless $type; > if ($ldap) { > $debug and print STDERR "## checkpw - checking LDAP\n"; >@@ -1778,11 +1778,11 @@ sub checkpw { > } > > # INTERNAL AUTH >- return checkpw_internal(@_) >+ return checkpw_internal( $dbh, $userid, $password, $no_set_userenv); > } > > sub checkpw_internal { >- my ( $dbh, $userid, $password ) = @_; >+ my ( $dbh, $userid, $password, $no_set_userenv ) = @_; > > $password = Encode::encode( 'UTF-8', $password ) > if Encode::is_utf8($password); >@@ -1812,7 +1812,7 @@ sub checkpw_internal { > if ( checkpw_hash( $password, $stored_hash ) ) { > > C4::Context->set_userenv( "$borrowernumber", $userid, $cardnumber, >- $firstname, $surname, $branchcode, $branchname, $flags ); >+ $firstname, $surname, $branchcode, $branchname, $flags ) unless $no_set_userenv; > return 1, $cardnumber, $userid; > } > } >@@ -1829,7 +1829,7 @@ sub checkpw_internal { > if ( checkpw_hash( $password, $stored_hash ) ) { > > C4::Context->set_userenv( $borrowernumber, $userid, $cardnumber, >- $firstname, $surname, $branchcode, $branchname, $flags ); >+ $firstname, $surname, $branchcode, $branchname, $flags ) unless $no_set_userenv; > return 1, $cardnumber, $userid; > } > } >diff --git a/C4/SIP/ILS/Patron.pm b/C4/SIP/ILS/Patron.pm >index d61b097..63a0a3a 100644 >--- a/C4/SIP/ILS/Patron.pm >+++ b/C4/SIP/ILS/Patron.pm >@@ -201,7 +201,7 @@ sub check_password { > > my $dbh = C4::Context->dbh; > my $ret = 0; >- ($ret) = checkpw( $dbh, $self->{userid}, $pwd ); >+ ($ret) = checkpw( $dbh, $self->{userid}, $pwd, undef, undef, 1 ); # dbh, userid, query, type, no_set_userenv > return $ret; > } > >diff --git a/t/db_dependent/Auth.t b/t/db_dependent/Auth.t >index 3077969..3499c79 100644 >--- a/t/db_dependent/Auth.t >+++ b/t/db_dependent/Auth.t >@@ -8,22 +8,24 @@ use Modern::Perl; > use CGI qw ( -utf8 ); > use Test::MockModule; > use List::MoreUtils qw/all any none/; >-use Test::More tests => 13; >+use Test::More tests => 18; > use Test::Warn; > use t::lib::Mocks; >+use t::lib::TestBuilder; >+ >+use C4::Auth qw(checkpw); > use C4::Members; > use Koha::AuthUtils qw/hash_password/; >+use Koha::Database; > > BEGIN { >- use_ok('C4::Auth'); >+ use_ok('C4::Auth'); > } > >-my $dbh = C4::Context->dbh; >- >-# Start transaction >-$dbh->{AutoCommit} = 0; >-$dbh->{RaiseError} = 1; >- >+my $schema = Koha::Database->schema; >+$schema->storage->txn_begin; >+my $builder = t::lib::TestBuilder->new; >+my $dbh = C4::Context->dbh; > > # get_template_and_user tests > >@@ -176,4 +178,18 @@ my $hash2 = hash_password('password'); > ok(C4::Auth::checkpw_hash('password', $hash1), 'password validates with first hash'); > ok(C4::Auth::checkpw_hash('password', $hash2), 'password validates with second hash'); > >-$dbh->rollback; >+my $patron = $builder->build( { source => 'Borrower' } ); >+changepassword( $patron->{userid}, $patron->{borrowernumber}, $hash1 ); >+my $library = $builder->build( >+ { >+ source => 'Branch', >+ } >+); >+ >+C4::Context->set_userenv(0,0,0,'firstname','surname', $library->{branchcode}, 'Library 1', 0, '', ''); >+is( C4::Context->userenv->{branch}, $library->{branchcode}, 'Userenv gives correct branch' ); >+ok( checkpw( $dbh, $patron->{userid}, 'password', undef, undef, 1 ), 'checkpw returns true' ); >+is( C4::Context->userenv->{branch}, $library->{branchcode}, 'Userenv branch is preserved if no_set_userenv is true' ); >+ok( checkpw( $dbh, $patron->{userid}, 'password', undef, undef, 0 ), 'checkpw still returns true' ); >+isnt( C4::Context->userenv->{branch}, $library->{branchcode}, 'Userenv branch is overwritten if no_set_userenv is false' ); >+ >-- >2.1.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 16492
:
51390
|
51391
|
51744
|
51745
|
51768
|
51797
|
52188
|
52189
|
52190