Bug 8753 - Various little things - removing new dependency, changes to errors, textua...

Koha already has a sub that creates salts, so lets use that instead of math::Random::secure, so as not to add a new dependency.

Made the references to "Forgotten password" consistent, including adding it to the title of the page.

Also removed the individual error for "this email doesn't belong to this account" as that could expose the existence of a login, which I think we'd rather not do.

Made some of the text more grammatically correct, and more library specific.

To test:

Apply on top of all of the other patches.

All the usual checks, plus make sure there are no typos in any text references.

Signed-off-by: Marc Veron <veron@veron.ch>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

diff --git a/C4/Passwordrecovery.pm b/C4/Passwordrecovery.pm
index 8090eeb..d8420db 100644 (file)
--- a/C4/Passwordrecovery.pm
+++ b/C4/Passwordrecovery.pm
@@ -19,7 +19,7 @@ package C4::Passwordrecovery;
 
 use Modern::Perl;
 use C4::Context;
-use Math::Random::Secure;
+use Crypt::Eksblowfish::Bcrypt qw(en_base64);
 
 use vars qw($VERSION @ISA @EXPORT);
 
@@ -111,8 +111,7 @@ sub SendPasswordRecoveryEmail {
 
     # generate UUID
     my @chars = ( "A" .. "Z", "a" .. "z", "0" .. "9" );
-    my $uuid_str;
-    $uuid_str .= $chars[ rand @chars ] for 1 .. 32;
+    my $uuid_str = '$2a$08$'.en_base64(Koha::AuthUtils::generate_salt('weak', 16));
 
     # insert into database
     my $expirydate =

diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-password-recovery.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-password-recovery.tt
index 8ffb3e3..91ac94d 100644 (file)
--- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-password-recovery.tt
+++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-password-recovery.tt
@@ -1,6 +1,6 @@
 [% USE Koha %]
 [% INCLUDE 'doc-head-open.inc' %]
-<title>[% IF ( LibraryNameTitle ) %][% LibraryNameTitle %][% ELSE %]Koha online[% END %] catalog</title>
+<title>[% IF ( LibraryNameTitle ) %][% LibraryNameTitle %] - Forgotten password recovery[% ELSE %]Koha online[% END %] catalog - Forgotten password recovery</title>
 [% INCLUDE 'doc-head-close.inc' %]
 [% BLOCK cssinclude %][% END %]
 [% BLOCK jsinclude %]
@@ -30,7 +30,7 @@
 <div class="main">
     <ul class="breadcrumb">
         <li><a href="/cgi-bin/koha/opac-main.pl">Home</a> <span class="divider">&rsaquo;</span></li>
-        <li><a href="#">Change your password</a></li>
+        <li><a href="#">Forgotten password recovery</a></li>
     </ul>
 
     <div class="container-fluid">
@@ -44,40 +44,31 @@
                 [% END %]
             </div>
             <div class="span10">
-                    <h3>Password recovery</h3>
+                    <h3>Forgotten password recovery</h3>
             [% IF (hasError) %]
                 <div class="alert alert-warning">
-                    <h3>An error occurred</h3>
+                    <h3>Oops!</h3>
                     <p>
                     [% IF (sendmailError) %]
                         An error has occurred while sending you the password recovery link.
                         <br/>Please try again later.
                     [% ELSIF (errNoBorrowerFound) %]
                         No account was found with the provided information.
-                        <br/>Check if you typed it correctly.
-                    [% ELSIF (errBadEmail) %]
-                        The provided email address is not tied to this account.
-                    [% ELSIF (errTooManyEmailFound) %]
-                        More than one account has been found for the email address: "<strong>[% email %]</strong>"
-                        <br/>Try to use your username or an alternative email if you have one.
-                    [% ELSIF (errNoBorrowerEmail) %]
-                        This account has no email address we can send the email to.
                     [% ELSIF (errAlreadyStartRecovery) %]
-                        The process of password recovery has already started for this account ("<strong>[% username %]</strong>")
-                        <br/>Check your emails; you should receive the link to reset your password.
-                        <br/>If you did not receive it, click <a href="/cgi-bin/koha/opac-password-recovery.pl?resendEmail=true&email=[% email %]&username=[% username %]">here</a> to get a new password recovery link.
+                        The process of password recovery has already been started for this account ("<strong>[% username %]</strong>")
+                        <br/>You should have received an email with a link to reset your password.
+                        <br/>If you did not receive this email, you can <a href="/cgi-bin/koha/opac-password-recovery.pl?resendEmail=true&email=[% email %]&username=[% username %]">request a new password recovery link.</a>
                     [% ELSIF (errPassNotMatch) %]
-                        The passwords entered does not match.
-                        <br/>Please try again.
+                        Oops! The passwords must match.
                     [% ELSIF (errPassTooShort) %]
-                        The password is too short.
+                        Your chosen password is too short.
                         <br/>The password must contain at least [% minPassLength %] characters.
                     [% ELSIF (errLinkNotValid) %]
-                        We could not authenticate you as the account owner.
-                        <br/>Be sure to use the link you received in your email.
+                        The link you clicked is either invalid, or expired.
+                        <br/>Be sure you used the link from the email, or contact library staff for assistance.
                     [% END %]
                     </p>
-                    <p>Please contact the staff if you need further assistance.</p>
+                    <p>Please contact the library if you need further assistance.</p>
                 </div>
             [% END %]
                 <div id="password-recovery">
@@ -87,8 +78,7 @@
                     <form action="/cgi-bin/koha/opac-password-recovery.pl" method="post">
                         <input type="hidden" name="koha_login_context" value="opac" />
                         <fieldset>
-                            <p>To reset your password, enter your username or email address.
-                            <br/>A link to reset your password will be sent at this address.</p>
+                            <p>To reset your password, enter your login and email address.
                             <label for="username">Login:</label>
                             <input type="text" id="username" size="40" name="username" value="[% username %]" />
                             <label for="email">Email:</label>
@@ -118,10 +108,10 @@
                     <div class="alert alert-info">
                         <p>
                             An email has been sent to "[% email %]".
-                            <br/>It contains a link to create a new password.
-                            <br/>This link will be valid for 2 days starting now.
+                            <br/>Please click the link in this email to finish the process of resetting your password.
+                            <br/>This link is valid for 2 days starting now.
                         </p>
-                        Click <a href="/cgi-bin/koha/opac-main.pl"">here</a> to return to the main page.
+                        <a href="/cgi-bin/koha/opac-main.pl"">Return to the main page</a>
                     </div>
 [% ELSIF (password_reset_done) %]
                     <div class="alert alert-success">

diff --git a/opac/opac-password-recovery.pl b/opac/opac-password-recovery.pl
index 1cef3ed..6cfc414 100755 (executable)
--- a/opac/opac-password-recovery.pl
+++ b/opac/opac-password-recovery.pl
@@ -64,14 +64,10 @@ if ( $query->param('sendEmail') || $query->param('resendEmail') ) {
     elsif ($email) {
         $search_results = [ Koha::Borrowers->search( { -or => { email => $email, emailpro => $email, B_email  => $email } } ) ];
     }
-    if ( not $search_results ) {
+    if ( not $search_results || scalar @$search_results > 1 ) {
         $hasError           = 1;
         $errNoBorrowerFound = 1;
     }
-    elsif ( scalar @$search_results > 1 ) {    # Many matching borrowers
-        $hasError             = 1;
-        $errTooManyEmailFound = 1;
-    }
     elsif ( $borrower = shift @$search_results ) {    # One matching borrower
         $username ||= $borrower->userid;
         my @emails = ( $borrower->email, $borrower->emailpro, $borrower->B_email );
@@ -79,7 +75,7 @@ if ( $query->param('sendEmail') || $query->param('resendEmail') ) {
         # Is the given email one of the borrower's ?
         if ( $email && !( grep { $_ eq $email } @emails ) ) {
             $hasError    = 1;
-            $errBadEmail = 1;
+            $errNoBorrowerFound = 1;
         }
 
 # If we dont have an email yet. Get one of the borrower's email or raise an error.
@@ -88,7 +84,7 @@ if ( $query->param('sendEmail') || $query->param('resendEmail') ) {
 # It's supposed to get a non-empty string from the @emails array. There's surely a simpler way
         elsif ( !$email && !( $email = shift [ grep { length() } @emails ] ) ) {
             $hasError           = 1;
-            $errNoBorrowerEmail = 1;
+            $errNoBorrowerFound = 1;
         }
 
 # Check if a password reset already issued for this borrower AND we are not asking for a new email