Skip to content

Commit 26fd906

Browse files
committed
Option to redirect to exact service url
Some clients will encode query parameters in their service url incorrectly. If SSP/php's query builder utils are used then the service url is parsed, and reconstructed differently then what is stored in the ticket. Sometimes badly encoded parameters are lost. Example: should space in a query param be encoded as '+' or '%20'
1 parent eee5fa2 commit 26fd906

File tree

4 files changed

+57
-5
lines changed

4 files changed

+57
-5
lines changed

config-templates/module_casserver.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,9 @@
3131
'attributes_to_transfer' => ['cn'],
3232
],
3333
],
34+
// Don't use php or SSP's built in methods for constructing queries. Default is false
35+
// Enabling this redirects the user back to the service with the exact service url provided.
36+
'noReencode' => false,
3437

3538
'legal_target_service_urls' => [
3639
//Any target service url string matching any of the following prefixes is accepted

tests/config/module_casserver.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,10 @@
2727
'|https://override.example.com/|' => [
2828
'attrname' => 'uid',
2929
'attributes_to_transfer' => ['cn'],
30+
],
31+
'https://buggy.edu' => [
32+
// Don't use php or SSP's built in methods for constructing queries.
33+
'noReencode' => true
3034
]
3135
],
3236

tests/www/LoginIntegrationTest.php

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,43 @@ public function testValidServiceUrl()
151151
);
152152
}
153153

154+
/**
155+
* Some clients don't correctly encode query parameters that are part their service
156+
* urls or encode a space in a different way then SSP will in a redirect. This workaround
157+
* is to allow those clients to work
158+
* @dataProvider buggyClientProvider
159+
* @return void
160+
*/
161+
public function testBuggyClientBadUrlEncodingWorkAround($service_url)
162+
{
163+
$this->authenticate();
164+
165+
/** @var array $resp */
166+
$resp = $this->server->get(
167+
self::$LINK_URL,
168+
['service' => $service_url],
169+
[
170+
CURLOPT_COOKIEJAR => $this->cookies_file,
171+
CURLOPT_COOKIEFILE => $this->cookies_file
172+
]
173+
);
174+
$this->assertEquals(302, $resp['code']);
175+
176+
$this->assertStringStartsWith(
177+
$service_url . '?ticket=ST-',
178+
$resp['headers']['Location'],
179+
'Ticket should be part of the redirect.'
180+
);
181+
}
182+
183+
public function buggyClientProvider(): array
184+
{
185+
return [
186+
['https://buggy.edu/kc/portal.do?solo&ct=Search%20Prot&curl=https://kc.edu/kc/IRB.do?se=1875*&runSearch=1'],
187+
['https://buggy.edu/kc'],
188+
];
189+
}
190+
154191

155192
/**
156193
* Test outputting user info instead of redirecting

www/login.php

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,15 +28,15 @@
2828
* language
2929
*/
3030

31+
use SimpleSAML\Configuration;
32+
use SimpleSAML\Locale\Language;
33+
use SimpleSAML\Logger;
34+
use SimpleSAML\Module;
3135
use SimpleSAML\Module\casserver\Cas\AttributeExtractor;
3236
use SimpleSAML\Module\casserver\Cas\Protocol\SamlValidateResponder;
3337
use SimpleSAML\Module\casserver\Cas\ServiceValidator;
3438
use SimpleSAML\Module\casserver\Cas\Ticket\TicketFactory;
3539
use SimpleSAML\Module\casserver\Cas\Ticket\TicketStore;
36-
use SimpleSAML\Configuration;
37-
use SimpleSAML\Locale\Language;
38-
use SimpleSAML\Logger;
39-
use SimpleSAML\Module;
4040
use SimpleSAML\Session;
4141
use SimpleSAML\Utils\HTTP;
4242

@@ -210,7 +210,15 @@
210210
echo '<pre>' . htmlspecialchars($casResponse) . '</pre>';
211211
}
212212
} elseif ($redirect) {
213-
HTTP::redirectTrustedURL(HTTP::addURLParameters($_GET['service'], $parameters));
213+
if ($casconfig->getBoolean('noReencode', false)) {
214+
// Some client encode query params wrong, and calling HTTP::addURLParameters
215+
// will reencode them resulting in service mismatches
216+
$extraParams = http_build_query($parameters);
217+
$redirectUrl = $_GET['service'] . (strpos('?', $_GET['service']) === false ? '?' : '&') . $extraParams;
218+
HTTP::redirectTrustedURL($redirectUrl);
219+
} else {
220+
HTTP::redirectTrustedURL(HTTP::addURLParameters($_GET['service'], $parameters));
221+
}
214222
} else {
215223
HTTP::submitPOSTData($_GET['service'], $parameters);
216224
}

0 commit comments

Comments
 (0)