Bug 42667: Enforce permission in Koha::Report

Prevents possible template bugs from allowing editing
of reports by others without the proper permissions.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>

Koha/Exceptions/Report.pm

@@ -2,8 +2,12 @@ package Koha::Exceptions::Report;
 
 use Modern::Perl;
 
+use Koha::Exception;
+
 use Exception::Class (
-    'Koha::Exceptions::Report'                   => { description => "Something went wrong!" },
+    'Koha::Exceptions::Report' => {
+        isa => 'Koha::Exception',
+    },
     'Koha::Exceptions::Report::DuplicateRunning' => {
         isa         => 'Koha::Exceptions::Report',
         description => "The configured per-user limit on simultaneous runs of this report has been reached",
@@ -18,7 +22,36 @@ use Exception::Class (
         isa         => 'Koha::Exceptions::Report',
         description => "The configured instance-wide limit on total simultaneous report runs has been reached",
         fields      => ['limit'],
-    }
+    },
+    'Koha::Exceptions::Report::EditPermission' => {
+        isa         => 'Koha::Exceptions::Report',
+        description => 'You do not have permission to edit this report',
+        fields      => ['report_id'],
+    },
 );
 
+=head1 NAME
+
+Koha::Exceptions::Report - Base class for report exceptions
+
+=head1 Exceptions
+
+=head2 Koha::Exceptions::Report
+
+Generic report exception.
+
+=head2 Koha::Exceptions::Report::DuplicateRunning
+
+The configured per-user limit on simultaneous runs of this report has been reached.
+
+=head2 Koha::Exceptions::Report::TotalRunning
+
+The configured per-user limit on total simultaneous report runs has been reached.
+
+=head2 Koha::Exceptions::Report::EditPermission
+
+Exception raised when the logged-in user may not edit a report.
+
+=cut
+
 1;

Koha/Report.pm

@@ -17,8 +17,10 @@ package Koha::Report;
 
 use Modern::Perl;
 
+use C4::Context;
 use Koha::Database;
 use Koha::Exceptions::Report;
+use Koha::Patrons;
 use Koha::Reports;
 
 use Koha::Object;
@@ -167,6 +169,59 @@ sub new_from_mana {
     Koha::Report->new($data)->store;
 }
 
+=head3 check_edit_permission
+
+    $report->check_edit_permission;
+
+Throws L<Koha::Exceptions::Report::EditPermission> when the patron in the
+current userenv may not edit this report. A patron may edit a report
+they created; editing a report owned by another librarian, or an
+ownerless report, requires the C<edit_all_reports> permission. When
+there is no logged-in user (eg. a cron job) no check is performed.
+
+=cut
+
+sub check_edit_permission {
+    my ($self) = @_;
+
+    my $userenv = C4::Context->userenv;
+    return 1 unless $userenv && $userenv->{number};
+
+    my $borrowernumber = $userenv->{number};
+    return 1 if $self->borrowernumber && $self->borrowernumber == $borrowernumber;
+
+    my $patron = Koha::Patrons->find($borrowernumber);
+    return 1 if $patron && $patron->has_permission( { reports => 'edit_all_reports' } );
+
+    Koha::Exceptions::Report::EditPermission->throw( report_id => $self->id );
+}
+
+=head3 store
+
+    $report->store;
+
+Overridden to enforce edit permissions. Updating an existing report
+runs L</check_edit_permission> first, so a librarian cannot modify a
+report owned by someone else without the C<edit_all_reports>
+permission. Creating a new report is not checked.
+
+Changing only C<mana_id> is exempt: recording a Mana identifier when
+a report is shared is not a change to the report's contents.
+
+=cut
+
+sub store {
+    my ($self) = @_;
+
+    if ( $self->in_storage ) {
+        my %dirty = $self->_result->get_dirty_columns;
+        delete $dirty{mana_id};
+        $self->check_edit_permission if %dirty;
+    }
+
+    return $self->SUPER::store;
+}
+
 =head3 prep_report
 
 Prep the report and return executable sql with parameters embedded and a list of header types

t/db_dependent/Koha/Reports.t

@@ -20,10 +20,11 @@ use Modern::Perl;
 use Test::Exception;
 use Test::MockModule;
 use Test::NoWarnings;
-use Test::More tests => 17;
+use Test::More tests => 18;
 
 use Koha::Report;
 use Koha::Reports;
+use Koha::Patrons;
 use Koha::Database;
 
 use t::lib::Mocks;
@@ -566,4 +567,57 @@ subtest 'running' => sub {
     );
 };
 
+subtest 'check_edit_permission() and store enforcement' => sub {
+    plan tests => 6;
+
+    my $owner  = $builder->build_object( { class => 'Koha::Patrons', value => { flags => 0 } } );
+    my $editor = $builder->build_object( { class => 'Koha::Patrons', value => { flags => 0 } } );
+
+    my $report = Koha::Report->new(
+        {
+            report_name    => 'permission_test',
+            savedsql       => 'SELECT 1',
+            borrowernumber => $owner->borrowernumber,
+        }
+    )->store;
+
+    my $context = Test::MockModule->new('C4::Context');
+    $context->mock( 'userenv', sub { undef } );
+    lives_ok { $report->savedsql('SELECT 2')->store }
+    'no logged-in user, store of an existing report is allowed';
+    $context->unmock('userenv');
+
+    t::lib::Mocks::mock_userenv( { patron => $owner } );
+    lives_ok { $report->savedsql('SELECT 3')->store }
+    'report creator can edit their own report';
+
+    t::lib::Mocks::mock_userenv( { patron => $editor } );
+    throws_ok { $report->savedsql('SELECT 4')->store }
+    'Koha::Exceptions::Report::EditPermission',
+        'a non-owner without edit_all_reports cannot edit the report';
+
+    my $shared = Koha::Reports->find( $report->id );
+    lives_ok { $shared->mana_id(12345)->store }
+    'a mana_id-only change is exempt from the edit check';
+
+    $builder->build(
+        {
+            source => 'UserPermission',
+            value  => {
+                borrowernumber => $editor->borrowernumber,
+                module_bit     => 16,
+                code           => 'edit_all_reports',
+            },
+        }
+    );
+    lives_ok { $report->savedsql('SELECT 5')->store }
+    'a non-owner with edit_all_reports can edit the report';
+
+    lives_ok {
+        Koha::Report->new(
+            { report_name => 'new_one', savedsql => 'SELECT 1', borrowernumber => $owner->borrowernumber } )->store;
+    }
+    'creating a new report is not subject to the edit check';
+};
+
 $schema->storage->txn_rollback;