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: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Tomás Cohen Arazi
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
Modified: 2020-01-06 20:15 UTC (History)
3 users (show)

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


Attachments
Bug 22048: Use set_password in onboarding.pl (915 bytes, patch)
2018-12-26 16:37 UTC, Tomás Cohen Arazi
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
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
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
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
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
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
Details | Diff | Splinter Review
Bug 22048: Use set_password in onboarding.pl (987 bytes, patch)
2019-01-02 14:01 UTC, Kyle M Hall
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
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
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
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
Details | Diff | Splinter Review
Bug 22048: Use set_password in tests (7.88 KB, patch)
2019-01-02 14:02 UTC, Kyle M Hall
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
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 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.