From d3687f0b62dd1d5e83ac5cbd4a12262c5c719508 Mon Sep 17 00:00:00 2001 From: David Cook Date: Fri, 23 Feb 2024 05:05:30 +0000 Subject: [PATCH] Bug 36148: Allow Koha::Middleware::CSRF to use error/exception middlewares This change allows Koha::Middleware::CSRF to use the ErrorDocument and HTTPExcetions middlewares to display the correct status codes and HTML documents. Leveraging Plack environmental variables, we're also able to pass along data to the error page handlers to show warnings indicating that there was a missing CSRF token. Signed-off-by: Jonathan Druart --- Koha/Middleware/CSRF.pm | 16 +++++++++------- debian/templates/plack.psgi | 4 ++-- errors/403.pl | 4 ++++ .../prog/en/modules/errors/errorpage.tt | 3 +++ .../bootstrap/en/modules/errors/errorpage.tt | 3 +++ opac/errors/403.pl | 4 ++++ 6 files changed, 25 insertions(+), 9 deletions(-) diff --git a/Koha/Middleware/CSRF.pm b/Koha/Middleware/CSRF.pm index 7b9d43590f..890f55ae79 100644 --- a/Koha/Middleware/CSRF.pm +++ b/Koha/Middleware/CSRF.pm @@ -18,8 +18,7 @@ package Koha::Middleware::CSRF; use Modern::Perl; use parent qw(Plack::Middleware); - -use Koha::Logger; +use Plack::Response; sub call { my ( $self, $env ) = @_; @@ -41,7 +40,7 @@ sub call { my $original_op = $req->param('op'); my $request_method = $req->method // q{}; - my ( $error ); + 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; @@ -70,10 +69,13 @@ sub call { } } - if ( $error ) { - Koha::Logger->get->warn( $error ); - $env->{KOHA_ERROR} = $error; - $env->{PATH_INFO} = '/intranet/errors/403.pl'; + if ( $error && !$env->{'plack.middleware.Koha.CSRF'} ) { + + #NOTE: Other Middleware will take care of logging to correct place, as Koha::Logger doesn't know where to go here + warn $error; + $env->{'plack.middleware.Koha.CSRF'} = "BAD_CSRF"; + my $res = Plack::Response->new( 403, [ 'Content-Type' => 'text/plain' ], ["Bad CSRF"] ); + return $res->finalize; } return $self->app->($env); diff --git a/debian/templates/plack.psgi b/debian/templates/plack.psgi index fd968da1a4..530c3b0962 100644 --- a/debian/templates/plack.psgi +++ b/debian/templates/plack.psgi @@ -77,7 +77,6 @@ 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', @@ -94,10 +93,10 @@ builder { enable 'Log4perl', category => 'plack-opac'; enable 'LogWarn'; } + enable "+Koha::Middleware::CSRF"; $opac; }; mount '/intranet' => builder { - enable "+Koha::Middleware::CSRF"; #NOTE: it is important that these are relative links enable 'ErrorDocument', 400 => 'errors/400.pl', @@ -114,6 +113,7 @@ builder { enable 'Log4perl', category => 'plack-intranet'; enable 'LogWarn'; } + enable "+Koha::Middleware::CSRF"; $intranet; }; mount '/api/v1/app.pl' => builder { diff --git a/errors/403.pl b/errors/403.pl index 33706a6f56..7c8a6993fd 100755 --- a/errors/403.pl +++ b/errors/403.pl @@ -37,6 +37,10 @@ $template->param ( admin => $admin, errno => 403, ); +my $csrf_error = $ENV{'plack.middleware.Koha.CSRF'}; +if ($csrf_error) { + $template->param( 'csrf_error' => 1 ); +} my $status = '403 Forbidden'; if ( C4::Context->is_internal_PSGI_request() ) { $status = '200 OK'; diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/errors/errorpage.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/errors/errorpage.tt index f5ffce0b28..16e0899fd5 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/errors/errorpage.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/errors/errorpage.tt @@ -36,6 +36,9 @@
  • You followed an outdated link e.g. from a search engine or a bookmark
  • You tried to access a page that needs authentication
  • An internal link in the client is broken and the page does not exist
    • + [% IF ( csrf_error ) %] +
  • A missing CSRF token
    • + [% END %]

    What's next?