From d68fe07bf806e3676fc637dddf0e63dee41055bb Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Wed, 28 Mar 2018 14:44:21 -0300 Subject: [PATCH] Bug 20489: Prevent DB user login Signed-off-by: Mark Tompsett Signed-off-by: Marcel de Rooy Signed-off-by: Jonathan Druart --- C4/Auth.pm | 47 +---------------------------------------- members/member-flags.pl | 7 ++---- members/memberentry.pl | 6 ++---- members/moremember.pl | 8 +++---- t/db_dependent/Auth.t | 2 +- 5 files changed, 9 insertions(+), 61 deletions(-) diff --git a/C4/Auth.pm b/C4/Auth.pm index 13c4652d2e..00dedcb22a 100644 --- a/C4/Auth.pm +++ b/C4/Auth.pm @@ -1048,10 +1048,8 @@ sub checkauth { } } - # $return: 1 = valid user, 2 = superlibrarian + # $return: 1 = valid user if ($return) { - # If DB user is logged in - $userid ||= $q_userid if $return == 2; #_session_log(sprintf "%20s from %16s logged in at %30s.\n", $userid,$ENV{'REMOTE_ADDR'},(strftime '%c', localtime)); if ( $flags = haspermission( $userid, $flagsrequired ) ) { @@ -1151,22 +1149,6 @@ sub checkauth { $session->param( 'shibboleth', $shibSuccess ); $debug and printf STDERR "AUTH_4: (%s)\t%s %s - %s\n", map { $session->param($_) } qw(cardnumber firstname surname branch); } - elsif ( $return == 2 ) { - - #We suppose the user is the superlibrarian - $borrowernumber = 0; - $session->param( 'number', 0 ); - $session->param( 'id', C4::Context->config('user') ); - $session->param( 'cardnumber', C4::Context->config('user') ); - $session->param( 'firstname', C4::Context->config('user') ); - $session->param( 'surname', C4::Context->config('user') ); - $session->param( 'branch', 'NO_LIBRARY_SET' ); - $session->param( 'branchname', 'NO_LIBRARY_SET' ); - $session->param( 'flags', 1 ); - $session->param( 'emailaddress', C4::Context->preference('KohaAdminEmailAddress') ); - $session->param( 'ip', $session->remote_addr() ); - $session->param( 'lasttime', time() ); - } $session->param('cas_ticket', $cas_ticket) if $cas_ticket; C4::Context->set_userenv( $session->param('number'), $session->param('id'), @@ -1598,20 +1580,6 @@ sub check_api_auth { $session->param( 'emailaddress', $emailaddress ); $session->param( 'ip', $session->remote_addr() ); $session->param( 'lasttime', time() ); - } elsif ( $return == 2 ) { - - #We suppose the user is the superlibrarian - $session->param( 'number', 0 ); - $session->param( 'id', C4::Context->config('user') ); - $session->param( 'cardnumber', C4::Context->config('user') ); - $session->param( 'firstname', C4::Context->config('user') ); - $session->param( 'surname', C4::Context->config('user') ); - $session->param( 'branch', 'NO_LIBRARY_SET' ); - $session->param( 'branchname', 'NO_LIBRARY_SET' ); - $session->param( 'flags', 1 ); - $session->param( 'emailaddress', C4::Context->preference('KohaAdminEmailAddress') ); - $session->param( 'ip', $session->remote_addr() ); - $session->param( 'lasttime', time() ); } $session->param( 'cas_ticket', $cas_ticket); C4::Context->set_userenv( @@ -1805,7 +1773,6 @@ sub checkpw { # 1 if auth is ok # 0 if auth is nok # -1 if user bind failed (LDAP only) - # 2 if DB user is used (internal only) if ( $patron and $patron->account_locked ) { # Nothing to check, account is locked @@ -1878,18 +1845,6 @@ sub checkpw_internal { $password = Encode::encode( 'UTF-8', $password ) if Encode::is_utf8($password); - if ( $userid && $userid eq C4::Context->config('user') ) { - if ( $password && $password eq C4::Context->config('pass') ) { - - # Koha superuser account - # C4::Context->set_userenv(0,0,C4::Context->config('user'),C4::Context->config('user'),C4::Context->config('user'),"",1); - return 2; - } - else { - return 0; - } - } - my $sth = $dbh->prepare( "select password,cardnumber,borrowernumber,userid,firstname,surname,borrowers.branchcode,branches.branchname,flags from borrowers join branches on borrowers.branchcode=branches.branchcode where userid=?" diff --git a/members/member-flags.pl b/members/member-flags.pl index 6f9780e161..14fb50a183 100755 --- a/members/member-flags.pl +++ b/members/member-flags.pl @@ -44,11 +44,8 @@ my ($template, $loggedinuser, $cookie) = get_template_and_user({ debug => 1, }); -my $userenv = C4::Context->userenv; -if ( $userenv and $userenv->{number} ) { # Allow DB user to create a superlibrarian patron - my $logged_in_user = Koha::Patrons->find( $loggedinuser ) or die "Not logged in"; - output_and_exit_if_error( $input, $cookie, $template, { module => 'members', logged_in_user => $logged_in_user, current_patron => $patron } ); -} +my $logged_in_user = Koha::Patrons->find( $loggedinuser ) or die "Not logged in"; +output_and_exit_if_error( $input, $cookie, $template, { module => 'members', logged_in_user => $logged_in_user, current_patron => $patron } ); my %member2; $member2{'borrowernumber'}=$member; diff --git a/members/memberentry.pl b/members/memberentry.pl index a1703d0f40..0a5e81b3de 100755 --- a/members/memberentry.pl +++ b/members/memberentry.pl @@ -151,10 +151,8 @@ $template->param( "quickadd" => 1 ) if ( $quickadd ); $template->param( "duplicate" => 1 ) if ( $op eq 'duplicate' ); $template->param( "checked" => 1 ) if ( defined($nodouble) && $nodouble eq 1 ); if ( $op eq 'modify' or $op eq 'save' or $op eq 'duplicate' ) { - if ( $patron and $userenv and $userenv->{number} ) { # Allow DB user to create a superlibrarian patron - my $logged_in_user = Koha::Patrons->find( $loggedinuser ) or die "Not logged in"; - output_and_exit_if_error( $input, $cookie, $template, { module => 'members', logged_in_user => $logged_in_user, current_patron => $patron } ); - } + my $logged_in_user = Koha::Patrons->find( $loggedinuser ) or die "Not logged in"; + output_and_exit_if_error( $input, $cookie, $template, { module => 'members', logged_in_user => $logged_in_user, current_patron => $patron } ); $borrower_data = $patron->unblessed; $borrower_data->{category_type} = $patron->category->category_type; diff --git a/members/moremember.pl b/members/moremember.pl index f898adb5f6..c5d13e85ac 100755 --- a/members/moremember.pl +++ b/members/moremember.pl @@ -122,11 +122,8 @@ my $error = $input->param('error'); $template->param( error => $error ) if ( $error ); my $patron = Koha::Patrons->find( $borrowernumber ); -my $userenv = C4::Context->userenv; -if ( $userenv and $userenv->{number} ) { # Allow DB user to create a superlibrarian patron - my $logged_in_user = Koha::Patrons->find( $loggedinuser ) or die "Not logged in"; - output_and_exit_if_error( $input, $cookie, $template, { module => 'members', logged_in_user => $logged_in_user, current_patron => $patron } ); -} +my $logged_in_user = Koha::Patrons->find( $loggedinuser ) or die "Not logged in"; +output_and_exit_if_error( $input, $cookie, $template, { module => 'members', logged_in_user => $logged_in_user, current_patron => $patron } ); my $issues = $patron->checkouts; my $balance = $patron->account->balance; @@ -206,6 +203,7 @@ if ( C4::Context->preference("IndependentBranches") ) { $samebranch = 1; } else { + my $userenv = C4::Context->userenv; $samebranch = ( $data->{'branchcode'} eq $userenv->{branch} ); } } diff --git a/t/db_dependent/Auth.t b/t/db_dependent/Auth.t index f1d1ddd702..c08060e93c 100644 --- a/t/db_dependent/Auth.t +++ b/t/db_dependent/Auth.t @@ -62,7 +62,7 @@ subtest 'checkauth() tests' => sub { else { return; } }); ( $userid, $cookie, $sessionID, $flags ) = C4::Auth::checkauth( $cgi, $authnotrequired ); - is ( $userid, $db_user_id, 'If DB user is logging in, it should be considered as logged in, i.e. checkauth return the relevant userid' ); + is ( $userid, undef, 'If DB user is used, it should not be logged in' ); C4::Context->_new_userenv; # For next tests }; -- 2.39.5