Option to redirect to exact service url

tests/config/module_casserver.php

@@ -30,6 +30,8 @@
         ],
         'http://changeTicketParam' => [
             'ticketName' => 'myTicket',
+        ],
+        'https://encoding.edu' => [
         ]
     ],
 

tests/www/LoginIntegrationTest.php

@@ -236,7 +236,7 @@ public function testValidTicketNameOverride()
                 CURLOPT_COOKIEFILE => $this->cookies_file
             ]
         );
-        $this->assertEquals(302, $resp['code']);
+        $this->assertEquals(302, $resp['code'], $resp['body']);
 
         $this->assertStringStartsWith(
             $service_url . '?myTicket=ST-',
@@ -245,6 +245,69 @@ public function testValidTicketNameOverride()
         );
     }
 
+    /**
+     * Some clients urls:
+     * 1. Incorrectly encode query parameters or fragments
+     * 2. Encode spaces as %20 which php rencodes to +
+     * 3. Special characters url encoded to lower case hexadecimal (some .net versions) are changed to upper case.
+     *
+     * Test to confirm these type of urls work
+     * @dataProvider encodingIssueProvider
+     * @param string $service_url The service url submitted by the client
+     * @param string $expectedStartsWith The redirect location is expected to start with this.
+     * @param string $expectedEndsWith The redirect location is expected to end with this. Used for testing #fragments.
+     * @return void
+     */
+    public function testEncodingIssued($service_url, $expectedStartsWith, $expectedEndsWith)
+    {
+        $this->authenticate();
+
+        /** @var array $resp */
+        $resp = $this->server->get(
+            self::$LINK_URL,
+            ['service' => $service_url],
+            [
+                CURLOPT_COOKIEJAR => $this->cookies_file,
+                CURLOPT_COOKIEFILE => $this->cookies_file
+            ]
+        );
+        $this->assertEquals(302, $resp['code'], $resp['body']);
+
+        $this->assertStringStartsWith(
+            $expectedStartsWith,
+            $resp['headers']['Location'],
+            'Ticket should be part of the redirect.'
+        );
+        if (!empty($expectedEndsWith)) {
+            $this->assertStringEndsWith(
+                $expectedEndsWith,
+                $resp['headers']['Location'],
+                'url fragments happen after the query params'
+            );
+        }
+    }
+
+    public function encodingIssueProvider(): array
+    {
+        $urlWithQuery = 'https://encoding.edu/bug/portal.do?solo&ct=Search%20Prot&curl=https://kc.edu/IRB.do?se=1875*&rs=1';
+        $urlNoQuery = 'https://encoding.edu/bug';
+        $urlMultiKeys = 'https://encoding.edu/bug?a=val1&a=val2';
+        $urlWithCaseDifference = 'https://encoding.edu?url=https%3a%2f%2Fencoding.edu%3Fa%3Db';
+        $urlWithSpace = 'https://encoding.edu?a=a%20space';
+        $urlWithRepeatParams = 'https://encoding.edu?a=b&a=c';
+        return [
+            [$urlWithQuery, $urlWithQuery . '&ticket=ST-', ''],
+            [$urlWithQuery . '#fragment', $urlWithQuery . '&ticket=ST-', '#fragment'],
+            [$urlMultiKeys, $urlMultiKeys . '&ticket=ST-', ''],
+            [$urlMultiKeys . '#fragment', $urlMultiKeys . '&ticket=ST-', '#fragment'],
+            [$urlNoQuery, $urlNoQuery. '?ticket=ST-', ''],
+            [$urlNoQuery . '#fragment', $urlNoQuery . '?ticket=ST-', '#fragment'],
+            [$urlWithCaseDifference, $urlWithCaseDifference. '&ticket=ST-', ''],
+            [$urlWithSpace, $urlWithSpace. '&ticket=ST-', ''],
+            [$urlWithRepeatParams, $urlWithRepeatParams. '&ticket=ST-', ''],
+        ];
+    }
+
 
     /**
      * Test outputting user info instead of redirecting

www/login.php

@@ -28,15 +28,15 @@
  *  language
  */
 
+use SimpleSAML\Configuration;
+use SimpleSAML\Locale\Language;
+use SimpleSAML\Logger;
+use SimpleSAML\Module;
 use SimpleSAML\Module\casserver\Cas\AttributeExtractor;
 use SimpleSAML\Module\casserver\Cas\Protocol\SamlValidateResponder;
 use SimpleSAML\Module\casserver\Cas\ServiceValidator;
 use SimpleSAML\Module\casserver\Cas\Ticket\TicketFactory;
 use SimpleSAML\Module\casserver\Cas\Ticket\TicketStore;
-use SimpleSAML\Configuration;
-use SimpleSAML\Locale\Language;
-use SimpleSAML\Logger;
-use SimpleSAML\Module;
 use SimpleSAML\Session;
 use SimpleSAML\Utils;
 
@@ -227,7 +227,7 @@
             echo '<pre>' . htmlspecialchars($casResponse) . '</pre>';
         }
     } elseif ($redirect) {
-        $httpUtils->redirectTrustedURL($httpUtils->addURLParameters($serviceUrl, $parameters));
+        $httpUtils->redirectTrustedURL($this->casAddURLParameters($serviceUrl, $parameters));
     } else {
         $httpUtils->submitPOSTData($serviceUrl, $parameters);
     }
@@ -236,3 +236,30 @@
         $httpUtils->addURLParameters(Module::getModuleURL('casserver/loggedIn.php'), $parameters)
     );
 }
+
+
+/**
+ * CAS wants to ensure that a service url provided in login matches exactly that provided in service validate.
+ * This method avoids SSP's built in redirect which can change that url in certain ways, such as
+ * * changing how a ' ' is encoded
+ * * not correctly handling url fragments (e.g. #)
+ * * not correctly handling query param keys occurring multiple times
+ * * changing lower case hexadecimal to upper case
+ * * some buggy clients don't encode query params correctly
+ * which results in either the wrong url being returned to the client, or a service mismatch
+ * @param string $url The url to adjust
+ * @param array $params The query parameters to add.
+ * @return string The url to return
+ */
+function casAddURLParameters($url, $params)
+{
+    $url_fragment = explode("#", $url);
+    if (strpos($url_fragment[0], "?") === false) {
+        $url_fragment[0] .= "?";
+    } else {
+        $url_fragment[0] .= "&";
+    }
+    $url_fragment[0] .= http_build_query($params);
+    $url = implode("#", $url_fragment);
+    return $url;
+}