Bugzilla – Attachment 162359 Details for
Bug 36148
Move CSRF check code outside of CGI->new
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 36148: Allow Koha::Middleware::CSRF to use error/exception middlewares
Bug-36148-Allow-KohaMiddlewareCSRF-to-use-errorexc.patch (text/plain), 5.81 KB, created by
David Cook
on 2024-02-23 05:07:31 UTC
(
hide
)
Description:
Bug 36148: Allow Koha::Middleware::CSRF to use error/exception middlewares
Filename:
MIME Type:
Creator:
David Cook
Created:
2024-02-23 05:07:31 UTC
Size:
5.81 KB
patch
obsolete
>From 660ddc13ce8972d8efaa22eaae1cc11cb59f8683 Mon Sep 17 00:00:00 2001 >From: David Cook <dcook@prosentient.com.au> >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. >--- > 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 @@ > <li>You followed an outdated link e.g. from a search engine or a bookmark</li> > <li>You tried to access a page that needs authentication</li> > <li>An internal link in the client is broken and the page does not exist</li> >+ [% IF ( csrf_error ) %] >+ <li>A missing CSRF token</li> >+ [% END %] > </ul> > <h3>What's next?</h3> > <ul style="margin-bottom: 1em; padding-bottom: 1em; border-bottom: 1px solid #CCC;"> >diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/errors/errorpage.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/errors/errorpage.tt >index 9a759c92f4..000ddf5ec1 100644 >--- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/errors/errorpage.tt >+++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/errors/errorpage.tt >@@ -48,6 +48,9 @@ > > [% IF ( errno == 403 ) %] > <li>You are forbidden to view this page.</li> >+ [% IF ( csrf_error ) %] >+ <li>A missing CSRF token</li> >+ [% END %] > [% END %] > > [% IF ( errno == 404 ) %] >diff --git a/opac/errors/403.pl b/opac/errors/403.pl >index ac3992fb6c..9b513b37a1 100755 >--- a/opac/errors/403.pl >+++ b/opac/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'; >-- >2.30.2
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 36148
:
162333
|
162336
|
162359
|
162360
|
162362
|
162363
|
162428
|
162505
|
162506
|
162507
|
162508
|
162509
|
162510