[Bug 29272] New: API not respecting $category->effective_change_password
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29272 Bug ID: 29272 Summary: API not respecting $category->effective_change_password Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: NEW Severity: major Priority: P5 - low Component: REST API Assignee: koha-bugs@lists.koha-community.org Reporter: tomascohen@gmail.com The API public routes for changing passwords were developed before bug 10796, and when it was pushed (before this routes), we forgot to adapt things in the API. It only checks OpacPasswordChange, but doesn't consider $category->effective_change_password -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29272 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |tomascohen@gmail.com |ity.org | CC| |jonathan.druart+koha@gmail. | |com Depends on| |10796, 22061 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10796 [Bug 10796] Allow password changes for logged in OPAC users by patron category https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22061 [Bug 22061] Add route to change patron's password (public) -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29272 --- Comment #1 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 126501 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=126501&action=edit Bug 29272: Regression tests This patch introduces regression tests for the described bug. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/api/v1/patrons_password.t => FAIL: Tests fail, excepted failures are considered success Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29272 --- Comment #2 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 126502 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=126502&action=edit Bug 29272: Make public password changing honour category constraints This patch makes the public API routes validate $user->category->effective_change_password before allowing the change. To test: 1. Apply the regression tests patch 2. Run: $ kshell k$ prove t/db_dependent/api/v1/patrons_password.t => FAIL: Tests fail, it allows the first change instead of returning 403. 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29272 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff CC| |kyle@bywatersolutions.com, | |martin.renvoize@ptfs-europe | |.com, | |nick@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29272 --- Comment #3 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Good catch, Jonathan! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29272 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29272 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #126501|0 |1 is obsolete| | --- Comment #4 from David Nind <david@davidnind.com> --- Created attachment 126632 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=126632&action=edit Bug 29272: Regression tests This patch introduces regression tests for the described bug. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/api/v1/patrons_password.t => FAIL: Tests fail, excepted failures are considered success Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29272 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #126502|0 |1 is obsolete| | --- Comment #5 from David Nind <david@davidnind.com> --- Created attachment 126633 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=126633&action=edit Bug 29272: Make public password changing honour category constraints This patch makes the public API routes validate $user->category->effective_change_password before allowing the change. To test: 1. Apply the regression tests patch 2. Run: $ kshell k$ prove t/db_dependent/api/v1/patrons_password.t => FAIL: Tests fail, it allows the first change instead of returning 403. 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29272 --- Comment #6 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Are you sure it's how it's suppose to work? I think you should remove the short-circuit part (checking for the syspref). You should only rely on effective_change_password return. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29272 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=29211 --- Comment #7 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Using the GUI you can have the pref OFF and the setting ON at category level, and the feature is enabled. See also discussion on bug 29211. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29272 --- Comment #8 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #6)
Are you sure it's how it's suppose to work?
I think you should remove the short-circuit part (checking for the syspref). You should only rely on effective_change_password return.
I just checked, and agree! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29272 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #9 from Katrin Fischer <katrin.fischer@bsz-bw.de> ---
From the last comments I understand that another change is required - moving to FQA for the follow-up.
-- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29272 --- Comment #10 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 126814 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=126814&action=edit Bug 29272: (QA follow-up) Category setup overrides syspref This patch removes an early short-circuit we had, which wasn't correct regarding the OpacPasswordChange syspref. If a patron category is allowed to change password, it overrides the syspref. To test: 1. Tests still pass. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29272 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29272 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #126632|0 |1 is obsolete| | --- Comment #11 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 126976 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=126976&action=edit Bug 29272: Regression tests This patch introduces regression tests for the described bug. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/api/v1/patrons_password.t => FAIL: Tests fail, excepted failures are considered success Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29272 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #126633|0 |1 is obsolete| | --- Comment #12 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 126977 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=126977&action=edit Bug 29272: Make public password changing honour category constraints This patch makes the public API routes validate $user->category->effective_change_password before allowing the change. To test: 1. Apply the regression tests patch 2. Run: $ kshell k$ prove t/db_dependent/api/v1/patrons_password.t => FAIL: Tests fail, it allows the first change instead of returning 403. 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29272 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #126814|0 |1 is obsolete| | --- Comment #13 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 126978 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=126978&action=edit Bug 29272: (QA follow-up) Category setup overrides syspref This patch removes an early short-circuit we had, which wasn't correct regarding the OpacPasswordChange syspref. If a patron category is allowed to change password, it overrides the syspref. To test: 1. Tests still pass. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29272 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #14 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- All works as expected now, good catch. Passing QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29272 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |21.11.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29272 --- Comment #15 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Pushed to master for 21.11, thanks to everybody involved! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29272 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |Pushed to stable CC| |fridolin.somers@biblibre.co | |m Version(s)|21.11.00 |21.11.00,21.05.05 released in| | --- Comment #16 from Fridolin Somers <fridolin.somers@biblibre.com> --- It is in 21.05.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29272 --- Comment #17 from Fridolin Somers <fridolin.somers@biblibre.com> --- Does not apply on 20.11.x Please provide a rebase if needed -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29272 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|21.11.00,21.05.05 |21.11.00,21.05.06 released in| | --- Comment #18 from Fridolin Somers <fridolin.somers@biblibre.com> --- It will be in 21.05.06 -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org