Commit graph

36 commits

Author SHA1 Message Date
9d6d641d1f Bug 17600: Standardize our EXPORT_OK
On bug 17591 we discovered that there was something weird going on with
the way we export and use subroutines/modules.
This patch tries to standardize our EXPORT to use EXPORT_OK only.

That way we will need to explicitely define the subroutine we want to
use from a module.

This patch is a squashed version of:
Bug 17600: After export.pl
Bug 17600: After perlimport
Bug 17600: Manual changes
Bug 17600: Other manual changes after second perlimports run
Bug 17600: Fix tests

And a lot of other manual changes.

export.pl is a dirty script that can be found on bug 17600.

"perlimport" is:
git clone https://github.com/oalders/App-perlimports.git
cd App-perlimports/
cpanm --installdeps .
export PERL5LIB="$PERL5LIB:/kohadevbox/koha/App-perlimports/lib"
find . \( -name "*.pl" -o -name "*.pm" \) -exec perl App-perlimports/script/perlimports --inplace-edit --no-preserve-unused --filename {} \;

The ideas of this patch are to:
* use EXPORT_OK instead of EXPORT
* perltidy the EXPORT_OK list
* remove '&' before the subroutine names
* remove some uneeded use statements
* explicitely import the subroutines we need within the controllers or
modules

Note that the private subroutines (starting with _) should not be
exported (and not used from outside of the module except from tests).

EXPORT vs EXPORT_OK (from
https://www.thegeekstuff.com/2010/06/perl-exporter-examples/)
"""
Export allows to export the functions and variables of modules to user’s namespace using the standard import method. This way, we don’t need to create the objects for the modules to access it’s members.

@EXPORT and @EXPORT_OK are the two main variables used during export operation.

@EXPORT contains list of symbols (subroutines and variables) of the module to be exported into the caller namespace.

@EXPORT_OK does export of symbols on demand basis.
"""

If this patch caused a conflict with a patch you wrote prior to its
push:
* Make sure you are not reintroducing a "use" statement that has been
removed
* "$subroutine" is not exported by the C4::$MODULE module
means that you need to add the subroutine to the @EXPORT_OK list
* Bareword "$subroutine" not allowed while "strict subs"
means that you didn't imported the subroutine from the module:
  - use $MODULE qw( $subroutine list );
You can also use the fully qualified namespace: C4::$MODULE::$subroutine

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-07-16 08:58:47 +02:00
6f204fdf96 Bug 28591: Don't pass debug to get_template_and_user
There is a "debug" parameter we are passing from the controller scripts
to C4::Auth::get_template_and_user, but it's not actually used!

Test plan:
Confirm the assumption
Review the changes from this patch

Generated with:
perl -p -i -e 's#\s*debug\s*=\>\s*(0|1),?\s*##gms' **/*.pl

git checkout misc/devel/update_dbix_class_files.pl # Wrong catch
+ Manual fix in acqui/neworderempty.pl

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-06-22 12:04:32 +02:00
470c760669 Bug 26941: Fix OPAC password recovery error messages
In OPAC password recovery perl opac/opac-password-recovery.pl
there are some error codes not in Template koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-password-recovery.tt

This patch fixes several bugs:
- remove 'use Koha::Patrons' defined twice
- remove vars $errTooManyEmailFound $errBadEmail, not used in any template
- add in template text for error 'errNoBorrowerEmail'

1) Create a patron A with login but no email
2) Create a patron B with login and valid email
3) Go to system preferences set 'OpacResetPassword' to ON
4) Make sure that OpacPasswordChange is also ON
5) Go to 'Forgot your password' in OPAC
6) Enter login if patron A and save
=> You get message 'This account has no email address we can send the email to.'
7) Enter login if patron B and save
=> Password recovery is send, no error message

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-12-21 10:08:02 +01:00
904e926ba0 Bug 26904: OPAC password recovery allows regexp in email
When using OPAC password recovery form, opac/opac-password-recovery.pl :
if one provides correct login and an email, there is a check that this email is one of patron's.

This check uses RegExp with case insensitive :
  if ( $email && !( any { /^$email$/i } @emails ) )

This is a security issue since one can simply enter '.*'.
Severity is normal because the login must be a correct.

I propose to use simple string compare with lowercase to be case insensitive.

Test plan :
1) Don't apply patch
2) Enable system preference 'OpacResetPassword'
3) Go to 'OPAC > Log in to your account > Forgot your password?'
4) Enter an existing userid or cardnumber and '.*' in 'Email'
5) The password recovery is created ! (check table 'borrower_password_recovery')
6) Apply patch
7) Enter an existing userid or cardnumber and '.*' in 'Email'
8) You get the message 'No account was found with the provided information.'
9) Enter an existing userid or cardnumber and in 'Email' the corresponding email but with different case
10) The password recovery is created (check table 'borrower_password_recovery')

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-11-09 16:18:25 +01:00
Julian Maurice
96cc447045 Bug 25898: Prohibit indirect object notation
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-10-15 12:56:30 +02:00
Timothy Alexis Vass
33c8969039 Bug 26266: Add jQuery validator to opac-password-recovery.tt
To test:
0) Activate password reset in system preferences.
1) Initiate a password recovery.
2) Try to submit with an invalid password.
3) Confirm that validation occurs.
4) Try to submit with mismatching passwords.
5) Confirm that validation occurs.
6) Sign off.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-10-01 10:32:15 +02:00
Agustin Moyano
5848da810e Bug 23816: Add minimum password length and require strong password overrides by category
This patch adds the capability to override minPasswordLenth and RequireStrongPassword settings by category

To test:
1. koha-shell kohadev
2. koha-mysql kohadev

3. drop database koha_kohadev;
4. create database koha_kohadev;

5. go to admin page and start webinstaller. There continue the steps until onboarding.
6. reach step 3 of onboarding and create a new administrator patron
CHECH => Password control woks as normal (Minimum length 3 and strong required)

7. finish Koha installation and enter admin with your new administrator
8. set minPasswordLength to 3 and RequireStrongPassword to “Don’t require”
9. Create a new category (CAT2 from now on.. CAT1 is the category you made in onboarding process) and set minimum password length to 8 and require strong password
10. Create two new patrons, one with CAT1(patron1) and one with CAT2 (patron2)
CHECK => In both cases, try different combinations of length and strength. For patron1 the only requirement is to have 3 letters, but for patron2 the minimum length will be 8 and will require strong password.
CHECK => Try changing patron category before saving. Password requirements will change with category change.

11. Edit CAT1 and set minimum password length to 5
12. Go to patron1 details page, and change password.
CHECH => Now password minimum length is 5, but still it doesn’t require strong password

13. Edit CAT1, leave blank minimum password length and set require strong password to yes.
14. Go to patron1 details page, and change password.
CHECH => Password minimum length is back to 3, but now strong password is required

15. Set minimum password length in CAT2 to 12.
16. Go to patron2 details page, and click to fill a random generated password
CHECK => generated password should be 12 characters length

17. Set PatronSelfRegistration to Allow in admin settings
18. Go to OPAC and fill self registration from.
CHECK => Play with patron category. For each change in category, password requirements are modified.
CHECK => Set CAT1 as patron category, set ‘aA1’ as password (or another valid password for CAT1) and before hitting submit button, change to CAT2. Form should enter invalid state, and CAT2 password requirements should be displayed as error in password input.

19. Create a patron for CAT1 and another for CAT2, leaving password blank
CHECK => For CAT1’s patron, generated password length is 8 (minimum length for generated passwords), but for CAT2’s patron should be 12

20. In admin set PatronSelfRegistrationVerifyByEmail to require
21. Fill self registration form again with CAT2 as category
CHECK => Password requirements works as previous case.
22. Leave password blank and click submit

23. select * from message_queue;
24. Copy the link in the message and paste it in OPAC
CHECH => Generated password is 12 characters long. (Copy user id for next steps)

25. In admin set OpacResetPassword to Allow
26. Go back to OPAC, reload and click on “Forgot password?” link
27. Paste user id and click submit
28. Repeat steps 23 and 24
CHECK => Info message says “Your password must contain at least 12 characters, including UPPERCASE, lowercase and numbers.”
CHECK => enter an invalid password and you’ll get the same message in warning.

29. Login OPAC with the last user and your newly created password
30. Go to “Change your password” option
CHECK => Info message says “Your password must contain at least 12 characters, including UPPERCASE, lowercase and numbers.”
CHECK => enter an invalid password and you’ll get the same message in below “New password” input.

31. prove t/db_dependent/AuthUtils.t t/db_dependent/Koha/Patron/Category.t

32. Sign off

Sponsored-by: Northeast Kansas Library - NEKLS

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-09-09 15:39:52 +02:00
d35cc83379 Bug 21533: Do not allow password recovery for administrative locked patrons
Depends on bug 21336 for the ADMINISTRATIVE_LOCKOUT constant.

This is a bit lazy solution (but good enough): The account will not be found
when recovering the password. The user should contact the library. Since
the library chose to lock the account, that seems appropriate.

Test plan:
Select a borrower and set login_attempts to -1. Via mysql command line or
with Koha::Patrons->find(borrowernumber)->lock.
Enable password recovery.
Try to recover password from OPAC. You should fail with 'Not found, contact
the library'.

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

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Bouzid Fergani <bouzid.fergani@inlibro.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2019-05-10 12:19:13 +00:00
6388ffd702 Bug 21890: (QA follow-up) Use List::Util::any
When possible it's a good idea to use `any` from List::Util to shortcut
on the first occurence of a truthy value.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2019-04-12 02:32:04 +00:00
376d1a7023 Bug 21890: Allow restricting password resets by patron category
This patch makes the templates relying on the OpacResetPassword syspref
use the introduced TT plugin method instead by changing:

[% IF Koha.Preference('OpacResetPassword') %]

=>

[% IF Categories.can_any_reset_password %]

To test:
- Verify that all the places in which the 'forgot password' link is
displayed in OPAC keep working, provided there's at least one category
that has the flag set
- Attempt to recover the password for a patron that belong to a valid
category (i.e. that has the flag set)
=> SUCCESS: You can go through the normal process
- Attempt to recover the password for a patron that belongs to a
category with the flag unset.
=> SUCCESS: Once Koha identifies your category, you are told you are not
allowed to do it
- Sign off :-D

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2019-04-12 02:32:03 +00:00
Liz Rea
a0b81b906d Bug 22559: Restore OPAC forgotten password functionality
To test:

Enable OpacResetPassword
Create a borrower with a username and password
Go to the OPAC, try to reset the password (I always get the reset link
        token out of the message_queue in the database rather than worry about
        receiving the actual email. You do you.)
Go to the link provided.
Attempt to set a password, this should fail.
Apply this patch
Go through the process again, password should be reset.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2019-03-22 18:56:14 +00:00
f822dee088 Bug 22048: Use set_password opac/opac-password-recovery.pl
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2019-01-25 20:15:31 +00:00
1885e6a214 Bug 21771: (follow-up) Revert the shift statement
Obviously you should never shift an items from an array if you need that
item later on :)

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Tested with entering userid as well as entering email..

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-11-06 17:42:20 +00:00
eec74a2cd1 Bug 20023: (QA follow-up) Tiny simplification
Remove empty emails from the list rightaway.
Would be tempted to lc the params->{email} at the start btw..

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-09-26 16:03:14 +00:00
Blou
cc3bc29207 Bug 20023: Password recovery using email should be case insensitive
When entering an email to recover the password, a user should not have to know or remember the casing of the given email.

Test
0) enable OpacResetPassword
1) On the OPAC, click "Forgot your password"
2) Enter your email address as in your user account BUT WITH A DIFFERENT CASING
3) Submit.  This will fail.
4) Apply the patch, redo with success.

Signed-off-by: Michal Denar <black23@gmail.com>

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-09-26 16:03:14 +00:00
2e6fb40ef8 Bug 21087: Hash passwords in ->update_password
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: John Doe <you@example.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2018-07-30 14:58:06 -03:00
Mark Tompsett
c4113dce70 Bug 18956: Fix empty to in message queue
Follow the test plan in comment #20.
Also tweaked string, because it was really 'or' before too.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Amended text in added comment.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-10-27 13:57:10 -03:00
6336e53aed Bug 18956: (QA follow-up) Resolve a CGI::Param in list context warn
From the plack-error.log:
CGI::param called in list context from package CGI::Compile::ROOT::usr_share_koha_masterclone_opac_opac_2dpassword_2drecovery_2epl line 129, this can lead to vulnerabilities. See the warning in "Fetching the value or values of a single named parameter" at /usr/share/perl5/CGI.pm line 436.

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-10-27 13:56:59 -03:00
Mark Tompsett
e08a0afa26 Bug 18956: Prevent leaking during password recovery
TEST PLAN
---------

It is assumed you have set the OpacResetPassword to 'allowed',
and likely in combination with OpacPasswordChange to 'Allowed'.

You will have two patrons: one with and another without
any email address entered. You will want to test this test plan
with both patrons.

$ git checkout -b bug_18956 origin/master

Prepend the following as understood between step sections:
opac -> forgot password and then enter...

correct login/cardnumber, it will email
delete from borrower_password_recovery;

correct email, it will email
delete from borrower_password_recovery;

correct login/cardnumber && correct email, it will email
delete from borrower_password_recovery;

wrong login/cardnumber && correct email, error page as expected
delete from borrower_password_recovery;

correct login/cardnumber && wrong email, error page as expected
delete from borrower_password_recovery;

wrong login/cardnumber && wrong email, error page as expected
delete from borrower_password_recovery;

submit empty -- INTERNAL SERVER ERROR?!
delete from borrower_password_recovery;

-- None of the above step sections displayed email.

correct login/cardnumber, it will email

correct login/cardnumber again, but it leaks email address!
delete from borrower_password_recovery;

correct email, it will email

correct email again, but it leaks login/cardnumber!
delete from borrower_password_recovery;

$ git bz apply 18956
-- choose interactive, and choose this counter patch.

repeat the same test set again
-- no leaks will occur, error message pages returned should
   be reasonable, code should read reasonably.

run koha qa test tools.

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-10-27 10:59:05 -03:00
3f9da34683 Bug 18298: Add server-side checks and refactor stuffs
Now that we have a check client-side, nothing prevents us from a smart guy to
bypass it and force an invalid password.
This patch adds two new subroutines to Koha::AuthUtils to check the
validity of passwords and generate a password server-side. It is used
only once (self-registration) but could be useful later.

Moreover the 3 different cases of password rejection (too leak, too
short, contains leading or trailing whitespaces) were not tested
everywhere. Now they are!

This patch makes things consistent everywhere and clean up some code.

Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-10-16 09:44:32 -03:00
ec4e666bc5 Bug 18298: minPaswordLength should not be < 3
Indeed if RequireStrongPassword is set we need at least 3 characters to
match 1 upper, 1 lower and 1 digit.
We could make things more complicated to allow minPasswordLength < 3
but, really, 3 is already too low...

Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-10-16 09:44:32 -03:00
David Bourgault
078eb77451 Bug 19345: Add missing error flag to template
Adds a missing error flag to the template->param { } call.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-10-06 12:09:55 -03:00
David Kuhn
e29163af4c Bug 18616 - The "Add forgot password link to OPAC" should allow patrons to use their library card number in addition to username
Allow patrons to enter either their library card number or user name in the "Log in" box for password recovery.

Most patrons at our library use their card number to log in and are unaware of what their userid is. However there are some who have set a

customized userid and would prefer to use that. This patch would allow either to be entered for password recovery.

To test:
1. Enable the password recovery feature.
2. In the OPAC, click on "Forgot you password?" link and enter a valid library card number.
3. The error message "No account found with the provided information" appears.
4. Apply the patch.
5. Repeat step 2. The recovery email is now sent.

Note: Moved patch from 16711 back here and re-tested.
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-07-06 14:52:53 -03:00
Marc Véron
3829020c26 Bug 16711: OPAC Password recovery: Handling if multiple accounts have the same mail address
To reproduce:
- Create 3 Accounts, login names are test01, test02, test03, Email is the same
for all.
- Go to OPAC -> Password recovery and indicate E-Mail only
- You will get an email for only one of the accounts above.

To test:
- Apply patch, restart memcached and plack
- Go to db, delete from borrower_password_recovery;
- Try steps above to reproduce. You will get an error message:
    Account identification with this email address only is ambiguous.
    Please use the field 'Login' as well.
- Verify that other cases work as before (provide valid / invalid login only,
  provide valid email for an existing account, provide unknown email, provide
  both login and email with all combinations of valid / invalid)

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Bug 16711: (QA-followup) Use count directly

See comment # 13

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-07-06 14:52:50 -03:00
a471ad80bb Bug 18025: Simplify logic and avoid 1 call to ValidateBorrowernumber
Signed-off-by: Liz Rea <liz@catalyst.net.nz>
This is fine with me.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-02-17 11:24:39 +00:00
Liz Rea
e87dab6411 Bug 18025 - Expired password recovery links cause sql crash
When a user gets an email, but doesn't act or visit it within two days,
     attempting to create a new one causes a collision. We should just
     delete the old one, assuming they still want to reset their
     password.

To test:
create yourself a borrower with a userid and password.
Attempt a password recovery on the OPAC
update the entry in the database for that user to have an expired token
e.g. update borrower_password_recovery set valid_until = '2017-01-25
03:25:26' where borrowernumber = 12;
Attempt another password recovery operation - should error
apply the patch
Try it again - no error, new token is generated and additional email
with new link is sent.

Issue reproduced - is resolved by patch
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-02-17 11:24:39 +00:00
afc008b2fa Bug 16853: Move changepassword to Koha::Patron->update_password
This patch moves the code from C4::Members::changepassword to
Koha::Patron->update_password

Test plan:
Change your password at the OPAC and the staff interface
This should work as before

Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
I rebased this on top of 16849 because they were conflicting.
Tests pass, code looks good (as usual) and I checked both OPAC
and staff password change work as expected.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-07-15 18:09:17 +00:00
0d690ca383 Bug 16707: Fix software error in OPAC password recovery when leaving form fields empty
The $search_results is considered as an arrayref but is not defined if
no patron matches the recovery infos.

Test plan:
- Set syspref OpacResetPassword to "Allow"
- Go to OPAC
- Click link "Forgot your password?
- On the following screen "Forgotten password recovery", do not fill in
  form fields, click "Submit"
=> Without this patch you got the software error
=> With this patch apply, you will get "No account was found with the
provided information."

Sign-off on counter patch.
Signed-off-by: Marc Véron <veron@veron.ch>

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-06-24 11:49:32 +00:00
charles
db0ecc3cc5 Bug 15585 - Move C4::Passwordrecovery to the new namespace Koha::Patron::Password::Reset
As promised, here is the long-awaited sequel to #8753.

What has changed :

    - The Koha::Patron::Password::Reset is now used in place of C4::Passwordrecovery
    - That ugly shift-grep contraption is no more (goodbye old friend)
    - The generated unique key won't end in a dot anymore

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Brendan Gallagher brendan@bywatersolutions.com
2016-03-22 23:08:21 +00:00
77e1e7c4ef Bug 15548 [QA Followup] - More new uses of Koha::Borrower
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Jesse Weaver <jweaver@bywatersolutions.com>
2016-03-03 14:39:00 -07:00
8ec7572d0c Bug 8753: [QA Follow-up] Primary key and collation
This patch includes:
[1] Adds primary key borrowernumber to new table.
[2] Fixes collation.
[3] Removes manual PK in DBIx schema file.
[4] Fixes typo CompletePasswordRevovery.
[5] Removes use strict from opac-password-recovery; Modern::Perl is used.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
2016-01-27 06:40:56 +00:00
Liz Rea
563688050c 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>
2016-01-27 06:40:54 +00:00
Charles Farmer
76d1509838 Bug 8753 - Smartmatch substitute, Math::Random::Secure, Perltidy, Passwordrecovery.t
This is a collection of changes taken from different comments (but mostly comment 21 and comment 122).

Passes qa and prove, on my machine at least.

There's also a new test file, Passwordrecovery.t, which covers every method of C4::Passwordrecovery.

To test:

All normal checks plus :

    1/ Receive the email
    2/ Click on the link
    3/ Change the pwd
    4/ Click again on the link
    5/ You should immediately get an error message

Problems with Math/Random/Secure.pm, is solved in following patch, signing off
Signed-off-by: Marc Veron <veron@veron.ch>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
2016-01-27 06:40:54 +00:00
mxbeaulieu
0f2aea716a Bug 8753 - Use Koha::Borrowers instead of C4::Members
Use the new library to search for borrowers.
Changed how the $borrower variable is used since it is now a Koha::Borrower object.

Removed the $protocol parameter from the generated link. It should be included in the OPACBaseURL syspref.

	modified:   C4/Passwordrecovery.pm
	modified:   opac/opac-password-recovery.pl

Signed-off-by: Marc Veron <veron@veron.ch>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
2016-01-27 06:40:53 +00:00
Liz Rea
b99f1dcbfc Bug 8753 - Followup - change value text on syspref
Followup changes text from "The user can reset | can not reset their password on OPAC" to "Library users are allowed | not allowed to recover their password via e-mail in the OPAC"

This change more clearly differentiates the purpose of this new preference from OpacPasswordChange.

Bug 8753 - followup - update text for link to match common UI paradigms, fixes OpacPublic disabled view

Also corrects OpacNav being included on the reset page on private catalogues.

Updated the link for forgotten passwords to more closely match common UI paradigms, i.e. Facebook and Twitter

To test:
apply all patches, link should now be the less verbose "Forgot your password?"
disable OpacPublic, anything in opacnav should not appear (you may need to add something to opacnav to test properly)

Bug 8753 - [followup] fix the title on opac-password-recovery.tt

The title stanza was missing a <title></title> around it, causing the extra text to appear.

To test, apply all patches and make sure it looks ok and there is no extra text at the top or bottom of the page.

Bug 8753 - [followup} Correcting spelling mistakes

Make sure it all still works

Bug 8753 - [followup] fix error when no information is provided

To test:

All normal checks plus make sure that a nice error is displayed when no data is provided.

fixing the deprecated thing

Signed-off-by: Marc Veron <veron@veron.ch>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
2016-01-27 06:40:53 +00:00
Maxime Beaulieu
d5abcbc8f3 Bug 8753 - Add forgot password link to OPAC
I've addressed a lot of Liz Rea's points.

1.  I have moved the code from updatedatabase.pl and kohastructure.sql to a file in the atomicupdates directory.
1a. The feature is now off by default when the atomicupdate is run.

2.  The password reset link is now visible on the home page, in the modal box and on opac-user.pl .

3.  The password recovery pages now use bootstrap markup.

4.  I am unsure here. I see "New Password:" and "Confirm new password:".

5.  This should still work :).

6.  I could not reproduce.

7.  I have added the userid field.
    You can now reset the password by submitting either your useid or email address.
    Both fields can be filled, but the email address must be one of the borrower's (email, emailpro or b_email).
    When entering only the email address and two borrowers use that same address, the system tells the user to try with another address or to specify his userid.

8.  The text is in the atomicupdate file. Have at it, anyone.

Concerning the email. It is inconvenient for the use to have to wait X minutes for the message queue the be processed.
Maybe we could add a sub in Letters.pm that:
    Takes the same argments as EnqueueLetter
    Sends the letter.
    Saves the letter in the message queue with a 'sent' status.

 TEST PLAN:

Setup)
    1) apply the patch
    2) go to system preferences OPAC>>Privacy and set 'OpacResetPassword' to ON.
    2b) make sure that OpacPasswordChange is also ON.
A)
    1) refresh front page, click on 'Forgot your password' and enter a VALID address
    1b) Also try an INVALID address (valid yet not in your koha db).  An error message will show up.
    2) An email should be received at that address with a link.
    3) Follow the link in the mail to fill the new password.
    Until a satisfactory new password is entered, the old password is not reset.
    4) Go to main page try the new password.
B)
    1) Repeat the password reset, this time use the userid (username) field.
    2) Try to reset the password using a userid and an email not linked to the account. An error appears.
    3) Make sure the borrower has many available email addresses.
    4) For each email, reset the password using both the userid and the email. The link should be sent to the specified address
C)
    1) Make sure two borrowers use the same email.
    2) Repeat the reset procedure in test case A). An error message appears

http://bugs.koha-community.org/show_bug.cgi?id=13068

 Author:    Maxime Beaulieu <maxime.beaulieu@inlibro.com>

Followed test plan. Works as described.
Signed-off-by: Marc Veron <veron@veron.ch>

New sign-off after testing all patches together
Signed-off-by: Marc Veron <veron@veron.ch>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
2016-01-27 06:40:53 +00:00