From 462d170c8eca959396659b738191acd580c3606a Mon Sep 17 00:00:00 2001
From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Date: Fri, 23 Feb 2024 08:53:46 +0100
Subject: [PATCH] Bug 36148: Improve error handling and restore programming
 errors

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
---
 C4/Auth.pm                                    |  4 ++--
 Koha/Middleware/CSRF.pm                       |  4 ++--
 errors/403.pl                                 |  6 ++----
 .../prog/en/includes/blocking_errors.inc      |  5 +++--
 .../prog/en/modules/errors/errorpage.tt       | 21 ++++++++++---------
 .../bootstrap/en/modules/errors/errorpage.tt  | 10 +++++++--
 opac/errors/403.pl                            |  6 ++----
 7 files changed, 30 insertions(+), 26 deletions(-)

diff --git a/C4/Auth.pm b/C4/Auth.pm
index a407ba1a3c..9c4c7842e6 100644
--- a/C4/Auth.pm
+++ b/C4/Auth.pm
@@ -640,8 +640,8 @@ sub get_template_and_user {
     $template->param( logged_in_user     => $patron );
     $template->param( sessionID          => $sessionID );
 
-    if ( $ENV{KOHA_ERROR} ) {
-        C4::Output::output_and_exit( $in->{query}, $cookie, $template, $ENV{KOHA_ERROR} );
+    if ( $ENV{'plack.middleware.Koha.CSRF'} ) {
+        C4::Output::output_and_exit( $in->{query}, $cookie, $template, $ENV{'plack.middleware.Koha.CSRF'} );
     }
 
     return ( $template, $borrowernumber, $cookie, $flags );
diff --git a/Koha/Middleware/CSRF.pm b/Koha/Middleware/CSRF.pm
index c9a30c97ba..2d4582f2c4 100644
--- a/Koha/Middleware/CSRF.pm
+++ b/Koha/Middleware/CSRF.pm
@@ -76,8 +76,8 @@ sub call {
 
         #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"] );
+        $env->{'plack.middleware.Koha.CSRF'} = $error;
+        my $res = Plack::Response->new( 403, [ 'Content-Type' => 'text/plain' ], ["Wrong CSRF token"] );
         return $res->finalize;
     }
 
diff --git a/errors/403.pl b/errors/403.pl
index 7c8a6993fd..b83c014de8 100755
--- a/errors/403.pl
+++ b/errors/403.pl
@@ -36,11 +36,9 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user(
 $template->param (
     admin => $admin,
     errno => 403,
+    csrf_error => $ENV{'plack.middleware.Koha.CSRF'},
 );
-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/includes/blocking_errors.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/blocking_errors.inc
index 1f48630b39..a4fd22249e 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/includes/blocking_errors.inc
+++ b/koha-tmpl/intranet-tmpl/prog/en/includes/blocking_errors.inc
@@ -18,14 +18,15 @@
     [% CASE 'order_cannot_be_edited' %]
         <div class="dialog message">This order cannot be edited, the basket is closed or the order was already received.</div>
     [% CASE 'wrong_csrf_token' %]
-        <div class="dialog message">The form submission failed (Wrong CSRF token). Try to come back, refresh the page, then try again.</div>
+        <div class="dialog alert">The form submission failed (Wrong CSRF token). Try to come back, refresh the page, then try again.</div>
     [% CASE 'budget_is_locked' %]
         <div class="dialog message">The budget is locked, fund creation is not possible.</div>
     [% CASE 'missing_es_modules' %]
         <div class="dialog message">Necessary Elasticsearch packages are not installed on your server. Please contact your server admin if you wish to configure Elasticsearch</div>
     [% CASE 'insufficient_permission' %]
         <div class="dialog message">You do not have sufficient permission to continue.</div>
-    [% CASE %][% blocking_error | html %]
+    [% CASE %]
+        <div class="dialog alert">[% blocking_error | html %]</div>
     [% END %]
 
     [% INCLUDE 'intranet-bottom.inc' %]
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 16e0899fd5..1737b9abee 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/errors/errorpage.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/errors/errorpage.tt
@@ -30,16 +30,17 @@
 
     <h1>An error has occurred!</h1>
     <h2><em>[% tx("Error {error_number}", {error_number = errno }) | html %]</em></h2>
-        <h3>This message may have been caused by any of the following reasons:</h3>
-        <ul style="padding-bottom: 0.5em;">
-            <li>You made use of an external link to an item that is no longer available</li>
-            <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>
+        [% IF ( csrf_error ) %]
+            [% INCLUDE 'blocking_errors.inc' blocking_error => csrf_error %]
+        [% ELSE %]
+            <h3>This message may have been caused by any of the following reasons:</h3>
+            <ul style="padding-bottom: 0.5em;">
+                <li>You made use of an external link to an item that is no longer available</li>
+                <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>
+            </ul>
+        [% END %]
         <h3>What's next?</h3>
         <ul style="margin-bottom: 1em; padding-bottom: 1em; border-bottom: 1px solid #CCC;">
             <li>Use top menu bar to navigate to another part of Koha.</li>
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 000ddf5ec1..768f824429 100644
--- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/errors/errorpage.tt
+++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/errors/errorpage.tt
@@ -47,9 +47,15 @@
                             [% END %]
 
                             [% IF ( errno == 403 ) %]
-                                <li>You are forbidden to view this page.</li>
                                 [% IF ( csrf_error ) %]
-                                    <li>A missing CSRF token</li>
+                                    [% IF csrf_error == 'wrong_csrf_token' %]
+                                        <li>The form submission failed (Wrong CSRF token). Try to come back, refresh the page, then try again.</li>
+                                    [% ELSE %]
+                                        [%# Programming errors, we do not want to display them at the OPAC %]
+                                        <li>An unexpected error occurred while processing your requets.</li>
+                                    [% END %]
+                                [% ELSE %]
+                                    <li>You are forbidden to view this page.</li>
                                 [% END %]
                             [% END %]
 
diff --git a/opac/errors/403.pl b/opac/errors/403.pl
index 9b513b37a1..4d7dc16d7f 100755
--- a/opac/errors/403.pl
+++ b/opac/errors/403.pl
@@ -36,11 +36,9 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user(
 $template->param (
     admin => $admin,
     errno => 403,
+    csrf_error => $ENV{'plack.middleware.Koha.CSRF'},
 );
-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.39.5