Bug 14686: Add allows_add_by to check permissions for uploading

The three permissions in tools/upload-file.pl are moved to a custom
routine in DBIx's UploadedFile.pm. An additional granular permission
upload_general_files is added. (The dbrev patch will contain it too.)
At some point in time this could be moved to a Koha::Object class.

The routine is tested in Upload.t.

Test plan:
[1] Run t/db_dependent/Upload.t
[2] If you only apply this patch, you can also test uploading with a
    user that has edit_catalogue but does not have tools or circulate.
    Upload status should say: Denied.

Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Brendan Gallagher <bredan@bywatersolutions.com>

diff --git a/Koha/Schema/Result/UploadedFile.pm b/Koha/Schema/Result/UploadedFile.pm
index 1eaab71..8e2106b 100644 (file)
--- a/Koha/Schema/Result/UploadedFile.pm
+++ b/Koha/Schema/Result/UploadedFile.pm
@@ -124,5 +124,19 @@ __PACKAGE__->set_primary_key("id");
 # DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:Cq/HSuTaBxZH2qSGEddoaQ
 
 
-# You can replace this text with custom code or comments, and it will be preserved on regeneration
+sub allows_add_by {
+    my ( $self, $userid ) = @_; # do not confuse with borrowernumber
+    my $flags = [
+        { tools      => 'upload_general_files' },
+        { circulate  => 'circulate_remaining_permissions' },
+        { tools      => 'stage_marc_import' },
+        { tools      => 'upload_local_cover_images' },
+    ];
+    require C4::Auth;
+    foreach( @$flags ) {
+        return 1 if C4::Auth::haspermission( $userid, $_ );
+    }
+    return;
+}
+
 1;

diff --git a/t/db_dependent/Upload.t b/t/db_dependent/Upload.t
index 1a273d5..31d8767 100644 (file)
--- a/t/db_dependent/Upload.t
+++ b/t/db_dependent/Upload.t
@@ -2,17 +2,20 @@
 
 use Modern::Perl;
 use File::Temp qw/ tempdir /;
-use Test::More tests => 7;
+use Test::More tests => 8;
 
 use Test::MockModule;
 use t::lib::Mocks;
+use t::lib::TestBuilder;
 
 use C4::Context;
+use Koha::Database;
 use Koha::Upload;
+use Koha::Schema::Result::UploadedFile;
 
+my $schema  = Koha::Database->new->schema;
+$schema->storage->txn_begin;
 my $dbh = C4::Context->dbh;
-$dbh->{AutoCommit} = 0;
-$dbh->{RaiseError} = 1;
 
 our $current_upload = 0;
 our $uploads = [
@@ -81,7 +84,11 @@ subtest 'Test07' => sub {
     plan tests => 2;
     test07();
 };
-$dbh->rollback;
+subtest 'Test08: UploadedFile->allows_add_by' => sub {
+    plan tests => 4;
+    test08();
+};
+$schema->storage->txn_rollback;
 
 sub test01 {
     # Delete existing records (for later tests)
@@ -169,6 +176,43 @@ sub test07 { #simple test for httpheaders and getCategories
     is( @$cat >= 1, 1, 'getCategories returned at least one category' );
 }
 
+sub test08 { # UploadedFile->allows_add_by
+    my $builder = t::lib::TestBuilder->new;
+    my $patron = $builder->build({
+        source => 'Borrower',
+        value  => { flags => 0 }, #no permissions
+    });
+    my $patronid = $patron->{borrowernumber};
+    is( Koha::Schema::Result::UploadedFile->allows_add_by( $patron->{userid} ),
+        undef, 'Patron is not allowed to do anything' );
+
+    # add some permissions: edit_catalogue
+    my $fl = 2**9; # edit_catalogue
+    $schema->resultset('Borrower')->find( $patronid )->update({ flags => $fl });
+    is( Koha::Schema::Result::UploadedFile->allows_add_by( $patron->{userid} ),
+        undef, 'Patron is still not allowed to add uploaded files' );
+
+    # replace flags by all tools
+    $fl = 2**13; # tools
+    $schema->resultset('Borrower')->find( $patronid )->update({ flags => $fl });
+    is( Koha::Schema::Result::UploadedFile->allows_add_by( $patron->{userid} ),
+        1, 'Patron should be allowed now to add uploaded files' );
+
+    # remove all tools and add upload_general_files only
+    $fl = 0; # no modules
+    $schema->resultset('Borrower')->find( $patronid )->update({ flags => $fl });
+    $builder->build({
+        source => 'UserPermission',
+        value  => {
+            borrowernumber => $patronid,
+            module_bit     => { module_bit => { flag => 'tools' } },
+            code           => 'upload_general_files',
+        },
+    });
+    is( Koha::Schema::Result::UploadedFile->allows_add_by( $patron->{userid} ),
+        1, 'Patron is still allowed to add uploaded files' );
+}
+
 sub newCGI {
     my ( $class, $hook ) = @_;
     my $read = 0;

diff --git a/tools/upload-file.pl b/tools/upload-file.pl
index 1994214..b410934 100755 (executable)
--- a/tools/upload-file.pl
+++ b/tools/upload-file.pl
@@ -28,6 +28,7 @@ use URI::Escape;
 use C4::Context;
 use C4::Auth qw/check_cookie_auth haspermission/;
 use Koha::Upload;
+use Koha::Schema::Result::UploadedFile;
 
 # upload-file.pl must authenticate the user
 # before processing the POST request,
@@ -37,25 +38,13 @@ use Koha::Upload;
 # requires that the session cookie already
 # has been created.
 
-my $flags_required = [
-    {circulate  => 'circulate_remaining_permissions'},
-    {tools      => 'stage_marc_import'},
-    {tools      => 'upload_local_cover_images'}
-];
-
 my %cookies = CGI::Cookie->fetch;
 my $sid = $cookies{'CGISESSID'}->value;
-
-my $auth_failure = 1;
 my ( $auth_status, $sessionID ) = check_cookie_auth( $sid );
 my $uid = C4::Auth::get_session($sid)->param('id');
-foreach my $flag_required ( @{$flags_required} ) {
-    if ( my $flags = haspermission( $uid, $flag_required ) ) {
-        $auth_failure = 0 if $auth_status eq 'ok';
-    }
-}
+my $allowed = Koha::Schema::Result::UploadedFile->allows_add_by( $uid );
 
-if ($auth_failure) {
+if( $auth_status ne 'ok' || !$allowed ) {
     send_reply( 'denied' );
     exit 0;
 }