From 4e02adb5dfee46aacadc74a054da26d0de83a73a Mon Sep 17 00:00:00 2001
From: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Date: Thu, 13 Jun 2019 17:02:33 +0100
Subject: [PATCH] Bug 23042: Only include GET params in return URL for
 Shibboleth

The shibboleth return target included POST parameters in the URL string,
this meant that a failed local login POST would include the username and
password used in the attemtped login in plaintext in the redirect URL
that is appended to the shibboleth login URL.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
---
 C4/Auth_with_shibboleth.pm | 32 ++++++++++++++++++++++++++------
 1 file changed, 26 insertions(+), 6 deletions(-)

diff --git a/C4/Auth_with_shibboleth.pm b/C4/Auth_with_shibboleth.pm
index 0c493f14b4..cbc3b6ec7b 100644
--- a/C4/Auth_with_shibboleth.pm
+++ b/C4/Auth_with_shibboleth.pm
@@ -54,18 +54,17 @@ sub shib_ok {
 sub logout_shib {
     my ($query) = @_;
     my $uri = _get_uri();
-    print $query->redirect( $uri . "/Shibboleth.sso/Logout?return=$uri" );
+    my $return = _get_return($query);
+    print $query->redirect( $uri . "/Shibboleth.sso/Logout?return=$return" );
 }
 
 # Returns Shibboleth login URL with callback to the requesting URL
 sub login_shib_url {
     my ($query) = @_;
 
-    my $param = _get_uri() . get_script_name();
-    if ( $query->query_string() ) {
-        $param = $param . '?' . $query->query_string();
-    }
-    my $uri = _get_uri() . "/Shibboleth.sso/Login?target=$param";
+    my $target = _get_return($query);
+    my $uri = _get_uri() . "/Shibboleth.sso/Login?target=" . $target;
+
     return $uri;
 }
 
@@ -194,6 +193,27 @@ sub _get_uri {
     return $return;
 }
 
+sub _get_return {
+    my ($query) = @_;
+
+    my $uri_base_part = _get_uri() . get_script_name();
+
+    my $uri_params_part = '';
+    foreach my $param ( $query->url_param() ) {
+        # url_param() always returns parameters that were deleted by delete()
+        # This additional check ensure that parameter was not deleted.
+        my $uriPiece = $query->param($param);
+        if ($uriPiece) {
+            $uri_params_part .= '&' if $uri_params_part;
+            $uri_params_part .= $param . '=';
+            $uri_params_part .= URI::Escape::uri_escape( $uriPiece );
+        }
+    }
+    $uri_base_part .= '%3F' if $uri_params_part;
+
+    return $uri_base_part . $uri_params_part;
+}
+
 sub _get_shib_config {
     my $config = C4::Context->config('shibboleth');
 
-- 
2.39.5