Koha-community/Koha - Koha - Koha: The world's first free and open source library system

Commit Graph

Author	SHA1	Message	Date
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>	7 years ago
Marcel de Rooy	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>	7 years ago
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>	7 years ago
Jonathan Druart	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>	7 years ago
Jonathan Druart	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>	7 years ago
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>	7 years ago
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>	7 years ago
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>	7 years ago
Jonathan Druart	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>	7 years ago
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>	7 years ago
Jonathan Druart	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>	8 years ago
Jonathan Druart	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>	8 years ago
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	8 years ago
Kyle Hall	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>	8 years ago
Marcel de Rooy	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>	8 years ago
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>	9 years ago
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>	9 years ago
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>	9 years ago
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>	9 years ago
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>	9 years ago

20 Commits (d10513dfc0d47846fdb874b0a62ce9c8c7107644)