[Bug 8753] New: Add forgot password link to OPAC
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Priority: P5 - low Change sponsored?: --- Bug ID: 8753 Assignee: oleonard@myacpl.org Summary: Add forgot password link to OPAC Severity: enhancement Classification: Unclassified OS: All Reporter: amit.gupta@osslabs.biz Hardware: All Status: NEW Version: rel_3_10 Component: OPAC Product: Koha If user forgot there password once click on add forgot password link to opac then system generate a new password and send to patron email address. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Amit Gupta <amit.gupta@osslabs.biz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |savitra.sirohi@osslabs.biz -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Amit Gupta <amit.gupta@osslabs.biz> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|oleonard@myacpl.org |amit.gupta@osslabs.biz -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Eric Bégin <eric.begin@inLibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |eric.begin@inLibro.com Assignee|amit.gupta@osslabs.biz |eric.begin@inLibro.com --- Comment #1 from Eric Bégin <eric.begin@inLibro.com> --- A better way to handle this is to send a uniquely generated link to the user which will allow him to reset its password. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Eric Bégin <eric.begin@inLibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nengard@gmail.com --- Comment #2 from Eric Bégin <eric.begin@inLibro.com> --- *** Bug 3059 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Version|3.10 |master -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |chris@bigballofwax.co.nz --- Comment #3 from Chris Cormack <chris@bigballofwax.co.nz> --- (In reply to comment #1)
A better way to handle this is to send a uniquely generated link to the user which will allow him to reset its password.
I agree, this is the common best practice, we should never be sending passwords to users (unless they are onetime use only) ... i'd like to remove the code that already does this in staff, rather than adding more. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #4 from Nicole C. Engard <nengard@gmail.com> --- This says it's assigned - is someone actually working on this? Nicole -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #5 from Eric Bégin <eric.begin@inLibro.com> --- We actually have this code running on our 3.10.x version. It's juste a matter of finding time to merge on the main branch... -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Blou <philippe.blouin@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |philippe.blouin@inlibro.com --- Comment #6 from Blou <philippe.blouin@inlibro.com> --- Created attachment 21502 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=21502&action=edit Add forgot password link to OPAC -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #7 from Blou <philippe.blouin@inlibro.com> --- TEST PLAN: 1) apply the patch 2) go to system preferences OPAC>>Privacy and set 'OpacResetPassword' to ON. That will cause the link 'Forgot your password' to show up on the welcome page, below connection box. 3) refresh front page, click on 'Forgot your password' and enter a VALID address (one that is associated to an entry in borrowers.email or borrowers.email_pro. 3b) Also try an INVALID address (valid yet not in your koha db). An error message will show up. 4) An email should be received at that address with a link. 5) Follow the link in the mail to fill the new password. Until a satisfactory new password is entered, the old password is not reset. 6) Go to main page try the new password. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Zeno Tajoli <z.tajoli@cineca.it> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff CC| |z.tajoli@cineca.it -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Blou <philippe.blouin@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|eric.begin@inLibro.com |philippe.blouin@inlibro.com -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Blou <philippe.blouin@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #21502|0 |1 is obsolete| | --- Comment #8 from Blou <philippe.blouin@inlibro.com> --- Created attachment 21512 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=21512&action=edit Add forgot password link to OPAC The commit message has been updated to match the standards. Sponsor added. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #9 from Owen Leonard <oleonard@myacpl.org> --- Looks like this requires an update to the database, but I don't see any changes in updatedatabase.pl. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #10 from Blou <philippe.blouin@inlibro.com> --- Created attachment 21521 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=21521&action=edit Bug 8753 - Add forgot password link to OPAC Includes the database scripts modifications. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #11 from Blou <philippe.blouin@inlibro.com> --- The second patch "Bug 8753 - Add forgot password link to OPAC" contains the db scripts. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Blou <philippe.blouin@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 I'm just a bot <gitbot@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |gitbot@bugs.koha-community. | |org When did the bot| |2013-09-29 last check this| | --- Comment #12 from I'm just a bot <gitbot@bugs.koha-community.org> --- Patch applied cleanly, go forth and signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Blou <philippe.blouin@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #21512|0 |1 is obsolete| | Attachment #21521|0 |1 is obsolete| | --- Comment #13 from Blou <philippe.blouin@inlibro.com> --- Created attachment 21849 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=21849&action=edit Add forgot password link to OPAC Following what I learned last week, I 1) fixed updatedatabase.pl ('' for escaping quotes) 2) put setprefs.sql (alphabetical order) 3) renamed a variable (very minor) 4) packaged it all in one new patch. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 JD@TeTakere <joanned@tetakere.org.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Patch doesn't apply CC| |joanned@tetakere.org.nz --- Comment #14 from JD@TeTakere <joanned@tetakere.org.nz> --- Patch didn't apply one the sandboxes. There was a conflict in updatedatabase.pl -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #21849|0 |1 is obsolete| | --- Comment #15 from Chris Cormack <chris@bigballofwax.co.nz> --- Created attachment 22175 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=22175&action=edit Rebased patch -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #16 from Blou <philippe.blouin@inlibro.com> --- Thank you sir! I barely had time to notice the issue email then set up my environment to rebase, that you had already solved it. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Blou <philippe.blouin@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Needs Signoff --- Comment #17 from Blou <philippe.blouin@inlibro.com> --- Changing the status from "Patch doesn't apply" to "Need signoff". Not sure if that was needed since rebaser knows the business, please let me if otherwise. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA CC| |kyle@bywatersolutions.com --- Comment #18 from Kyle M Hall <kyle@bywatersolutions.com> --- Looks like the table borrower_password_recovery was left out of the database update by accident. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #19 from Blou <philippe.blouin@inlibro.com> --- Created attachment 22430 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=22430&action=edit Bug 8753 - Add forgot password link to OPAC Added the missing updatedatabase.pl script to add the table borrower_password_recovery and the preference OpacResetPassword. Modified kohastructure.sql to use InnoDB -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Blou <philippe.blouin@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff --- Comment #20 from Blou <philippe.blouin@inlibro.com> --- Well, seems I'm fully forgot updatedatabase.pl in first patch. Rebased and fixed. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@biblibre.co | |m --- Comment #21 from Jonathan Druart <jonathan.druart@biblibre.com> --- Early QA comment: 1/ new routine needs unit tests 2/ remove tab characters 3/ perltidy new pl script 4/ use Modern::Perl; (you only use strict) 5/ only 1 syspref is added to sysprefs.sql 6/ why don't you sent the email using utf8? 7/ on generating the new password char, maybe should you use the alphabet syspref (+ numbers) 8/ why don't you use koha opac header for opac-send-password-recovery template file? 9/ use the tt plugin to access to syspref value (Koha.Preference("mypref")). 10/ what is the 'UNAUTHORIZ' check (in opac-password-recovery.pl)? 10 is a good number. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 marjorie barry-vila <marjorie.barry-vila@ccsr.qc.ca> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |marjorie.barry-vila@ccsr.qc | |.ca -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Blou <philippe.blouin@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #22175|0 |1 is obsolete| | Attachment #22430|0 |1 is obsolete| | --- Comment #22 from Blou <philippe.blouin@inlibro.com> --- Created attachment 24586 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=24586&action=edit Bug 8753 - Add forgot password link to OPAC This includes fix following the most recent comments, as well as full bootstrap and rebasing to 3.15. TEST PLAN: 1) apply the patch 2) go to system preferences OPAC>>Privacy and set 'OpacResetPassword' to ON. That will cause the link 'Forgot your password' to show up on the welcome page, below connection box. 3) refresh front page, click on 'Forgot your password' and enter a VALID address (one that is associated to an entry in borrowers.email or borrowers.email_pro. 3b) Also try an INVALID address (valid yet not in your koha db). An error message will show up. 4) An email should be received at that address with a link. 5) Follow the link in the mail to fill the new password. Until a satisfactory new password is entered, the old password is not reset. 6) Go to main page try the new password. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #23 from M. Tompsett <mtompset@hotmail.com> --- Comment on attachment 24586 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=24586 Bug 8753 - Add forgot password link to OPAC Review of attachment 24586: --> (http://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=8753&attachment=24586) ----------------------------------------------------------------- ::: C4/Auth.pm @@ +1068,5 @@
PatronSelfRegistrationDefaultCategory => C4::Context->preference("PatronSelfRegistrationDefaultCategory"), persona => C4::Context->preference("Persona"), opac_css_override => $ENV{'OPAC_CSS_OVERRIDE'}, + OpacResetPassword => C4::Context->preference("OpacResetPassword"), + OpacPasswordChange => C4::Context->preference("OpacPasswordChange"),
No longer necessary. ::: C4/Members.pm @@ +2273,5 @@
+ my $uuid_str; + $uuid_str .= $chars[rand @chars] for 1..32; + + #insert into database + my $sth = $dbh->prepare( 'INSERT INTO borrower_password_recovery VALUES (? ,? , ADDDATE(NOW(), INTERVAL 2 DAY) )' );
Is this ANSI SQL? @@ +2281,5 @@
+ #define to/from emails + my $kohaEmail = C4::Context->preference( 'KohaAdminEmailAddress' ); + + #create link + my $uuidLink = "http://" . C4::Context->preference( 'OPACBaseURL' ) . "/cgi-bin/koha/opac-password-recovery.pl?uniqueKey=$uuid_str";
Does someone have that funky code to determine http vs https in a URL handy? Please don't hard code http. ::: koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref @@ +556,5 @@
+ default: 1 + choices: + yes: "On" + no: "Off" + - ". If On, the user can reset his password on OPAC."
There are prettier ways to format this. - - "The user " - pref: OpacResetPassword default: 1 choices: yes: "can reset" no: "can not reset" - " their password on OPAC." ::: koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-password-recovery.tt @@ +48,5 @@
+ More than one account has been found for the email address: "<strong>[% email %]</strong>" + <br/>Try yo use your alternative email if you have another. + [% ELSIF (errAlreadyStartRecovery) %] + The process of password recovery has already started for this account ("<strong>[% email %]</strong>") + <br/>Check your emails; you should recieve the link to reset your password.
receive typo. @@ +74,5 @@
+ +[% ELSIF (new_password) %] + [% IF (errLinkNotValid) %] + <span class="TxtErreur"><h6> + We could not authentify you as the account owner.
authenticate ::: koha-tmpl/opac-tmpl/prog/en/modules/opac-auth.tt @@ +86,5 @@
<input type="submit" value="Log In" class="submit" /> +[% IF Koha.Preference('OpacPasswordChange') && Koha.Preference('OpacResetPassword') %] + <p><a href="/cgi-bin/koha/opac-password-recovery.pl">Forgot your password?</a></p> +[% END %]
Could this be in the nologininstructions div? ::: koha-tmpl/opac-tmpl/prog/en/modules/opac-password-recovery.tt @@ +46,5 @@
+ More than one account has been found for the email address: "<strong>[% email %]</strong>" + <br/>Try yo use your alternative email if you have another. + [% ELSIF (errAlreadyStartRecovery) %] + The process of password recovery has already started for this account ("<strong>[% email %]</strong>") + <br/>Check your emails; you should recieve the link to reset your password.
receive typo. @@ +72,5 @@
+ +[% ELSIF (new_password) %] + [% IF (errLinkNotValid) %] + <span class="TxtErreur"><h6> + We could not authentify you as the account owner.
authenticate ::: opac/opac-main.pl @@ +58,4 @@
koha_news_count => $koha_news_count, display_daily_quote => C4::Context->preference('QuoteOfTheDay'), daily_quote => $quote, + OpacResetPassword => C4::Context->preference('OpacResetPassword'),
You tweaked opac-main.tt to use the Koha.Preference method of grabbing system preferences in the template. There's no need to modify this file. ::: opac/opac-password-recovery.pl @@ +48,5 @@
+my $errPassTooShort; + +my $dbh = C4::Context->dbh; + +$template->param( OpacResetPassword => C4::Context->preference("OpacResetPassword") );
This is not needed. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA CC| |mtompset@hotmail.com --- Comment #24 from M. Tompsett <mtompset@hotmail.com> --- Please look at my eyeball comments, and deal with this: FAIL koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-auth.tt FAIL forbidden patterns forbidden pattern: tab char (line 98) forbidden pattern: tab char (line 100) forbidden pattern: tab char (line 99) OK tt_valid OK valid_template -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #25 from M. Tompsett <mtompset@hotmail.com> --- Comment on attachment 24586 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=24586 Bug 8753 - Add forgot password link to OPAC Review of attachment 24586: --> (http://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=8753&attachment=24586) ----------------------------------------------------------------- ::: opac/opac-password-recovery.pl @@ +102,5 @@
+ email => HTML::Entities::encode($email), + ); + } + else { + #generate uuid and send recovery email
Bug 11575 has: ($in->{'query'}->https() ? "https://" : "http://") $query->https() may be what you need. Some way of passing that to SendPasswordRecoveryEmail, so you don't hardcode http or https. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #26 from Blou <philippe.blouin@inlibro.com> --- Created attachment 24780 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=24780&action=edit Fixes all comments specified by mtompsett. Same testing procedure. Rebased. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Blou <philippe.blouin@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #24586|0 |1 is obsolete| | Attachment #24780|0 |1 is obsolete| | --- Comment #27 from Blou <philippe.blouin@inlibro.com> --- Created attachment 24781 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=24781&action=edit Bug 8753 - Add forgot password link to OPAC This includes fix following the most recent comments and a rebasing to 3.15.15 TEST PLAN: 1) apply the patch 2) go to system preferences OPAC>>Privacy and set 'OpacResetPassword' to ON. That will cause the link 'Forgot your password' to show up on the welcome page, below connection box. 3) refresh front page, click on 'Forgot your password' and enter a VALID address (one that is associated to an entry in borrowers.email or borrowers.email_pro. 3b) Also try an INVALID address (valid yet not in your koha db). An error message will show up. 4) An email should be received at that address with a link. 5) Follow the link in the mail to fill the new password. Until a satisfactory new password is entered, the old password is not reset. 6) Go to main page try the new password. Fixes all comments specified by mtompsett. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #28 from Chris Cormack <chris@bigballofwax.co.nz> --- Comment on attachment 24781 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=24781 Bug 8753 - Add forgot password link to OPAC Review of attachment 24781: --> (http://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=8753&attachment=24781) ----------------------------------------------------------------- ::: opac/opac-password-recovery.pl @@ +135,5 @@
+ if ( ($borrower_number) + && ( $password eq $repeatPassword ) + && ( length($password) >= $minPassLength ) ) + { #apply changes + changepassword( $username, $borrower_number, md5_base64($password) );
We no longer md5 hash the password in Koha, you should do use Koha::AuthUtils; hash_password($password) instead. (It's using bcrypt with a salt). Old passwords still work, we check both, but anytime people change their password it should be hashed with the much more secure bcrypt -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Blou <philippe.blouin@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #24781|0 |1 is obsolete| | --- Comment #29 from Blou <philippe.blouin@inlibro.com> --- Created attachment 24846 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=24846&action=edit Bug 8753 - Add forgot password link to OPAC Removed md5 usage. Replaced with AuthUtils' hash_password. System preference 'OPACBaseUrl' must be set for this feature to work properly (otherwise, the link sent in the email will be invalid). -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #30 from M. Tompsett <mtompset@hotmail.com> --- Comment on attachment 24846 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=24846 Bug 8753 - Add forgot password link to OPAC Review of attachment 24846: --> (http://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=8753&attachment=24846) ----------------------------------------------------------------- ::: C4/Auth.pm @@ +1068,4 @@
PatronSelfRegistrationDefaultCategory => C4::Context->preference("PatronSelfRegistrationDefaultCategory"), persona => C4::Context->preference("Persona"), opac_css_override => $ENV{'OPAC_CSS_OVERRIDE'}, + OpacResetPassword => C4::Context->preference("OpacResetPassword"),
You added OpacResetPassword, and all the templates use the Koha system preference grabbing plugin. There's no need for this. @@ +1068,5 @@
PatronSelfRegistrationDefaultCategory => C4::Context->preference("PatronSelfRegistrationDefaultCategory"), persona => C4::Context->preference("Persona"), opac_css_override => $ENV{'OPAC_CSS_OVERRIDE'}, + OpacResetPassword => C4::Context->preference("OpacResetPassword"), + OpacPasswordChange => C4::Context->preference("OpacPasswordChange"),
This wasn't here, so I don't see a reason to add it, since all the template changes you made use the Koha system preference grabbing plugin. ::: opac/opac-main.pl @@ +58,4 @@
koha_news_count => $koha_news_count, display_daily_quote => C4::Context->preference('QuoteOfTheDay'), daily_quote => $quote, + OpacResetPassword => C4::Context->preference('OpacResetPassword'),
There's no point to have this. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #31 from Blou <philippe.blouin@inlibro.com> --- Created attachment 24853 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=24853&action=edit Removed unnecessary references in template calls. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #32 from M. Tompsett <mtompset@hotmail.com> --- Comment on attachment 24846 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=24846 Bug 8753 - Add forgot password link to OPAC Review of attachment 24846: --> (http://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=8753&attachment=24846) ----------------------------------------------------------------- ::: C4/Members.pm @@ +2266,5 @@
+ +sub SendPasswordRecoveryEmail { + my $borrowernumber = shift; + my $email = shift; + my $query = shift;
Perhaps with the code later you were trying to handle: my $query = shift // CGI->new(); @@ +2285,5 @@
+ #define to/from emails + my $kohaEmail = C4::Context->preference( 'KohaAdminEmailAddress' ); + + #create link + my $https = $query->https() ? "https://" : "http://";
I believe I have seen others call this protocol, but changing this isn't necessary. @@ +2290,5 @@
+ my $uuidLink = $https . C4::Context->preference( 'OPACBaseURL' ) . "/cgi-bin/koha/opac-password-recovery.pl?uniqueKey=$uuid_str"; + #warn $uuidLink; + + #build email content + my $query = new CGI;
Why do you need this?! You have line 2270. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #33 from Blou <philippe.blouin@inlibro.com> --- Created attachment 24854 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=24854&action=edit Removed unnecessary argument in call to SendPasswordRecoveryEmail. Improved var naming -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Blou <philippe.blouin@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #24846|0 |1 is obsolete| | Attachment #24853|0 |1 is obsolete| | Attachment #24854|0 |1 is obsolete| | --- Comment #34 from Blou <philippe.blouin@inlibro.com> --- Created attachment 24855 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=24855&action=edit Bug 8753 - Add forgot password link to OPAC This includes fix following the most recent comments and a rebasing to 3.15.15 Fixes all comments specified by mtompsett. TEST PLAN: 1) apply the patch 2) go to system preferences OPAC>>Privacy and set 'OpacResetPassword' to ON. That will cause the link 'Forgot your password' to show up on the welcome page, below connection box. 3) refresh front page, click on 'Forgot your password' and enter a VALID address (one that is associated to an entry in borrowers.email or borrowers.email_pro. 3b) Also try an INVALID address (valid yet not in your koha db). An error message will show up. 4) An email should be received at that address with a link. 5) Follow the link in the mail to fill the new password. Until a satisfactory new password is entered, the old password is not reset. 6) Go to main page try the new password. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #35 from M. Tompsett <mtompset@hotmail.com> --- Comment on attachment 24855 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=24855 Bug 8753 - Add forgot password link to OPAC Review of attachment 24855: --> (http://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=8753&attachment=24855) ----------------------------------------------------------------- ::: C4/Auth.pm @@ +1068,5 @@
PatronSelfRegistrationDefaultCategory => C4::Context->preference("PatronSelfRegistrationDefaultCategory"), persona => C4::Context->preference("Persona"), opac_css_override => $ENV{'OPAC_CSS_OVERRIDE'}, + OpacResetPassword => C4::Context->preference("OpacResetPassword"), + OpacPasswordChange => C4::Context->preference("OpacPasswordChange"),
They're back. :( ::: opac/opac-main.pl @@ +58,4 @@
koha_news_count => $koha_news_count, display_daily_quote => C4::Context->preference('QuoteOfTheDay'), daily_quote => $quote, + OpacResetPassword => C4::Context->preference('OpacResetPassword'),
Oops. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Blou <philippe.blouin@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #24855|0 |1 is obsolete| | --- Comment #36 from Blou <philippe.blouin@inlibro.com> --- Created attachment 24862 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=24862&action=edit Bug 8753 - Add forgot password link to OPAC This includes fix following the most recent comments and a rebasing to 3.15.15 Fixes all comments specified by mtompsett. TEST PLAN: 1) apply the patch 2) go to system preferences OPAC>>Privacy and set 'OpacResetPassword' to ON. That will cause the link 'Forgot your password' to show up on the welcome page, below connection box. 3) refresh front page, click on 'Forgot your password' and enter a VALID address (one that is associated to an entry in borrowers.email or borrowers.email_pro. 3b) Also try an INVALID address (valid yet not in your koha db). An error message will show up. 4) An email should be received at that address with a link. 5) Follow the link in the mail to fill the new password. Until a satisfactory new password is entered, the old password is not reset. 6) Go to main page try the new password. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #37 from M. Tompsett <mtompset@hotmail.com> --- You forget to delete the password reset entry after resetting the password. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #38 from M. Tompsett <mtompset@hotmail.com> --- Please test with /cgi-bin/koha/opac-password-recovery.pl?uniqueKey=OneThatDoesNotExist 404.pl is not the correct behaviour, in my opinion. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #39 from Blou <philippe.blouin@inlibro.com> --- (In reply to M. Tompsett from comment #37)
You forget to delete the password reset entry after resetting the password.
You mean from table borrower_password_recovery ? I validated it does delete the entry. And I can't reproduce the 404 of your next comment. When I append your line to my opac base url, it sends me to a message: " We could not authenticate you as the account owner. Be sure to use the link you received in your email. " Are you sure the patch was correctly applied? Thanks again for your time! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #40 from Blou <philippe.blouin@inlibro.com> --- Created attachment 24906 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=24906&action=edit Improved UGLY code (sic). -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #41 from Blou <philippe.blouin@inlibro.com> --- Created attachment 24907 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=24907&action=edit hash vs hashref issue -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Blou <philippe.blouin@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #24862|0 |1 is obsolete| | Attachment #24906|0 |1 is obsolete| | Attachment #24907|0 |1 is obsolete| | --- Comment #42 from Blou <philippe.blouin@inlibro.com> --- Created attachment 24908 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=24908&action=edit Bug 8753 - Add forgot password link to OPAC This includes fix following the most recent comments and a rebasing to 3.15.15 Fixes all comments specified by mtompsett. TEST PLAN: 1) apply the patch 2) go to system preferences OPAC>>Privacy and set 'OpacResetPassword' to ON. That will cause the link 'Forgot your password' to show up on the welcome page, below connection box. 2b) make sure that OpacPasswordChange is also ON. 3) refresh front page, click on 'Forgot your password' and enter a VALID address (one that is associated to an entry in borrowers.email or borrowers.email_pro. 3b) Also try an INVALID address (valid yet not in your koha db). An error message will show up. 4) An email should be received at that address with a link. 5) Follow the link in the mail to fill the new password. Until a satisfactory new password is entered, the old password is not reset. 6) Go to main page try the new password. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #43 from M. Tompsett <mtompset@hotmail.com> --- The generate a new link process can create multiple links (potentially huge number) for a single user. It should overwrite the existing one. No more than one entry per borrowernumber. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #44 from Blou <philippe.blouin@inlibro.com> --- Created attachment 25038 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=25038&action=edit Bug 8753 - Add forgot password link to OPAC (patch) When asking for a new link when one was already sent now causes an update of the one in the DB. This - makes the previous link invalid - reset the timestamp to allow for two more days. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Blou <philippe.blouin@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #24908|0 |1 is obsolete| | Attachment #25038|0 |1 is obsolete| | --- Comment #45 from Blou <philippe.blouin@inlibro.com> --- Created attachment 25039 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=25039&action=edit Bug 8753 - Add forgot password link to OPAC This includes fix following the most recent comments and a rebasing through db 3.15.00.16 Fixes all comments specified by mtompsett. TEST PLAN: 1) apply the patch 2) go to system preferences OPAC>>Privacy and set 'OpacResetPassword' to ON. That will cause the link 'Forgot yo 2b) make sure that OpacPasswordChange is also ON. 3) refresh front page, click on 'Forgot your password' and enter a VALID address (one that is associated to an en 3b) Also try an INVALID address (valid yet not in your koha db). An error message will show up. 4) An email should be received at that address with a link. 5) Follow the link in the mail to fill the new password. Until a satisfactory new password is entered, the old password is not reset. 6) Go to main page try the new password. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #46 from Blou <philippe.blouin@inlibro.com> --- Created attachment 25051 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=25051&action=edit Delete the exprired links after (any) successful reset -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #47 from M. Tompsett <mtompset@hotmail.com> --- Vast improvement on the DELETE. Sadly, still have two problems. "Framing Problem" Click forget password link Type in username and password to log in to OPAC Click link in email. (screen 1) Log out of OPAC Click link in email. (screen 2) Both screens are framed differently. "Email Problem" http://irc.koha-community.org/koha/2014-02-04#i_1460899 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #48 from Owen Leonard <oleonard@myacpl.org> --- I notice this feature is enabled by default. I think it should be disabled by default. There are many problems with the template which I'll work on a follow-up for. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #49 from Owen Leonard <oleonard@myacpl.org> --- For what it's worth, I tested this and didn't get a password recovery email. I noticed in trying to understand how the emails should be sent that the feature doesn't use notice templates or the message queue. Should we be adding a feature that uses yet another method for handling emails? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Francois Charbonnier <francharb@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |francharb@gmail.com -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #50 from M. Tompsett <mtompset@hotmail.com> --- My Ubuntu VM has email set up in a manner similar to https://help.ubuntu.com/community/Postfix This uses the Mail::SendMail library directly, which is already part of the dependencies for Koha. Checkout the code in C4/Members.pm around line 2314 after applying the patch. The "if ( sendmail %mail )" is what actually does the emailing. If you don't have postfix configured, then nullmailer is installed. Hopefully, that is configured correctly. I have yet to read sensible and useful documentation on it. Some mail hosts delay sending/receiving when mail is relayed. Patience may be required while testing this. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #51 from David Cook <dcook@prosentient.com.au> --- (In reply to M. Tompsett from comment #50)
My Ubuntu VM has email set up in a manner similar to https://help.ubuntu.com/community/Postfix
This uses the Mail::SendMail library directly, which is already part of the dependencies for Koha. Checkout the code in C4/Members.pm around line 2314 after applying the patch. The "if ( sendmail %mail )" is what actually does the emailing.
If you don't have postfix configured, then nullmailer is installed. Hopefully, that is configured correctly. I have yet to read sensible and useful documentation on it.
Some mail hosts delay sending/receiving when mail is relayed. Patience may be required while testing this.
While other parts of Koha do rely on Sendmail directly, I think Owen has a very good point. We should definitely be using the notice templates, and we should probably be using the message queue. The notice templates allow libraries to provide their own wording/translations to the message. I think that's a *must*. Using the message queue also helps Koha administrators to track the messages going out, since they might not have direct access to the MTA or mail server. Owen: I'm not sure what your testing scenario is but I noticed recently that my Koha VM's MTA is configured out of the box not to send email outside of its local domain. I don't know if that's relevant for you though. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Heather Braum <hbraum@nekls.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |hbraum@nekls.org --- Comment #52 from Heather Braum <hbraum@nekls.org> --- I came into Koha today looking to see if there was any work on a patron being able to reset a forgotten password, and was happy to see this bug in progress. I want to +1 on the use of the existing notice templates and built-in messaging system in Koha for this feature. As the manager of a large, multi-branch system, that just gained access to the ability to finally have ALL notices sent from the branch email not the admin email (thank you developers for bugfix #5544!), I would hate to see password reset messages separately again come from the admin email address. Using the message queue will also be able to help us troubleshoot and work with patrons who repeatedly say "I reset my password but it didn't work." I hope this patch is eventually successful! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Magnus Enger <magnus@enger.priv.no> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |magnus@enger.priv.no -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Blou <philippe.blouin@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #25039|0 |1 is obsolete| | Attachment #25051|0 |1 is obsolete| | --- Comment #53 from Blou <philippe.blouin@inlibro.com> --- Created attachment 29111 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=29111&action=edit Bug 8753 - Add forgot password link to OPAC This redo includes the following - usage of the template letters for the email instead of a .tt (that feature didn't exist when the functionnality was initially coded for our client) - removal of the prog version - removal of the code that was initially put into Members.pm - Added a letter into updatedatabase.pl and sample_notices.sql - Of course, rebase to latest master. The rest remain unchanged since the previous comments/approvals. As such, what worked before should still work. TEST PLAN: 1) apply the patch 2) go to system preferences OPAC>>Privacy and set 'OpacResetPassword' to ON. That will cause the link 'Forgot yo 2b) make sure that OpacPasswordChange is also ON. 3) refresh front page, click on 'Forgot your password' and enter a VALID address (one that is associated to an en 3b) Also try an INVALID address (valid yet not in your koha db). An error message will show up. 4) An email should be received at that address with a link. 5) Follow the link in the mail to fill the new password. Until a satisfactory new password is entered, the old password is not reset. 6) Go to main page try the new password. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Liz Rea <liz@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff CC| |liz@catalyst.net.nz -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |ASSIGNED --- Comment #54 from M. Tompsett <mtompset@hotmail.com> --- With the moved to DBIx, the Schema file needs to be created as well. Hence, the change back to ASSIGNED. Sorry. Wasn't me that changed the underlying technologies in mid-stream. :( -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Jacek Ablewicz <abl@biblos.pk.edu.pl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |abl@biblos.pk.edu.pl -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 dmin <daniel.minuck@gov.mb.ca> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |daniel.minuck@gov.mb.ca -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Indranil Das Gupta <indradg@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |indradg@gmail.com -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #55 from simith.doliveira@inlibro.com --- Created attachment 33175 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=33175&action=edit Bug 8753 - Add forgot password link to OPAC - Updated -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #56 from simith.doliveira@inlibro.com --- Created attachment 33176 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=33176&action=edit Bug 8753 - Add forgot password link to OPAC - DBIx Schema Added -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 simith.doliveira@inlibro.com changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff CC| |simith.doliveira@inlibro.co | |m -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 M. de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #57 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- Interesting! Just some thoughts: The SQL code in the opac-recovery script will not make it pass QA. Please move it to module level (in DBIx?). Can you unit test SendPasswordRecoveryEmail? I would not mind a mail with a library password; other info is more sensitive. If you can read/intercept the password from the mail, you can also read the unique userid for the reset password form. Same result: a hacked account. This approach is fine with me, feels more safe but is not per se safer imo. Thinking out loud: if you do not include sensitive keywords as "password" in your mail (so rename your script?), would that be little more safe? Would not attract attention? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #58 from Chris Cormack <chris@bigballofwax.co.nz> --- (In reply to M. de Rooy from comment #57)
Interesting! Just some thoughts: The SQL code in the opac-recovery script will not make it pass QA. Please move it to module level (in DBIx?). Can you unit test SendPasswordRecoveryEmail?
I would not mind a mail with a library password; other info is more sensitive. If you can read/intercept the password from the mail, you can also read the unique userid for the reset password form. Same result: a hacked account. This approach is fine with me, feels more safe but is not per se safer imo.
I disagree, a token should be time based, and will require the person to set a password. This password only they will know. It's true that if someone intercepts the token and uses it to set your password for you, that is still a problem. However when you go to use the token and find out it has been used you will know this. If they intercept your password, they could use it for a bunch of time without you ever knowing. Sending passwords in the clear is a bad idea. Please do not do it. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de --- Comment #59 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I agree with Chris, I think. Also we have a similar approach for the patron verification - sending a link to a page where you learn about your password. It seems a good way to do this. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #60 from Eric Bégin <eric.begin@inLibro.com> --- The token sent to the user is valid only for 2 days or if the link is clicked. Most of the time, a use who requests a password will click the link in the following minutes (if not seconds). Of course, this doesn't prevent your email to be intercepted. As far as I know, this feature was implemented the way most of the « Forgot your password ? » links are implemented. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #61 from Liz Rea <liz@catalyst.net.nz> --- Also, if you send the password in the clear, if it is intercepted once, and you are one of (more people than we'd like to admit) those people who uses the same password in multiple accounts, it's not only your library account that is now possibly hacked. We should not ever, ever, ever show a password in the clear. Ever. I am glad this works the way it does, it is the proper way. Liz -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #62 from Eric Bégin <eric.begin@inLibro.com> --- Juste wondering how this clear text password came in the discussion. That was never the case with this development. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #63 from Liz Rea <liz@catalyst.net.nz> --- Comment 57, 2nd paragraph - when someone doing QA suggests mailing a cleartext password might be ok, it seems right to emphatically tell them "no, that's a bad idea." I for one appreciate the fact that it was never part of this dev, thanks for doing it properly. :) Liz -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #64 from M. Tompsett <mtompset@hotmail.com> --- (In reply to M. de Rooy from comment #57)
Interesting! Just some thoughts: The SQL code in the opac-recovery script will not make it pass QA. Please move it to module level (in DBIx?). Can you unit test SendPasswordRecoveryEmail?
Though, I would make sure this comment didn't get missed in the midst of everyone discussing password logic. M. de Rooy, are you talking about just the SELECT? What about the DELETE? Could you clarify which bits you mean by commenting on a splinter review? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #65 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- We don't want any SQL code in the .pl files - so I'd say both. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #66 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Liz Rea from comment #63)
Comment 57, 2nd paragraph - when someone doing QA suggests mailing a cleartext password might be ok, it seems right to emphatically tell them "no, that's a bad idea."
Please keep it in context. Hope I do not intercept any undertone about QA work here? For completeness: I completely agree that sending password by mail is theoretically very bad. However, we probably do not have a 100% safe solution for this. So therefore in a pragmatic view the sensitivity argument etc. And to add: As long as many OPACs are going over HTTP, we send the password clear text all over the place.. But again, also comment 57 This approach is fine with me! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #67 from David Cook <dcook@prosentient.com.au> --- (In reply to M. de Rooy from comment #66)
(In reply to Liz Rea from comment #63)
Comment 57, 2nd paragraph - when someone doing QA suggests mailing a cleartext password might be ok, it seems right to emphatically tell them "no, that's a bad idea."
Please keep it in context. Hope I do not intercept any undertone about QA work here?
For completeness: I completely agree that sending password by mail is theoretically very bad. However, we probably do not have a 100% safe solution for this. So therefore in a pragmatic view the sensitivity argument etc. And to add: As long as many OPACs are going over HTTP, we send the password clear text all over the place..
But again, also comment 57 This approach is fine with me!
When it comes to security/safety, there is never a 100% guarantee. However, there are better and worse ways of doing things. It sounds like the approach used in these patches is a reasonable one. As Eric mentioned, it's the "standard" way of doing it. As for sending passwords in the clear when using HTTP, that's not an issue with Koha's code. That's an end-user configuration issue. If people are using HTTP instead of HTTPS, that's their responsibility. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #68 from Owen Leonard <oleonard@myacpl.org> --- If you use the same password recover link twice you get an error which is inaccurate: "We could not authenticate you as the account owner Be sure to use the link you received in your email." It might make sense to retain entries in the borrower_password_recovery table for a certain amount of time after they have been "used" so that we can accurately tell the user that they have followed a link which is out of date. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #69 from Owen Leonard <oleonard@myacpl.org> --- opac/opac-password-recovery.pl line 163: warn "INLIBRO username $username"; The script contains a lot of SQL, which is a violation of coding guidelines: http://wiki.koha-community.org/wiki/Coding_Guidelines#SQL8:_SQL_code_in_.pl_... The template contains a check for the variable "errTooManyEmailFound" but I don't see where that variable is created. In fact there doesn't seems to be any handling of multiple identical email addresses. This is a problem for my library where parents often put their email addresses on their kids' accounts. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #70 from simith.doliveira@inlibro.com --- Created attachment 34472 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=34472&action=edit SQL code in the .pl files removed. New .pm and .t files created. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 simith.doliveira@inlibro.com changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |veron@veron.ch -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #71 from Marc Véron <veron@veron.ch> --- Does not apply Applying: Bug 8753 - Add forgot password link to OPAC Using index info to reconstruct a base tree... <stdin>:471: trailing whitespace. <stdin>:500: trailing whitespace. ); <stdin>:511: trailing whitespace. } ); <stdin>:25: new blank line at EOF. + <stdin>:126: new blank line at EOF. + warning: squelched 1 whitespace error warning: 6 lines add whitespace errors. Falling back to patching base and 3-way merge... Auto-merging koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-auth.tt Auto-merging koha-tmpl/opac-tmpl/bootstrap/en/includes/masthead.inc Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref Auto-merging installer/data/mysql/updatedatabase.pl CONFLICT (content): Merge conflict in installer/data/mysql/updatedatabase.pl Auto-merging installer/data/mysql/sysprefs.sql Auto-merging installer/data/mysql/kohastructure.sql Failed to merge in the changes. Patch failed at 0001 Bug 8753 - Add forgot password link to OPAC -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Patch doesn't apply -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #72 from M. Tompsett <mtompset@hotmail.com> --- simith.doliveira@inlibro.com, could you squash your patches together too? Just my preference. :) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #73 from Owen Leonard <oleonard@myacpl.org> --- (In reply to M. Tompsett from comment #72)
Just my preference. :)
Last I heard it was not the RM's preference, but that may have been a previous RM. Perhaps this is something we should add to the coding guidelines? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #74 from M. Tompsett <mtompset@hotmail.com> --- The patches were never signed off. I figure a squash isn't so bad at this stage. :) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 simith.doliveira@inlibro.com changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #29111|0 |1 is obsolete| | --- Comment #75 from simith.doliveira@inlibro.com --- Created attachment 36819 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=36819&action=edit Bug 8753 - Add forgot password link to OPAC This redo includes the following - usage of the template letters for the email instead of a .tt (that feature didn't exist when the functionnality was initially coded for our client) - removal of the prog version - removal of the code that was initially put into Members.pm - Added a letter into updatedatabase.pl and sample_notices.sql - Of course, rebase to latest master. The rest remain unchanged since the previous comments/approvals. As such, what worked before should still work. TEST PLAN: 1) apply the patch 2) go to system preferences OPAC>>Privacy and set 'OpacResetPassword' to ON. That will cause the link 'Forgot yo 2b) make sure that OpacPasswordChange is also ON. 3) refresh front page, click on 'Forgot your password' and enter a VALID address (one that is associated to an en 3b) Also try an INVALID address (valid yet not in your koha db). An error message will show up. 4) An email should be received at that address with a link. 5) Follow the link in the mail to fill the new password. Until a satisfactory new password is entered, the old password is not reset. 6) Go to main page try the new password. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 simith.doliveira@inlibro.com changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #34472|0 |1 is obsolete| | --- Comment #76 from simith.doliveira@inlibro.com --- Comment on attachment 34472 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=34472 SQL code in the .pl files removed. New .pm and .t files created.
From b775b68292a4c9757f7d0b93d2675d9165c18cc4 Mon Sep 17 00:00:00 2001 From: simith <simith@inlibro.com> Date: Tue, 16 Dec 2014 13:22:26 -0500 Subject: [PATCH] SQL code in the .pl files removed. New .pm and .t files created.
http://bugs.koha-community.org/show_bug.cgi?id=8753 --- C4/Passwordrecovery.pm | 159 +++++++++++++++++++++ .../data/mysql/en/mandatory/sample_notices.sql | 2 +- opac/opac-password-recovery.pl | 100 ++----------- 3 files changed, 173 insertions(+), 88 deletions(-) create mode 100644 C4/Passwordrecovery.pm
diff --git a/C4/Passwordrecovery.pm b/C4/Passwordrecovery.pm new file mode 100644 index 0000000..f1b26e9 --- /dev/null +++ b/C4/Passwordrecovery.pm @@ -0,0 +1,159 @@ +package C4::Passwordrecovery; + +# Copyright 2014 PTFS Europe +# +# This file is part of Koha. +# +# Koha is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# Koha is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Koha; if not, see <http://www.gnu.org/licenses>. + +use Modern::Perl; +use C4::Context; + +use vars qw($VERSION @ISA @EXPORT); + +BEGIN { + # set the version for version checking + $VERSION = 3.07.00.049; + require Exporter; + @ISA = qw(Exporter); + push @EXPORT, qw( + &ValidateBorrowernumber + &SendPasswordRecoveryEmail + &GetValidLinkInfo + ); +} + +=head1 NAME + +C4::Passwordrecovery - Koha password recovery module + +=head1 SYNOPSIS + +use C4::Passwordrecovery; + +=head1 FUNCTIONS + +=head2 ValidateBorrowernumber + +$alread = ValidateBorrowernumber( $borrower_number ); + +Check if the system already start recovery + +Returns true false + +=cut + +sub ValidateBorrowernumber { + my ($borrower_number) = @_; + my $schema = Koha::Database->new->schema; + + my $rs = $schema->resultset('BorrowerPasswordRecovery')->search( + { + borrowernumber => $borrower_number, + valid_until => \'> NOW()' + }, { + columns => 'borrowernumber' + }); + + if ($rs->next){ + return 1; + } + + return 0; +} + +=head2 GetValidLinkInfo + + Check if the link is still valid and return some info. + +=cut + +sub GetValidLinkInfo { + my ($uniqueKey) = @_; + my $dbh = C4::Context->dbh; + my $query = ' + SELECT borrower_password_recovery.borrowernumber, userid + FROM borrower_password_recovery, borrowers + WHERE borrowers.borrowernumber = borrower_password_recovery.borrowernumber + AND NOW() < valid_until + AND uuid = ? + '; + my $sth = $dbh->prepare($query); + $sth->execute($uniqueKey); + return $sth->fetchrow; +} + +=head2 SendPasswordRecoveryEmail + + It creates an email using the templates and send it to the user, using the specified email + +=cut + +sub SendPasswordRecoveryEmail { + my $borrower = shift; # from GetMember + my $userEmail = shift; #to_address (the one specified in the request) + my $protocol = shift; #only required to determine if 'http' or 'https' + my $update = shift; + + my $schema = Koha::Database->new->schema; + + # generate UUID + my @chars = ("A".."Z", "a".."z", "0".."9"); + my $uuid_str; + $uuid_str .= $chars[rand @chars] for 1..32; + + # insert into database + my $expirydate = DateTime->now(time_zone => C4::Context->tz())->add( days => 2 ); + if($update){ + my $rs = $schema->resultset('BorrowerPasswordRecovery')->search( + { + borrowernumber => $borrower->{'borrowernumber'}, + }); + $rs->update({uuid => $uuid_str, valid_until => $expirydate->datetime()}); + } else { + my $rs = $schema->resultset('BorrowerPasswordRecovery')->create({ + borrowernumber=>$borrower->{'borrowernumber'}, + uuid => $uuid_str, + valid_until=> $expirydate->datetime() + }); + } + + # create link + my $uuidLink = $protocol . C4::Context->preference( 'OPACBaseURL' ) . "/cgi-bin/koha/opac-password-recovery.pl?uniqueKey=$uuid_str"; + + # prepare the email + my $letter = C4::Letters::GetPreparedLetter ( + module => 'members', + letter_code => 'PASSWORD_RESET', + branchcode => $borrower->{branchcode}, + substitute => {passwordreseturl => $uuidLink, user => $borrower->{userid} }, + ); + + # define to/from emails + my $kohaEmail = C4::Context->preference( 'KohaAdminEmailAddress' ); # from + + C4::Letters::EnqueueLetter( { + letter => $letter, + borrowernumber => $borrower->{borrowernumber}, + to_address => $userEmail, + from_address => $kohaEmail, + message_transport_type => 'email', + } ); + + return 1; +} + +END { } # module clean-up code here (global destructor) + +1; \ No newline at end of file diff --git a/installer/data/mysql/en/mandatory/sample_notices.sql b/installer/data/mysql/en/mandatory/sample_notices.sql index 37e9d9f..67c8b34 100644 --- a/installer/data/mysql/en/mandatory/sample_notices.sql +++ b/installer/data/mysql/en/mandatory/sample_notices.sql @@ -144,5 +144,5 @@ Your library.' );
INSERT INTO `letter` (module, code, branchcode, name, is_html, title, content, message_transport_type) -VALUES ('members','PASSWORD_RESET','','Online password reset',1,'Koha password recovery','<html>\r\n<p>This email has been sent in response to your password recovery request for the account <strong><< borrowers.userid>></strong>.\r\n</p>\r\n<p>\r\nYou can now create your new password using the following link:\r\n<br/><a href=\"<<passwordreseturl>>\"><<passwordreseturl>></a>\r\n</p>\r\n<p>This link will be valid for 2 days from this email\'s reception, then you must reapply if you do not change your password.</p>\r\n<p>Thank you.</p>\r\n</html>\r\n','email' +VALUES ('members','PASSWORD_RESET','','Online password reset',1,'Koha password recovery','<html>\r\n<p>This email has been sent in response to your password recovery request for the account <strong><<user>></strong>.\r\n</p>\r\n<p>\r\nYou can now create your new password using the following link:\r\n<br/><a href=\"<<passwordreseturl>>\"><<passwordreseturl>></a>\r\n</p>\r\n<p>This link will be valid for 2 days from this email\'s reception, then you must reapply if you do not change your password.</p>\r\n<p>Thank you.</p>\r\n</html>\r\n','email' ); diff --git a/opac/opac-password-recovery.pl b/opac/opac-password-recovery.pl index 27c8ea6..b5cd2c2 100755 --- a/opac/opac-password-recovery.pl +++ b/opac/opac-password-recovery.pl @@ -9,6 +9,7 @@ use C4::Koha; use C4::Members qw(changepassword GetMember GetMemberDetails ); use C4::Output; use C4::Context; +use C4::Passwordrecovery qw(SendPasswordRecoveryEmail ValidateBorrowernumber GetValidLinkInfo); use Koha::AuthUtils qw(hash_password); my $query = new CGI; use HTML::Entities; @@ -44,11 +45,8 @@ my $errLinkNotValid; my $errPassNotMatch; my $errPassTooShort;
-my $dbh = C4::Context->dbh; - if ( $query->param('sendEmail') || $query->param('resendEmail') ) { - #send mail + confirmation - + my $protocol = $query->https() ? "https://" : "http://"; #try with the main email $email ||= ''; # avoid undef my $borrower_infos = GetMember( email => $email ); @@ -63,11 +61,9 @@ if ( $query->param('sendEmail') || $query->param('resendEmail') ) { $errNoEmailFound = 1; } elsif ( !$query->param('resendEmail') ) { - my $sth = $dbh->prepare( -"SELECT borrowernumber FROM borrower_password_recovery WHERE NOW() < valid_until AND borrowernumber = ?" - ); - $sth->execute($borrower_number); - if ( my $already = $sth->fetchrow ) { + my $already = ValidateBorrowernumber( $borrower_number ); + + if ( $already ) { $hasError = 1; $errAlreadyStartRecovery = 1; } @@ -82,7 +78,7 @@ if ( $query->param('sendEmail') || $query->param('resendEmail') ) { email => HTML::Entities::encode($email), ); } - elsif ( SendPasswordRecoveryEmail( $borrower_infos, $email, $query, $query->param('resendEmail') ) ) {#generate uuid and send recovery email + elsif ( SendPasswordRecoveryEmail( $borrower_infos, $email, $protocol, $query->param('resendEmail') ) ) {#generate uuid and send recovery email $template->param( mail_sent => 1, email => $email @@ -96,18 +92,7 @@ if ( $query->param('sendEmail') || $query->param('resendEmail') ) { } } elsif ( $query->param('passwordReset') ) { - #new password form - #check if the link is still valid - my $sth = $dbh->prepare( - "SELECT borrower_password_recovery.borrowernumber, userid - FROM borrower_password_recovery, borrowers - WHERE borrowers.borrowernumber = borrower_password_recovery.borrowernumber - AND NOW() < valid_until - AND uuid = ?" - ); - $sth->execute($uniqueKey); - ( $borrower_number, $username ) = $sth->fetchrow; - + ( $borrower_number, $username ) = GetValidLinkInfo($uniqueKey); #validate password length & match if ( ($borrower_number) && ( $password eq $repeatPassword ) @@ -116,8 +101,9 @@ elsif ( $query->param('passwordReset') ) { changepassword( $username, $borrower_number, hash_password($password) );
#remove entry - my $sth = $dbh->prepare("DELETE FROM borrower_password_recovery WHERE uuid = ? or NOW() > valid_until"); - $sth->execute($uniqueKey); + my $schema = Koha::Database->new->schema; + my $rs = $schema->resultset('BorrowerPasswordRecovery')->search({-or => [uuid => $uniqueKey, valid_until => \'< NOW()']}); + $rs->delete;
$template->param( password_reset_done => 1, @@ -148,19 +134,12 @@ elsif ( $query->param('passwordReset') ) { } elsif ($uniqueKey) { #reset password form #check if the link is valid - my $sth = $dbh->prepare( - "SELECT borrower_password_recovery.borrowernumber, userid - FROM borrower_password_recovery, borrowers - WHERE borrowers.borrowernumber = borrower_password_recovery.borrowernumber - AND NOW() < valid_until - AND uuid = ?" - ); - $sth->execute($uniqueKey); - ( $borrower_number, $username ) = $sth->fetchrow; + ( $borrower_number, $username ) = GetValidLinkInfo($uniqueKey); + if ( !$borrower_number ) { $errLinkNotValid = 1; } -warn "INLIBRO username $username"; + $template->param( new_password => 1, minPassLength => $minPassLength, @@ -175,56 +154,3 @@ else { #password recovery form (to send email) }
output_html_with_http_headers $query, $cookie, $template->output; - -# -# It creates an email using the templates and send it to the user, using the specified email -# -sub SendPasswordRecoveryEmail { - my $borrower = shift; # from GetMember - my $userEmail = shift; #to_address (the one specified in the request) - my $query = shift; #only required to determine if 'http' or 'https' - my $update = shift; - - my $dbh = C4::Context->dbh; - - # generate UUID - my @chars = ("A".."Z", "a".."z", "0".."9"); - my $uuid_str; - $uuid_str .= $chars[rand @chars] for 1..32; - - # insert into database - my $expirydate = DateTime->now(time_zone => C4::Context->tz())->add( days => 2 ); - if($update){ - my $sth = $dbh->prepare( 'UPDATE borrower_password_recovery set uuid=?, valid_until=? where borrowernumber=? '); - $sth->execute($uuid_str, $expirydate->datetime(), $borrower->{'borrowernumber'}); - } else { - my $sth = $dbh->prepare( 'INSERT INTO borrower_password_recovery VALUES (?, ?, ?)'); - $sth->execute($borrower->{'borrowernumber'}, $uuid_str, $expirydate->datetime()); - } - - # create link - my $protocol = $query->https() ? "https://" : "http://"; - my $uuidLink = $protocol . C4::Context->preference( 'OPACBaseURL' ) . "/cgi-bin/koha/opac-password-recovery.pl?uniqueKey=$uuid_str"; - - # prepare the email - my $letter = C4::Letters::GetPreparedLetter ( - module => 'members', - letter_code => 'PASSWORD_RESET', - branchcode => $borrower->{branchcode}, - tables => {borrowers => $borrower}, - substitute => {passwordreseturl => $uuidLink}, - ); - - # define to/from emails - my $kohaEmail = C4::Context->preference( 'KohaAdminEmailAddress' ); # from - - C4::Letters::EnqueueLetter( { - letter => $letter, - borrowernumber => $borrower->{'borrowernumber'}, - to_address => $userEmail, - from_address => $kohaEmail, - message_transport_type => 'email', - } ); - - return 1; -} -- 1.9.1
-- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 simith.doliveira@inlibro.com changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #33176|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 simith.doliveira@inlibro.com changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #33175|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 simith.doliveira@inlibro.com changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Needs Signoff --- Comment #77 from simith.doliveira@inlibro.com --- Sorry for #76 Conflit fixed and patches squashed -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Jason Burds <jburds@dubuque.lib.ia.us> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA CC| |jburds@dubuque.lib.ia.us --- Comment #78 from Jason Burds <jburds@dubuque.lib.ia.us> --- The sandbox you've requested is not ready. Some problems occurred applying patches from bug 8753: <h1>Something went wrong !</h1>Applying: Bug 8753 - Add forgot password link to OPAC Using index info to reconstruct a base tree... Falling back to patching base and 3-way merge... Auto-merging koha-tmpl/opac-tmpl/bootstrap/en/includes/masthead.inc Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref Auto-merging installer/data/mysql/updatedatabase.pl CONFLICT (content): Merge conflict in installer/data/mysql/updatedatabase.pl Auto-merging installer/data/mysql/sysprefs.sql Auto-merging installer/data/mysql/kohastructure.sql Auto-merging installer/data/mysql/en/mandatory/sample_notices.sql CONFLICT (content): Merge conflict in installer/data/mysql/en/mandatory/sample_notices.sql Failed to merge in the changes. Patch failed at 0001 Bug 8753 - Add forgot password link to OPAC When you have resolved this problem run git bz apply --continue. If you would prefer to skip this patch, instead run git bz apply --skip. To restore the original branch and stop patching run git bz apply --abort. Bug 8753 - Add forgot password link to OPAC 36819 - Bug 8753 - Add forgot password link to OPAC Apply? [(y)es, (n)o, (i)nteractive] Patch left in /tmp/Bug-8753---Add-forgot-password-link-to-OPAC-VCLXC4.patch . -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 dmin <dminuck@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC|dminuck@gmail.com | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Blou <philippe.blouin@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #36819|0 |1 is obsolete| | --- Comment #79 from Blou <philippe.blouin@inlibro.com> --- Created attachment 37492 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=37492&action=edit Bug 8753 - Add forgot password link to OPAC This redo includes the following - usage of the template letters for the email instead of a .tt (that feature didn't exist when the functionnality was initially coded for our client) - removal of the prog version - removal of the code that was initially put into Members.pm - Added a letter into updatedatabase.pl and sample_notices.sql - Of course, rebase to latest master. The rest remain unchanged since the previous comments/approvals. As such, what worked before should still work. TEST PLAN: 1) apply the patch 2) go to system preferences OPAC>>Privacy and set 'OpacResetPassword' to ON. That will cause the link 'Forgot yo 2b) make sure that OpacPasswordChange is also ON. 3) refresh front page, click on 'Forgot your password' and enter a VALID address (one that is associated to an en 3b) Also try an INVALID address (valid yet not in your koha db). An error message will show up. 4) An email should be received at that address with a link. 5) Follow the link in the mail to fill the new password. Until a satisfactory new password is entered, the old password is not reset. 6) Go to main page try the new password. http://bugs.koha-community.org/show_bug.cgi?id=13068 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Arturo <library@sll.texas.gov> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |library@sll.texas.gov --- Comment #80 from Arturo <library@sll.texas.gov> --- Hi, all. My library recently switched to Koha, and this is my first attempt at testing a patch, but I am not sure if I am doing this correctly or if I've truly run into an issue. I followed the instructions at http://wiki.koha-community.org/wiki/Sandboxes to test this patch, bug 8753. I set up Sandbox 6 from BibLibre succesfully, logged in correctly, and did not see any error messages in the 'news' area, so it seems the patch was applied successfully. Before I could follow the test plan listed in this thread, I randomly selected a patron (I chose Colin Campbell), set a password for him, and set his e-mail address to my e-mail address so that I'd receive the e-mail notifications. Then I followed the test plan. Everything seemed OK except that I never received the e-mail to reset the pw. I then remembered that by default, Koha installations do not have e-mail enabled. Could that be the reason? Or would it be safe to assume that e-mail is enabled in the BibLibre sandboxes? I couldn't find that information anywhere. Thanks for any assistance anyonce can provide! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #81 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to Arturo from comment #80)
Hi, all. My library recently switched to Koha, and this is my first attempt at testing a patch, but I am not sure if I am doing this correctly or if I've truly run into an issue.
I followed the instructions at http://wiki.koha-community.org/wiki/Sandboxes to test this patch, bug 8753. I set up Sandbox 6 from BibLibre succesfully, logged in correctly, and did not see any error messages in the 'news' area, so it seems the patch was applied successfully.
I have just tested to apply this patch on sandbox 6, I received an email within 5 minutes: sandbox is not ready ! There is a conflict with the patch. Did you see the pref OpacResetPassword? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #82 from Arturo <library@sll.texas.gov> --- (In reply to Jonathan Druart from comment #81)
(In reply to Arturo from comment #80)
Hi, all. My library recently switched to Koha, and this is my first attempt at testing a patch, but I am not sure if I am doing this correctly or if I've truly run into an issue.
I followed the instructions at http://wiki.koha-community.org/wiki/Sandboxes to test this patch, bug 8753. I set up Sandbox 6 from BibLibre succesfully, logged in correctly, and did not see any error messages in the 'news' area, so it seems the patch was applied successfully.
I have just tested to apply this patch on sandbox 6, I received an email within 5 minutes: sandbox is not ready !
There is a conflict with the patch.
Did you see the pref OpacResetPassword?
Yes, I did see the OpacResetPassword preference, turned it on along with the pref to allow patrons to change their passwords, and went to the homepage and saw the "Forgot my password" link. Below is the result I got when I set up the sandbox. It does seem to start off with a warning, perhaps ("Use of uninitialized value"), but the message does say that the patch for bug 8753 were applied successfully. The sandbox you've requested is now ready. The patches attached to bugzilla 8753 have been applied, and the result is: Updatedatabase: Use of uninitialized value in substitution (s///) at /home/koha/src/installer/data/mysql/updatedatabase.pl line 9676. Upgrade to 3.19.00.004 done (Bug 13346: OpacExportOptions is now multiple) Upgrade to 3.19.00.005 done (Bug 13379 - Modify authorised_values.category to varchar(32)) Upgrade to 3.19.00.006 done (Bug 11944 - Convert DB tables to utf8_unicode_ci) Upgrade to 3.19.00.007 done (Bug 12905: Check budget integrity: OK) Upgrade to 3.19.00.008 done (Bug 12601 - Add new foreign key aqorders.budget_id) Upgrade to 3.19.00.009 done (Bug 13007 - Add new foreign key suggestions.budgetid) Upgrade to 3.19.00.010 done (Bug 5511 - SessionRestrictionByIP) Upgrade to 3.19.00.011 done (Bug 13417: Add permission to delete public lists) Upgrade to 3.19.00.012 done (Bug 13523 - Remove NOT NULL restriction on field marcxml due to mysql STRICT_TRANS_TABLES) Upgrade to 3.19.00.013 done (Bug 11395: Add permission tools_records_batchmod) Upgrade to 3.19.00.014 done (Bug 12648: Add letter ACQ_NOTIF_ON_RECEIV ) Upgrade to 3.19.00.015 done (Bug 11430 - Add primary key for search_history) Upgrade to 3.19.00.016 done (Bug 13380: Add the ORDER_CANCELLATION_REASON authorised value) Upgrade to 3.19.00.017 done (Bug 5786 - Move AllowOnShelfHolds to circulation matrix; Move OPACItemHolds system preference to circulation matrix) Upgrade to 3.19.00.018 done (Bug 10328: Rename opaccolorstylesheet to OpacAdditionalStylesheet Upgrade to 3.19.00.019 done (Bug 9580: Cover image from Coce, a remote image URL cache) Upgrade to XXX done (Bug 8753: Add forgot password link to OPAC) If you use this system for the 1st time, you may find usefull to reach the wiki page http://wiki.koha-community.org/wiki/Sandboxes to have some information about those results, to check if everything went well & if you can test the patch properly -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Fridolin SOMERS <fridolyn.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolyn.somers@biblibre.co | |m -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #83 from Fridolin SOMERS <fridolyn.somers@biblibre.com> --- I test on sandbox 13, I have a conflict on installer/data/mysql/updatedatabase.pl But i solved it manually -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #84 from Fridolin SOMERS <fridolyn.somers@biblibre.com> --- I've found : opac-sendbasket.pl: Error sending mail: Bad or missing From address: '' You need to specify an address in syspref KohaAdminEmailAddress before testing sending emails. In UNIMARC database at least. Works for me with a cart. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #85 from Fridolin SOMERS <fridolyn.somers@biblibre.com> --- (In reply to Fridolin SOMERS from comment #84)
You need to specify an address in syspref KohaAdminEmailAddress before testing sending emails. In UNIMARC database at least. Same for MARC21 database
-- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #86 from techservspec@gmail.com --- Patch not loading on sandbox: The sandbox you've requested is not ready. Some problems occurred applying patches from bug 8753: <h1>Something went wrong !</h1>Applying: Bug 8753 - Add forgot password link to OPAC Using index info to reconstruct a base tree... Falling back to patching base and 3-way merge... Auto-merging installer/data/mysql/updatedatabase.pl CONFLICT (content): Merge conflict in installer/data/mysql/updatedatabase.pl Auto-merging installer/data/mysql/sysprefs.sql Auto-merging installer/data/mysql/kohastructure.sql Failed to merge in the changes. Patch failed at 0001 Bug 8753 - Add forgot password link to OPAC When you have resolved this problem run git bz apply --continue. If you would prefer to skip this patch, instead run git bz apply --skip. To restore the original branch and stop patching run git bz apply --abort. Bug 8753 - Add forgot password link to OPAC 37492 - Bug 8753 - Add forgot password link to OPAC Apply? [(y)es, (n)o, (i)nteractive] Patch left in /tmp/Bug-8753---Add-forgot-password-link-to-OPAC-QS82WH.patch . -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #87 from Liz Rea <liz@catalyst.net.nz> --- Hi, I had a chance to take a look at this today. My comments are not code-level, they are "I applied this and had a look at the actual function" level. This is definitely a desirable feature, please don't give up. :) I saw several issues with this when I was testing: 1. The patch won't apply, it has easily resolved conflicts in database structure and update database. I fixed them for my testing purposes, just wanted you to be aware. 1a. As noted elsewhere, the feature should go into Koha turned off, instead of turned on. We don't want surprises for our users. 2. In the bootstrap theme, we will want to have the "Forgot password" link on the home page beneath the login, on /cgi-bin/koha/opac-user.pl, as well as in the modal box. This can be a follow up. 3. The markup on the opac-password-recovery pages is not consistent with the rest of the OPAC in the Bootstrap theme. This will need to be fixed. 4. On the "reset password" phase ("Password recovery form") - I don't understand why the first field is "Surname in lower case" and the second field is "Confirm password" - the standard behaviour for functions like this is to have something like "Type password" and "repeat password." The function works, but it wasn't clear to me when testing that the "surname" field was actually supposed to be the initial go at the password. It may be a relic from the implementation for your client, but I don't think we want it to be default Koha behaviour for it to suggest the surname as a password. Please fix. 5. *Good thing* submitting the form, (when I realised that both fields are for passwords) works just fine, and the password is reset, and I was able to log in with no issue. 6. Having a mismatch in the passwords on the reset form seems to pop up a box asking me to confirm which user I am changing the password for - this doesn't seem right to me, please check. It doesn't seem to actually *work* but it's a bit scary to see, as it suggests that I could possibly change the password for a user that is not "me." 7. This functionality does not work when you have more than one borrower with the same email address, it presents "We chould not authenticate you as the account owner." I think we may need to use a combination of userid and email address to authenticate users. Userid's do have to be unique, maybe use that instead and pop an error if the user does not have a registered email address. 8. *Good thing* the email seems to work fine. Maybe minor updates to the text from a native English speaker are needed, but the message and the link are fine. Any changes can come in a follow up. I understand this is a lot of things to deal with - I think everyone in the community wants this patch/function to be successful, so please say if you need help or if you have any questions. If you feel like you would rather not continue on with this development, please say so that someone else will feel better about picking up where you left off. Cheers, Liz Rea -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #88 from Liz Rea <liz@catalyst.net.nz> --- A couple of other things that I saw just after I posted the last message - - Please check the copyright - should it be InLibro or PTFS Europe? - It occurs to me that by default, the message queue sends things every 15 minutes and that's a pretty long time in password-reset land. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Tim Hannah <timdhannah@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |timdhannah@gmail.com -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Maxime Beaulieu <maxime.beaulieu@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Maxime Beaulieu <maxime.beaulieu@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #37492|0 |1 is obsolete| | --- Comment #89 from Maxime Beaulieu <maxime.beaulieu@inlibro.com> --- Created attachment 39049 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=39049&action=edit 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 Author: Maxime Beaulieu <maxime.beaulieu@inlibro.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Maxime Beaulieu <maxime.beaulieu@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |maxime.beaulieu@inlibro.com Assignee|philippe.blouin@inlibro.com |maxime.beaulieu@inlibro.com -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #90 from Liz Rea <liz@catalyst.net.nz> --- Hi, Spectacular, thanks I'll have another look. :) Cheers, Liz -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #91 from Marc Véron <veron@veron.ch> --- Created attachment 39144 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=39144&action=edit [Signed-off] 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 Véron <veron@veron.ch> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #39049|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #92 from Liz Rea <liz@catalyst.net.nz> --- Hi, I tested this with opacpublic enabled and disabled - we probably need to modify the templates to not include the opacnav blocks if opacpublic is disabled. This can be a followup. I'm not super keen on the syspref text, I may send a followup with text that more clearly differentiates the purpose of this new syspref from OpacPasswordChange. Otherwise, thank you very much for the updated patch! Liz -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #93 from Liz Rea <liz@catalyst.net.nz> --- Created attachment 39165 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=39165&action=edit 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 Véron <veron@veron.ch> I tested this with opacpublic enabled and disabled - we probably need to modify the templates to not include opacnav if opacpublic is disabled. I'm not super keen on the syspref text, I may send a followup with text that more clearly differentiates the purpose of this new syspref from OpacPasswordChange Signed-off-by: Liz Rea <liz@catalyst.net.nz> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #94 from Liz Rea <liz@catalyst.net.nz> --- Created attachment 39166 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=39166&action=edit 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. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #95 from Liz Rea <liz@catalyst.net.nz> --- Created attachment 39167 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=39167&action=edit Bug 8753 - followup - update text for link to match common UI paradigms 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?" -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Liz Rea <liz@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #39167|0 |1 is obsolete| | --- Comment #96 from Liz Rea <liz@catalyst.net.nz> --- Created attachment 39168 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=39168&action=edit 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) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |josef.moravec@gmail.com -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #39165|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #97 from Jonathan Druart <jonathan.druart@biblibre.com> --- FAIL C4/Passwordrecovery.pm FAIL pod *** ERROR: Spurious text after =cut in file C4/Passwordrecovery.pm *** ERROR: Spurious =cut command in file C4/Passwordrecovery.pm FAIL opac/opac-password-recovery.pl FAIL valid shift on reference is experimental Smartmatch is experimental FAIL koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-auth.tt FAIL forbidden patterns forbidden pattern: tab char (line 155) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #98 from Rémi Mayrand-Provencher <remi.mayrand-provencher@inLibro.com> --- Created attachment 39827 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=39827&action=edit Bug 8753 - Fixed the forbidden pattern error in opac-auth.tt I could only reproduce the forbidden pattern error from jonathan Druart's comment#97 locally, so I fixed it. TEST PLAN 1)Apply all patches 2)Forbidden pattern error should not be there anymore -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Rémi Mayrand-Provencher <remi.mayrand-provencher@inLibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff CC| |remi.mayrand-provencher@inL | |ibro.com -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 techservspec@gmail.com changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |techservspec@gmail.com --- Comment #99 from techservspec@gmail.com --- Following test plan, entered a valid email address and received this software error: Undefined subroutine &C4::Members::Search called at /home/koha/src/opac/opac-password-recovery.pl line 63. The same happens with a valid user name. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #100 from Liz Rea <liz@catalyst.net.nz> --- I get this error as well now too. I believe it was introduced by the pushing of bug 12633, which removed C4::Members::Search. Cheers, Liz -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Liz Rea <liz@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |12633 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Liz Rea <liz@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Amit <amitddng135@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |amitddng135@gmail.com -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #101 from Liz Rea <liz@catalyst.net.nz> --- Hi, Just wondering if you all at inLibro were going to come back to this bug? Cheers, Liz -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #102 from Blou <philippe.blouin@inlibro.com> --- I think this bug's history shows we are ALWAYS coming back. We shall fight on the beach, we shall fight on the tests, we shall fight on the bugs, we shall never surrender. (bur our resources might be stuck on another beachhead right now) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #103 from Liz Rea <liz@catalyst.net.nz> --- (In reply to Blou from comment #102)
I think this bug's history shows we are ALWAYS coming back.
We shall fight on the beach, we shall fight on the tests, we shall fight on the bugs, we shall never surrender.
(bur our resources might be stuck on another beachhead right now)
This is possibly the best bug reply ever. :D -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #104 from Maxime Beaulieu <maxime.beaulieu@inlibro.com> --- Created attachment 41181 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=41181&action=edit 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 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Blou <philippe.blouin@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #105 from Marc Véron <veron@veron.ch> --- Created attachment 41192 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=41192&action=edit Stray text appears above header Above OPAC header, I get stray text on all pages related to password recovery, see screenshot. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #106 from Liz Rea <wizzyrea@gmail.com> --- Created attachment 41193 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=41193&action=edit 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. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #107 from Tim Hannah <timdhannah@gmail.com> --- Had a look at this, first time using kohadevbox and git bz, so could be something I did or didn't do. Looked good, no stray text, but when I tried resetting a password I got error message DBIx::Class::ResultSet::next(): Table koha_kohadev.borrower_password_recovery' doesn't exist at /home/vagrant/kohaclone/C4/Passwordrecovery.pm line 70 Any clues? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #108 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Hi Tim, not sure if that's the reason - but did you run the database update? you can run updatedatabase.pl from the command line or change kohaversion.pl to +1 and then after the update is run change it back to the number it had before. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #109 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- ... that should read 'add 1 to the version number at the end...' or similar :) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Aleisha Amohia <aleishaamohia@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA CC| |aleishaamohia@hotmail.com --- Comment #110 from Aleisha Amohia <aleishaamohia@hotmail.com> --- The system breaks for me if I don't provide an email or username (leave both spaces empty) to send the reset link to. Get this error: opac-password-recovery.pl: Can't use an undefined value as an ARRAY reference at /home/vagrant/kohaclone/opac/opac-password-recovery.pl line 66 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #111 from Aleisha Amohia <aleishaamohia@hotmail.com> --- Also could be a good idea to fix the title in the metadata - at the moment it says <title>Waitaki District Libraries catalog -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #112 from Liz Rea <wizzyrea@gmail.com> --- Created attachment 41455 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=41455&action=edit 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. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #113 from Liz Rea <liz@catalyst.net.nz> --- I wasn't able to replicate Aleisha's 2nd issue, it may be a local issue to her environment. Liz -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Liz Rea <liz@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Aleisha Amohia <aleishaamohia@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off --- Comment #114 from Aleisha Amohia <aleishaamohia@hotmail.com> --- Signing off! Works for me -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #115 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- FAIL opac/opac-password-recovery.pl FAIL spelling doesnt ==> doesn't FAIL valid defined(@array) is deprecated Smartmatch is experimental shift on reference is experimental (Maybe you should just omit the defined()?) FAIL C4/Passwordrecovery.pm FAIL pod *** ERROR: Spurious text after =cut in file C4/Passwordrecovery.pm *** ERROR: Spurious =cut command in file C4/Passwordrecovery.pm FAIL koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-password-recovery.tt FAIL spelling occured ==> occurred -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #116 from Liz Rea <liz@catalyst.net.nz> --- Created attachment 41686 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=41686&action=edit Bug 8753 - [followup} Correcting spelling mistakes Make sure it all still works -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Liz Rea <liz@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #41455|0 |1 is obsolete| | --- Comment #117 from Liz Rea <liz@catalyst.net.nz> --- Created attachment 41687 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=41687&action=edit 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 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #118 from Liz Rea <liz@catalyst.net.nz> --- Created attachment 41688 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=41688&action=edit Bug 8753 - [followup] POD cleanup Check to make sure the messages from the qa checker are gone. Note I'm not sure about this one because my test tools are apparently broken, and I can't get the error to manifest. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #119 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Does the status should be Signoff? I would say we should not provide any information if the user does not fill correct login or email. The message should be generic "Bad info" instead of "Bad login" or "Bad email". -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #120 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> ---
From perldoc rand:
rand() is not cryptographically secure. You should not rely on it in security-sensitive situations. As of this writing, a number of third-party CPAN modules offer random number generators intended by their authors to be cryptographically secure, including: Data::Entropy, Crypt::Random, Math::Random::Secure, and Math::TrulyRandom. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #41688|0 |1 is obsolete| | --- Comment #121 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 41706 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=41706&action=edit Bug 8753 - [followup] POD cleanup Check to make sure the messages from the qa checker are gone. Note I'm not sure about this one because my test tools are apparently broken, and I can't get the error to manifest. Amended patch by Jonathan Druart: Add a blank line before =head2 It now passes. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #122 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- 1/ Receive the email 2/ Click on the link 3/ Change the pwd 4/ Click again on the link 5/ Change the pwd 6/ You get an error The error should appear at step 5 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #123 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- comment 21 did not get an answer. For instance tests are still missing. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Chris Rohde <crohde@roseville.ca.us> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |crohde@roseville.ca.us -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Nick Clemens <nick@quecheelibrary.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nick@quecheelibrary.org -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #124 from Chris Rohde <crohde@roseville.ca.us> --- Glad to see recent movement on this. Realize it's already made it to QA, but if there's anything I/my library can do to help or just encourage, I'm up for it. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Charles Farmer <charles.farmer@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #125 from Charles Farmer <charles.farmer@inlibro.com> --- Created attachment 43017 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=43017&action=edit 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 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Charles Farmer <charles.farmer@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|maxime.beaulieu@inlibro.com |charles.farmer@inlibro.com CC| |charles.farmer@inlibro.com -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #126 from Liz Rea <liz@catalyst.net.nz> --- Created attachment 43183 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=43183&action=edit 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. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #127 from David Kuhn <techservspec@gmail.com> --- I get the following error message when I try to install patch: Some problems occurred applying patches from bug 8753: <h1>Something went wrong !</h1>Applying: Bug 8753 - Add forgot password link to OPAC Using index info to reconstruct a base tree... Falling back to patching base and 3-way merge... Auto-merging koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-main.tt Auto-merging koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-auth.tt Auto-merging koha-tmpl/opac-tmpl/bootstrap/en/includes/masthead.inc Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref Auto-merging installer/data/mysql/en/mandatory/sample_notices.sql CONFLICT (content): Merge conflict in installer/data/mysql/en/mandatory/sample_notices.sql Failed to merge in the changes. Patch failed at 0001 Bug 8753 - Add forgot password link to OPAC -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #128 from Liz Rea <liz@catalyst.net.nz> --- Created attachment 43576 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=43576&action=edit 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 Véron <veron@veron.ch> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Liz Rea <liz@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #39144|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #129 from Liz Rea <liz@catalyst.net.nz> --- That latest one should resolve the merge issue. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Liz Rea <liz@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #39166|0 |1 is obsolete| | Attachment #39168|0 |1 is obsolete| | Attachment #39827|0 |1 is obsolete| | Attachment #41181|0 |1 is obsolete| | Attachment #41193|0 |1 is obsolete| | Attachment #41686|0 |1 is obsolete| | Attachment #41687|0 |1 is obsolete| | Attachment #41706|0 |1 is obsolete| | Attachment #43017|0 |1 is obsolete| | Attachment #43183|0 |1 is obsolete| | Attachment #43576|0 |1 is obsolete| | --- Comment #130 from Liz Rea <liz@catalyst.net.nz> --- Created attachment 44305 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=44305&action=edit 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 Véron <veron@veron.ch> Current status: Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #131 from Liz Rea <liz@catalyst.net.nz> --- Created attachment 44306 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=44306&action=edit 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 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #132 from Liz Rea <liz@catalyst.net.nz> --- Created attachment 44307 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=44307&action=edit Bug 8753 - Fixed the forbidden pattern error in opac-auth.tt I could only reproduce the forbidden pattern error from jonathan Druart's comment#97 locally, so I fixed it. TEST PLAN 1)Apply all patches 2)Forbidden pattern error should not be there anymore -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #133 from Liz Rea <liz@catalyst.net.nz> --- Created attachment 44308 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=44308&action=edit 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 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #134 from Liz Rea <liz@catalyst.net.nz> --- Created attachment 44309 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=44309&action=edit Bug 8753 - [followup] POD cleanup Check to make sure the messages from the qa checker are gone. Note I'm not sure about this one because my test tools are apparently broken, and I can't get the error to manifest. Amended patch by Jonathan Druart: Add a blank line before =head2 It now passes. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #135 from Liz Rea <liz@catalyst.net.nz> --- Created attachment 44310 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=44310&action=edit 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 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #136 from Liz Rea <liz@catalyst.net.nz> --- Created attachment 44311 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=44311&action=edit 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. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #137 from Liz Rea <liz@catalyst.net.nz> --- I just squashed my followups to make this a bit tidier, where I could. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #41192|0 |1 is obsolete| | --- Comment #138 from Marc Véron <veron@veron.ch> --- Comment on attachment 41192 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=41192 Stray text appears above header This is fixed. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #44305|0 |1 is obsolete| | --- Comment #139 from Marc Véron <veron@veron.ch> --- Created attachment 44323 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=44323&action=edit 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 Véron <veron@veron.ch> New sign-off after testing all patches together Signed-off-by: Marc Véron <veron@veron.ch> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #44306|0 |1 is obsolete| | --- Comment #140 from Marc Véron <veron@veron.ch> --- Created attachment 44324 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=44324&action=edit 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 Véron <veron@veron.ch> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #44307|0 |1 is obsolete| | --- Comment #141 from Marc Véron <veron@veron.ch> --- Created attachment 44325 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=44325&action=edit Bug 8753 - Fixed the forbidden pattern error in opac-auth.tt I could only reproduce the forbidden pattern error from jonathan Druart's comment#97 locally, so I fixed it. TEST PLAN 1)Apply all patches 2)Forbidden pattern error should not be there anymore Signed-off-by: Marc Véron <veron@veron.ch> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #44308|0 |1 is obsolete| | --- Comment #142 from Marc Véron <veron@veron.ch> --- Created attachment 44326 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=44326&action=edit 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 Véron <veron@veron.ch> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #44309|0 |1 is obsolete| | --- Comment #143 from Marc Véron <veron@veron.ch> --- Created attachment 44327 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=44327&action=edit Bug 8753 - [followup] POD cleanup Check to make sure the messages from the qa checker are gone. Note I'm not sure about this one because my test tools are apparently broken, and I can't get the error to manifest. Amended patch by Jonathan Druart: Add a blank line before =head2 It now passes. Signed-off-by: Marc Véron <veron@veron.ch> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #44310|0 |1 is obsolete| | --- Comment #144 from Marc Véron <veron@veron.ch> --- Created attachment 44329 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=44329&action=edit 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 Véron <veron@veron.ch> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #44311|0 |1 is obsolete| | --- Comment #145 from Marc Véron <veron@veron.ch> --- Created attachment 44330 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=44330&action=edit 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 Véron <veron@veron.ch> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Jesse Weaver <jweaver@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jweaver@bywatersolutions.co | |m -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 sally.healey@cheshirewestandchester.gov.uk changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |sally.healey@cheshirewestan | |dchester.gov.uk -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Liz Rea <liz@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Priority|P5 - low |P1 - high --- Comment #146 from Liz Rea <liz@catalyst.net.nz> --- This is a must for 3.24. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Patch doesn't apply --- Comment #147 from Josef Moravec <josef.moravec@gmail.com> --- I agree, but rebase needed... -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #148 from Blou <philippe.blouin@inlibro.com> --- Will be done today. We will never surrender... -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Charles Farmer <charles.farmer@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Charles Farmer <charles.farmer@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #44323|0 |1 is obsolete| | --- Comment #149 from Charles Farmer <charles.farmer@inlibro.com> --- Created attachment 45209 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=45209&action=edit 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 Véron <veron@veron.ch> New sign-off after testing all patches together Signed-off-by: Marc Véron <veron@veron.ch> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Charles Farmer <charles.farmer@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #44324|0 |1 is obsolete| | Attachment #44325|0 |1 is obsolete| | Attachment #44326|0 |1 is obsolete| | Attachment #44327|0 |1 is obsolete| | Attachment #44329|0 |1 is obsolete| | Attachment #44330|0 |1 is obsolete| | --- Comment #150 from Charles Farmer <charles.farmer@inlibro.com> --- Created attachment 45210 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=45210&action=edit 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 Véron <veron@veron.ch> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #151 from Charles Farmer <charles.farmer@inlibro.com> --- Created attachment 45211 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=45211&action=edit Bug 8753 - Fixed the forbidden pattern error in opac-auth.tt I could only reproduce the forbidden pattern error from jonathan Druart's comment#97 locally, so I fixed it. TEST PLAN 1)Apply all patches 2)Forbidden pattern error should not be there anymore Signed-off-by: Marc Véron <veron@veron.ch> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #152 from Charles Farmer <charles.farmer@inlibro.com> --- Created attachment 45212 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=45212&action=edit 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 Véron <veron@veron.ch> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #153 from Charles Farmer <charles.farmer@inlibro.com> --- Created attachment 45213 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=45213&action=edit Bug 8753 - [followup] POD cleanup Check to make sure the messages from the qa checker are gone. Note I'm not sure about this one because my test tools are apparently broken, and I can't get the error to manifest. Amended patch by Jonathan Druart: Add a blank line before =head2 It now passes. Signed-off-by: Marc Véron <veron@veron.ch> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #154 from Charles Farmer <charles.farmer@inlibro.com> --- Created attachment 45214 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=45214&action=edit 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 Véron <veron@veron.ch> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #155 from Charles Farmer <charles.farmer@inlibro.com> --- Created attachment 45215 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=45215&action=edit 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 Véron <veron@veron.ch> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #156 from Charles Farmer <charles.farmer@inlibro.com> --- I had to resubmit every patches because the first one had the merge conflicts. If anybody knows how to obsolete the first patch of a queue without messing the queue order, let me know, it would be appreciated. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #157 from Josef Moravec <josef.moravec@gmail.com> --- I think that files kohastructure.sql and sysprefs.sql, should be updated too, not only to add atomicupdate sql file... -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Charles Farmer <charles.farmer@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #45209|0 |1 is obsolete| | --- Comment #158 from Charles Farmer <charles.farmer@inlibro.com> --- Created attachment 45223 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=45223&action=edit 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 Véron <veron@veron.ch> New sign-off after testing all patches together Signed-off-by: Marc Véron <veron@veron.ch> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Charles Farmer <charles.farmer@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #45210|0 |1 is obsolete| | --- Comment #159 from Charles Farmer <charles.farmer@inlibro.com> --- Created attachment 45224 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=45224&action=edit 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 Véron <veron@veron.ch> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Charles Farmer <charles.farmer@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #45211|0 |1 is obsolete| | --- Comment #160 from Charles Farmer <charles.farmer@inlibro.com> --- Created attachment 45225 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=45225&action=edit Bug 8753 - Fixed the forbidden pattern error in opac-auth.tt I could only reproduce the forbidden pattern error from jonathan Druart's comment#97 locally, so I fixed it. TEST PLAN 1)Apply all patches 2)Forbidden pattern error should not be there anymore Signed-off-by: Marc Véron <veron@veron.ch> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Charles Farmer <charles.farmer@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #45212|0 |1 is obsolete| | --- Comment #161 from Charles Farmer <charles.farmer@inlibro.com> --- Created attachment 45226 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=45226&action=edit 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 Véron <veron@veron.ch> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Charles Farmer <charles.farmer@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #45213|0 |1 is obsolete| | --- Comment #162 from Charles Farmer <charles.farmer@inlibro.com> --- Created attachment 45227 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=45227&action=edit Bug 8753 - [followup] POD cleanup Check to make sure the messages from the qa checker are gone. Note I'm not sure about this one because my test tools are apparently broken, and I can't get the error to manifest. Amended patch by Jonathan Druart: Add a blank line before =head2 It now passes. Signed-off-by: Marc Véron <veron@veron.ch> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Charles Farmer <charles.farmer@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #45214|0 |1 is obsolete| | --- Comment #163 from Charles Farmer <charles.farmer@inlibro.com> --- Created attachment 45228 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=45228&action=edit 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 Véron <veron@veron.ch> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Charles Farmer <charles.farmer@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #45215|0 |1 is obsolete| | --- Comment #164 from Charles Farmer <charles.farmer@inlibro.com> --- Created attachment 45229 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=45229&action=edit 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 Véron <veron@veron.ch> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #165 from Charles Farmer <charles.farmer@inlibro.com> --- Created attachment 45230 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=45230&action=edit Bug 8753 - propagating the changes to kohastructure.sql and sysprefs.sql -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #166 from Charles Farmer <charles.farmer@inlibro.com> --- (In reply to Josef Moravec from comment #157)
I think that files kohastructure.sql and sysprefs.sql, should be updated too, not only to add atomicupdate sql file...
The changes brought by the atomicupdate file are now visible in the kohastructure and sysprefs files. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Liz Rea <liz@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #45230|0 |1 is obsolete| | --- Comment #167 from Liz Rea <liz@catalyst.net.nz> --- Created attachment 46379 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=46379&action=edit Bug 8753 - propagating the changes to kohastructure.sql and sysprefs.sql Signed-off-by: Liz Rea <liz@catalyst.net.nz> Looks good with a new install. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Liz Rea <liz@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #168 from Kyle M Hall <kyle@bywatersolutions.com> --- I don't think C4::Passwordrecovery is a good namespace. Should this not be something like Koha::Patron::Password::Recovery or Koha::Borrower::Password::Recovery? We should not be adding modules to the C4 namespace at this point. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #169 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Koha::Patron please :) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Karl Holten <kholten@switchinc.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kholten@switchinc.org -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Karl Holten <kholten@switchinc.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jschmidt@switchinc.org -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #170 from Liz Rea <liz@catalyst.net.nz> --- May we have this heavily tested version in, and move it to the new namespace in a follow up? This is an egregiously missing feature, and one that we have been waiting a long, long time for. It's for the users, let's get it in. Liz -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #171 from David Cook <dcook@prosentient.com.au> --- As the local pedant in the Koha community, I'm with Liz on this one. While we shouldn't be adding anything to the C4 namespace, I think perhaps an exception should be made for this particular patch set, as the feature is rather important in modernizing Koha. Out in the world, there's pretty much no service where you can't reset your own password. Yet, you can't do it in Koha. Surely this frustrates users and puts a burden on librarians. It's an aspect of the modern web experience which Koha fails at. This patch set has been a long time coming. While it should be fairly easy to do a search and replace through the patches, mistakes do happen, and we'd probably need to re-test all the patches again to make sure that nothing was missed. It seems like it's already been tested to the Nth degree, so perhaps a pass should be made this one time. I suppose it would qualify as technical debt, and I hate technical debt. But this does seem to be an extraordinary case. If it were anything but a password reset, I'd say change the namespace. But this is such a missing piece of functionality in Koha, that I think the faster we get it in the better. Of course, I defer to the QA team and the RM. But just adding that perspective to the comments. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #172 from Tim Hannah <timdhannah@gmail.com> --- As a librarian I can confirm that the lack of this feature frustrates me, my colleagues and our users. We've been waiting for this for a while and would dearly love to have it sooner rather than later. I can't pretend to understand the ramifications of an imperfect namespace choice, but I know we currently lose users due to them being locked out and going elsewhere rather than approaching us for resets or reminders. If it can be released as is and fixed later, as Liz suggests, I for one would be very grateful. (And thanks for everyone's work on it. Looking forward to it, whenever we get it). Cheers -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 JD@TeTakere <joanned@tetakere.org.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC|joanned@tetakere.org.nz | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #173 from Marc Véron <veron@veron.ch> --- +1 for getting it in! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #174 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- After thinking this over for a bit I would be in favor of pushing it as it is. This is a long needed development that probably predates most of the work in the Koha namespace. Changing the namespace now would require more testing and it might give us a lot of conflicts in other places. So it might be easier to fix the namespace separately after this is pushed. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #175 from Magnus Enger <magnus@libriotech.no> --- +1 for getting this in ASAP. :-) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #176 from Blou <philippe.blouin@inlibro.com> --- In the name of the whole personnel at inLibro, which I've cycled through to support this patch in the past 2.5 years (so many good people fell on the battlefield, see comment #102), I say "yeah! it's about time!". Of course, we're now at 180 comments and counting, so there's a side of me that would like to see that keep increasing and beat a record of some sort. ;-) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #177 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Katrin Fischer from comment #174)
After thinking this over for a bit I would be in favor of pushing it as it is. This is a long needed development that probably predates most of the work in the Koha namespace. Changing the namespace now would require more testing and it might give us a lot of conflicts in other places. So it might be easier to fix the namespace separately after this is pushed.
If it doesn't get fixed now, it won't get fixed. This patch was submitted long after the creation of the Koha namespace. Changing the namespace is not that onerous a request. We can't allow code in that isn't fit just because we like the feature itself. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |In Discussion -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #178 from Blou <philippe.blouin@inlibro.com> --- Noooooo!!! Not the dreaded "In Discussion" !! I would have prefered Failed QA. Not sure what is left to discuss. Everyone already chimed in. But I respect the QA's right to fail it. At this point, since it would require "full retest", I will ask Charles to fix it and SQUASH everything. Considering the amount of support this feature has, I suppose a re-signoff wouldn't be long. *sigh* -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |ASSIGNED --- Comment #179 from Chris Cormack <chris@bigballofwax.co.nz> --- *sigh* Oh well, can't be helped I guess. I will test the new patch as soon as it is up. Hopefully this new super strict QA standard will be across the board and I won't see people pushing patches they wrote skipping qa steps anymore. Switching back to assigned, while we wait for the new patch. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #180 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Chris Cormack from comment #179)
Hopefully this new super strict QA standard will be across the board and I won't see people pushing patches they wrote skipping qa steps anymore.
I do not favor a super strict QA standard. Common_sense++ -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #181 from Chris Cormack <chris@bigballofwax.co.nz> --- (In reply to Marcel de Rooy from comment #180)
(In reply to Chris Cormack from comment #179)
Hopefully this new super strict QA standard will be across the board and I won't see people pushing patches they wrote skipping qa steps anymore.
I do not favor a super strict QA standard. Common_sense++
Neither do I, but if we are going to be super strict about this one, we need to at least be consistent. That was my point. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #182 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Kyle, can we imagine to make an exception for this one, open a new bug report to move the new module to the Koha namespace and ask for the author to commit to submit a patch? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #183 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Blou from comment #176)
Of course, we're now at 180 comments and counting, so there's a side of me that would like to see that keep increasing and beat a record of some sort. ;-)
Don't worry about that, we won't reach it (there are 247 comments on bug 7167 and it as been abandoned, there is another one with more than 500 comments iirc). -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |In Discussion --- Comment #184 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Jonathan Druart from comment #182)
Kyle, can we imagine to make an exception for this one, open a new bug report to move the new module to the Koha namespace and ask for the author to commit to submit a patch?
Yes! I set this bug to in discussion while we were actually discussing the issue. I purposefully did not set it to failed qa. My fear is that if this is pushed as is, there is no impetus for the namespace to be fixed. All I'd like to see is a commitment from someone that the patches to move this to a better namespace *will be submitted*, preferably before the next major release if possible. So far I haven't seen anyone make that commitment. I should have written that more clearly than I had. Philippe, if you can vouch that Charles or someone will submit the patch to move this module to a new namespace, I have no problem with it proceeding as is. Now would be the time to file the followup bug report and assign it to the proper person. If this works for you please go ahead and reset the status to Signed Off. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Blou <philippe.blouin@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |Signed Off --- Comment #185 from Blou <philippe.blouin@inlibro.com> --- This is not a problem for us. To be sure, just create the new bug and assign it to me immediately. I assure you you'll get a patch within a week after this has been pushed to master. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #186 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Blou from comment #185)
This is not a problem for us. To be sure, just create the new bug and assign it to me immediately. I assure you you'll get a patch within a week after this has been pushed to master.
Thanks! I'll go ahead and take care of that! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #187 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Chris Cormack from comment #179)
Hopefully this new super strict QA standard will be across the board and I won't see people pushing patches they wrote skipping qa steps anymore.
I'd like to address this. I'm not in favor of super strict QA standards. What I'm in favor of are transparent, standardized, and consistently applied QA rules. My experience as a developer has informed my work as a QA team member. I have had to rewrite C4 modules as Koha modules, I've asked others to rewrite C4 modules as Koha modules. Code quality guidelines should be completely divorced from the necessity or popularity of a bugfix or feature. I think our coding guidelines need a bit of a revamp to make sure all of the things we've taken for granted as being necessary to pass QA are actually in the coding guidelines. If a QA team member fails QA on a patch, the QA'er should be able to link directly to the reason and explanation for it failing QA. If the QA'er believes the patch should fail QA but the reason is not part of the coding guidelines, then a meeting should be called, a vote should be made, and a new rule added to the guidelines. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #188 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- While I have put coding guideline additions on the agenda a lot of times in the past, I don't believe that we can ever have a complete set of rules that will fit every situation and possible question. Another common practice we QA has used where they haven't been sure about something is asking for more opinions - I think this has helped in a lot of cases to get more views and a better picture of the problem and possible solutions. That said, I am happy with the 2 step approach here. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |15585 Referenced Bugs: http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15585 [Bug 15585] Move C4::Passwordrecovery to Koha::Patron::Password::Reset -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #189 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Katrin Fischer from comment #188)
While I have put coding guideline additions on the agenda a lot of times in the past, I don't believe that we can ever have a complete set of rules that will fit every situation and possible question.
Another common practice we QA has used where they haven't been sure about something is asking for more opinions - I think this has helped in a lot of cases to get more views and a better picture of the problem and possible solutions.
That simply highlights the problem. With this methodology, a developer will not know if code will pass qa or not until it has been written and submitted. I agree, we will never have a complete set of rules, but we can build that set as we go. The more we have written down for developers to be aware in advance, the better. We need documented guidelines. The approach of "this doesn't feel right to me" just doesn't work any more. It can only lead to future frustration for developers. We can't have one set of rules for bug A and another set of rules for bug B. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #190 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I think we are actually not in disagreement - I just want to warn about wanting to solve everything with rules. That will be trying for the impossible. And a 20 pages catalog of guidelines... is kind of scaring away devs too. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #191 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Katrin Fischer from comment #190)
I think we are actually not in disagreement - I just want to warn about wanting to solve everything with rules. That will be trying for the impossible. And a 20 pages catalog of guidelines... is kind of scaring away devs too.
Lol, yeah, I agree with that too. I just think at the moment we have too few guidelines on paper and too many guidelines only in the heads of the QA team ; ) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #192 from Kyle M Hall <kyle@bywatersolutions.com> --- I also want to add that the guidelines would be an explanation of they something failed qa. Instead of "Here is a link to the community decided reason" it is currently more of a "because I said so" situation. I personally dislike failing qa on something without having those reasons. I'm far more apt to submit follow ups to avoid doing so. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact| |m.de.rooy@rijksmuseum.nl --- Comment #193 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- QA: Working on this one.. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #45223|0 |1 is obsolete| | Attachment #45224|0 |1 is obsolete| | Attachment #45225|0 |1 is obsolete| | Attachment #45226|0 |1 is obsolete| | Attachment #45227|0 |1 is obsolete| | Attachment #45228|0 |1 is obsolete| | Attachment #45229|0 |1 is obsolete| | Attachment #46379|0 |1 is obsolete| | --- Comment #194 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 47164 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=47164&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #195 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 47165 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=47165&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #196 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 47166 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=47166&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #197 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 47167 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=47167&action=edit Bug 8753 - [followup] POD cleanup Check to make sure the messages from the qa checker are gone. Note I'm not sure about this one because my test tools are apparently broken, and I can't get the error to manifest. Amended patch by Jonathan Druart: Add a blank line before =head2 It now passes. Signed-off-by: Marc Veron <veron@veron.ch> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #198 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 47168 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=47168&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #199 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 47169 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=47169&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #200 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 47170 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=47170&action=edit Bug 8753 - propagating the changes to kohastructure.sql and sysprefs.sql Signed-off-by: Liz Rea <liz@catalyst.net.nz> Looks good with a new install. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #201 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 47171 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=47171&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8753 --- Comment #202 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- QA Comment: With reference to the discussion above (and time passed), I am moving the status to Passed QA. Looks good to me. Added a small follow-up. An important detail to fix before the new release is: New notice should be added in all language files! Please fix that asap in a follow-up report after this one has been pushed. This would normally be a reason to Fail QA on itself... Note to RM: Watch the conflict with bug 15548 (Borrower vs Patron); and the promised fix under 15585. Minor points: Spotted "FIXME: That ugly shift-grep contraption". The term "Forgotten password recovery" sounds funny to me (no native speaker :) Is Password recovery not sufficient? Are dots in the unique key (especially at the end) user friendly? Passed QA -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org