Bug 21547 - Use set_password in opac-passwd and remove sub goodkey
Summary: Use set_password in opac-passwd and remove sub goodkey
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: Marcel de Rooy
QA Contact: Tomás Cohen Arazi
URL:
Keywords:
Depends on: 21178
Blocks: 22047 22048
  Show dependency treegraph
 
Reported: 2018-10-11 09:56 UTC by Marcel de Rooy
Modified: 2020-01-06 20:14 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Part of the ongoing effort to improve the maintainability of our codebase.
Version(s) released in:


Attachments
Bug 21547: Use update_password in opac-passwd and remove sub goodkey (2.94 KB, patch)
2018-10-11 10:07 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 21547: Use update_password in opac-passwd and remove sub goodkey (3.00 KB, patch)
2018-10-11 10:19 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 21547: Use set_password in opac-passwd and remove sub goodkey (3.04 KB, patch)
2018-10-11 10:48 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 21547: Use set_password in opac-passwd and remove sub goodkey (3.05 KB, patch)
2018-12-05 17:08 UTC, Owen Leonard
Details | Diff | Splinter Review
erg goed! (3.11 KB, patch)
2018-12-05 17:56 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 21547: Use set_password in opac-passwd and remove sub goodkey (3.11 KB, patch)
2018-12-05 17:56 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 21547: (follow-up) Rely on Koha::Patron->set_password checks (2.55 KB, patch)
2018-12-26 15:31 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Marcel de Rooy 2018-10-11 09:56:15 UTC

    
Comment 1 Marcel de Rooy 2018-10-11 10:07:10 UTC Comment hidden (obsolete)
Comment 2 Marcel de Rooy 2018-10-11 10:19:48 UTC Comment hidden (obsolete)
Comment 3 Marcel de Rooy 2018-10-11 10:21:59 UTC
Tomas, care to sign this one ?
Comment 4 Marcel de Rooy 2018-10-11 10:48:50 UTC
Created attachment 80400 [details] [review]
Bug 21547: Use set_password in opac-passwd and remove sub goodkey

Remove sql statement to change password by calling set_password.
Remove sub goodkey by calling C4::Auth::checkpw_hash.
Adding the scalar before param Oldkey (from bug 21036).

Rebased on top of 21178 (using set_password instead of update_password).

Test plan:
Try to change password in OPAC with good and bad pw.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 5 Marcel de Rooy 2018-10-11 10:50:08 UTC
(In reply to Marcel de Rooy from comment #3)
> Tomas, care to sign this one ?

Especially since it is now using set_password :)
Note that this is no behavior change, since the sql statement did not touch userid either..
Comment 6 Owen Leonard 2018-12-05 17:08:56 UTC Comment hidden (obsolete)
Comment 7 Tomás Cohen Arazi 2018-12-05 17:56:02 UTC Comment hidden (obsolete)
Comment 8 Tomás Cohen Arazi 2018-12-05 17:56:47 UTC
Created attachment 82883 [details] [review]
Bug 21547: Use set_password in opac-passwd and remove sub goodkey

Remove sql statement to change password by calling set_password.
Remove sub goodkey by calling C4::Auth::checkpw_hash.
Adding the scalar before param Oldkey (from bug 21036).

Rebased on top of 21178 (using set_password instead of update_password).

Test plan:
Try to change password in OPAC with good and bad pw.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 9 Tomás Cohen Arazi 2018-12-05 17:57:07 UTC
erg goed!
Comment 10 Marcel de Rooy 2018-12-06 07:11:38 UTC
(In reply to Tomás Cohen Arazi from comment #9)
> erg goed!

nudo de lenguaje ?

Thx
Comment 11 Tomás Cohen Arazi 2018-12-26 15:31:52 UTC
Created attachment 83490 [details] [review]
Bug 21547: (follow-up) Rely on Koha::Patron->set_password checks

This patch makes the controller just call $patron->set_password and use
the exceptions it might raise instead of manually checking the passwor
strength.

No behaviour change should be expected. It also removes some leftovers.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 12 Nick Clemens 2018-12-26 20:56:44 UTC
Awesome work all!

Pushed to master for 19.05
Comment 13 Martin Renvoize 2019-01-03 14:30:59 UTC
Pushed to 18.11.x for 18.11.02

I chose to backport this architectural change as it's simple cleanup and may aid in future backports.