Bug 22048

Summary: Use set_password instead of update_password in the codebase
Product: Koha Reporter: Tomás Cohen Arazi <tomascohen>
Component: Architecture, internals, and plumbingAssignee: Tomás Cohen Arazi <tomascohen>
Status: CLOSED FIXED QA Contact: Josef Moravec <josef.moravec>
Severity: enhancement    
Priority: P5 - low CC: josef.moravec, martin.renvoize, nick
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 21547, 22047    
Bug Blocks: 21992, 22559    
Attachments: Bug 22048: Use set_password in onboarding.pl
Bug 22048: Use set_password in member-password.pl
Bug 22048: Use set_password in memberentry.pl
Bug 22048: Use set_password in create_superlibrarian.pl
Bug 22048: Use set_password opac/opac-password-recovery.pl
Bug 22048: Use set_password in tests
Bug 22048: Use set_password in Auth_with_ldap.pm
Bug 22048: Use set_password in onboarding.pl
Bug 22048: Use set_password in member-password.pl
Bug 22048: Use set_password in memberentry.pl
Bug 22048: Use set_password in create_superlibrarian.pl
Bug 22048: Use set_password opac/opac-password-recovery.pl
Bug 22048: Use set_password in tests
Bug 22048: Use set_password in Auth_with_ldap.pm
Bug 22048: Use set_password in onboarding.pl
Bug 22048: Use set_password in member-password.pl
Bug 22048: Use set_password in memberentry.pl
Bug 22048: Use set_password in create_superlibrarian.pl
Bug 22048: Use set_password opac/opac-password-recovery.pl
Bug 22048: Use set_password in tests
Bug 22048: Use set_password in Auth_with_ldap.pm

Description Tomás Cohen Arazi 2018-12-26 16:30:28 UTC
update_password has been deprecated and will be removed by bug 21992. We need to switch all the uses in the codebase into using set_password.
Comment 1 Tomás Cohen Arazi 2018-12-26 16:37:42 UTC
Created attachment 83501 [details] [review]
Bug 22048: Use set_password in onboarding.pl

To test, verify the onboarding process sets the password correctly.
Comment 2 Tomás Cohen Arazi 2018-12-26 16:37:46 UTC
Created attachment 83502 [details] [review]
Bug 22048: Use set_password in member-password.pl

This patch makes member-password.pl use $patron->set_password instead of
update_password. The side effect is that setting password and userid
become separate steps in the code.

For the password all the initial checks are the same, but password
strength is checked on calling set_password and an exception is thrown.
So instead of checking the password quality, we just wait for exceptions
and behave the same as before.

Bonus: you will notice I reused the initially fetched $patron object.
Things get simpler :-D

To test:
- Verify that changing the password / userid for a patron works as usual
Comment 3 Tomás Cohen Arazi 2018-12-26 16:37:49 UTC
Created attachment 83503 [details] [review]
Bug 22048: Use set_password in memberentry.pl

To test:
- Verify that changing the password and userid of a patron by globally
editing they works,
Comment 4 Tomás Cohen Arazi 2018-12-26 16:37:52 UTC
Created attachment 83504 [details] [review]
Bug 22048: Use set_password in create_superlibrarian.pl

Use $patron->set_password instead of update_password. In this case the
'skip_validation' param needs to be set as no password strength checks
are expected.
Comment 5 Tomás Cohen Arazi 2018-12-26 16:37:55 UTC
Created attachment 83505 [details] [review]
Bug 22048: Use set_password opac/opac-password-recovery.pl
Comment 6 Tomás Cohen Arazi 2018-12-26 16:37:58 UTC
Created attachment 83506 [details] [review]
Bug 22048: Use set_password in tests

To test:
- Apply this patch
- Run:
  $ kshell
 k$ prove t/db_dependent/Auth.t \
          t/db_dependent/Koha/Patrons.t \
          t/db_dependent/Search/History.t \
          t/db_dependent/selenium/authentication.t \
          t/db_dependent/selenium/regressions.t
=> SUCCESS: Tests pass!
- Sign off :-D
Comment 7 Tomás Cohen Arazi 2018-12-26 16:38:02 UTC
Created attachment 83507 [details] [review]
Bug 22048: Use set_password in Auth_with_ldap.pm
Comment 8 Kyle M Hall 2019-01-02 14:01:40 UTC
Created attachment 83585 [details] [review]
Bug 22048: Use set_password in onboarding.pl

To test, verify the onboarding process sets the password correctly.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 9 Kyle M Hall 2019-01-02 14:02:28 UTC
Created attachment 83586 [details] [review]
Bug 22048: Use set_password in member-password.pl

This patch makes member-password.pl use $patron->set_password instead of
update_password. The side effect is that setting password and userid
become separate steps in the code.

For the password all the initial checks are the same, but password
strength is checked on calling set_password and an exception is thrown.
So instead of checking the password quality, we just wait for exceptions
and behave the same as before.

Bonus: you will notice I reused the initially fetched $patron object.
Things get simpler :-D

To test:
- Verify that changing the password / userid for a patron works as usual

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 10 Kyle M Hall 2019-01-02 14:02:31 UTC
Created attachment 83587 [details] [review]
Bug 22048: Use set_password in memberentry.pl

To test:
- Verify that changing the password and userid of a patron by globally
editing they works,

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 11 Kyle M Hall 2019-01-02 14:02:34 UTC
Created attachment 83588 [details] [review]
Bug 22048: Use set_password in create_superlibrarian.pl

Use $patron->set_password instead of update_password. In this case the
'skip_validation' param needs to be set as no password strength checks
are expected.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 12 Kyle M Hall 2019-01-02 14:02:36 UTC
Created attachment 83589 [details] [review]
Bug 22048: Use set_password opac/opac-password-recovery.pl

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 13 Kyle M Hall 2019-01-02 14:02:39 UTC
Created attachment 83590 [details] [review]
Bug 22048: Use set_password in tests

To test:
- Apply this patch
- Run:
  $ kshell
 k$ prove t/db_dependent/Auth.t \
          t/db_dependent/Koha/Patrons.t \
          t/db_dependent/Search/History.t \
          t/db_dependent/selenium/authentication.t \
          t/db_dependent/selenium/regressions.t
=> SUCCESS: Tests pass!
- Sign off :-D

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 14 Kyle M Hall 2019-01-02 14:02:42 UTC
Created attachment 83591 [details] [review]
Bug 22048: Use set_password in Auth_with_ldap.pm

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 15 Josef Moravec 2019-01-02 20:34:35 UTC
Created attachment 83597 [details] [review]
Bug 22048: Use set_password in onboarding.pl

To test, verify the onboarding process sets the password correctly.

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

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 16 Josef Moravec 2019-01-02 20:34:40 UTC
Created attachment 83598 [details] [review]
Bug 22048: Use set_password in member-password.pl

This patch makes member-password.pl use $patron->set_password instead of
update_password. The side effect is that setting password and userid
become separate steps in the code.

For the password all the initial checks are the same, but password
strength is checked on calling set_password and an exception is thrown.
So instead of checking the password quality, we just wait for exceptions
and behave the same as before.

Bonus: you will notice I reused the initially fetched $patron object.
Things get simpler :-D

To test:
- Verify that changing the password / userid for a patron works as usual

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

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 17 Josef Moravec 2019-01-02 20:34:44 UTC
Created attachment 83599 [details] [review]
Bug 22048: Use set_password in memberentry.pl

To test:
- Verify that changing the password and userid of a patron by globally
editing they works,

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

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 18 Josef Moravec 2019-01-02 20:34:47 UTC
Created attachment 83600 [details] [review]
Bug 22048: Use set_password in create_superlibrarian.pl

Use $patron->set_password instead of update_password. In this case the
'skip_validation' param needs to be set as no password strength checks
are expected.

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

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 19 Josef Moravec 2019-01-02 20:34:51 UTC
Created attachment 83601 [details] [review]
Bug 22048: Use set_password opac/opac-password-recovery.pl

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

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 20 Josef Moravec 2019-01-02 20:34:56 UTC
Created attachment 83602 [details] [review]
Bug 22048: Use set_password in tests

To test:
- Apply this patch
- Run:
  $ kshell
 k$ prove t/db_dependent/Auth.t \
          t/db_dependent/Koha/Patrons.t \
          t/db_dependent/Search/History.t \
          t/db_dependent/selenium/authentication.t \
          t/db_dependent/selenium/regressions.t
=> SUCCESS: Tests pass!
- Sign off :-D

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

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 21 Josef Moravec 2019-01-02 20:34:59 UTC
Created attachment 83603 [details] [review]
Bug 22048: Use set_password in Auth_with_ldap.pm

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

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 22 Nick Clemens 2019-01-25 20:50:22 UTC
Awesome work all!

Pushed to master for 19.05
Comment 23 Martin Renvoize 2019-01-31 16:57:07 UTC
Enhancement, will not be backported at this time to 18.11.x series.