From c6c8b66b74c5bc7ee7e715a55bfa3f451f48c4ea Mon Sep 17 00:00:00 2001
From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Date: Mon, 12 Feb 2024 17:36:44 +0100
Subject: [PATCH] Bug 36084: C4::Auth+plack.psgi for svc?

Suggestion to move the CSRF check to CGI->new so that we will check it
for every request, and it will cover svc scripts as well (they are not
using get_template_and_user).

The token will be retrieve from the param list *or the csrf_token
header* (do we want to name it x-koha-csrf-token instead?).

This will be done for *every* request that are not GET: CSRF token is now
required everywhere CGI is used (side-effects possible?).

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
---
 C4/Auth.pm                  | 34 ++++++++--------------------------
 debian/templates/plack.psgi | 35 ++++++++++++++++++++++++++++++-----
 2 files changed, 38 insertions(+), 31 deletions(-)

diff --git a/C4/Auth.pm b/C4/Auth.pm
index a0a93f15c6..156626e488 100644
--- a/C4/Auth.pm
+++ b/C4/Auth.pm
@@ -648,20 +648,9 @@ sub get_template_and_user {
     $template->param( logged_in_user     => $patron );
     $template->param( sessionID          => $sessionID );
 
-    my $op = $in->{query}->param('op');
-    if ( defined $op && $op =~ m{^cud-} ) {
-        unless (
-            Koha::Token->new->check_csrf(
-                {
-                    session_id => scalar $in->{query}->cookie('CGISESSID'),
-                    token      => scalar $in->{query}->param('csrf_token'),
-                }
-            )
-            )
-        {
-            Koha::Logger->get->debug("The form submission failed (Wrong CSRF token).");
-            C4::Output::output_and_exit( $in->{query}, $cookie, $template, 'wrong_csrf_token' );
-        }
+    if ( $in->{query}->param('invalid_csrf_token') ) {
+        Koha::Logger->get->debug("The form submission failed (Wrong CSRF token).");
+        C4::Output::output_and_exit( $in->{query}, $cookie, $template, 'wrong_csrf_token' );
     }
 
     return ( $template, $borrowernumber, $cookie, $flags );
@@ -1362,23 +1351,16 @@ sub checkauth {
         my $patron = $userid ? Koha::Patrons->find({ userid => $userid }) : undef;
         $patron->update_lastseen('login') if $patron;
 
+        # FIXME This is only needed for scripts not using plack
         my $op = $query->param('op');
         if ( defined $op && $op =~ m{^cud-} ) {
             die "Cannot use GET for this request"
                 if $request_method eq 'GET';
+        }
 
-            unless (
-                $skip_csrf_check
-                || Koha::Token->new->check_csrf(
-                    {
-                        session_id => scalar $query->cookie('CGISESSID'),
-                        token      => scalar $query->param('csrf_token'),
-                    }
-                )
-                )
-            {
-                Koha::Exceptions::Token::WrongCSRFToken->throw;
-            }
+
+        if ( !$skip_csrf_check && $query->param('invalid_csrf_token') ) {
+            Koha::Exceptions::Token::WrongCSRFToken->throw;
         }
 
         # In case, that this request was a login attempt, we want to prevent that users can repost the opac login
diff --git a/debian/templates/plack.psgi b/debian/templates/plack.psgi
index eeeeca2b08..f651de493a 100644
--- a/debian/templates/plack.psgi
+++ b/debian/templates/plack.psgi
@@ -65,13 +65,38 @@ use CGI qw(-utf8 ); # we will loose -utf8 under plack, otherwise
         my $original_op    = $q->param('op');
         my $request_method = $q->request_method // q{};
         if ( $stateless_methods{$request_method} && defined $original_op && $original_op =~ m{^cud-} ) {
-            warn "Programming error - op '$original_op' must not start with 'cud-' for $request_method";
+            Koha::Logger->get->warn("Programming error - op '$original_op' must not start with 'cud-' for $request_method");
             $q->param( 'op', '' );
             $q->param( 'debug_programming_error', "'$original_op' must not start with 'cud-' for $request_method" );
-        } elsif ( $stateful_methods{$request_method} && defined $q->param('op') && $original_op !~ m{^cud-} ) {
-            warn "Programming error - op '$original_op' must start with 'cud-' for $request_method";
-            $q->param( 'op', '' );
-            $q->param( 'debug_programming_error', "'$original_op' must start with 'cud-' for $request_method" );
+        } elsif ( $stateful_methods{$request_method} ) {
+            # Get the CSRF token from the param list or the header
+            my $csrf_token = $q->param('csrf_token') || $q->http('HTTP_CSRF_TOKEN');
+
+            if ( defined $q->param('op') && $original_op !~ m{^cud-} ) {
+                Koha::Logger->get->warn("Programming error - op '$original_op' must start with 'cud-' for $request_method");
+                $q->param( 'op', '' );
+                $q->param( 'debug_programming_error', "'$original_op' must start with 'cud-' for $request_method" );
+            }
+
+            if ( $csrf_token ) {
+                unless (
+                    Koha::Token->new->check_csrf(
+                        {
+                            session_id => scalar $q->cookie('CGISESSID'),
+                            token      => $csrf_token,
+                        }
+                    )
+                    )
+                {
+                    Koha::Logger->get->debug("The form submission failed (Wrong CSRF token).");
+                    $q->param( 'op', '' );
+                    $q->param( 'invalid_csrf_token', 1);
+                }
+            } else {
+                Koha::Logger->get->warn("Programming error - No CSRF token passed for $request_method");
+                $q->param( 'op', '' );
+                $q->param( 'debug_programming_error', "No CSRF token passed for $request_method" );
+            }
         }
 
         return $q;
-- 
2.39.5