[Koha-patches] [PATCH] Bug 8515 - OPAC password change does not obey OpacPasswordChange

[Owen Leonard]

The OPAC change password template enforces the OpacPasswordChange
preference by preventing the form from appearing. However, the
script doesn't contain any check for OpacPasswordChange so it is
vulnerable to someone submitting data to it by some other means.

This patch adds a check for OpacPasswordChange to the script and
revises the template logic in order to show the right warning
in all circumstances.

To test, turn off OpacPasswordChange and navigate manually to
opac-passwd.pl. You should see a warning that you can't change
your password.

Turn on OpacPasswordChange load the change password page and
save the page to your desktop. Turn off OpacPasswordChange and
submit a password change via the saved page. Without the patch
this would result in a password change. After the patch it
should not.
---
 koha-tmpl/opac-tmpl/prog/en/modules/opac-passwd.tt |   10 +--
 opac/opac-passwd.pl                                |   81 ++++++++++----------
 2 files changed, 46 insertions(+), 45 deletions(-)

diff --git a/koha-tmpl/opac-tmpl/prog/en/modules/opac-passwd.tt b/koha-tmpl/opac-tmpl/prog/en/modules/opac-passwd.tt
index 907835f..d10ab7b 100644
--- a/koha-tmpl/opac-tmpl/prog/en/modules/opac-passwd.tt
+++ b/koha-tmpl/opac-tmpl/prog/en/modules/opac-passwd.tt
@@ -26,18 +26,18 @@
         </p></div>
     [% END %]
     
-    [% IF ( Ask_data ) %]
-        [% IF ( OpacPasswordChange ) %]
+    [% IF ( OpacPasswordChange ) %]
+        [% IF ( Ask_data ) %]
         <form action="/cgi-bin/koha/opac-passwd.pl" name="mainform" id="mainform" method="post"><fieldset class="brief">
 			[% UNLESS ( ShortPass ) %]<div class="hint">Your password must be at least [% minpasslen %] characters long.</div>[% END %]
           <ol>  <li><label for="Oldkey">Current password:</label> <input type="password" id="Oldkey" size="25"  name="Oldkey" /></li>
             <li><label for="Newkey">New password:</label> <input type="password" id="Newkey"  size="25"  name="Newkey" /></li>
             <li><label for="Confirm">Re-type new password:</label> <input type="password"  id="Confirm" size="25" name="Confirm" /></li></ol></fieldset>
             <fieldset class="action"><input type="submit" value="Submit changes" class="submit" /> <a href="/cgi-bin/koha/opac-user.pl" class="cancel">Cancel</a></fieldset>
-    </form>
-        [% ELSE %]
-            <div class="dialog alert">You can't change your password.</div>
+        </form>
         [% END %]
+    [% ELSE %]
+        <div class="dialog alert">You can't change your password.</div>
     [% END %]
     
     [% IF ( password_updated ) %]
diff --git a/opac/opac-passwd.pl b/opac/opac-passwd.pl
index a52d40a..19425f7 100755
--- a/opac/opac-passwd.pl
+++ b/opac/opac-passwd.pl
@@ -46,57 +46,58 @@ my ( $template, $borrowernumber, $cookie ) = get_template_and_user(
 
 # get borrower information ....
 my ( $borr ) = GetMemberDetails( $borrowernumber );
-my $sth =  $dbh->prepare("UPDATE borrowers SET password = ? WHERE borrowernumber=?");
 my $minpasslen = C4::Context->preference("minPasswordLength");
-if (   $query->param('Oldkey')
-    && $query->param('Newkey')
-    && $query->param('Confirm') )
-{
-    if ( goodkey( $dbh, $borrowernumber, $query->param('Oldkey') ) ) {
-        if ( $query->param('Newkey') eq $query->param('Confirm')
-            && length( $query->param('Confirm') ) >= $minpasslen )
-        {    # Record password
-            my $clave = md5_base64( $query->param('Newkey') );
-            $sth->execute( $clave, $borrowernumber );
-            $template->param( 'password_updated' => '1' );
-            $template->param( 'borrowernumber'   => $borrowernumber );
+if ( C4::Context->preference("OpacPasswordChange") ) {
+    my $sth =  $dbh->prepare("UPDATE borrowers SET password = ? WHERE borrowernumber=?");
+    if (   $query->param('Oldkey')
+        && $query->param('Newkey')
+        && $query->param('Confirm') )
+    {
+        if ( goodkey( $dbh, $borrowernumber, $query->param('Oldkey') ) ) {
+            if ( $query->param('Newkey') eq $query->param('Confirm')
+                && length( $query->param('Confirm') ) >= $minpasslen )
+            {    # Record password
+                my $clave = md5_base64( $query->param('Newkey') );
+                $sth->execute( $clave, $borrowernumber );
+                $template->param( 'password_updated' => '1' );
+                $template->param( 'borrowernumber'   => $borrowernumber );
+            }
+            elsif ( $query->param('Newkey') ne $query->param('Confirm') ) {
+                $template->param( 'Ask_data'       => '1' );
+                $template->param( 'Error_messages' => '1' );
+                $template->param( 'PassMismatch'   => '1' );
+            }
+            elsif ( length( $query->param('Confirm') ) < $minpasslen ) {
+                $template->param( 'Ask_data'       => '1' );
+                $template->param( 'Error_messages' => '1' );
+                $template->param( 'ShortPass'      => '1' );
+            }
+            else {
+                $template->param( 'Error_messages' => '1' );
+            }
         }
-        elsif ( $query->param('Newkey') ne $query->param('Confirm') ) {
+        else {
             $template->param( 'Ask_data'       => '1' );
             $template->param( 'Error_messages' => '1' );
-            $template->param( 'PassMismatch'   => '1' );
+            $template->param( 'WrongPass'      => '1' );
         }
-        elsif ( length( $query->param('Confirm') ) < $minpasslen ) {
-            $template->param( 'Ask_data'       => '1' );
+    }
+    else {
+
+        # Called Empty, Ask for data.
+        $template->param( 'Ask_data' => '1' );
+        if (!$query->param('Oldkey') && ($query->param('Newkey') || $query->param('Confirm'))){
+            # Old password is empty but one of the others isnt
             $template->param( 'Error_messages' => '1' );
-            $template->param( 'ShortPass'      => '1' );
+            $template->param( 'WrongPass'      => '1' );
         }
-        else {
+        elsif ($query->param('Oldkey') && (!$query->param('Newkey') || !$query->param('Confirm'))){
+            # Oldpassword is entered but one of the other fields is empty
             $template->param( 'Error_messages' => '1' );
+            $template->param( 'PassMismatch'   => '1' );
         }
     }
-    else {
-        $template->param( 'Ask_data'       => '1' );
-        $template->param( 'Error_messages' => '1' );
-        $template->param( 'WrongPass'      => '1' );
-    }
-}
-else {
-   
-    # Called Empty, Ask for data.
-    $template->param( 'Ask_data' => '1' );
-	if (!$query->param('Oldkey') && ($query->param('Newkey') || $query->param('Confirm'))){
-		# Old password is empty but one of the others isnt
-		$template->param( 'Error_messages' => '1' );
-		$template->param( 'WrongPass'      => '1' );
-	}
-	elsif ($query->param('Oldkey') && (!$query->param('Newkey') || !$query->param('Confirm'))){
-		# Oldpassword is entered but one of the other fields is empty
-		$template->param( 'Error_messages' => '1' );
-		$template->param( 'PassMismatch'   => '1' );
-	}
 }
-
 $template->param(firstname => $borr->{'firstname'},
 							surname => $borr->{'surname'},
 							minpasslen => $minpasslen,
-- 
1.7.9.5