| Summary: | authenticate_api_request should stash the reason access is granted | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Tomás Cohen Arazi (tcohen) <tomascohen> |
| Component: | REST API | Assignee: | Tomás Cohen Arazi (tcohen) <tomascohen> |
| Status: | CLOSED INVALID | QA Contact: | |
| Severity: | enhancement | ||
| Priority: | P5 - low | CC: | alex.arnaud, josef.moravec, kyle, martin.renvoize |
| Version: | Main | ||
| Hardware: | All | ||
| OS: | All | ||
| Change sponsored?: | --- | Patch complexity: | --- |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: | Version(s) released in: | ||
| Circulation function: | |||
| Bug Depends on: | 22071 | ||
| Bug Blocks: | |||
| Attachments: |
Bug 21198: Unit tests
Bug 21198: Make authenticate_api_request stash the reason access is granted |
||
|
Description
Tomás Cohen Arazi (tcohen)
2018-08-09 19:11:13 UTC
Created attachment 83683 [details] [review] Bug 21198: Unit tests Created attachment 83684 [details] [review] Bug 21198: Make authenticate_api_request stash the reason access is granted This patch makes the authenticate_api_request method stash the reason the request or has been granted access to the resource. Possible values are: - permissions - owner - guarantor This will allow the controllers to avoid querying on their own for this info. To test: - Run: $ kshell k$ prove t/db_dependent/api/v1/auth_authenticate_api_request.t => SUCCESS: Tests pass! - Sign off :-D When I thought about this, I was writing the /patrons/:patron_id/password endpoint, which was an hybrid endpoint: it was intended for the user to change its own password, and also an admin user to change a patron's password. It required the controller to check what was the reason to grant access to the resource. We decided to create a /public namespace for endpoints that would be used by unprivileged users instead. I decided to still submit this, because - We might need it - People writing endpoints through plugins could take advantage of this I hope you'll agree with this enhancement.
# Failed test '200 OK'
# at t/db_dependent/api/v1/auth_authenticate_api_request.t line 161.
# got: '403'
# expected: '200'
not ok 9 - grant_reason is 'guarantor'
# Failed test 'grant_reason is 'guarantor''
# at t/db_dependent/api/v1/auth_authenticate_api_request.t line 164.
# got: undef
# expected: 'guarantor'
At this point I'm not sure this really serves any purpose.. I'm going to mark it as invalid for now. We can always revisit it in the future should we choose to do so. In reality, what I believe would be considerably more useful is for us to implement the FIXME in C4::Auth->haspermission `#FIXME - This fcn should return the failed permission so a suitable error msg can be delivered.` Passing back the permission one is missing, perhaps as a catch-able Koha::Exception or something would be a distinctly more useful pattern. |