Bug 22132

Summary: Add Basic authentication to the REST API
Product: Koha Reporter: Tomás Cohen Arazi (tcohen) <tomascohen>
Component: REST APIAssignee: Tomás Cohen Arazi (tcohen) <tomascohen>
Status: CLOSED FIXED QA Contact: Martin Renvoize (ashimema) <martin.renvoize>
Severity: new feature    
Priority: P5 - low CC: alex.arnaud, dcook, fridolin.somers, josef.moravec, julian.maurice, kyle, lucas, martin.renvoize, nick, oleonard, tomascohen
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
This adds http BASIC authentication as an optional auth method to the RESTful APIs. This greatly aids developers when developing against our APIs.
Version(s) released in:
Circulation function:
Bug Depends on: 22071    
Bug Blocks: 13895, 22206, 24003    
Attachments: Bug 22132: Add RESTBasicAuth syspref
Bug 22132: Unit tests
Bug 22132: Add Basic authentication to the API
Bug 22132: Add RESTBasicAuth syspref
Bug 22132: Unit tests
Bug 22132: Add Basic authentication to the API
Bug 22132: Add RESTBasicAuth syspref
Bug 22132: Unit tests
Bug 22132: Add Basic authentication to the API
Bug 22132: (QA follow-up) Tests - use Mojo builtin for auth
Bug 22132: Add RESTBasicAuth syspref
Bug 22132: Unit tests
Bug 22132: Add Basic authentication to the API
Bug 22132: (QA follow-up) Tests - use Mojo builtin for auth
Bug 22132: (QA follow-up) Tests - use Mojo builtin for auth
Bug 22132: (QA follow-up) Tests - use Mojo builtin for auth
Bug 22132: (QA follow-up) Tests - use Mojo builtin for auth
Bug 22132: Add RESTBasicAuth syspref
Bug 22132: Unit tests
Bug 22132: Add Basic authentication to the API
Bug 22132: (QA follow-up) Tests - use Mojo builtin for auth
Bug 22132: (QA follow-up) set_password now expects a hashref
[18.05.x] Bug 22132: (follow-up) Make tests use Koha::Patron->update_password

Description Tomás Cohen Arazi (tcohen) 2019-01-15 11:11:47 UTC
It would help testing using tools like Postman or Rested easier.
Comment 1 Tomás Cohen Arazi (tcohen) 2019-01-15 16:27:55 UTC Comment hidden (obsolete)
Comment 2 Tomás Cohen Arazi (tcohen) 2019-01-15 16:27:59 UTC Comment hidden (obsolete)
Comment 3 Tomás Cohen Arazi (tcohen) 2019-01-15 16:28:05 UTC Comment hidden (obsolete)
Comment 4 Tomás Cohen Arazi (tcohen) 2019-01-15 17:47:17 UTC
Created attachment 83993 [details] [review]
Bug 22132: Add RESTBasicAuth syspref
Comment 5 Tomás Cohen Arazi (tcohen) 2019-01-15 17:47:20 UTC
Created attachment 83994 [details] [review]
Bug 22132: Unit tests
Comment 6 Tomás Cohen Arazi (tcohen) 2019-01-15 17:47:23 UTC
Created attachment 83995 [details] [review]
Bug 22132: Add Basic authentication to the API

This patch implements Basic authentication for the API to aid testers
and developers with tools like Postman.

To test:
- Apply this patches
- Run:
  $ kshell
 k$ prove t/db_dependent/api/v1/auth_basic.t
=> SUCCESS: Tests pass!
- Open Postman or your favourite tool
- In Authorization, choose Basic auth.
- Enter the username and password of a known privileged user.
- Hit an endpoint with Postman, for example:
  ] GET http://kohadev-intra.myDNSname.org:8081/api/v1/patrons
=> SUCCESS: Basic authentication is great!
- Sign off :-D
Comment 7 Kyle M Hall (khall) 2019-01-24 14:59:28 UTC
Created attachment 84353 [details] [review]
Bug 22132: Add RESTBasicAuth syspref

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 8 Kyle M Hall (khall) 2019-01-24 14:59:41 UTC
Created attachment 84354 [details] [review]
Bug 22132: Unit tests

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 9 Kyle M Hall (khall) 2019-01-24 14:59:45 UTC
Created attachment 84355 [details] [review]
Bug 22132: Add Basic authentication to the API

This patch implements Basic authentication for the API to aid testers
and developers with tools like Postman.

To test:
- Apply this patches
- Run:
  $ kshell
 k$ prove t/db_dependent/api/v1/auth_basic.t
=> SUCCESS: Tests pass!
- Open Postman or your favourite tool
- In Authorization, choose Basic auth.
- Enter the username and password of a known privileged user.
- Hit an endpoint with Postman, for example:
  ] GET http://kohadev-intra.myDNSname.org:8081/api/v1/patrons
=> SUCCESS: Basic authentication is great!
- Sign off :-D

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 10 Martin Renvoize (ashimema) 2019-01-24 16:35:55 UTC
Created attachment 84361 [details] [review]
Bug 22132: (QA follow-up) Tests - use Mojo builtin for auth

Mojolicious has built in handling for encoding/decoding of of basic auth
paramenters. We should use it to simplify our test here.
Comment 11 Martin Renvoize (ashimema) 2019-01-24 16:37:05 UTC
Created attachment 84362 [details] [review]
Bug 22132: Add RESTBasicAuth syspref

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 12 Martin Renvoize (ashimema) 2019-01-24 16:37:09 UTC
Created attachment 84363 [details] [review]
Bug 22132: Unit tests

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 13 Martin Renvoize (ashimema) 2019-01-24 16:37:13 UTC
Created attachment 84364 [details] [review]
Bug 22132: Add Basic authentication to the API

This patch implements Basic authentication for the API to aid testers
and developers with tools like Postman.

To test:
- Apply this patches
- Run:
  $ kshell
 k$ prove t/db_dependent/api/v1/auth_basic.t
=> SUCCESS: Tests pass!
- Open Postman or your favourite tool
- In Authorization, choose Basic auth.
- Enter the username and password of a known privileged user.
- Hit an endpoint with Postman, for example:
  ] GET http://kohadev-intra.myDNSname.org:8081/api/v1/patrons
=> SUCCESS: Basic authentication is great!
- Sign off :-D

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 14 Martin Renvoize (ashimema) 2019-01-24 16:37:16 UTC
Created attachment 84365 [details] [review]
Bug 22132: (QA follow-up) Tests - use Mojo builtin for auth

Mojolicious has built in handling for encoding/decoding of of basic auth
paramenters. We should use it to simplify our test here.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 15 Martin Renvoize (ashimema) 2019-01-24 16:38:16 UTC
All works for me and seems like sane and sensible code.

I've added a QA follow-up with a little code golf on the tests, I'll let the RM decide whether it's worthwhile or not keeping that final patch.

Passing QA
Comment 16 Martin Renvoize (ashimema) 2019-01-24 16:41:25 UTC
Created attachment 84368 [details] [review]
Bug 22132: (QA follow-up) Tests - use Mojo builtin for auth

Mojolicious has built in handling for encoding/decoding of of basic auth
paramenters. We should use it to simplify our test here.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 17 Tomás Cohen Arazi (tcohen) 2019-01-24 16:42:43 UTC
(In reply to Martin Renvoize from comment #15)
> All works for me and seems like sane and sensible code.
> 
> I've added a QA follow-up with a little code golf on the tests, I'll let the
> RM decide whether it's worthwhile or not keeping that final patch.
> 
> Passing QA

Thanks!! I like the changes! I will adopt that style myself
Comment 18 Martin Renvoize (ashimema) 2019-01-24 16:43:13 UTC
Created attachment 84369 [details] [review]
Bug 22132: (QA follow-up) Tests - use Mojo builtin for auth

Mojolicious has built in handling for encoding/decoding of of basic auth
paramenters. We should use it to simplify our test here.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 19 Martin Renvoize (ashimema) 2019-01-24 17:02:06 UTC
Created attachment 84371 [details] [review]
Bug 22132: (QA follow-up) Tests - use Mojo builtin for auth

Mojolicious has built in handling for encoding/decoding of of basic auth
paramenters. We should use it to simplify our test here.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 20 Martin Renvoize (ashimema) 2019-01-24 17:05:35 UTC
The followup also happens to clarify why this addition is a good thing.. it should make writing tests simpler as it requires less scaffolding for authenticated api tests.
Comment 21 Tomás Cohen Arazi (tcohen) 2019-01-30 18:47:38 UTC
Created attachment 84535 [details] [review]
Bug 22132: Add RESTBasicAuth syspref

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 22 Tomás Cohen Arazi (tcohen) 2019-01-30 18:47:43 UTC
Created attachment 84536 [details] [review]
Bug 22132: Unit tests

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 23 Tomás Cohen Arazi (tcohen) 2019-01-30 18:47:48 UTC
Created attachment 84537 [details] [review]
Bug 22132: Add Basic authentication to the API

This patch implements Basic authentication for the API to aid testers
and developers with tools like Postman.

To test:
- Apply this patches
- Run:
  $ kshell
 k$ prove t/db_dependent/api/v1/auth_basic.t
=> SUCCESS: Tests pass!
- Open Postman or your favourite tool
- In Authorization, choose Basic auth.
- Enter the username and password of a known privileged user.
- Hit an endpoint with Postman, for example:
  ] GET http://kohadev-intra.myDNSname.org:8081/api/v1/patrons
=> SUCCESS: Basic authentication is great!
- Sign off :-D

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 24 Tomás Cohen Arazi (tcohen) 2019-01-30 18:47:52 UTC
Created attachment 84538 [details] [review]
Bug 22132: (QA follow-up) Tests - use Mojo builtin for auth

Mojolicious has built in handling for encoding/decoding of of basic auth
paramenters. We should use it to simplify our test here.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 25 Tomás Cohen Arazi (tcohen) 2019-01-30 18:47:56 UTC
Created attachment 84539 [details] [review]
Bug 22132: (QA follow-up) set_password now expects a hashref

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 26 Kyle M Hall (khall) 2019-01-31 12:21:38 UTC
This is definitely an API enhancement I'd love to see backported as far as possible!
Comment 27 Nick Clemens (kidclamp) 2019-01-31 13:38:17 UTC
Awesome work all!

Pushed to master for 19.05
Comment 28 Martin Renvoize (ashimema) 2019-02-01 15:47:49 UTC
Pushed to 18.11.x for 18.11.03 as per my policy on API non-breaking change api enhancements.
Comment 29 Lucas Gass (lukeg) 2019-02-08 00:33:44 UTC
pushed to 18.05.x for 18.05.09
Comment 30 Lucas Gass (lukeg) 2019-02-12 16:24:33 UTC
running into problems with set_password in 18.05. reverting this patchset. it will NOT be in 18.05.x
Comment 31 Martin Renvoize (ashimema) 2019-02-12 16:46:47 UTC
I believe set_password is only used in the test is t it... Wasn't there a simple function that came before it... Perhaps Tomas would provide you a patch for the 18.05 branch.. or I could probably.. if we're keen to have it.. Kyle seemed very keen ﷐[U+1F609]﷑
Comment 32 Tomás Cohen Arazi (tcohen) 2019-02-12 17:19:45 UTC
Created attachment 85024 [details] [review]
[18.05.x] Bug 22132: (follow-up) Make tests use Koha::Patron->update_password

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 33 David Cook 2020-03-12 06:29:57 UTC
Unfortunately, RESTBasicAuth doesn't work with CGI. 

Admittedly, it would be an unfortunate idea to use the API with CGI, but it's worth noting. 

To test (w/ koha-testing-docker):
1. koha-plack --disable kohadev
2. restart_all
3. curl -u koha:koha http://localhost:8080/api/v1/patrons
4. Observe the following response:
{"error":"Authentication failure."}

After a bit of debugging, it appears that Mojolicious doesn't detect the Authorization header. It'll just have the accept, user-agent, and host headers.