Description
Alex Buckley
2017-10-23 11:24:58 UTC
Created attachment 68386 [details] [review] Bug 19514 - Password restrictions implemented into onboarding tool Test plan: 1. Drop and recreate your database 2. Restart memcached 3. Go through the web installer 4. In the onboarding tool create a patron with a password of only 2 characters in length 5. Notice the patron is successfully created and no warning message is displayed 6. Repeat step 1,2,3 and create a patron with a password of 3 characters none of which are a uppercase letter or number and notice the patron is successfully created and no warning message is displayed 7. Apply patch 8. Repeat steps 1,2,3 and create a patron with a password consisting of 2 characters, notice that after submitting the form the same form is loaded again and there is a warning message at the top of the page informing you the patron wasn't created 9. Repeat steps 1,2,3 and create a patron with a password consisting of 3 characters (all lower case) and submit the form, notice the same form is reloaded and a warning message at the top of the page informs you that the patron wasn't created because the password was weak 10. Repeat steps 1,2,3 and create a patron with a password consisting of 3 characters (one lower case letter, one upper case letter and one number) and submit the form and notice this time the next form in the onboarding is displayed with the message at the top of the screen informing you that the patron was successfully created Sponsored-By: Catalyst IT Created attachment 68387 [details] [review] Bug 19514 - Password restrictions implemented into onboarding tool Test plan: 1. Drop and recreate your database 2. Restart memcached 3. Go through the web installer 4. In the onboarding tool create a patron with a password of only 2 characters in length 5. Notice the patron is successfully created and no warning message is displayed 6. Repeat step 1,2,3 and create a patron with a password of 3 characters none of which are a uppercase letter or number and notice the patron is successfully created and no warning message is displayed 7. Apply patch 8. Repeat steps 1,2,3 and create a patron with a password consisting of 2 characters, notice that after submitting the form the same form is loaded again and there is a warning message at the top of the page informing you the patron wasn't created 9. Repeat steps 1,2,3 and create a patron with a password consisting of 3 characters (all lower case) and submit the form, notice the same form is reloaded and a warning message at the top of the page informs you that the patron wasn't created because the password was weak 10. Repeat steps 1,2,3 and create a patron with a password consisting of 3 characters (one lower case letter, one upper case letter and one number) and submit the form and notice this time the next form in the onboarding is displayed with the message at the top of the screen informing you that the patron was successfully created Sponsored-By: Catalyst IT Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> As discussed on IRC: Did you plan to change the setting of minimum password length in onboarding too on another patch ? Hi Marcel Yes I plan to have a patch attached to bug 18308 for setting the minimum password length in the onboarding tool Alex, it works but there is no client-side validation Take a look at commit 7cc65af6ffdabbabe7ae3463f51096de375216ad Bug 18298: Use the validate jQuery plugin We want this form to behave like others. Hi Jonathan I have tried implemented the client side validation by doing the following: 1. Adding the following pragma at the start of onboardingstep3.tt: [% USE Koha %] 2. Adding the following to the bottom of the same file: [% PROCESS 'password_check.inc' %] [% PROCESS 'add_password_check' new_password => 'newpassword' %] 3. Added the following to the validation in the ../../../js/onboarding.js password: required: true, password_strong: true password_no_spaces: true },? password2: { required: true, password_match: true } . I notice that I get the JS error 'TypeError: _(...).format is not a function. This error is not thrown by the memberentrygen.tt file which uses the same client side validation. Can you give me any suggestions as to how to fix this issue Created attachment 68982 [details] [review] Bug 19514: Add client-side check Created attachment 69208 [details] [review] Bug 19514 - Password restrictions implemented into onboarding tool Test plan: 1. Drop and recreate your database 2. Restart memcached 3. Go through the web installer 4. In the onboarding tool create a patron with a password of only 2 characters in length 5. Notice the patron is successfully created and no warning message is displayed 6. Repeat step 1,2,3 and create a patron with a password of 3 characters none of which are a uppercase letter or number and notice the patron is successfully created and no warning message is displayed 7. Apply patch 8. Repeat steps 1,2,3 and create a patron with a password consisting of 2 characters, notice that after submitting the form the same form is loaded again and there is a warning message at the top of the page informing you the patron wasn't created 9. Repeat steps 1,2,3 and create a patron with a password consisting of 3 characters (all lower case) and submit the form, notice the same form is reloaded and a warning message at the top of the page informs you that the patron wasn't created because the password was weak 10. Repeat steps 1,2,3 and create a patron with a password consisting of 3 characters (one lower case letter, one upper case letter and one number) and submit the form and notice this time the next form in the onboarding is displayed with the message at the top of the screen informing you that the patron was successfully created Sponsored-By: Catalyst IT Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: David Bourgault <david.bourgault@inlibro.com> Created attachment 69209 [details] [review] Bug 19514: Add client-side check Signed-off-by: David Bourgault <david.bourgault@inlibro.com> Tested working on Firefox 57, Chrome 62 and Opera 49. Did not work on Firefox 58 from the mozillateam/firefox-next PPA (ubuntu). Marking as signed-off anyway since Firefox 58 is considered beta, and this provides both server-side and client-side validation. QA tool green. Lowering severity, this does not appear critical to me. Created attachment 69796 [details] [review] Bug 19514 - Password restrictions implemented into onboarding tool Test plan: 1. Drop and recreate your database 2. Restart memcached 3. Go through the web installer 4. In the onboarding tool create a patron with a password of only 2 characters in length 5. Notice the patron is successfully created and no warning message is displayed 6. Repeat step 1,2,3 and create a patron with a password of 3 characters none of which are a uppercase letter or number and notice the patron is successfully created and no warning message is displayed 7. Apply patch 8. Repeat steps 1,2,3 and create a patron with a password consisting of 2 characters, notice that after submitting the form the same form is loaded again and there is a warning message at the top of the page informing you the patron wasn't created 9. Repeat steps 1,2,3 and create a patron with a password consisting of 3 characters (all lower case) and submit the form, notice the same form is reloaded and a warning message at the top of the page informs you that the patron wasn't created because the password was weak 10. Repeat steps 1,2,3 and create a patron with a password consisting of 3 characters (one lower case letter, one upper case letter and one number) and submit the form and notice this time the next form in the onboarding is displayed with the message at the top of the screen informing you that the patron was successfully created Sponsored-By: Catalyst IT Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: David Bourgault <david.bourgault@inlibro.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 69797 [details] [review] Bug 19514: Add client-side check Signed-off-by: David Bourgault <david.bourgault@inlibro.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Pushed to master for 18.05, thanks to everybody involved! Awesome work all! Pushed to stable for 17.11.02 Pushed to 17.05.x for v17.05.08 Ouch depends on Bug 18298 (In reply to Fridolin SOMERS from comment #16) > Pushed to 17.05.x for v17.05.08 I choose to revert since depends on new 17.11 feature Bug 18298 |