Bug 24861 - Unable to setPatronPasswordPublic using REST API
Summary: Unable to setPatronPasswordPublic using REST API
Status: CLOSED INVALID
Alias: None
Product: Koha
Classification: Unclassified
Component: REST API (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Bugs List
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-03-12 22:20 UTC by David Cook
Modified: 2020-11-30 21:45 UTC (History)
2 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description David Cook 2020-03-12 22:20:38 UTC
I've tried POST /api/v1/public/patrons/{patron_id}/password with RESTPublicAPI enabled (using koha-testing-docker with Plack enabled), but I keep getting 401 errors.

curl http://localhost:8080/api/v1/public/patrons/42/password -d '{"old_password":"OLD","password":"NEW","password_repeated":"NEW"}'

{"error":"Authentication failure."}

With RESTBasicAuth enabled and using the username and password for patron #42:

curl -u <username:password> http://localhost:8080/api/v1/public/patrons/42/password -d '{"old_password":"OLD","password":"NEW","password_repeated":"NEW"}'

{"error":"Authorization failure. Missing required permission(s).","required_permissions":null}

Perhaps I'm doing something wrong but this looks like a bug to me?
Comment 1 Jonathan Druart 2020-04-07 10:59:42 UTC
(In reply to David Cook from comment #0)
> I've tried POST /api/v1/public/patrons/{patron_id}/password with
> RESTPublicAPI enabled (using koha-testing-docker with Plack enabled), but I
> keep getting 401 errors.
> 
> curl http://localhost:8080/api/v1/public/patrons/42/password -d
> '{"old_password":"OLD","password":"NEW","password_repeated":"NEW"}'
> 
> {"error":"Authentication failure."}

What are you expecting here? To modify the password without auth?

> With RESTBasicAuth enabled and using the username and password for patron
> #42:
> 
> curl -u <username:password>
> http://localhost:8080/api/v1/public/patrons/42/password -d
> '{"old_password":"OLD","password":"NEW","password_repeated":"NEW"}'
> 
> {"error":"Authorization failure. Missing required
> permission(s).","required_permissions":null}
> 
> Perhaps I'm doing something wrong but this looks like a bug to me?

It works for me on master.
Comment 2 Tomás Cohen Arazi 2020-04-07 12:06:29 UTC
The message could be better. Being an error and the code (401) is the expected behavior without authentication.
Comment 3 David Cook 2020-04-07 23:16:14 UTC
(In reply to Jonathan Druart from comment #1)
> What are you expecting here? To modify the password without auth?
> 

I wouldn't expect to modify the password without authentication, as that seems crazy, but I was trying every permutation. 

What's the difference between a "public" and "non-public" Koha API endpoint? I haven't been able to observe any difference. 

> > With RESTBasicAuth enabled and using the username and password for patron
> > #42:
> > 
> > curl -u <username:password>
> > http://localhost:8080/api/v1/public/patrons/42/password -d
> > '{"old_password":"OLD","password":"NEW","password_repeated":"NEW"}'
> > 
> > {"error":"Authorization failure. Missing required
> > permission(s).","required_permissions":null}
> > 
> > Perhaps I'm doing something wrong but this looks like a bug to me?
> 
> It works for me on master.

With that same syntax? The documentation isn't clear.

I'll have to try again. I've noticed RESTBasicAuth doesn't work with CGI, so maybe I was trying it on a non-Plack Koha, and that's why it was failing.
Comment 4 David Cook 2020-04-07 23:17:39 UTC
(In reply to Tomás Cohen Arazi from comment #2)
> The message could be better. Being an error and the code (401) is the
> expected behavior without authentication.

I'll have to test again, but it seems that I was being authenticated, but that I wasn't being authorized:

{"error":"Authorization failure. Missing required permission(s).","required_permissions":null}

But it might be that I was being caught out by RESTBasicAuth not working for CGI. 

Maybe I'll just close this one for the time being, until I have a chance to test again.
Comment 5 Tomás Cohen Arazi 2020-04-07 23:25:43 UTC
(In reply to David Cook from comment #3)
> 
> What's the difference between a "public" and "non-public" Koha API endpoint?
> I haven't been able to observe any difference. 

Public routes are not expected to have 'permissions' required (i.e. they don't need privileged access). For example, a patron should be able to see some of its checkouts information, but can only see others if it has admin rights to see that. A possible approach was to have a big IF in the controller for /checkouts, but we decided to have /public/patrons/:patron_id/checkouts which wouldn't require 'permissions' (in Koha's sense) but requires the requestor to be the resources owner (i.e. identified as a real patron and matching the patron_id with its borrowernumber).

> > > With RESTBasicAuth enabled and using the username and password for patron
> > > #42:
> > > 
> > > curl -u <username:password>
> > > http://localhost:8080/api/v1/public/patrons/42/password -d
> > > '{"old_password":"OLD","password":"NEW","password_repeated":"NEW"}'
> > > 
> > > {"error":"Authorization failure. Missing required
> > > permission(s).","required_permissions":null}
> > > 
> > > Perhaps I'm doing something wrong but this looks like a bug to me?
> > 
> > It works for me on master.
> 
> With that same syntax? The documentation isn't clear.

You shouldn't need any different syntax. How are you testing this? cURL? Postman? Try Postman with Basic authentication, just in case.

> I'll have to try again. I've noticed RESTBasicAuth doesn't work with CGI, so
> maybe I was trying it on a non-Plack Koha, and that's why it was failing.

I submitted a patch to fix your non-Plack issue, or I meant to and forgot. Can you double check?
Comment 6 Tomás Cohen Arazi 2020-04-07 23:28:29 UTC
(In reply to David Cook from comment #4)
> (In reply to Tomás Cohen Arazi from comment #2)
> > The message could be better. Being an error and the code (401) is the
> > expected behavior without authentication.
> 
> I'll have to test again, but it seems that I was being authenticated, but
> that I wasn't being authorized:
> 
> {"error":"Authorization failure. Missing required
> permission(s).","required_permissions":null}

That's a generic handling of an exception that seems to work for the non-public use case and should be handled better for the public case (i.e. it shouldn't mention required permissions). But the exception is raised correctly by allow_owner(). So this is an error message bug.
Comment 7 Tomás Cohen Arazi 2020-04-07 23:30:07 UTC
(In reply to Tomás Cohen Arazi from comment #6)
> (In reply to David Cook from comment #4)
> > (In reply to Tomás Cohen Arazi from comment #2)
> > > The message could be better. Being an error and the code (401) is the
> > > expected behavior without authentication.
> > 
> > I'll have to test again, but it seems that I was being authenticated, but
> > that I wasn't being authorized:
> > 
> > {"error":"Authorization failure. Missing required
> > permission(s).","required_permissions":null}
> 
> That's a generic handling of an exception that seems to work for the
> non-public use case and should be handled better for the public case (i.e.
> it shouldn't mention required permissions). But the exception is raised
> correctly by allow_owner(). So this is an error message bug.

I mean the last fallback exception in authenticate_api_request(), which is reached because allow_owner() returns false in the previous 'if'.