Bug 17989: Centralize bad template check

The bad template check in get_template_and_user can be removed, since
this check has been added in gettemplate on bug 18010.

The check moves here to a new sub in C4::Templates. And will be extended
in a follow-up.

Removed unused variable $path in gettemplatefile.

Test plan:
[1] Run t/db_dependent/Auth.t (get_template_and_user bad calls).
[2] Run t/db_dependent/Templates.t (bad call to gettemplate).

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

diff --git a/C4/Auth.pm b/C4/Auth.pm
index 11f57b2..f2b5f6c 100644 (file)
--- a/C4/Auth.pm
+++ b/C4/Auth.pm
@@ -161,10 +161,9 @@ sub get_template_and_user {
 
     C4::Context->interface( $in->{type} );
 
-    my $safe_chars = 'a-zA-Z0-9_\-\/';
-    die "bad template path" unless $in->{'template_name'} =~ m/^[$safe_chars]+\.tt$/ig; #sanitize input
-
     $in->{'authnotrequired'} ||= 0;
+
+    # the following call includes a bad template check; might croak
     my $template = C4::Templates::gettemplate(
         $in->{'template_name'},
         $in->{'type'},

diff --git a/C4/Templates.pm b/C4/Templates.pm
index 17746e8..94c5baf 100644 (file)
--- a/C4/Templates.pm
+++ b/C4/Templates.pm
@@ -37,6 +37,7 @@ use C4::Languages qw(getTranslatedLanguages get_bidi regex_lang_subtags language
 use C4::Context;
 
 use Koha::Cache::Memory::Lite;
+use Koha::Exceptions;
 
 __PACKAGE__->mk_accessors(qw( theme activethemes preferredtheme lang filename htdocs interface vars));
 
@@ -167,15 +168,30 @@ sub _get_template_file {
     return ($htdocs, $theme, $lang, $filename);
 }
 
+=head2 badtemplatecheck
+
+    badtemplatecheck( $template_path );
+
+    The sub will throw an exception if the template path is not allowed.
+
+    Note: At this moment the sub is actually a helper routine for
+    sub gettemplate.
+
+=cut
+
+sub badtemplatecheck {
+    my ( $template ) = @_;
+    Koha::Exceptions::NoPermission->throw( 'bad template path' )
+        unless $template =~ m/^[a-zA-Z0-9_\-\/]+\.(tt|pref)$/;
+}
 
 sub gettemplate {
     my ( $tmplbase, $interface, $query, $is_plugin ) = @_;
     ($query) or warn "no query in gettemplate";
-    die "bad template path" unless $tmplbase =~ m/^[a-zA-Z0-9_\-\/]+\.(tt|pref)$/; # Will be extended on bug 17989
-    my $path = C4::Context->preference('intranet_includes') || 'includes';
     my ($htdocs, $theme, $lang, $filename)
        =  _get_template_file($tmplbase, $interface, $query);
     $filename = $tmplbase if ( $is_plugin );
+    badtemplatecheck( $filename ); # single trip for bad templates
     my $template = C4::Templates->new($interface, $filename, $tmplbase, $query);
 
 # NOTE: Commenting these out rather than deleting them so that those who need

diff --git a/Koha/Exceptions.pm b/Koha/Exceptions.pm
index 528a151..9d7f397 100644 (file)
--- a/Koha/Exceptions.pm
+++ b/Koha/Exceptions.pm
@@ -33,6 +33,10 @@ use Exception::Class (
         isa => 'Koha::Exceptions::Exception',
         description => 'One or more parameters are wrong',
     },
+    'Koha::Exceptions::NoPermission' => {
+        isa => 'Koha::Exceptions::Exception',
+        description => 'You do not have permission for this action',
+    },
     'Koha::Exceptions::CannotAddLibraryLimit' => {
         isa => 'Koha::Exceptions::Exception',
         description => 'General problem adding a library limit'