From 90307bcbfaf4c2c8907be94eabca249ac173442e 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.

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
---
 C4/Auth.pm            | 10 +++++-----
 C4/SIP/ILS/Patron.pm  |  2 +-
 t/db_dependent/Auth.t | 33 ++++++++++++++++++++++++---------
 3 files changed, 30 insertions(+), 15 deletions(-)

diff --git a/C4/Auth.pm b/C4/Auth.pm
index c06fc1aea6..a5cf72ccfb 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 9eb249c606..93b6c546c4 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 307796953a..b5e1ef1f72 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,17 @@ 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.39.5