[Bug 14509] New: Koha still allows spaces at the end, beginning or as your password
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14509 Bug ID: 14509 Summary: Koha still allows spaces at the end, beginning or as your password Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: major Priority: P5 - low Component: Authentication Assignee: gmcharlt@gmail.com Reporter: katrin.fischer@bsz-bw.de QA Contact: testopia@bugs.koha-community.org CC: dpavlin@rot13.org This is a follow-up bug for bug 10177 which fixed the 'change password' page to not allow your password to have spaces at the beginning or ending or to consist solely of spaces. But there are still 2 other pages to fix: - Patron account > Edit patron - OPAC > Change password -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14509 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=10177 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14509 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14509 --- Comment #1 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 40993 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=40993&action=edit Bug 14509: Reject invalid passwords Bug 10177 rejects password with leading or trailing whitespaces, but only on the member-password page. It's not consistent to only do this check on 1 place. This patch adds the check for the 2 other places: memberentry and at the OPAC. Test plan: 1/ Edit a patron and set a password with leading and/or trailing whitespaces. You should not be allowed to do it (no server side check). 2/ Same at the OPAC ("Change you password" tab). Here there is a server side check. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14509 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|gmcharlt@gmail.com |jonathan.druart@bugs.koha-c | |ommunity.org CC| |jonathan.druart@bugs.koha-c | |ommunity.org -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14509 --- Comment #2 from Marc Véron <veron@veron.ch> --- Created attachment 41295 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=41295&action=edit [Signed-off] Bug 14509: Reject invalid passwords Bug 10177 rejects password with leading or trailing whitespaces, but only on the member-password page. It's not consistent to only do this check on 1 place. This patch adds the check for the 2 other places: memberentry and at the OPAC. Test plan: 1/ Edit a patron and set a password with leading and/or trailing whitespaces. You should not be allowed to do it (no server side check). 2/ Same at the OPAC ("Change you password" tab). Here there is a server side check. Followed test plan. Works as expected. 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=14509 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |veron@veron.ch Attachment #40993|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=14509 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=14509 Joonas Kylmälä <j.kylmala@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |j.kylmala@gmail.com --- Comment #3 from Joonas Kylmälä <j.kylmala@gmail.com> --- Attachment 41295 uses client side checking for the whitespaces in the staff side password changing view. Shouldn't we change it to server side as it looks like all the other fields are checked that way. I think it would be more consistent. What do you think? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14509 --- Comment #4 from Owen Leonard <oleonard@myacpl.org> --- (In reply to Joonas Kylmälä from comment #3)
Shouldn't we change it to server side as it looks like all the other fields are checked that way.
There are changes to the template and to opac-passwd.pl. To confirm, disable JavaScript and see if the server-side check is working correctly as well. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14509 --- Comment #5 from Joonas Kylmälä <j.kylmala@gmail.com> --- (In reply to Owen Leonard from comment #4)
(In reply to Joonas Kylmälä from comment #3)
Shouldn't we change it to server side as it looks like all the other fields are checked that way.
There are changes to the template and to opac-passwd.pl. To confirm, disable JavaScript and see if the server-side check is working correctly as well.
opac-passwd.pl is for the end user server side check, right? I disabled JavaScript in the staff side and there wasn't any check from the server side, it allowed to change the password (this was also mentioned in the test plan). So should we change the code so that it checks for the whitespaces in the server side when the password is changed from the staff view? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14509 --- Comment #6 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Joonas Kylmälä from comment #5)
(In reply to Owen Leonard from comment #4)
(In reply to Joonas Kylmälä from comment #3)
Shouldn't we change it to server side as it looks like all the other fields are checked that way.
There are changes to the template and to opac-passwd.pl. To confirm, disable JavaScript and see if the server-side check is working correctly as well.
opac-passwd.pl is for the end user server side check, right? I disabled JavaScript in the staff side and there wasn't any check from the server side, it allowed to change the password (this was also mentioned in the test plan). So should we change the code so that it checks for the whitespaces in the server side when the password is changed from the staff view?
If you like, you can provide it. But I don't know why a librarian will force a password if the application denies it. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14509 --- Comment #7 from Joonas Kylmälä <j.kylmala@gmail.com> --- (In reply to Jonathan Druart from comment #6)
(In reply to Joonas Kylmälä from comment #5)
(In reply to Owen Leonard from comment #4)
(In reply to Joonas Kylmälä from comment #3)
Shouldn't we change it to server side as it looks like all the other fields are checked that way.
There are changes to the template and to opac-passwd.pl. To confirm, disable JavaScript and see if the server-side check is working correctly as well.
opac-passwd.pl is for the end user server side check, right? I disabled JavaScript in the staff side and there wasn't any check from the server side, it allowed to change the password (this was also mentioned in the test plan). So should we change the code so that it checks for the whitespaces in the server side when the password is changed from the staff view?
If you like, you can provide it. But I don't know why a librarian will force a password if the application denies it.
Librarian doesn't need to force the password, only use a browser where the JavaScript is disabled. So I give one vote for the server side check. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14509 --- Comment #8 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- The intranet side does not work if JS is disabled. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14509 --- Comment #9 from Joonas Kylmälä <j.kylmala@gmail.com> --- Well it doesn't (In reply to Jonathan Druart from comment #8)
The intranet side does not work if JS is disabled.
It works and looks actually pretty ok, from the whole process of finding the patron with the search bar in the top to changing the password. But I also think it is not so urgent to do the server side check. However, there is bug 14620, so if it goes to upstream then maybe the password validation could go there and then it could check also for the spaces :) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14509 Joonas Kylmälä <j.kylmala@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=14620 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14509 --- Comment #10 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Maybe it could be a good enh to add something like: "Your password hasn't been changed." to the message in the OPAC. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14509 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #11 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Opac works great, but I can't trigger the message changing my password from the 'Change password' page in staff (http://localhost:8080/cgi-bin/koha/members/member-password.pl?member=1) It seems to work alright without this patch. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14509 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14509 --- Comment #12 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 42194 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=42194&action=edit Bug 14509: (follow-up) Reject invalid passwords The members.js was not included in member-password.tt And it cannot, there are specific code which cannot be used outside of memberentry.tt -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14509 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA CC| |m.de.rooy@rijksmuseum.nl --- Comment #13 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- member-password and member-entry are not consistent: member-password does not allow trailing space, but member-entry does ... member-entry: 1234<space> becomes $2a$08$se1EC5tmOPHRh2NKoHlVVu4UWzq.SBTh1zY/qjQ94qzZPOd20HcW2 1234 becomes $2a$08$0nW80AYBaRFf5MEVoesp0uzMAGYlTZiZDlpTl4bjQNx7bhIGGS6FW FQA -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14509 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |m.de.rooy@rijksmuseum.nl |y.org | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14509 --- Comment #14 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Marcel, you should get the JS alert preventing the form to be submitted. Are you sure you have correctly refreshed the cache? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14509 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14509 --- Comment #15 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 42324 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=42324&action=edit Bug 14509: Reject invalid passwords Bug 10177 rejects password with leading or trailing whitespaces, but only on the member-password page. It's not consistent to only do this check on 1 place. This patch adds the check for the 2 other places: memberentry and at the OPAC. Test plan: 1/ Edit a patron and set a password with leading and/or trailing whitespaces. You should not be allowed to do it (no server side check). 2/ Same at the OPAC ("Change you password" tab). Here there is a server side check. Followed test plan. Works as expected. Signed-off-by: Marc Véron <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=14509 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #42194|0 |1 is obsolete| | --- Comment #16 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 42325 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=42325&action=edit Bug 14509: (follow-up) Reject invalid passwords The members.js was not included in member-password.tt And it cannot, there are specific code which cannot be used outside of memberentry.tt 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=14509 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=14509 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #41295|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=14509 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master CC| |tomascohen@gmail.com --- Comment #17 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Patches pushed to master. Thanks Jonathan! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14509 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |chris@bigballofwax.co.nz Status|Pushed to Master |Pushed to Stable --- Comment #18 from Chris Cormack <chris@bigballofwax.co.nz> --- Pushed to 3.20.x will be in 3.20.5 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14509 Liz Rea <liz@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |liz@catalyst.net.nz --- Comment #19 from Liz Rea <liz@catalyst.net.nz> --- Pushed to 3.18.x, is in 3.18.12. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14509 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Koha still allows spaces at |Don't allow spaces at the |the end, beginning or as |end, beginning or as your |your password |password -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org