[Bug 22522] New: ILL API breaks with updated Mojolicious version

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

[Bug 22522] New: ILL API breaks with updated Mojolicious version

bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522

            Bug ID: 22522
           Summary: ILL API breaks with updated Mojolicious version
 Change sponsored?: ---
           Product: Koha
           Version: 18.05
          Hardware: All
                OS: Linux
            Status: NEW
          Severity: normal
          Priority: P5 - low
         Component: ILL
          Assignee: [hidden email]
          Reporter: [hidden email]
                CC: [hidden email],
                    [hidden email]
  Target Milestone: ---

When we tried to setup the ILL module using FreeForm as its backend, the ILL
requests page in our staff client would not load any of the requests. We would
get an error message saying that there was a problem with missing content in
the Datable.

Upon investigation, we discovered that the calls made to the API were returning
"Malformed query" errors. When validating the query, Auth.pm checks for
authorized parameters in openapi.op_spec, which wasn't defined anywhere in our
case. After a quick discussion with Andrew Isherwood on IRC, we downgraded
Mojolicious (from 8.02 to 7.21), JSON::Validator (from 3.02 to 0.97) and
Mojolicious::Plugin::OpenAPI (from 2.08 to 1.15) and everything worked.

Isn't this problematic ? If these modules aren't installed or don't meet the
minimum version required by Koha, cpan will install the most recent versions
and the API won't work. While this was experienced with the ILL module, I
assume that other API calls can and will break in the same way.

--
You are receiving this mail because:
You are the assignee for the bug.
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 22522] ILL API breaks with updated Mojolicious version

bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522

Andrew Isherwood <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[hidden email]-c
                   |                            |ommunity.org,
                   |                            |[hidden email]

--- Comment #1 from Andrew Isherwood <[hidden email]> ---
I agree with José-Mario, if Mojolicious is introducing breaking changes like
this we should be pinning the version to be installed.

Looking at the release changelog
(https://github.com/mojolicious/mojo/blob/master/Changes) 7.21 is now over two
years old, so we should probably be making gradual moves forward on that too.

It seems to be a very fast moving project, so pinning the max version is
probably the best thing to do in the short term.

--
You are receiving this mail because:
You are watching all bug changes.
You are the assignee for the bug.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 22522] API authentication breaks with updated Mojolicious version

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522

Andrew Isherwood <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|ILL API breaks with updated |API authentication breaks
                   |Mojolicious version         |with updated Mojolicious
                   |                            |version

--
You are receiving this mail because:
You are watching all bug changes.
You are the assignee for the bug.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 22522] API authentication breaks with updated Mojolicious version

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522

Josef Moravec <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[hidden email]

--
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 22522] API authentication breaks with updated Mojolicious version

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522

--- Comment #2 from José-Mario Monteiro-Santos <[hidden email]> ---
I've noticed today that with newer Mojolicious versions, the authentication is
basically skipped as x-koha-authorization is never defined. It seems that this
makes it so all endpoints do not require authorization to be accessed. This is
a major security flaw, since anybody can for example access patron's
information.

--
You are receiving this mail because:
You are watching all bug changes.
You are the assignee for the bug.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 22522] API authentication breaks with updated Mojolicious version

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522

--- Comment #3 from José-Mario Monteiro-Santos <[hidden email]> ---
Created attachment 86756
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=86756&action=edit
Bug 22522 - Update API specs' access in Auth.pm

With newer versions of Mojolicious and its plugins, endpoints' specs
could no longer be accessed, thus bypassing authorization checks
and failing to validate query parameters.

Test plan:
1. Without being logged in to Koha, access an endpoint directly
   (such as /api/v1/patrons/{patron_id})
2. Notice results are received (which is bad since we're not authenticated)
3. Try again with an endpoint that accepts query parameters
   (such as /api/v1/patrons?firstname=something)
4. Notice that the query is not accepted (even with correct parameters)

5. Apply the patch

6. Repeat step 1
7. Notice that the access is denied
8. Login as a user with proper access rights
9. Repeat step 1
10. Notice that you can now get results
11. Repeat step 3
12. Notice that the query is now accepted
13. Repeat step 3 but with an absurd parameter
14. Notice the query is correctly rejected

15. Ideally, check if other API calls were not broken

--
You are receiving this mail because:
You are the assignee for the bug.
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 22522] API authentication breaks with updated Mojolicious version

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522

José-Mario Monteiro-Santos <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #86756|Bug 22522 - Update API      |Fix for newer
        description|specs' access in Auth.pm    |Mojolicious/OpenAPI
                   |                            |versions

--- Comment #4 from José-Mario Monteiro-Santos <[hidden email]> ---
Comment on attachment 86756
  --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=86756
Fix for newer Mojolicious/OpenAPI versions

>From 9398e9ce1ddb545b031c4accad4148f743eddadd Mon Sep 17 00:00:00 2001
>From: Jose-Mario Monteiro-Santos <[hidden email]>
>Date: Tue, 19 Mar 2019 11:55:45 -0400
>Subject: [PATCH] Bug 22522 - Update API specs' access in Auth.pm
>
>With newer versions of Mojolicious and its plugins, endpoints' specs
>could no longer be accessed, thus bypassing authorization checks
>and failing to validate query parameters.
>
>Test plan:
>1. Without being logged in to Koha, access an endpoint directly
>   (such as /api/v1/patrons/{patron_id})
>2. Notice results are received (which is bad since we're not authenticated)
>3. Try again with an endpoint that accepts query parameters
>   (such as /api/v1/patrons?firstname=something)
>4. Notice that the query is not accepted (even with correct parameters)
>
>5. Apply the patch
>
>6. Repeat step 1
>7. Notice that the access is denied
>8. Login as a user with proper access rights
>9. Repeat step 1
>10. Notice that you can now get results
>11. Repeat step 3
>12. Notice that the query is now accepted
>13. Repeat step 3 but with an absurd parameter
>14. Notice the query is correctly rejected
>
>15. Ideally, check if other API calls were not broken
>---
> Koha/REST/V1/Auth.pm | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/Koha/REST/V1/Auth.pm b/Koha/REST/V1/Auth.pm
>index 53c6bac..d0ecd13 100644
>--- a/Koha/REST/V1/Auth.pm
>+++ b/Koha/REST/V1/Auth.pm
>@@ -130,7 +130,7 @@ sub authenticate_api_request {
>
>     my $user;
>
>-    my $spec = $c->match->endpoint->pattern->defaults->{'openapi.op_spec'};
>+    my $spec = $c->openapi->spec;
>     my $authorization = $spec->{'x-koha-authorization'};
>
>     my $authorization_header = $c->req->headers->authorization;
>--
>2.7.4

--
You are receiving this mail because:
You are watching all bug changes.
You are the assignee for the bug.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
Reply | Threaded
Open this post in threaded view
|

[Bug 22522] API authentication breaks with updated Mojolicious version

bugzilla-daemon
In reply to this post by bugzilla-daemon
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22522

--- Comment #5 from José-Mario Monteiro-Santos <[hidden email]> ---
Slightly modifying Auth.pm restores the original functionnality. It may be
useful as a temporary fix for those who have this problem.

--
You are receiving this mail because:
You are the assignee for the bug.
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[hidden email]
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/