Summary: | GET /patrons should use Koha::Patrons->search_limited | ||
---|---|---|---|
Product: | Koha | Reporter: | Tomás Cohen Arazi <tomascohen> |
Component: | REST API | Assignee: | Tomás Cohen Arazi <tomascohen> |
Status: | CLOSED FIXED | QA Contact: | |
Severity: | normal | ||
Priority: | P5 - low | CC: | 1joynelson, andrewfh, fridolin.somers, jonathan.druart, katrin.fischer, kyle, lucas, m.de.rooy, martin.renvoize, nick, victor |
Version: | unspecified | ||
Hardware: | All | ||
OS: | All | ||
See Also: | https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29523 | ||
Change sponsored?: | --- | Patch complexity: | --- |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: |
22.05.00,21.11.02,21.05.08
|
|
Bug Depends on: | |||
Bug Blocks: | 29506 | ||
Attachments: |
Bug 29503: Regression tests
Bug 29503: Make GET /patrons use Koha::Patrons->search_limited Bug 29503: Regression tests Bug 29503: Make GET /patrons use Koha::Patrons->search_limited Bug 29503: Regression tests Bug 29503: Make GET /patrons use Koha::Patrons->search_limited |
Description
Tomás Cohen Arazi
2021-11-17 14:22:00 UTC
Created attachment 127735 [details] [review] Bug 29503: Regression tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 127736 [details] [review] Bug 29503: Make GET /patrons use Koha::Patrons->search_limited This patch makes the controller method use Koha::Patrons->search_limited so filters based on configuration and permissions apply when fetching patrons. To test: 1. Apply the regression tests patch 2. Run: $ kshell k$ prove t/db_dependent/api/v1/patrons.t => FAIL: Boo, you get more patrons than you should 3. Apply the patch 4. Repeat 2 => SUCCESS: Yay! Things are filtered as expected (i.e. using Koha::Patron->libraries_where_can_see_patrons) 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> I made it like this, mainly because of backportability. But I will file a separate bug to update objects.search to apply ->search_filtered when appropriate (i.e. when $result_set->can('search_filtered')). Created attachment 127737 [details] [review] Bug 29503: Regression tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 127738 [details] [review] Bug 29503: Make GET /patrons use Koha::Patrons->search_limited This patch makes the controller method use Koha::Patrons->search_limited so filters based on configuration and permissions apply when fetching patrons. To test: 1. Apply the regression tests patch 2. Run: $ kshell k$ prove t/db_dependent/api/v1/patrons.t => FAIL: Boo, you get more patrons than you should 3. Apply the patch 4. Repeat 2 => SUCCESS: Yay! Things are filtered as expected (i.e. using Koha::Patron->libraries_where_can_see_patrons) 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Great work, works as described and contains unit tests. Signing off. I'm lowering the severity because, as bug 29509 highlights, we actually require all _borrowers_ permissions to use the route (iincluding view_borrower_infos_from_any_libraries). But those permissions are too high for using the routes on the API, and this bug (and bug 29506) will prevent leakages when time comes. Created attachment 128666 [details] [review] Bug 29503: Regression tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 128667 [details] [review] Bug 29503: Make GET /patrons use Koha::Patrons->search_limited This patch makes the controller method use Koha::Patrons->search_limited so filters based on configuration and permissions apply when fetching patrons. To test: 1. Apply the regression tests patch 2. Run: $ kshell k$ prove t/db_dependent/api/v1/patrons.t => FAIL: Boo, you get more patrons than you should 3. Apply the patch 4. Repeat 2 => SUCCESS: Yay! Things are filtered as expected (i.e. using Koha::Patron->libraries_where_can_see_patrons) 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> (In reply to Tomás Cohen Arazi from comment #7) > I'm lowering the severity because, as bug 29509 highlights, we actually > require all _borrowers_ permissions to use the route (iincluding > view_borrower_infos_from_any_libraries). > > But those permissions are too high for using the routes on the API, and this > bug (and bug 29506) will prevent leakages when time comes. I wrote this and forgot to change the 'Product'. Sorry for that Pushed to master for 22.05, thanks to everybody involved 🦄 Pushed to 21.11.x for 21.11.02 Pushed to 21.05.x for 21.05.08 Not backported to oldoldstable (20.11.x). Feel free to ask if it's needed. |