Attachment 123402 Details for Bug 28786 – Bug 28786: Improve readability in C4::Auth::checkauth

[patch] Bug 28786: Improve readability in C4::Auth::checkauth

Bug-28786-Improve-readability-in-C4Authcheckauth.patch (text/plain), 5.64 KB, created by David Nind on 2021-08-02 19:53:30 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: David Nind

Created: 2021-08-02 19:53:30 UTC

Size: 5.64 KB

patch

obsolete

>From 01ccbb4a8d9b4f3f92df7778609f9defdc2a634a Mon Sep 17 00:00:00 2001
>From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
>Date: Fri, 30 Jul 2021 12:36:46 +0200
>Subject: [PATCH] Bug 28786: Improve readability in C4::Auth::checkauth
>
>Sponsored-by: Orex Digital
>
>Signed-off-by: David Nind <david@davidnind.com>
>---
> C4/Auth.pm | 37 ++++++++++++++++++++-----------------
> 1 file changed, 20 insertions(+), 17 deletions(-)
>
>diff --git a/C4/Auth.pm b/C4/Auth.pm
>index db56ecdeda..5b9f6037dc 100644
>--- a/C4/Auth.pm
>+++ b/C4/Auth.pm
>@@ -832,6 +832,7 @@ sub checkauth {
> 
>     # state variables
>     my $loggedin = 0;
>+    my $auth_state = 'failed';
>     my %info;
>     my ( $userid, $cookie, $sessionID, $flags );
>     my $logout = $query->param('logout.x');
>@@ -846,7 +847,6 @@ sub checkauth {
>     my $session;
>     my $invalid_otp_token;
>     my $require_2FA = ( C4::Context->preference('TwoFactorAuthentication') && $type ne "OPAC" ) ? 1 : 0;
>-    my $auth_challenge_complete;
> 
>     # Basic authentication is incompatible with the use of Shibboleth,
>     # as Shibboleth may return REMOTE_USER as a Shibboleth attribute,
>@@ -882,12 +882,15 @@ sub checkauth {
>             $userid = $session->param('id');
>         }
> 
>-        $additional_auth_needed = ( $return eq 'additional-auth-needed' ) ? 1 : 0;
>+        $auth_state =
>+            $return eq 'ok'                     ? 'completed'
>+          : $return eq 'additional-auth-needed' ? 'additional-auth-needed'
>+          :                                       'failed';
> 
>         # We are at the second screen if the waiting-for-2FA is set in session
>         # and otp_token param has been passed
>         if (   $require_2FA
>-            && $additional_auth_needed
>+            && $auth_state eq 'additional-auth-needed'
>             && ( my $otp_token = $query->param('otp_token') ) )
>         {
>             my $patron    = Koha::Patrons->find( { userid => $userid } );
>@@ -896,10 +899,8 @@ sub checkauth {
>             $auth->clear;
>             if ( $verified ) {
>                 # The token is correct, the user is fully logged in!
>-                $additional_auth_needed = 0;
>+                $auth_state = 'completed';
>                 $session->param( 'waiting-for-2FA', 0 );
>-                $return = "ok";
>-                $auth_challenge_complete = 1;
> 
>                # This is an ugly trick to pass the test
>                # $query->param('koha_login_context') && ( $q_userid ne $userid )
>@@ -911,7 +912,7 @@ sub checkauth {
>             }
>         }
> 
>-        if ( $return eq 'ok' ) {
>+        if ( $auth_state eq 'completed' ) {
>             Koha::Logger->get->debug(sprintf "AUTH_SESSION: (%s)\t%s %s - %s", map { $session->param($_) || q{} } qw(cardnumber firstname surname branch));
> 
>             if ( ( $query->param('koha_login_context') && ( $q_userid ne $userid ) )
>@@ -937,9 +938,8 @@ sub checkauth {
>                 my $sessiontype = $session->param('sessiontype') || '';
>                 unless ( $sessiontype && $sessiontype eq 'anon' ) {    #if this is an anonymous session, we want to update the session, but not behave as if they are logged in...
>                     $flags = haspermission( $userid, $flagsrequired );
>-                    if ($flags) {
>-                        $loggedin = 1;
>-                    } else {
>+                    unless ( $flags ) {
>+                        $auth_state = 'failed';
>                         $info{'nopermission'} = 1;
>                     }
>                 }
>@@ -955,7 +955,7 @@ sub checkauth {
>         }
>     }
> 
>-    if ( ( !$loggedin && !$additional_auth_needed ) || $logout ) {
>+    if ( $auth_state eq 'failed' || $logout ) {
>         $sessionID = undef;
>         $session   = undef;
>         $userid    = undef;
>@@ -984,7 +984,7 @@ sub checkauth {
>         $sessionID = undef;
>         $session   = undef;
>         $userid    = undef;
>-        $additional_auth_needed = 0;
>+        $auth_state = 'logout';
>     }
> 
>     unless ( $userid ) {
>@@ -1134,7 +1134,7 @@ sub checkauth {
>             if ($return) {
> 
>                 if ( $flags = haspermission( $userid, $flagsrequired ) ) {
>-                    $loggedin = 1;
>+                    $auth_state = "logged_in";
>                 }
>                 else {
>                     $info{'nopermission'} = 1;
>@@ -1276,18 +1276,21 @@ sub checkauth {
>         }
>     }    # END unless ($userid)
> 
>-    if ( $require_2FA && ( $loggedin && !$auth_challenge_complete)) {
>+
>+    if ( $require_2FA && $auth_state eq 'logged_in' ) {
>         my $patron = Koha::Patrons->find({userid => $userid});
>         if ( $patron->auth_method eq 'two-factor' ) {
>             # Ask for the OTP token
>-            $additional_auth_needed = 1;
>+            $auth_state = 'additional-auth-needed';
>             $session->param('waiting-for-2FA', 1);
>             %info = ();# We remove the warnings/errors we may have set incorrectly before
>+        } else {
>+            $auth_state = 'completed';
>         }
>     }
> 
>     # finished authentification, now respond
>-    if ( ( $loggedin || $authnotrequired ) && !$additional_auth_needed ) {
>+    if ( $auth_state eq 'completed' || $authnotrequired ) {
>         # successful login
>         unless ($cookie) {
>             $cookie = $query->cookie(
>@@ -1381,7 +1384,7 @@ sub checkauth {
>     $template->param( SCI_login => 1 ) if ( $query->param('sci_user_login') );
>     $template->param( OpacPublic => C4::Context->preference("OpacPublic") );
>     $template->param( loginprompt => 1 ) unless $info{'nopermission'};
>-    if ( $additional_auth_needed ) {
>+    if ( $auth_state eq 'additional-auth-needed' ) {
>         $template->param(
>             TwoFA_prompt => 1,
>             invalid_otp_token => $invalid_otp_token,
>-- 
>2.20.1

Actions: View | Diff | Splinter Review

Attachments on bug 28786: 123375 | 123376 | 123377 | 123378 | 123379 | 123380 | 123381 | 123397 | 123398 | 123399 | 123400 | 123401 | 123402 | 123403 | 124569 | 124770 | 124771 | 124844 | 124845 | 124846 | 124847 | 128841 | 128842 | 129383 | 129384 | 129474 | 129521 | 129557 | 129558 | 129559 | 129560 | 129561 | 129562 | 129563 | 129564 | 129565 | 129566 | 129567 | 129568 | 129569 | 129654 | 129655 | 129656 | 129657 | 129658 | 129659 | 129660 | 129661 | 129662 | 129665 | 131940 | 131941 | 131942 | 131943 | 131944 | 131945 | 131946 | 131947 | 131948 | 131949 | 131950 | 131951 | 131952 | 131953 | 131954 | 131955 | 131956 | 131957 | 131958 | 132142 | 132143 | 132144 | 132145 | 132146 | 132147 | 132148 | 132149 | 132150 | 132153 | 132154 | 132155 | 132156 | 132157 | 133222 | 133516 | 133522 | 133531 | 133533 | 133541 | 133548 | 133643 | 133686