[Bug 22071] New: Make authenticate_api_request stash koha.user in OAuth use case
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22071 Bug ID: 22071 Summary: Make authenticate_api_request stash koha.user in OAuth use case Change sponsored?: --- Product: Koha Version: master 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 Target Milestone: --- The patron that made the request is not being stashed in OAuth authentication. Regression tests and fixes to be submitted. -- 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=22071 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |21198 Status|NEW |ASSIGNED Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21198 [Bug 21198] authenticate_api_request should stash the reason access is granted -- 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=22071 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |oleonard@myacpl.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22071 --- Comment #1 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 83638 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=83638&action=edit Bug 22071: Regression tests The authenticate_api_request() behaviour needs more tests. This patch adds tests for the stashed Koha::Patron object. It highlights the bug in the OAuth authentication case. To test: - Apply this patch - Run: $ kshell k$ prove t/db_dependent/api/v1/auth_authenticate_api_request.t => FAIL: Tests fail in the OAuth case, pass in the cookie-based auth case. -- 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=22071 --- Comment #2 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 83639 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=83639&action=edit Bug 22071: Make authenticate_api_request stash koha.user in OAuth use case This patch makes the OAuth code stash the requestion Koha::Patron. To test: - Apply the tests patch: - Run: $ kshell k$ prove t/db_dependent/api/v1/auth_authenticate_api_request.t => FAIL: Tests fail :-( - Apply this patch - Run: k$ prove t/db_dependent/api/v1/auth_authenticate_api_request.t => SUCCESS: Tests pass :-D - Sign off =D -- 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=22071 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff Patch complexity|--- |Trivial patch -- 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=22071 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |tomascohen@gmail.com |ity.org | -- 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=22071 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #83638|0 |1 is obsolete| | --- Comment #3 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 83641 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=83641&action=edit Bug 22071: Regression tests The authenticate_api_request() behaviour needs more tests. This patch adds tests for the stashed Koha::Patron object. It highlights the bug in the OAuth authentication case. To test: - Apply this patch - Run: $ kshell k$ prove t/db_dependent/api/v1/auth_authenticate_api_request.t => FAIL: Tests fail in the OAuth case, pass in the cookie-based auth case. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22071 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #83639|0 |1 is obsolete| | --- Comment #4 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 83642 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=83642&action=edit Bug 22071: Make authenticate_api_request stash koha.user in OAuth use case This patch makes the OAuth code stash the requestion Koha::Patron. To test: - Apply the tests patch: - Run: $ kshell k$ prove t/db_dependent/api/v1/auth_authenticate_api_request.t => FAIL: Tests fail :-( - Apply this patch - Run: k$ prove t/db_dependent/api/v1/auth_authenticate_api_request.t => SUCCESS: Tests pass :-D - Sign off =D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22071 Charles Farmer <charles.farmer@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off CC| |charles.farmer@inlibro.com --- Comment #5 from Charles Farmer <charles.farmer@inlibro.com> --- I was able to get the same behavior as described in the test plan: "not ok 1 - token-based tests" but "ok 2 - cookie-based tests" The following patch made all tests successful. Had to download Net::OAuth2::AuthorizationServer and activate the syspref RESTOAuth2ClientCredentials, if anybody needs additional instructions. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22071 Charles Farmer <charles.farmer@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #83641|0 |1 is obsolete| | --- Comment #6 from Charles Farmer <charles.farmer@inlibro.com> --- Created attachment 83651 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=83651&action=edit Bug 22071: Regression tests The authenticate_api_request() behaviour needs more tests. This patch adds tests for the stashed Koha::Patron object. It highlights the bug in the OAuth authentication case. To test: - Apply this patch - Run: $ kshell k$ prove t/db_dependent/api/v1/auth_authenticate_api_request.t => FAIL: Tests fail in the OAuth case, pass in the cookie-based auth case. Signed-off-by: Charles Farmer <charles.farmer@inLibro.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22071 Charles Farmer <charles.farmer@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #83642|0 |1 is obsolete| | --- Comment #7 from Charles Farmer <charles.farmer@inlibro.com> --- Created attachment 83652 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=83652&action=edit Bug 22071: Make authenticate_api_request stash koha.user in OAuth use case This patch makes the OAuth code stash the requestion Koha::Patron. To test: - Apply the tests patch: - Run: $ kshell k$ prove t/db_dependent/api/v1/auth_authenticate_api_request.t => FAIL: Tests fail :-( - Apply this patch - Run: k$ prove t/db_dependent/api/v1/auth_authenticate_api_request.t => SUCCESS: Tests pass :-D - Sign off =D Signed-off-by: Charles Farmer <charles.farmer@inLibro.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22071 --- Comment #8 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 83676 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=83676&action=edit Bug 22071: (follow-up) Simplify code In order to add features to this method, the current code would force us to do it for each authentication method. There's duplicated code that could be simplified. This patch makes the authentication code just set $user on each block (oauth and cookie authentication) and moves the final permissions check to the end of the authenticate_api_request method. Overall, the behaviour remains unchanged. To test: - Run: $ kshell k$ prove t/db_dependent/api/v1/auth_authenticate_api_request.t \ t/db_dependent/api/v1/oauth.t => SUCCESS: Tests pass! Nothing changed! - Sign off :-D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22071 --- Comment #9 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 83677 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=83677&action=edit Bug 22071: (follow-up) Add POD for validate_query_parameters 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=22071 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact| |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=22071 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #83651|0 |1 is obsolete| | --- Comment #10 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 83686 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=83686&action=edit Bug 22071: Regression tests The authenticate_api_request() behaviour needs more tests. This patch adds tests for the stashed Koha::Patron object. It highlights the bug in the OAuth authentication case. To test: - Apply this patch - Run: $ kshell k$ prove t/db_dependent/api/v1/auth_authenticate_api_request.t => FAIL: Tests fail in the OAuth case, pass in the cookie-based auth case. Signed-off-by: Charles Farmer <charles.farmer@inLibro.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=22071 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #83652|0 |1 is obsolete| | --- Comment #11 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 83687 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=83687&action=edit Bug 22071: Make authenticate_api_request stash koha.user in OAuth use case This patch makes the OAuth code stash the requestion Koha::Patron. To test: - Apply the tests patch: - Run: $ kshell k$ prove t/db_dependent/api/v1/auth_authenticate_api_request.t => FAIL: Tests fail :-( - Apply this patch - Run: k$ prove t/db_dependent/api/v1/auth_authenticate_api_request.t => SUCCESS: Tests pass :-D - Sign off =D Signed-off-by: Charles Farmer <charles.farmer@inLibro.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=22071 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #83676|0 |1 is obsolete| | --- Comment #12 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 83688 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=83688&action=edit Bug 22071: (follow-up) Simplify code In order to add features to this method, the current code would force us to do it for each authentication method. There's duplicated code that could be simplified. This patch makes the authentication code just set $user on each block (oauth and cookie authentication) and moves the final permissions check to the end of the authenticate_api_request method. Overall, the behaviour remains unchanged. To test: - Run: $ kshell k$ prove t/db_dependent/api/v1/auth_authenticate_api_request.t \ t/db_dependent/api/v1/oauth.t => SUCCESS: Tests pass! Nothing changed! - Sign off :-D 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=22071 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #83677|0 |1 is obsolete| | --- Comment #13 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 83689 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=83689&action=edit Bug 22071: (follow-up) Add POD for validate_query_parameters 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=22071 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> --- A solid improvement to the existing code and good additional of tests. No regressions found and no qa failures found. Passing QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22071 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nick@bywatersolutions.com Status|Passed QA |Pushed to Master --- Comment #15 from Nick Clemens <nick@bywatersolutions.com> --- Awesome work all! Pushed to master for 19.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22071 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |Pushed to Stable --- Comment #16 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Pushed to 18.11.x for 18.11.02 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22071 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Make |authenticate_api_request |authenticate_api_request |does not stash koha.user in |stash koha.user in OAuth |the OAuth use case |use case | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22071 Jesse Maseto <jesse@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jesse@bywatersolutions.com --- Comment #17 from Jesse Maseto <jesse@bywatersolutions.com> --- Pushed to 18.05.x for 18.05.08 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22071 Fridolin SOMERS <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m Depends on| |20568 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20568 [Bug 20568] Add API key management interface for patrons -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22071 Fridolin SOMERS <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED Status|Pushed to Stable |RESOLVED --- Comment #18 from Fridolin SOMERS <fridolin.somers@biblibre.com> --- Depends on Bug 20568 not in 17.11.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22071 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |22132 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22132 [Bug 22132] Add Basic authentication to the REST API -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org