The "Delete" link is hidden but the controller does not do the necessary checks. /cgi-bin/koha/circ/del_message.pl?message_id=2&borrowernumber=5&from=moremember
Created attachment 129594 [details] [review] Bug 29903: Prevent messages to be deleted from unauthorised users The "Delete" link is hidden but the controller does not do the necessary checks. /cgi-bin/koha/circ/del_message.pl?message_id=1&borrowernumber=5&from=moremember Test plan: Create a message, see the "Delete" link, don't click it but copy it Change logged in library and use the link If AllowAllMessageDeletion is off you should be redirected to 403
I think you want &&, not || - with your patch I can't delete any messages I also think we need another bug, the delete link only shows if the branchcode of the patron matches the branchcode of the message. I.e. you only get a link if a Centerville patron has a note from Centerville - it should probably check the logged in branchcode against the message branchcode: koha-tmpl/intranet-tmpl/prog/en/includes/patron_messages.inc 220 [% IF patron_message.branchcode == patron.branchcode OR Koha.Preference('AllowAllMessageDeleti on') %]
(In reply to Nick Clemens from comment #2) > I think you want &&, not || - with your patch I can't delete any messages Oops yes, last minute change switching from "unless" to "if"... > I also think we need another bug, the delete link only shows if the > branchcode of the patron matches the branchcode of the message. I.e. you > only get a link if a Centerville patron has a note from Centerville - it > should probably check the logged in branchcode against the message > branchcode: > > koha-tmpl/intranet-tmpl/prog/en/includes/patron_messages.inc > 220 [% IF patron_message.branchcode == patron.branchcode > OR Koha.Preference('AllowAllMessageDeleti on') %] It's bug 29889.
Created attachment 129611 [details] [review] Bug 29903: Prevent messages to be deleted from unauthorised users The "Delete" link is hidden but the controller does not do the necessary checks. /cgi-bin/koha/circ/del_message.pl?message_id=1&borrowernumber=5&from=moremember Test plan: Create a message, see the "Delete" link, don't click it but copy it Change logged in library and use the link If AllowAllMessageDeletion is off you should be redirected to 403
This patch checks the homebranch of the logged in librarian - bug 29889 checks the currently logged in library This means a staff from centerville can only delete centerville messages, no matter where they are logged in. The link shows though if you are logged in at the branch that left the message
Created attachment 129613 [details] [review] Bug 29903: Prevent messages to be deleted from unauthorised users The "Delete" link is hidden but the controller does not do the necessary checks. /cgi-bin/koha/circ/del_message.pl?message_id=1&borrowernumber=5&from=moremember Test plan: Create a message, see the "Delete" link, don't click it but copy it Change logged in library and use the link If AllowAllMessageDeletion is off you should be redirected to 403
Created attachment 129614 [details] [review] Bug 29903: Prevent messages to be deleted from unauthorised users The "Delete" link is hidden but the controller does not do the necessary checks. /cgi-bin/koha/circ/del_message.pl?message_id=1&borrowernumber=5&from=moremember Test plan: Create a message, see the "Delete" link, don't click it but copy it Change logged in library and use the link If AllowAllMessageDeletion is off you should be redirected to 403 Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Straightforward, passing QA
I've pushed Bug 29889 so that behavior is consistent when this one is pushed.
Backported: Pushed to 20.11.x branch for 20.11.14
*20.11.x-security branch
Backported to 19.11.x-security branch on the Security repo for 19.11.25 security release.
*** Bug 28724 has been marked as a duplicate of this bug. ***