Attachment 100735 Details for Bug 24862 – Bug 24862: Handle annonymous sessions gracefuly

[patch] Bug 24862: Handle annonymous sessions gracefuly

Bug-24862-Handle-annonymous-sessions-gracefuly.patch (text/plain), 3.08 KB, created by Nick Clemens (kidclamp) on 2020-03-16 11:22:41 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Nick Clemens (kidclamp)

Created: 2020-03-16 11:22:41 UTC

Size: 3.08 KB

patch

obsolete

>From 3013373c419f76edc4079871990a8ebba21c135d Mon Sep 17 00:00:00 2001
>From: Tomas Cohen Arazi <tomascohen@theke.io>
>Date: Fri, 13 Mar 2020 12:03:03 -0300
>Subject: [PATCH] Bug 24862: Handle annonymous sessions gracefuly
>
>This patch introduces code to detect (cookie) annonymous sessions and
>act as expected.
>
>Right now, as check_cookie_auth is not passed the required permissions
>(because there aren't always required permissions, and the code to check
>permissions is shared with other authentication mechanisms) it returns
>'ok' and the session id. This use case was overlooked when this was
>coded, and yeilds unexpected error codes (500) when the user logs out
>and the annonymous session cookie is used to hit the API. The end result
>doesn't pose any security issue (i.e. the resource access is rejected)
>but the returned error code is not correct and should be fixed.
>
>This patch verifies for an anonymous session (and avoids querying the
>corresponding patron) and then verifies if there is an authorization
>config on the route and if the patron object is defined.
>
>To test:
>1. Apply the tests patch
>2. Run:
>   $ kshell
>  k$ prove t/db_dependent/api/v1/auth_authenticate_api_request.t
>=> FAIL: Tests fail, 500 instead of the expected 401
>3. Apply this patch
>4. Repeat 2
>=> SUCCESS: Tests pass!
>5. Repeat the original 'steps to reproduce' from the bug report using
>   the browser
>=> SUCCESS: Problem solved!
>6. Sign off :-D
>
>Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
>Sponsored-by: ByWater Solutions
>
>Signed-off-by: David Nind <david@davidnind.com>
>
>Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
>---
> Koha/REST/V1/Auth.pm | 16 +++++++++++++---
> 1 file changed, 13 insertions(+), 3 deletions(-)
>
>diff --git a/Koha/REST/V1/Auth.pm b/Koha/REST/V1/Auth.pm
>index ca3e914e5e..07792d7c94 100644
>--- a/Koha/REST/V1/Auth.pm
>+++ b/Koha/REST/V1/Auth.pm
>@@ -207,7 +207,9 @@ sub authenticate_api_request {
>                                                 { remote_addr => $remote_addr });
>         if ($status eq "ok") {
>             my $session = get_session($sessionID);
>-            $user = Koha::Patrons->find($session->param('number'));
>+            $user = Koha::Patrons->find( $session->param('number') )
>+              unless $session->param('sessiontype')
>+                 and $session->param('sessiontype') eq 'anon';
>         }
>         elsif ($status eq "maintenance") {
>             Koha::Exceptions::UnderMaintenance->throw(
>@@ -233,12 +235,20 @@ sub authenticate_api_request {
> 
>     $c->stash('koha.user' => $user);
> 
>-    # We do not need any authorization
>-    unless ($authorization) {
>+    if ( !$authorization ) {
>+        # We do not need any authorization
>         # Check the parameters
>         validate_query_parameters( $c, $spec );
>         return 1;
>     }
>+    else {
>+        # We are required authorizarion, there needs
>+        # to be an identified user
>+        Koha::Exceptions::Authentication::Required->throw(
>+            error => 'Authentication failure.' )
>+          unless $user;
>+    }
>+
> 
>     my $permissions = $authorization->{'permissions'};
>     # Check if the user is authorized
>-- 
>2.11.0

Actions: View | Diff | Splinter Review

Attachments on bug 24862: 100683 | 100684 | 100692 | 100693 | 100734 | 100735