Bug 36148: Move CSRF check to a Plack middleware

The easiest here is to not empty 'op' but instead redirect to an error
page.

Minor changes: to keep the patch simple it removed the 'dev only' error and
display the error for non-dev installs. It should not be a problem
anyway and will prevent errors to be hidden in the log.
We could make KOHA_ERROR an arrayref, but later (we don't need it now
anyway).

Note that the OPAC still not benefit from a friendly specific error for
invalid token.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This commit is contained in:
Jonathan Druart 2024-02-22 15:16:08 +01:00
parent 82359cf085
commit 4bd4b367dd
No known key found for this signature in database
GPG Key ID: A085E712BEF0E0F0
4 changed files with 86 additions and 63 deletions

View File

@ -484,14 +484,6 @@ sub get_template_and_user {
OPACBaseURL => C4::Context->preference('OPACBaseURL'),
minPasswordLength => $minPasswordLength,
);
if ( C4::Context->config('dev_install') ) {
# Dealing with debug_programming_error ( set from plack.psgi)
$template->param(
dev_install => 1,
debug_programming_error => scalar $in->{query}->param('debug_programming_error'),
);
$in->{query}->delete('debug_programming_error');
}
if ( $in->{'type'} eq "intranet" ) {
$template->param(
@ -648,9 +640,8 @@ sub get_template_and_user {
$template->param( logged_in_user => $patron );
$template->param( sessionID => $sessionID );
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' );
if ( $ENV{KOHA_ERROR} ) {
C4::Output::output_and_exit( $in->{query}, $cookie, $template, $ENV{KOHA_ERROR} );
}
return ( $template, $borrowernumber, $cookie, $flags );

82
Koha/Middleware/CSRF.pm Normal file
View File

@ -0,0 +1,82 @@
package Koha::Middleware::CSRF;
# This file is part of Koha.
#
# Koha is free software; you can redistribute it and/or modify it
# under the terms of the GNU General Public License as published by
# the Free Software Foundation; either version 3 of the License, or
# (at your option) any later version.
#
# Koha is distributed in the hope that it will be useful, but
# WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with Koha; if not, see <http://www.gnu.org/licenses>.
use Modern::Perl;
use parent qw(Plack::Middleware);
use Koha::Logger;
sub call {
my ( $self, $env ) = @_;
my $req = Plack::Request->new($env);
my %stateless_methods = (
GET => 1,
HEAD => 1,
OPTIONS => 1,
TRACE => 1,
);
my %stateful_methods = (
POST => 1,
PUT => 1,
DELETE => 1,
PATCH => 1,
);
my $original_op = $req->param('op');
my $request_method = $req->method // q{};
my ( $error );
if ( $stateless_methods{$request_method} && defined $original_op && $original_op =~ m{^cud-} ) {
$error = sprintf "Programming error - op '%s' must not start with 'cud-' for %s", $original_op,
$request_method;
} elsif ( $stateful_methods{$request_method} ) {
# Get the CSRF token from the param list or the header
my $csrf_token = $req->param('csrf_token') || $req->header('HTTP_CSRF_TOKEN');
if ( defined $req->param('op') && $original_op !~ m{^cud-} ) {
$error = sprintf "Programming error - op '%s' must start with 'cud-' for %s", $original_op,
$request_method;
} elsif ( !$csrf_token ) {
$error = sprintf "Programming error - No CSRF token passed for %s", $request_method;
} else {
unless (
Koha::Token->new->check_csrf(
{
session_id => scalar $req->cookies->{CGISESSID},
token => $csrf_token,
}
)
)
{
$error = "wrong_csrf_token";
}
}
}
if ( $error ) {
Koha::Logger->get->warn( $error );
$env->{KOHA_ERROR} = $error;
$env->{PATH_INFO} = '/intranet/errors/403.pl';
}
return $self->app->($env);
}
1;

View File

@ -47,55 +47,6 @@ use CGI qw(-utf8 ); # we will loose -utf8 under plack, otherwise
$CGI::PARAM_UTF8 = 1;
Koha::Caches->flush_L1_caches();
Koha::Cache::Memory::Lite->flush();
my %stateless_methods = (
GET => 1,
HEAD => 1,
OPTIONS => 1,
TRACE => 1,
);
my %stateful_methods = (
POST => 1,
PUT => 1,
DELETE => 1,
PATCH => 1,
);
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-} ) {
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} ) {
# 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" );
}
die "Programming error - No CSRF token passed for $request_method"
unless $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);
}
}
return $q;
};
}
@ -126,6 +77,7 @@ builder {
enable "+Koha::Middleware::RealIP";
mount '/opac' => builder {
enable "+Koha::Middleware::CSRF";
#NOTE: it is important that these are relative links
enable 'ErrorDocument',
400 => 'errors/400.pl',
@ -145,6 +97,7 @@ builder {
$opac;
};
mount '/intranet' => builder {
enable "+Koha::Middleware::CSRF";
#NOTE: it is important that these are relative links
enable 'ErrorDocument',
400 => 'errors/400.pl',

View File

@ -1,6 +1,3 @@
<div id="messages">
[% INCLUDE 'blocking_errors.inc' %]
[% IF dev_install && debug_programming_error %]
<div class="dialog alert">Programming error: [% debug_programming_error %]</div>
[% END %]
</div>