Attachment 171122 Details for Bug 36598 – Bug 36598: Improve documentation and error message in CSRF plugin

[patch] Bug 36598: Improve documentation and error message in CSRF plugin

Bug-36598-Improve-documentation-and-error-message-.patch (text/plain), 3.33 KB, created by Marcel de Rooy on 2024-09-06 07:40:27 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Marcel de Rooy

Created: 2024-09-06 07:40:27 UTC

Size: 3.33 KB

patch

obsolete

>From 3d2a6da592046ffcb363ab62310e1e984135b753 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
>Content-Type: text/plain; charset=utf-8
>
>Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
>
>Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
>---
> 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

Actions: View | Diff | Splinter Review

Attachments on bug 36598: 164847 | 164848 | 164870 | 165075 | 165076 | 166094 | 166634 | 166673 | 166675 | 166727 | 166728 | 166729 | 166730 | 166731 | 166732 | 166733 | 171119 | 171120 | 171121 | 171122 | 171123 | 171124 | 171125