Bug 22048 - Use set_password instead of update_password in the codebase
Summary: Use set_password instead of update_password in the codebase
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Tomás Cohen Arazi (tcohen)
QA Contact: Josef Moravec
URL:
Keywords:
Depends on: 21547 22047
Blocks: 21992 22559
  Show dependency treegraph
 
Reported: 2018-12-26 16:30 UTC by Tomás Cohen Arazi (tcohen)
Modified: 2020-01-06 20:15 UTC (History)
3 users (show)

See Also:
GIT URL:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 22048: Use set_password in onboarding.pl (915 bytes, patch)
2018-12-26 16:37 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 22048: Use set_password in member-password.pl (6.33 KB, patch)
2018-12-26 16:37 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 22048: Use set_password in memberentry.pl (1.31 KB, patch)
2018-12-26 16:37 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 22048: Use set_password in create_superlibrarian.pl (926 bytes, patch)
2018-12-26 16:37 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 22048: Use set_password opac/opac-password-recovery.pl (1.91 KB, patch)
2018-12-26 16:37 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 22048: Use set_password in tests (7.81 KB, patch)
2018-12-26 16:37 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 22048: Use set_password in Auth_with_ldap.pm (946 bytes, patch)
2018-12-26 16:38 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 22048: Use set_password in onboarding.pl (987 bytes, patch)
2019-01-02 14:01 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 22048: Use set_password in member-password.pl (6.40 KB, patch)
2019-01-02 14:02 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 22048: Use set_password in memberentry.pl (1.38 KB, patch)
2019-01-02 14:02 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 22048: Use set_password in create_superlibrarian.pl (998 bytes, patch)
2019-01-02 14:02 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 22048: Use set_password opac/opac-password-recovery.pl (1.98 KB, patch)
2019-01-02 14:02 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 22048: Use set_password in tests (7.88 KB, patch)
2019-01-02 14:02 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 22048: Use set_password in Auth_with_ldap.pm (1017 bytes, patch)
2019-01-02 14:02 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 22048: Use set_password in onboarding.pl (1020 bytes, patch)
2019-01-02 20:34 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 22048: Use set_password in member-password.pl (6.44 KB, patch)
2019-01-02 20:34 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 22048: Use set_password in memberentry.pl (1.41 KB, patch)
2019-01-02 20:34 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 22048: Use set_password in create_superlibrarian.pl (1.01 KB, patch)
2019-01-02 20:34 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 22048: Use set_password opac/opac-password-recovery.pl (2.02 KB, patch)
2019-01-02 20:34 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 22048: Use set_password in tests (7.90 KB, patch)
2019-01-02 20:34 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 22048: Use set_password in Auth_with_ldap.pm (1.03 KB, patch)
2019-01-02 20:34 UTC, Josef Moravec
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi (tcohen) 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 (tcohen) 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 (tcohen) 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 (tcohen) 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 (tcohen) 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 (tcohen) 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 (tcohen) 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 (tcohen) 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 (khall) 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 (khall) 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 (khall) 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 (khall) 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 (khall) 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 (khall) 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 (khall) 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 (kidclamp) 2019-01-25 20:50:22 UTC
Awesome work all!

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