diff --git a/Koha/Exceptions/Report.pm b/Koha/Exceptions/Report.pm index b854587c74..882484b86e 100644 --- a/Koha/Exceptions/Report.pm +++ b/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; diff --git a/Koha/Report.pm b/Koha/Report.pm index e172d2a03a..e69869fc94 100644 --- a/Koha/Report.pm +++ b/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 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 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 first, so a librarian cannot modify a +report owned by someone else without the C +permission. Creating a new report is not checked. + +Changing only C 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 diff --git a/t/db_dependent/Koha/Reports.t b/t/db_dependent/Koha/Reports.t index a6ae63a8f3..d95ecd9e1a 100755 --- a/t/db_dependent/Koha/Reports.t +++ b/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;