Bug 37286 - Fix REST API authentication when using Mojo apps
Summary: Fix REST API authentication when using Mojo apps
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: REST API (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Julian Maurice
QA Contact: Martin Renvoize (ashimema)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-07-09 11:40 UTC by Julian Maurice
Modified: 2025-03-17 07:28 UTC (History)
7 users (show)

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


Attachments
Bug 37286: Fix REST API authentication when using Mojo apps (4.52 KB, patch)
2024-07-09 11:41 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 37286: Fix REST API authentication when using Mojo apps (4.58 KB, patch)
2025-01-15 10:34 UTC, Magnus Enger
Details | Diff | Splinter Review
Bug 37286: Add tests for REST authentication through Mojo apps (2.39 KB, patch)
2025-01-24 09:59 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 37286: Fix REST API authentication when using Mojo apps (4.60 KB, patch)
2025-03-10 14:03 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 37286: Add tests for REST authentication through Mojo apps (2.42 KB, patch)
2025-03-10 14:03 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 37286: Fix REST API authentication when using Mojo apps (4.67 KB, patch)
2025-03-10 17:31 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 37286: Add tests for REST authentication through Mojo apps (2.49 KB, patch)
2025-03-10 17:31 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Julian Maurice 2024-07-09 11:40:17 UTC
Koha::REST::V1::Auth checks the request URL path and do various things depending on how it looks.
For instance, it allows everyone to access paths starting with "/api/v1/oauth/"

But because of how Koha::REST::V1 was written, when using mojolicious applications Koha::App::Intranet and Koha::App::Opac, paths had to add a path prefix ("/api"), which means the final path as seen by Koha::REST::V1::Auth looked like this: "/api/api/v1/oauth"

I said "had to", but actually there is another way that does not require this path manipulation and that's what this bug is about
Comment 1 Julian Maurice 2024-07-09 11:41:10 UTC Comment hidden (obsolete)
Comment 2 David Nind 2024-10-27 23:21:52 UTC
Any hints on how to set things up for testing this?

In koha-testing-docker, I get this when running step 2 of the test plan:

  bin/intranet daemon -l http://*:8080
  Can't create listen socket: Address already in use at /usr/share/perl5/Mojo/IOLoop.pm line 124

PS I don't know what a "Mojo app" is, so maybe I'm not the right person to test this bug 8-)
Comment 3 Magnus Enger 2025-01-15 10:34:41 UTC
Created attachment 176544 [details] [review]
Bug 37286: Fix REST API authentication when using Mojo apps

Koha::REST::V1::Auth checks the request URL path and do various things
depending on how it looks.
For instance, it allows everyone to access paths starting with
"/api/v1/oauth/"

But because of how Koha::REST::V1 was written, when using mojolicious
applications Koha::App::Intranet and Koha::App::Opac, paths had to add a
path prefix ("/api"), which means the final path as seen by
Koha::REST::V1::Auth looked like this: "/api/api/v1/oauth".

I said "had to", but actually there is another way that does not require
this path manipulation and that's what this patch does:

Koha::REST::V1 now accepts a configuration parameter that allows to
change the base path for API routes, which allows
Koha::App::Plugin::RESTV1 (used by Koha::App::Intranet and
Koha::App::Opac) to generate the right routes.

This configuration parameter defaults to "/api/v1", so when outside of
Koha::App::Intranet and Koha::App::Opac (when using
debian/templates/plack.psgi for instance), the behavior is unchanged.

Test plan:
1. Do not apply the patch yet
2. Run `bin/intranet daemon -l http://*:8080`
3. Run `curl -i -d{} http://127.0.0.1:8080/api/v1/oauth/token`
   It should return a 403 error, with an error message "Authentication
   failure".
4. Stop `bin/intranet daemon -l http://*:8080` by hitting Ctrl-C on the
   terminal you started it
5. Apply the patch
6. Run `bin/intranet daemon -l http://*:8080` again
7. Run `curl -i -d{} http://127.0.0.1:8080/api/v1/oauth/token`
   This time it should return a 400 error with an error message saying
   the "grant_type" property is missing.
   This error is normal as we did not send any data in the POST request
   body, and seeing this means Koha allowed us to use that route because
   it recognized '/api/v1/oauth/' at the start of the URL path
8. You can do the same test with `bin/opac`

Signed-off-by: Magnus Enger <magnus@libriotech.no>
Works as advertised. See Bugzilla for notes from testing.
Comment 4 Magnus Enger 2025-01-15 10:41:19 UTC
Initially, I had the same problem as David (in KTD), but I got around it by replacing :8080 with :8083 in the test plan. Here is what I got: 

=== Server ===
$ bin/intranet daemon -l http://*:8083
[2025-01-15 10:28:23.65073] [11469] [info] Listening at "http://*:8083"
Web application available at http://127.0.0.1:8083

=== Client === 
$ curl -i -d{} http://127.0.0.1:8083/api/v1/oauth/token
HTTP/1.1 401 Unauthorized
Server: Mojolicious (Perl)
Date: Wed, 15 Jan 2025 10:29:10 GMT
Content-Type: application/json; charset=utf8
Content-Length: 35
{"error":"Authentication failure."}

=== Server ===
[2025-01-15 10:29:10.88254] [11469] [trace] [smrnH4W71gxh] POST "/api/api/v1/oauth/token"
[2025-01-15 10:29:10.88285] [11469] [trace] [smrnH4W71gxh] Routing to application "Koha::REST::V1"
[2025-01-15 10:29:10.88386] [11469] [trace] [smrnH4W71gxh] POST "/api/api/v1/oauth/token"
[2025-01-15 10:29:10.88987] [11469] [trace] [smrnH4W71gxh] Routing to controller "Koha::REST::V1::Auth" and action "under"
[2025-01-15 10:29:10.89101] [11469] [trace] [smrnH4W71gxh] 401 Unauthorized (0.007137s, 140.115/s)

Applied the patch and restarted all the things.

=== Server ===
$ bin/intranet daemon -l http://*:8083
[2025-01-15 10:30:57.89162] [12106] [info] Listening at "http://*:8083"
Web application available at http://127.0.0.1:8083

=== Client ===
$ curl -i -d{} http://127.0.0.1:8083/api/v1/oauth/token
HTTP/1.1 400 Bad Request
Server: Mojolicious (Perl)
Content-Length: 79
Content-Type: application/json;charset=UTF-8
Date: Wed, 15 Jan 2025 10:31:04 GMT
{"errors":[{"message":"Missing property.","path":"\/grant_type"}],"status":400}

=== Server ===
[2025-01-15 10:31:04.33156] [12106] [trace] [QQTeoJ7i-Vbe] POST "/api/v1/oauth/token"
[2025-01-15 10:31:04.33186] [12106] [trace] [QQTeoJ7i-Vbe] Routing to application "Koha::REST::V1"
[2025-01-15 10:31:04.33434] [12106] [trace] [QQTeoJ7i-Vbe] POST "/api/v1/oauth/token"
[2025-01-15 10:31:04.34088] [12106] [trace] [QQTeoJ7i-Vbe] Routing to controller "Koha::REST::V1::Auth" and action "under"
[2025-01-15 10:31:04.34195] [12106] [trace] [QQTeoJ7i-Vbe] Routing to controller "Koha::REST::V1::OAuth" and action "token"
[2025-01-15 10:31:04.34253] [12106] [trace] [QQTeoJ7i-Vbe] 400 Bad Request (0.008181s, 122.234/s)

So I have verified this works according to the tese plan, but it will definitely need a more thorough review of the code. :-)
Comment 5 Jonathan Druart 2025-01-23 10:48:30 UTC
Julian, do you think you could provide a test?
Comment 6 Julian Maurice 2025-01-24 09:59:12 UTC
Created attachment 177101 [details] [review]
Bug 37286: Add tests for REST authentication through Mojo apps
Comment 7 Julian Maurice 2025-01-24 10:00:12 UTC
(In reply to Jonathan Druart from comment #5)
> Julian, do you think you could provide a test?
Done!
Comment 8 Jonathan Druart 2025-03-10 14:03:27 UTC
Created attachment 179117 [details] [review]
Bug 37286: Fix REST API authentication when using Mojo apps

Koha::REST::V1::Auth checks the request URL path and do various things
depending on how it looks.
For instance, it allows everyone to access paths starting with
"/api/v1/oauth/"

But because of how Koha::REST::V1 was written, when using mojolicious
applications Koha::App::Intranet and Koha::App::Opac, paths had to add a
path prefix ("/api"), which means the final path as seen by
Koha::REST::V1::Auth looked like this: "/api/api/v1/oauth".

I said "had to", but actually there is another way that does not require
this path manipulation and that's what this patch does:

Koha::REST::V1 now accepts a configuration parameter that allows to
change the base path for API routes, which allows
Koha::App::Plugin::RESTV1 (used by Koha::App::Intranet and
Koha::App::Opac) to generate the right routes.

This configuration parameter defaults to "/api/v1", so when outside of
Koha::App::Intranet and Koha::App::Opac (when using
debian/templates/plack.psgi for instance), the behavior is unchanged.

Test plan:
1. Do not apply the patch yet
2. Run `bin/intranet daemon -l http://*:8080`
3. Run `curl -i -d{} http://127.0.0.1:8080/api/v1/oauth/token`
   It should return a 403 error, with an error message "Authentication
   failure".
4. Stop `bin/intranet daemon -l http://*:8080` by hitting Ctrl-C on the
   terminal you started it
5. Apply the patch
6. Run `bin/intranet daemon -l http://*:8080` again
7. Run `curl -i -d{} http://127.0.0.1:8080/api/v1/oauth/token`
   This time it should return a 400 error with an error message saying
   the "grant_type" property is missing.
   This error is normal as we did not send any data in the POST request
   body, and seeing this means Koha allowed us to use that route because
   it recognized '/api/v1/oauth/' at the start of the URL path
8. You can do the same test with `bin/opac`

Signed-off-by: Magnus Enger <magnus@libriotech.no>
Works as advertised. See Bugzilla for notes from testing.
Comment 9 Jonathan Druart 2025-03-10 14:03:30 UTC
Created attachment 179118 [details] [review]
Bug 37286: Add tests for REST authentication through Mojo apps

Amended-by: Jonathan Druart
Add +x
Comment 10 Martin Renvoize (ashimema) 2025-03-10 17:31:47 UTC
Created attachment 179129 [details] [review]
Bug 37286: Fix REST API authentication when using Mojo apps

Koha::REST::V1::Auth checks the request URL path and do various things
depending on how it looks.
For instance, it allows everyone to access paths starting with
"/api/v1/oauth/"

But because of how Koha::REST::V1 was written, when using mojolicious
applications Koha::App::Intranet and Koha::App::Opac, paths had to add a
path prefix ("/api"), which means the final path as seen by
Koha::REST::V1::Auth looked like this: "/api/api/v1/oauth".

I said "had to", but actually there is another way that does not require
this path manipulation and that's what this patch does:

Koha::REST::V1 now accepts a configuration parameter that allows to
change the base path for API routes, which allows
Koha::App::Plugin::RESTV1 (used by Koha::App::Intranet and
Koha::App::Opac) to generate the right routes.

This configuration parameter defaults to "/api/v1", so when outside of
Koha::App::Intranet and Koha::App::Opac (when using
debian/templates/plack.psgi for instance), the behavior is unchanged.

Test plan:
1. Do not apply the patch yet
2. Run `bin/intranet daemon -l http://*:8080`
3. Run `curl -i -d{} http://127.0.0.1:8080/api/v1/oauth/token`
   It should return a 403 error, with an error message "Authentication
   failure".
4. Stop `bin/intranet daemon -l http://*:8080` by hitting Ctrl-C on the
   terminal you started it
5. Apply the patch
6. Run `bin/intranet daemon -l http://*:8080` again
7. Run `curl -i -d{} http://127.0.0.1:8080/api/v1/oauth/token`
   This time it should return a 400 error with an error message saying
   the "grant_type" property is missing.
   This error is normal as we did not send any data in the POST request
   body, and seeing this means Koha allowed us to use that route because
   it recognized '/api/v1/oauth/' at the start of the URL path
8. You can do the same test with `bin/opac`

Signed-off-by: Magnus Enger <magnus@libriotech.no>
Works as advertised. See Bugzilla for notes from testing.
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 11 Martin Renvoize (ashimema) 2025-03-10 17:31:50 UTC
Created attachment 179130 [details] [review]
Bug 37286: Add tests for REST authentication through Mojo apps

Amended-by: Jonathan Druart
Add +x

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 12 Martin Renvoize (ashimema) 2025-03-10 17:32:39 UTC
I'm also happy here, so adding my SO stamp.. I've also drawn Tomas's attention to it.. but I reckon we can PQA here now.
Comment 13 Katrin Fischer 2025-03-17 06:49:23 UTC
Additional tests and a lot of removed FIXMES... :)
Comment 14 Katrin Fischer 2025-03-17 07:28:05 UTC
Pushed for 25.05!

Well done everyone, thank you!