Attachment 156740 Details for Bug 25672 – Bug 25672: Restore redirect on restricted upload

[patch] Bug 25672: Restore redirect on restricted upload

Bug-25672-Restore-redirect-on-restricted-upload.patch (text/plain), 8.76 KB, created by Martin Renvoize (ashimema) on 2023-10-09 11:44:16 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Martin Renvoize (ashimema)

Created: 2023-10-09 11:44:16 UTC

Size: 8.76 KB

patch

obsolete

>From a07e685124dfadb97edf2909fea6fc9f413395b0 Mon Sep 17 00:00:00 2001
>From: Martin Renvoize <martin.renvoize@ptfs-europe.com>
>Date: Mon, 9 Oct 2023 12:26:02 +0100
>Subject: [PATCH] Bug 25672: Restore redirect on restricted upload
>
>This patch restores the immediate redirect from plugins-upload should
>uploads be restricted and an uploadlocation not be specified.
>
>Amended test plan:
>1) Apply the full patchset
>2) Confirm <enable_plugins>1</enable_plugins> is present in koha-conf.xml
>3) Add <plugins_restricted>1</plugins_restricted> to koha-conf.xml
>4) Confirm that the <plugin_repos> block us not commented and contains at
>   least one trusted organisation in koha-conf.xml
>5) Run restart_all (in koha-testing-docker)
>6) Go to /cgi-bin/koha/plugins/plugins-home.pl and note that you don't see
>   an option to upload plugins
>7) You should however see a search option and upon search you should have
>   results returned from the chosen trusted organisations listed in the
>   <plugin_repos> block mentioned above.
>8) Clicking install on one of the results should work as expected and install
>   the plugin.
>9) Go directly to /cgi-bin/koha/plugins/plugins-upload.pl and note that it says
>   "Plugin upload is restricted to only those plugins listed by your server
>   administrator" and gives instructions on how to enable unrestricted browser
>   upload.
>---
> .../prog/en/modules/plugins/plugins-upload.tt |   2 +-
> plugins/plugins-upload.pl                     | 113 +++++++++---------
> 2 files changed, 56 insertions(+), 59 deletions(-)
>
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/plugins/plugins-upload.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/plugins/plugins-upload.tt
>index b7b2ab8939d..eb6bd9d32b3 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/plugins/plugins-upload.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/plugins/plugins-upload.tt
>@@ -46,7 +46,7 @@
>                 [% END %]
>                 <form method="post" action="/cgi-bin/koha/plugins/plugins-upload.pl" enctype="multipart/form-data">
>                     <fieldset class="brief">
>-                         <div class="hint"><strong>NOTE:</strong> Only KPZ file format is supported.</div>
>+                        <div class="hint"><strong>NOTE:</strong> Only KPZ file format is supported.</div>
>                         <ol>
>                             <li>
>                                 <label for="uploadfile">Select the file to upload: </label><input type="file" id="uploadfile" name="uploadfile" accept=".kpz" />
>diff --git a/plugins/plugins-upload.pl b/plugins/plugins-upload.pl
>index 25255f03c2f..dff5a7536a7 100755
>--- a/plugins/plugins-upload.pl
>+++ b/plugins/plugins-upload.pl
>@@ -45,93 +45,90 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user(
>     }
> );
> 
>-if ($plugins_enabled) {
>+my $uploadlocation = $input->param('uploadlocation');
>+
>+# Early exists if uploads are not enabled direct upload attempted when uploads are restricted
>+if (!$plugins_enabled) {
>+    output_html_with_http_headers $input, $cookie, $template->output;
>+} elsif ( $plugins_restricted && !$uploadlocation ) {
>     $template->param( plugins_restricted => $plugins_restricted );
>-} else {
>-    # Exit early if uploads are not enabled
>     output_html_with_http_headers $input, $cookie, $template->output;
> }
> 
> my $uploadfilename = $input->param('uploadfile');
> my $uploadfile     = $input->upload('uploadfile');
>-my $uploadlocation = $input->param('uploadlocation');
> my $op             = $input->param('op') || q{};
> 
> my ( $tempfile, $tfh );
> 
> my %errors;
> 
>-if ($plugins_enabled) {
>-    if ( ( $op eq 'Upload' ) && ( $uploadfile || $uploadlocation ) ) {
>-        my $plugins_dir = C4::Context->config("pluginsdir");
>-        $plugins_dir = ref($plugins_dir) eq 'ARRAY' ? $plugins_dir->[0] : $plugins_dir;
>+if ( ( $op eq 'Upload' ) && ( $uploadfile || $uploadlocation ) ) {
>+    my $plugins_dir = C4::Context->config("pluginsdir");
>+    $plugins_dir = ref($plugins_dir) eq 'ARRAY' ? $plugins_dir->[0] : $plugins_dir;
> 
>-        my $dirname = File::Temp::tempdir( CLEANUP => 1 );
>+    my $dirname = File::Temp::tempdir( CLEANUP => 1 );
> 
>-        my $filesuffix;
>-        $filesuffix = $1 if $uploadfilename =~ m/(\..+)$/i;
>-        ( $tfh, $tempfile ) = File::Temp::tempfile( SUFFIX => $filesuffix, UNLINK => 1 );
>+    my $filesuffix;
>+    $filesuffix = $1 if $uploadfilename =~ m/(\..+)$/i;
>+    ( $tfh, $tempfile ) = File::Temp::tempfile( SUFFIX => $filesuffix, UNLINK => 1 );
> 
>-        $errors{'NOTKPZ'}         = 1 if ( $uploadfilename !~ /\.kpz$/i );
>-        $errors{'NOWRITETEMP'}    = 1 unless ( -w $dirname );
>-        $errors{'NOWRITEPLUGINS'} = 1 unless ( -w $plugins_dir );
>+    $errors{'NOTKPZ'}         = 1 if ( $uploadfilename !~ /\.kpz$/i );
>+    $errors{'NOWRITETEMP'}    = 1 unless ( -w $dirname );
>+    $errors{'NOWRITEPLUGINS'} = 1 unless ( -w $plugins_dir );
> 
>-        if ($uploadlocation) {
>-            my $do_get = 1;
>-            if ( $plugins_restricted ) {
>-                my $repos = C4::Context->config('plugin_repos');
>-                
>-                # Fix data structure if only one repo defined
>-                if ( ref($repos->{repo}) eq 'HASH' ) {
>-                    $repos = { repo => [ $repos->{repo} ] };
>-                }
>+    if ($uploadlocation) {
>+        my $do_get = 1;
>+        if ($plugins_restricted) {
>+            my $repos = C4::Context->config('plugin_repos');
> 
>-                $do_get = any { index($uploadlocation, $_->{org_name}) != -1 } @{ $repos->{repo} };
>+            # Fix data structure if only one repo defined
>+            if ( ref( $repos->{repo} ) eq 'HASH' ) {
>+                $repos = { repo => [ $repos->{repo} ] };
>             }
> 
>-            if ( $do_get ) {
>-                my $ua = Mojo::UserAgent->new( max_redirects => 5 );
>-                my $tx = $ua->get($uploadlocation);
>-                $tx->result->content->asset->move_to($tempfile);
>-            } else {
>-                $errors{'RESTRICTED'} = 1;
>-            }
>-        } else {
>-            $errors{'RESTRICTED'} = 1 unless ( !$plugins_restricted );
>-            $errors{'EMPTYUPLOAD'} = 1 unless ( length($uploadfile) > 0 );
>+            $do_get = any { index( $uploadlocation, $_->{org_name} ) != -1 } @{ $repos->{repo} };
>         }
> 
>-        if (%errors) {
>-            $template->param( ERRORS => [ \%errors ] );
>+        if ($do_get) {
>+            my $ua = Mojo::UserAgent->new( max_redirects => 5 );
>+            my $tx = $ua->get($uploadlocation);
>+            $tx->result->content->asset->move_to($tempfile);
>         } else {
>-            if ($uploadfile && !$plugins_restricted) {
>-                while (<$uploadfile>) {
>-                    print $tfh $_;
>-                }
>-                close $tfh;
>-            }
>+            $errors{'RESTRICTED'} = 1;
>+        }
>+    } else {
>+        $errors{'RESTRICTED'}  = 1 unless ( !$plugins_restricted );
>+        $errors{'EMPTYUPLOAD'} = 1 unless ( length($uploadfile) > 0 );
>+    }
> 
>-            my $ae = Archive::Extract->new( archive => $tempfile, type => 'zip' );
>-            unless ( $ae->extract( to => $plugins_dir ) ) {
>-                warn "ERROR: " . $ae->error;
>-                $errors{'UZIPFAIL'} = $uploadfilename;
>-                $template->param( ERRORS => [ \%errors ] );
>-                output_html_with_http_headers $input, $cookie, $template->output;
>-                exit;
>+    if (%errors) {
>+        $template->param( ERRORS => [ \%errors ] );
>+    } else {
>+        if ( $uploadfile && !$plugins_restricted ) {
>+            while (<$uploadfile>) {
>+                print $tfh $_;
>             }
>+            close $tfh;
>+        }
> 
>-            Koha::Plugins->new()->InstallPlugins();
>+        my $ae = Archive::Extract->new( archive => $tempfile, type => 'zip' );
>+        unless ( $ae->extract( to => $plugins_dir ) ) {
>+            warn "ERROR: " . $ae->error;
>+            $errors{'UZIPFAIL'} = $uploadfilename;
>+            $template->param( ERRORS => [ \%errors ] );
>+            output_html_with_http_headers $input, $cookie, $template->output;
>+            exit;
>         }
>-    } elsif ( ( $op eq 'Upload' ) && !$uploadfile && !$uploadlocation ) {
>-        warn "Problem uploading file or no file uploaded.";
>-    }
> 
>-    if ( ( $uploadfile || $uploadlocation ) && !%errors && !$template->param('ERRORS') ) {
>-        print $input->redirect("/cgi-bin/koha/plugins/plugins-home.pl");
>-    } else {
>-        output_html_with_http_headers $input, $cookie, $template->output;
>+        Koha::Plugins->new()->InstallPlugins();
>     }
>+} elsif ( ( $op eq 'Upload' ) && !$uploadfile && !$uploadlocation ) {
>+    warn "Problem uploading file or no file uploaded.";
>+}
> 
>+if ( ( $uploadfile || $uploadlocation ) && !%errors && !$template->param('ERRORS') ) {
>+    print $input->redirect("/cgi-bin/koha/plugins/plugins-home.pl");
> } else {
>     output_html_with_http_headers $input, $cookie, $template->output;
> }
>-- 
>2.41.0

Actions: View | Diff | Splinter Review

Attachments on bug 25672: 106651 | 106671 | 106673 | 106674 | 107172 | 107173 | 107174 | 153824 | 153825 | 153826 | 153827 | 153925 | 154423 | 154424 | 154425 | 154426 | 154427 | 154453 | 154455 | 154464 | 154465 | 154466 | 154467 | 154468 | 154474 | 154475 | 154476 | 154477 | 154478 | 156715 | 156716 | 156717 | 156718 | 156719 | 156720 | 156734 | 156735 | 156736 | 156737 | 156738 | 156739 | 156740 | 157730 | 157810 | 157811 | 157812 | 157813 | 157814 | 157815 | 157816 | 157817 | 157836 | 157837 | 157838 | 157839 | 157840 | 157841 | 157842 | 157843 | 157846 | 157847 | 157848 | 157849 | 157850 | 157851 | 157852 | 157853