Bug 22216 - Make GET /patrons/{patron_id} staff only
Summary: Make GET /patrons/{patron_id} staff only
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: REST api (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Tomás Cohen Arazi
QA Contact: Josef Moravec
URL:
Keywords:
Depends on: 22061
Blocks: 22218
  Show dependency treegraph
 
Reported: 2019-01-28 14:29 UTC by Tomás Cohen Arazi
Modified: 2019-02-26 09:24 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 22216: Make GET /patrons/{patron_id} staff only (2.77 KB, patch)
2019-01-29 14:10 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 22216: Make GET /patrons/{patron_id} staff only (2.83 KB, patch)
2019-02-13 11:09 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 22216: Make GET /patrons/{patron_id} staff only (2.88 KB, patch)
2019-02-20 11:00 UTC, Josef Moravec
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi 2019-01-28 14:29:57 UTC
Now that we voted to have a /public namespace for unprivileged access endpoints, the existing /patrons endpoint needs to be adapted.
Comment 1 Tomás Cohen Arazi 2019-01-29 14:10:04 UTC
Created attachment 84506 [details] [review]
Bug 22216: Make GET /patrons/{patron_id} staff only

This patch removes the possibility to access the patron object
identified by patron_id by the patron itself, or a guarantor.

It does so by removing the permissions from the spec. The tests are
adjusted to remove that use case.

To test:
- Apply this patch
- Run:
  $ kshell
 k$ prove t/db_dependent/api/v1/patrons.t
=> SUCCESS: Tests pass!
- Sign off :-D
Comment 2 Martin Renvoize 2019-02-13 11:09:29 UTC
Created attachment 85059 [details] [review]
Bug 22216: Make GET /patrons/{patron_id} staff only

This patch removes the possibility to access the patron object
identified by patron_id by the patron itself, or a guarantor.

It does so by removing the permissions from the spec. The tests are
adjusted to remove that use case.

To test:
- Apply this patch
- Run:
  $ kshell
 k$ prove t/db_dependent/api/v1/patrons.t
=> SUCCESS: Tests pass!
- Sign off :-D

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 3 Martin Renvoize 2019-02-13 11:09:49 UTC
Works well for me, signing off
Comment 4 Marcel de Rooy 2019-02-15 11:21:42 UTC
Looks good but this patch removes those lines that tested allow-owner functionality. And now we have this 'feature' but no tests?
Comment 5 Tomás Cohen Arazi 2019-02-15 12:31:45 UTC
(In reply to Marcel de Rooy from comment #4)
> Looks good but this patch removes those lines that tested allow-owner
> functionality. And now we have this 'feature' but no tests?

The resources in /patrons/ are no longer accessible by unprivileged users. New endpoints will be added that will use it, in the /public namespace. There's uno already in master: bug 22061.
Comment 6 Josef Moravec 2019-02-20 11:00:35 UTC
Created attachment 85319 [details] [review]
Bug 22216: Make GET /patrons/{patron_id} staff only

This patch removes the possibility to access the patron object
identified by patron_id by the patron itself, or a guarantor.

It does so by removing the permissions from the spec. The tests are
adjusted to remove that use case.

To test:
- Apply this patch
- Run:
  $ kshell
 k$ prove t/db_dependent/api/v1/patrons.t
=> SUCCESS: Tests pass!
- Sign off :-D

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 7 Nick Clemens 2019-02-22 15:07:59 UTC
Awesome work all!

Pushed to master for 19.05
Comment 8 Martin Renvoize 2019-02-26 09:24:38 UTC
Change of behaviour, will not be backported to the stable 18.11.x series.