From bc822009481b7ad38db06c2d55b64ec7e0c25ae9 Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi 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 Sponsored-by: ByWater Solutions Signed-off-by: Tomas Cohen Arazi Signed-off-by: David Nind Signed-off-by: Nick Clemens Signed-off-by: Martin Renvoize (cherry picked from commit de07356028d5b98af3a7cd7cbae02a7ad6402a43) Signed-off-by: Aleisha Amohia (cherry picked from commit 99a3d1193ebfcb1ae5046bf36d60b1e53f8c2e93) Signed-off-by: Victor Grousset/tuxayo --- Koha/REST/V1/Auth.pm | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/Koha/REST/V1/Auth.pm b/Koha/REST/V1/Auth.pm index 5e505cb47b..7184bc4f50 100644 --- a/Koha/REST/V1/Auth.pm +++ b/Koha/REST/V1/Auth.pm @@ -195,8 +195,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')); - # $c->stash('koha.user' => $user); + $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( @@ -222,12 +223,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.20.1