From bc7034a0b21ad9166a5868c96d83c3525c915c5c Mon Sep 17 00:00:00 2001 From: Owen Leonard Date: Fri, 2 Nov 2012 12:40:08 -0400 Subject: [PATCH] Bug 8515 - OPAC password change does not obey OpacPasswordChange 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. Signed-off-by: Melia Meggs Signed-off-by: Katrin Fischer Confirmed bug and made sure patch fixes it. Passes all tests and perlcritic. Signed-off-by: Jared Camins-Esakov Signed-off-by: Chris Cormack --- .../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 3be0ef8021..f510d6ecb1 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 @@

[% END %] - [% IF ( Ask_data ) %] - [% IF ( OpacPasswordChange ) %] + [% IF ( OpacPasswordChange ) %] + [% IF ( Ask_data ) %]
[% UNLESS ( ShortPass ) %]
Your password must be at least [% minpasslen %] characters long.
[% END %]
Cancel
-
- [% ELSE %] -
You can't change your password.
+ [% END %] + [% ELSE %] +
You can't change your password.
[% END %] [% IF ( password_updated ) %] diff --git a/opac/opac-passwd.pl b/opac/opac-passwd.pl index a52d40a596..19425f7937 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, -- 2.39.5