Bug 18275: Do not rely on CGI param userid to log a user in if auth is not required
authorJonathan Druart <jonathan.druart@bugs.koha-community.org>
Wed, 15 Mar 2017 17:52:49 +0000 (14:52 -0300)
committerKyle M Hall <kyle@bywatersolutions.com>
Mon, 20 Mar 2017 13:00:02 +0000 (13:00 +0000)
From opac/opac-memberentry.pl, authnotrequired is set.
That means a patron can access the page without being logged in. It is
used on this page for the self registration feature.

From C4::Auth::get_template_and_user, we have
  $userid = $q_userid;
$q_userid is previously set to the 'userid' CGI param.

We end up here if authonotrequired is set AND CGISESSID does not exist.

Test plan:
- Run:
  $ prove t/db_dependent/Auth.t
=> FAIL: Regression test for checkauth fails
- Apply this patch
- Run:
  $ prove t/db_dependent/Auth.t
=> SUCCESS: Tests pass.
- Sign off :-D

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

C4/Auth.pm

index 20d64a5..5dcf1cd 100644 (file)
@@ -914,14 +914,13 @@ sub checkauth {
             -value    => $session->id,
             -HttpOnly => 1
         );
-        $userid = $q_userid;
         my $pki_field = C4::Context->preference('AllowPKIAuth');
         if ( !defined($pki_field) ) {
             print STDERR "ERROR: Missing system preference AllowPKIAuth.\n";
             $pki_field = 'None';
         }
         if ( ( $cas && $query->param('ticket') )
-            || $userid
+            || $q_userid
             || ( $shib && $shib_login )
             || $pki_field ne 'None' )
         {
@@ -935,7 +934,7 @@ sub checkauth {
                 my $retuserid;
 
                 # Do not pass password here, else shib will not be checked in checkpw.
-                ( $return, $cardnumber, $retuserid ) = checkpw( $dbh, $userid, undef, $query );
+                ( $return, $cardnumber, $retuserid ) = checkpw( $dbh, $q_userid, undef, $query );
                 $userid      = $retuserid;
                 $shibSuccess = $return;
                 $info{'invalidShibLogin'} = 1 unless ($return);
@@ -986,7 +985,7 @@ sub checkauth {
                 else {
                     my $retuserid;
                     ( $return, $cardnumber, $retuserid ) =
-                      checkpw( $dbh, $userid, $password, $query, $type );
+                      checkpw( $dbh, $q_userid, $password, $query, $type );
                     $userid = $retuserid if ($retuserid);
                     $info{'invalid_username_or_password'} = 1 unless ($return);
                 }
@@ -1131,7 +1130,7 @@ sub checkauth {
                 $session->param( 'ip',       $session->remote_addr() );
                 $session->param( 'sessiontype', 'anon' );
             }
-        }    # END if ( $userid    = $query->param('userid') )
+        }    # END if ( $q_userid
         elsif ( $type eq "opac" ) {
 
             # if we are here this is an anonymous session; add public lists to it and a few other items...