Bug 22047

Summary: set_password should have a 'skip_validation' param
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, lucas, martin.renvoize, nick, tomascohen
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: 21178, 21547    
Bug Blocks: 17006, 22048    
Attachments: Bug 22047: Unit tests
Bug 22047: Add 'skip_validation' param to Koha::Patron->set_password
Bug 22047: Fix opac/opac-passwd.pl call
Bug 22047: Unit tests
Bug 22047: Add 'skip_validation' param to Koha::Patron->set_password
Bug 22047: Fix opac/opac-passwd.pl call
Bug 22047: Unit tests
Bug 22047: Add 'skip_validation' param to Koha::Patron->set_password
Bug 22047: Fix opac/opac-passwd.pl call
Bug 22047: Unit tests
Bug 22047: Add 'skip_validation' param to Koha::Patron->set_password
Bug 22047: Fix opac/opac-passwd.pl call

Description Tomás Cohen Arazi 2018-12-26 14:25:49 UTC
On removing traces of update_password in the codebase, I noticed the use case in Auth_with_ldap.pm which basically skips any password quality check. I'm not sure if that 'feature' is or should be used. For now, we should provide this alternate behaviour. The best approach is adding an 'unsafe' param.
Comment 1 Tomás Cohen Arazi 2018-12-26 15:47:25 UTC
Created attachment 83491 [details] [review]
Bug 22047: Unit tests
Comment 2 Tomás Cohen Arazi 2018-12-26 15:47:28 UTC
Created attachment 83492 [details] [review]
Bug 22047: Add 'skip_validation' param to Koha::Patron->set_password

This patch makes Koha::Patron->set_password expect a hashref as param
and adds support for an 'skip_validation' param to be passed. Its purpose is to
make the method skip the relevant password strength checks if required.

It targets the Auth_with_ldap.pm usage when the 'update_password' flag
is set in the configuration.

The tests on this bug cover this use case so, to test:

- Apply the tests patch
- Run:
  $ kshell
 k$ prove t/db_dependent/Koha/Patrons.t
=> FAIL: Tests fail, code doesn't work as expected
- Apply this patch
- Run:
 k$ prove t/db_dependent/Koha/Patrons.t
=> SUCCESS: Tests pass! Yay!
- Sign off :-D
Comment 3 Tomás Cohen Arazi 2018-12-26 15:47:31 UTC
Created attachment 83493 [details] [review]
Bug 22047: Fix opac/opac-passwd.pl call
Comment 4 Kyle M Hall 2019-01-02 13:41:00 UTC
Created attachment 83582 [details] [review]
Bug 22047: Unit tests

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 5 Kyle M Hall 2019-01-02 13:41:33 UTC
Created attachment 83583 [details] [review]
Bug 22047: Add 'skip_validation' param to Koha::Patron->set_password

This patch makes Koha::Patron->set_password expect a hashref as param
and adds support for an 'skip_validation' param to be passed. Its purpose is to
make the method skip the relevant password strength checks if required.

It targets the Auth_with_ldap.pm usage when the 'update_password' flag
is set in the configuration.

The tests on this bug cover this use case so, to test:

- Apply the tests patch
- Run:
  $ kshell
 k$ prove t/db_dependent/Koha/Patrons.t
=> FAIL: Tests fail, code doesn't work as expected
- Apply this patch
- Run:
 k$ prove t/db_dependent/Koha/Patrons.t
=> SUCCESS: Tests pass! Yay!
- Sign off :-D

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 6 Kyle M Hall 2019-01-02 13:41:35 UTC
Created attachment 83584 [details] [review]
Bug 22047: Fix opac/opac-passwd.pl call

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 7 Josef Moravec 2019-01-02 20:05:25 UTC
Created attachment 83594 [details] [review]
Bug 22047: Unit tests

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

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 8 Josef Moravec 2019-01-02 20:05:29 UTC
Created attachment 83595 [details] [review]
Bug 22047: Add 'skip_validation' param to Koha::Patron->set_password

This patch makes Koha::Patron->set_password expect a hashref as param
and adds support for an 'skip_validation' param to be passed. Its purpose is to
make the method skip the relevant password strength checks if required.

It targets the Auth_with_ldap.pm usage when the 'update_password' flag
is set in the configuration.

The tests on this bug cover this use case so, to test:

- Apply the tests patch
- Run:
  $ kshell
 k$ prove t/db_dependent/Koha/Patrons.t
=> FAIL: Tests fail, code doesn't work as expected
- Apply this patch
- Run:
 k$ prove t/db_dependent/Koha/Patrons.t
=> SUCCESS: Tests pass! Yay!
- Sign off :-D

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

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 9 Josef Moravec 2019-01-02 20:05:33 UTC
Created attachment 83596 [details] [review]
Bug 22047: Fix opac/opac-passwd.pl call

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

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 10 Nick Clemens 2019-01-25 17:56:29 UTC
Minor rebase needed
Comment 11 Tomás Cohen Arazi 2019-01-25 19:00:37 UTC
Created attachment 84422 [details] [review]
Bug 22047: Unit tests

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 12 Tomás Cohen Arazi 2019-01-25 19:00:42 UTC
Created attachment 84423 [details] [review]
Bug 22047: Add 'skip_validation' param to Koha::Patron->set_password

This patch makes Koha::Patron->set_password expect a hashref as param
and adds support for an 'skip_validation' param to be passed. Its purpose is to
make the method skip the relevant password strength checks if required.

It targets the Auth_with_ldap.pm usage when the 'update_password' flag
is set in the configuration.

The tests on this bug cover this use case so, to test:

- Apply the tests patch
- Run:
  $ kshell
 k$ prove t/db_dependent/Koha/Patrons.t
=> FAIL: Tests fail, code doesn't work as expected
- Apply this patch
- Run:
 k$ prove t/db_dependent/Koha/Patrons.t
=> SUCCESS: Tests pass! Yay!
- Sign off :-D

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

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 13 Tomás Cohen Arazi 2019-01-25 19:00:46 UTC
Created attachment 84424 [details] [review]
Bug 22047: Fix opac/opac-passwd.pl call

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 14 Nick Clemens 2019-01-25 20:50:36 UTC
Awesome work all!

Pushed to master for 19.05
Comment 15 Martin Renvoize 2019-01-31 16:47:44 UTC
Invisible enhancement, I'm choosing to backport this to the 18.11.x series for 18.11.03
Comment 16 Lucas Gass 2019-02-01 22:48:29 UTC
missing dependency for 18.05.x will not backport