From 775dcf214476b9b33bfb9f0cb0820ad304c7f699 Mon Sep 17 00:00:00 2001
From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Date: Wed, 19 Jun 2019 12:12:15 -0500
Subject: [PATCH] Bug 23042: Correct shib param escaping

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
---
 C4/Auth_with_shibboleth.pm                            |  4 ++--
 koha-tmpl/intranet-tmpl/prog/en/modules/auth.tt       |  2 +-
 .../opac-tmpl/bootstrap/en/includes/masthead.inc      |  2 +-
 koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-auth.tt |  2 +-
 t/Auth_with_shibboleth.t                              | 11 +++++++----
 5 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/C4/Auth_with_shibboleth.pm b/C4/Auth_with_shibboleth.pm
index 54813ba6f6..94190b20b3 100644
--- a/C4/Auth_with_shibboleth.pm
+++ b/C4/Auth_with_shibboleth.pm
@@ -201,12 +201,12 @@ sub _get_return {
         if ($uriPiece) {
             $uri_params_part .= '&' if $uri_params_part;
             $uri_params_part .= $param . '=';
-            $uri_params_part .= URI::Escape::uri_escape( $uriPiece );
+            $uri_params_part .= $uriPiece;
         }
     }
     $uri_base_part .= '%3F' if $uri_params_part;
 
-    return $uri_base_part . $uri_params_part;
+    return $uri_base_part . URI::Escape::uri_escape_utf8($uri_params_part);
 }
 
 sub _get_shib_config {
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/auth.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/auth.tt
index 01d27ac2aa..0826ba8f9a 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/auth.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/auth.tt
@@ -54,7 +54,7 @@
 [% IF (invalidShibLogin ) %]
 <div id="login_error"><Strong>Error: </strong>Shibboleth login failed</div>
 [% END %]
-<p>If you have a shibboleth account, please <a href="[% shibbolethLoginUrl | url %]">click here</a> to login.</p>
+<p>If you have a shibboleth account, please <a href="[% shibbolethLoginUrl | $raw %]">click here</a> to login.</p>
 [% END %]
 
 <!-- login prompt time-->
diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/includes/masthead.inc b/koha-tmpl/opac-tmpl/bootstrap/en/includes/masthead.inc
index 9466c2d9c6..88d8fa1072 100644
--- a/koha-tmpl/opac-tmpl/bootstrap/en/includes/masthead.inc
+++ b/koha-tmpl/opac-tmpl/bootstrap/en/includes/masthead.inc
@@ -311,7 +311,7 @@
                         </div>
                     [% ELSE %]
                         <h4>Shibboleth login</h4>
-                        <p>If you have a Shibboleth account, please <a href="[% shibbolethLoginUrl | url %]">click here to login</a>.</p>
+                        <p>If you have a Shibboleth account, please <a href="[% shibbolethLoginUrl | $raw %]">click here to login</a>.</p>
                         <h4>Local Login</h4>
                     [% END %]
                 [% END %]
diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-auth.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-auth.tt
index 02359a341a..61a09c2034 100644
--- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-auth.tt
+++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-auth.tt
@@ -82,7 +82,7 @@
                                     </div>
                             [% ELSE %]
                                 <h4>Shibboleth Login</h4>
-                                <p><a href="[% shibbolethLoginUrl | url %]">If you have a Shibboleth account, please click here to log in.</a></p>
+                                <p><a href="[% shibbolethLoginUrl | $raw %]">If you have a Shibboleth account, please click here to log in.</a></p>
                             [% END %]
                             [% IF ( casAuthentication ) %]
                                 <h4>CAS login</h4>
diff --git a/t/Auth_with_shibboleth.t b/t/Auth_with_shibboleth.t
index 73d1173b3d..44a30fe633 100644
--- a/t/Auth_with_shibboleth.t
+++ b/t/Auth_with_shibboleth.t
@@ -24,7 +24,8 @@ use Test::MockModule;
 use Test::Warn;
 use File::Temp qw(tempdir);
 
-use CGI;
+use utf8;
+use CGI qw(-utf8 );
 use C4::Context;
 
 BEGIN {
@@ -128,7 +129,9 @@ subtest "shib_ok tests" => sub {
 subtest "login_shib_url tests" => sub {
     plan tests => 2;
 
-    my $query_string = 'language=en-GB';
+    my $string = 'language=en-GB&param="heh❤"';
+    my $query_string = Encode::encode('UTF-8', $string);
+    my $query_string_uri_escaped = URI::Escape::uri_escape_utf8('?'.$string);
 
     local $ENV{REQUEST_METHOD} = 'GET';
     local $ENV{QUERY_STRING}   = $query_string;
@@ -138,8 +141,8 @@ subtest "login_shib_url tests" => sub {
         login_shib_url($query),
         'https://testopac.com'
           . '/Shibboleth.sso/Login?target='
-          . 'https://testopac.com/cgi-bin/koha/opac-user.pl' . '%3F'
-          . $query_string,
+          . 'https://testopac.com/cgi-bin/koha/opac-user.pl'
+          . $query_string_uri_escaped,
         "login shib url"
     );
 
-- 
2.39.5