From 13a61279523b35370f7b3adeb0f47ad30bfa937d Mon Sep 17 00:00:00 2001
From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Date: Tue, 9 Aug 2016 22:18:14 +0100
Subject: [PATCH] Bug 17097: Add a confirmation page when deleting a patron

It won't hurt to have a confirmation page when deleting a patron.
Moreover it's the more easy way to protect against CSRF attacks :)

Test plan:
Make sure you get a confirmation page when deleting a patron
Confirm that approving or denying the confirmation work as expected

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
---
 .../prog/en/includes/members-toolbar.inc      | 10 +--------
 .../prog/en/modules/members/deletemem.tt      | 14 +++++++++++++
 members/deletemem.pl                          | 21 ++++++++++---------
 3 files changed, 26 insertions(+), 19 deletions(-)

diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/members-toolbar.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/members-toolbar.inc
index 7f7fe5c4ea..9eb1f65f12 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/includes/members-toolbar.inc
+++ b/koha-tmpl/intranet-tmpl/prog/en/includes/members-toolbar.inc
@@ -25,9 +25,7 @@ $(document).ready(function(){
             });
         [% ELSE %]
             $("#deletepatron").click(function(){
-                confirm_deletion();
-                $(".btn-group").removeClass("open");
-                return false;
+                window.location='/cgi-bin/koha/members/deletemem.pl?member=[% borrowernumber | url%]';
             });
         [% END %]
         $("#renewpatron").click(function(){
@@ -71,12 +69,6 @@ $(document).ready(function(){
         return false;
     })
 });
-function confirm_deletion() {
-    var is_confirmed = window.confirm(_("Are you sure you want to delete this patron? This cannot be undone."));
-    if (is_confirmed) {
-        window.location='/cgi-bin/koha/members/deletemem.pl?member=[% borrowernumber %]';
-    }
-}
 function confirm_local_deletion() {
     var is_confirmed = window.confirm(_("Are you sure you want to delete this patron from the local database? This cannot be undone."));
     if (is_confirmed) {
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/deletemem.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/deletemem.tt
index 4572f8240b..9e24bd6a46 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/deletemem.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/deletemem.tt
@@ -28,6 +28,20 @@
             [% END %]
             </ul>
     </div>
+    [% ELSIF op == 'delete_confirm' and borrowernumber %]
+        [%# TODO add "patron does not exist" unless borrowernumber %]
+        <div class="dialog alert">
+            <h3>Are you sure you want to delete the patron [% firstname %] [% surname %]? This cannot be undone.</h3>
+            <form action="/cgi-bin/koha/members/deletemem.pl">
+                <input type="hidden" name="borrowernumber" value="[% borrowernumber %]"/>
+                <input type="hidden" name="op" value="delete_confirmed" />
+                <button type="submit" class="approve"><i class="fa fa-fw fa-check"></i> Yes, delete</button>
+            </form>
+            <form action="/cgi-bin/koha/members/moremember.pl">
+                <input type="hidden" name="borrowernumber" value="[% borrowernumber %]"/>
+                <button type="submit" class="deny"><i class="fa fa-fw fa-remove"></i> No, do not delete</button>
+            </form>
+        </div>
     [% END %]
     [% IF ( keeplocal ) %]
         <div class="dialog message">
diff --git a/members/deletemem.pl b/members/deletemem.pl
index fd53eccd70..173d70e334 100755
--- a/members/deletemem.pl
+++ b/members/deletemem.pl
@@ -101,13 +101,10 @@ if (C4::Context->preference("IndependentBranches")) {
     }
 }
 
+my $op = $input->param('op') || 'delete_confirm';
 my $dbh = C4::Context->dbh;
-my $sth=$dbh->prepare("Select * from borrowers where guarantorid=?");
-$sth->execute($member);
-my $data=$sth->fetchrow_hashref;
-if ($countissues > 0 or $flags->{'CHARGES'}  or $data->{'borrowernumber'} or $deletelocal == 0){
-    #   print $input->header;
-
+my $is_guarantor = $dbh->selectrow_array("SELECT COUNT(*) FROM borrowers WHERE guarantorid=?", undef, $member);
+if ( $op eq 'delete_confirm' or $countissues > 0 or $flags->{'CHARGES'}  or $is_guarantor or $deletelocal == 0){
     my $patron_image = Koha::Patron::Images->find($bor->{borrowernumber});
     $template->param( picture => 1 ) if $patron_image;
 
@@ -137,19 +134,23 @@ if ($countissues > 0 or $flags->{'CHARGES'}  or $data->{'borrowernumber'} or $de
     if ($flags->{'CHARGES'} ne '') {
         $template->param(charges => $flags->{'CHARGES'}->{'amount'});
     }
-    if ($data->{'borrowernumber'}) {
+    if ($is_guarantor) {
         $template->param(guarantees => 1);
     }
     if ($deletelocal == 0) {
         $template->param(keeplocal => 1);
     }
-output_html_with_http_headers $input, $cookie, $template->output;
-
-} else {
+    # This is silly written but reflect the same conditions as above
+    if ( not $countissues > 0 and not $flags->{CHARGES} ne '' and not $is_guarantor and not $deletelocal == 0 ) {
+        $template->param( op => 'delete_confirm' );
+    }
+}elsif ( $op eq 'delete_confirmed' ) {
     MoveMemberToDeleted($member);
     C4::Members::HandleDelBorrower($member);
     DelMember($member);
+    # TODO Tell the user everything went ok
     print $input->redirect("/cgi-bin/koha/members/members-home.pl");
 }
 
+output_html_with_http_headers $input, $cookie, $template->output;
 
-- 
2.39.5