From 7960ba30dfdea9c917e80d35bc0e51eed47d2325 Mon Sep 17 00:00:00 2001
From: Julian Maurice <julian.maurice@biblibre.com>
Date: Mon, 13 May 2024 15:06:04 +0200
Subject: [PATCH] Bug 36598: Improve documentation and error message in CSRF
 plugin

---
 Koha/App/Plugin/CSRF.pm    | 18 +++++++++++-------
 t/db_dependent/mojo/csrf.t |  4 ++--
 2 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/Koha/App/Plugin/CSRF.pm b/Koha/App/Plugin/CSRF.pm
index 579d0178bd..ab84834919 100644
--- a/Koha/App/Plugin/CSRF.pm
+++ b/Koha/App/Plugin/CSRF.pm
@@ -43,14 +43,18 @@ use Koha::Token;
 
 Called by Mojolicious when the plugin is loaded.
 
-Defines an `around_action` hook that will return a 403 response if CSRF token
-is missing or invalid.
+Defines an `around_action` hook that will prevent CSRF attacks.
 
-This verification occurs only for HTTP methods POST, PUT, DELETE and PATCH.
+If the HTTP request method is safe (GET, HEAD, OPTIONS, TRACE) and the request
+contains an `op` parameter whose value starts with "cud-" (which means it's a
+Create, Update or Delete operation), it will immediately return a 400 response.
 
-If CGISESSID cookie is missing, it means that we are not authenticated or we
-are authenticated to the API by another method (HTTP basic or OAuth2). In this
-case, no verification is done.
+If the HTTP request method is unsafe (POST, PUT, DELETE, PATCH, CONNECT) and
+the CGISESSID cookie is set, the CSRF token is checked. A 403 response is
+immediately returned if the CSRF token is missing or invalid.
+If the CGISESSID cookie is missing, it means that we are not authenticated or
+we are authenticated to the API by another method (HTTP basic or OAuth2). In
+this case, no verification is done.
 
 =cut
 
@@ -65,7 +69,7 @@ sub register {
             if ( $method eq 'GET' || $method eq 'HEAD' || $method eq 'OPTIONS' || $method eq 'TRACE' ) {
                 my $op = $c->req->param('op');
                 if ( $op && $op =~ /^cud-/ ) {
-                    return $c->reply->exception('Wrong HTTP method')->rendered(400);
+                    return $c->reply->exception('Incorrect use of a safe HTTP method with a "cud-" op parameter')->rendered(400);
                 }
             } else {
                 if ( $c->cookie('CGISESSID') && !$self->is_csrf_valid( $c->req ) ) {
diff --git a/t/db_dependent/mojo/csrf.t b/t/db_dependent/mojo/csrf.t
index d52dd77b8c..4c67bfc2ef 100755
--- a/t/db_dependent/mojo/csrf.t
+++ b/t/db_dependent/mojo/csrf.t
@@ -44,7 +44,7 @@ subtest 'CSRF - Intranet' => sub {
         plan tests => 3;
 
         $t->get_ok('/cgi-bin/koha/mainpage.pl?op=cud-login')->status_is(400)
-            ->content_like( qr/Wrong HTTP method/, 'Body contains "Wrong HTTP method"' );
+            ->content_like( qr/Incorrect use of a safe HTTP method with a/, 'Body contains correct error message' );
     }
 };
 
@@ -85,6 +85,6 @@ subtest 'CSRF - OPAC' => sub {
         plan tests => 3;
 
         $t->get_ok('/cgi-bin/koha/opac-user.pl?op=cud-login')->status_is(400)
-            ->content_like( qr/Wrong HTTP method/, 'Body contains "Wrong HTTP method"' );
+            ->content_like( qr/Incorrect use of a safe HTTP method with a/, 'Body contains correct error message' );
     }
 };
-- 
2.30.2