Merge pull request #28 from cirrusidentity/bug/non-greedy-jessionid

Make jsession sanitize non-greedy

Author: pradtke

lib/Cas/TicketValidator.php

@@ -97,15 +97,17 @@ public function validateAndDeleteTicket($ticket, $service)
 
 
     /**
-     * @param string $parameter
-     * @return string
+     * Java CAS clients are inconsistent with their sending of jsessionid, so remove it to
+     * avoid service url matching issues.
+     * @param string $parameter The service url to sanitize
+     * @return string The sanitized url
      */
     public static function sanitize($parameter)
     {
         return preg_replace(
-            '/;jsessionid=.*[^?].*$/',
+            '/;jsessionid=.*[^?].*$/U',
             '',
-            preg_replace('/;jsessionid=.*[?]/', '?', urldecode($parameter))
+            preg_replace('/;jsessionid=.*[?]/U', '?', urldecode($parameter))
         );
     }
 }

tests/lib/TicketValidatorTest.php

@@ -10,12 +10,6 @@
 use SimpleSAML\Module\casserver\Cas\TicketValidator;
 use SimpleSAML\Utils\Random;
 
-/**
- * Created by PhpStorm.
- * User: patrick
- * Date: 8/23/19
- * Time: 12:13 PM
- */
 class TicketValidatorTest extends TestCase
 {
     /**
@@ -35,6 +29,8 @@ class TicketValidatorTest extends TestCase
     protected function setUp(): void
     {
         parent::setUp();
+        Configuration::clearInternalState();
+        putenv('SIMPLESAMLPHP_CONFIG_DIR=' . dirname(__DIR__) . '/config');
         $casConfig = Configuration::loadFromArray([
             'ticketstore' => [
                 'class' => 'casserver:FileSystemTicketStore',
@@ -131,6 +127,39 @@ public function testExpiredTicket()
         $this->assertNull($this->ticketStore->getTicket($id), "ticket deleted after loading");
     }
 
+    /**
+     * @dataProvider urlSanitizationProvider
+     * @param string $serviceUrl The service url that will get sanitized
+     * @param string $expectedSanitzedUrl The expected result
+     * @return void
+     */
+    public function testUrlSanitization(string $serviceUrl, string $expectedSanitzedUrl): void
+    {
+        $this->assertEquals($expectedSanitzedUrl, TicketValidator::sanitize($serviceUrl));
+    }
+
+    /**
+     * Urls to test
+     * @return array
+     */
+    public function urlSanitizationProvider()
+    {
+        return [
+            [
+                'https://example.edu/kc/portal.do;jsessionid=99AC064A12?a=b',
+                'https://example.edu/kc/portal.do?a=b',
+            ],
+            [
+                'https://example.edu/kc/portal.do?a=b',
+                'https://example.edu/kc/portal.do?a=b',
+            ],
+            [
+                'https://k.edu/kc/portal.do;jsessionid=99AC064A127?ct=Search&cu=https://k.edu/kc/as.do?ssf=456*&rsol=1',
+                'https://k.edu/kc/portal.do?ct=Search&cu=https://k.edu/kc/as.do?ssf=456*&rsol=1',
+            ]
+        ];
+    }
+
 
     /**
      * Create a ticket to use for testing

www/utility/urlUtils.php

@@ -23,6 +23,7 @@
 
 use SimpleSAML\Configuration;
 use SimpleSAML\Module\casserver\Cas\ServiceValidator;
+use SimpleSAML\Module\casserver\Cas\TicketValidator;
 
 /**
  * @deprecated
@@ -46,5 +47,5 @@ function checkServiceURL($service, array $legal_service_urls)
  */
 function sanitize($parameter)
 {
-    return preg_replace('/;jsessionid=.*[^?].*$/', '', preg_replace('/;jsessionid=.*[?]/', '?', urldecode($parameter)));
+    return TicketValidator::sanitize($parameter);
 }