Bug 8753 - Various little things - removing new dependency, changes to errors, textual updates

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>
This commit is contained in:
Liz Rea 2015-10-07 15:08:10 +13:00 committed by Brendan A Gallagher
parent 76d1509838
commit 563688050c
3 changed files with 21 additions and 36 deletions

View file

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

View file

@ -1,6 +1,6 @@
[% USE Koha %] [% USE Koha %]
[% INCLUDE 'doc-head-open.inc' %] [% 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' %] [% INCLUDE 'doc-head-close.inc' %]
[% BLOCK cssinclude %][% END %] [% BLOCK cssinclude %][% END %]
[% BLOCK jsinclude %] [% BLOCK jsinclude %]
@ -30,7 +30,7 @@
<div class="main"> <div class="main">
<ul class="breadcrumb"> <ul class="breadcrumb">
<li><a href="/cgi-bin/koha/opac-main.pl">Home</a> <span class="divider">&rsaquo;</span></li> <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> </ul>
<div class="container-fluid"> <div class="container-fluid">
@ -44,40 +44,31 @@
[% END %] [% END %]
</div> </div>
<div class="span10"> <div class="span10">
<h3>Password recovery</h3> <h3>Forgotten password recovery</h3>
[% IF (hasError) %] [% IF (hasError) %]
<div class="alert alert-warning"> <div class="alert alert-warning">
<h3>An error occurred</h3> <h3>Oops!</h3>
<p> <p>
[% IF (sendmailError) %] [% IF (sendmailError) %]
An error has occurred while sending you the password recovery link. An error has occurred while sending you the password recovery link.
<br/>Please try again later. <br/>Please try again later.
[% ELSIF (errNoBorrowerFound) %] [% ELSIF (errNoBorrowerFound) %]
No account was found with the provided information. 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) %] [% ELSIF (errAlreadyStartRecovery) %]
The process of password recovery has already started for this account ("<strong>[% username %]</strong>") The process of password recovery has already been started for this account ("<strong>[% username %]</strong>")
<br/>Check your emails; you should receive the link to reset your password. <br/>You should have received an email with a 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. <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) %] [% ELSIF (errPassNotMatch) %]
The passwords entered does not match. Oops! The passwords must match.
<br/>Please try again.
[% ELSIF (errPassTooShort) %] [% ELSIF (errPassTooShort) %]
The password is too short. Your chosen password is too short.
<br/>The password must contain at least [% minPassLength %] characters. <br/>The password must contain at least [% minPassLength %] characters.
[% ELSIF (errLinkNotValid) %] [% ELSIF (errLinkNotValid) %]
We could not authenticate you as the account owner. The link you clicked is either invalid, or expired.
<br/>Be sure to use the link you received in your email. <br/>Be sure you used the link from the email, or contact library staff for assistance.
[% END %] [% END %]
</p> </p>
<p>Please contact the staff if you need further assistance.</p> <p>Please contact the library if you need further assistance.</p>
</div> </div>
[% END %] [% END %]
<div id="password-recovery"> <div id="password-recovery">
@ -87,8 +78,7 @@
<form action="/cgi-bin/koha/opac-password-recovery.pl" method="post"> <form action="/cgi-bin/koha/opac-password-recovery.pl" method="post">
<input type="hidden" name="koha_login_context" value="opac" /> <input type="hidden" name="koha_login_context" value="opac" />
<fieldset> <fieldset>
<p>To reset your password, enter your username or email address. <p>To reset your password, enter your login and email address.
<br/>A link to reset your password will be sent at this address.</p>
<label for="username">Login:</label> <label for="username">Login:</label>
<input type="text" id="username" size="40" name="username" value="[% username %]" /> <input type="text" id="username" size="40" name="username" value="[% username %]" />
<label for="email">Email:</label> <label for="email">Email:</label>
@ -118,10 +108,10 @@
<div class="alert alert-info"> <div class="alert alert-info">
<p> <p>
An email has been sent to "[% email %]". An email has been sent to "[% email %]".
<br/>It contains a link to create a new password. <br/>Please click the link in this email to finish the process of resetting your password.
<br/>This link will be valid for 2 days starting now. <br/>This link is valid for 2 days starting now.
</p> </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> </div>
[% ELSIF (password_reset_done) %] [% ELSIF (password_reset_done) %]
<div class="alert alert-success"> <div class="alert alert-success">

View file

@ -64,14 +64,10 @@ if ( $query->param('sendEmail') || $query->param('resendEmail') ) {
elsif ($email) { elsif ($email) {
$search_results = [ Koha::Borrowers->search( { -or => { email => $email, emailpro => $email, B_email => $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; $hasError = 1;
$errNoBorrowerFound = 1; $errNoBorrowerFound = 1;
} }
elsif ( scalar @$search_results > 1 ) { # Many matching borrowers
$hasError = 1;
$errTooManyEmailFound = 1;
}
elsif ( $borrower = shift @$search_results ) { # One matching borrower elsif ( $borrower = shift @$search_results ) { # One matching borrower
$username ||= $borrower->userid; $username ||= $borrower->userid;
my @emails = ( $borrower->email, $borrower->emailpro, $borrower->B_email ); 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 ? # Is the given email one of the borrower's ?
if ( $email && !( grep { $_ eq $email } @emails ) ) { if ( $email && !( grep { $_ eq $email } @emails ) ) {
$hasError = 1; $hasError = 1;
$errBadEmail = 1; $errNoBorrowerFound = 1;
} }
# If we dont have an email yet. Get one of the borrower's email or raise an error. # 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 # 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 ] ) ) { elsif ( !$email && !( $email = shift [ grep { length() } @emails ] ) ) {
$hasError = 1; $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 # Check if a password reset already issued for this borrower AND we are not asking for a new email