Bug 22216

Summary: Make GET /patrons/{patron_id} staff only
Product: Koha Reporter: Tomás Cohen Arazi <tomascohen>
Component: REST APIAssignee: Tomás Cohen Arazi <tomascohen>
Status: CLOSED FIXED QA Contact: Josef Moravec <josef.moravec>
Severity: normal    
Priority: P5 - low CC: josef.moravec, katrin.fischer, m.de.rooy, martin.renvoize, nick
Version: master   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 22061    
Bug Blocks: 22218    
Attachments: Bug 22216: Make GET /patrons/{patron_id} staff only
Bug 22216: Make GET /patrons/{patron_id} staff only
Bug 22216: Make GET /patrons/{patron_id} staff only

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.