From 2347cdf33d37bdfb1307712121ff46d160499241 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: Nick Clemens --- 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 7b9d43590f9..890f55ae795 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 fd968da1a4a..530c3b0962b 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 33706a6f568..7c8a6993fdc 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 f5ffce0b286..16e0899fd58 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?