Created attachment 83993 [details] [review]
Bug 22132: Add RESTBasicAuth syspref

Created attachment 83994 [details] [review]
Bug 22132: Unit tests

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

Created attachment 84353 [details] [review]
Bug 22132: Add RESTBasicAuth syspref

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Created attachment 84354 [details] [review]
Bug 22132: Unit tests

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

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>

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.

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>

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>

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>

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>

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

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>

(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

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>

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>

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.

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>

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>

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>

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>

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>

This is definitely an API enhancement I'd love to see backported as far as possible!

Awesome work all!

Pushed to master for 19.05

Pushed to 18.11.x for 18.11.03 as per my policy on API non-breaking change api enhancements.

pushed to 18.05.x for 18.05.09

running into problems with set_password in 18.05. reverting this patchset. it will NOT be in 18.05.x

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]﷑

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>

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.