Skip to content

Commit c52e219

Browse files
ioigoumeioigoume
authored
slow post ux improvement (#2516)
* slow post ux improvement * Add tests * use getOptionalInteger
1 parent 614b754 commit c52e219

4 files changed

Lines changed: 224 additions & 28 deletions

File tree

‎config/config.php.dist‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -567,6 +567,16 @@ $config = [
567567
'saml' => true,
568568
],
569569

570+
/*************************************
571+
| POST/REDIRECT UI CONFIGURATION |
572+
*************************************/
573+
574+
/*
575+
* Delay (in milliseconds) before revealing the slow-post message on the auto-submit POST page.
576+
* If unset, defaults to 30000 ms. Negative values are coerced to 30000.
577+
* Set to 0 to reveal immediately.
578+
*/
579+
//'slow_post_delay_ms' => 30000,
570580

571581
/*************************
572582
| SESSION CONFIGURATION |

‎src/SimpleSAML/Utils/HTTP.php‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -265,7 +265,9 @@ private function redirect(string $url, array $parameters = []): void
265265
echo '</html>';
266266

267267
// end script execution
268-
exit;
268+
if (!defined('SIMPLESAMLPHP_TEST_NOEXIT')) {
269+
exit;
270+
}
269271
}
270272

271273

@@ -1187,12 +1189,23 @@ public function submitPOSTData(string $destination, array $data): void
11871189
if ($allowed && preg_match("#^http:#", $destination) && $this->isHTTPS()) {
11881190
// we need to post the data to HTTP
11891191
$this->redirect($this->getSecurePOSTRedirectURL($destination, $data));
1192+
return;
11901193
}
11911194

11921195
$p = new Template($config, 'post.twig');
11931196
$p->data['destination'] = $destination;
11941197
$p->data['post'] = $data;
1198+
1199+
// Read optional config override; default to 30s, ensure non-negative integer
1200+
$delay = $config->getOptionalInteger('slow_post_delay_ms', 30000);
1201+
if ($delay < 0) {
1202+
$delay = 30000;
1203+
}
1204+
$p->data['slow_post_delay_ms'] = $delay;
1205+
11951206
$p->send();
1196-
exit(0);
1207+
if (!defined('SIMPLESAMLPHP_TEST_NOEXIT')) {
1208+
exit(0);
1209+
}
11971210
}
11981211
}

‎templates/post.twig‎

Lines changed: 68 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,40 +1,84 @@
11
<!DOCTYPE html>
2-
<html>
3-
<head>
2+
<html lang="{{ appLocale|default(locale|default('en')) }}">
3+
<head>
44
<meta charset="utf-8">
5+
<meta name="viewport" content="width=device-width, initial-scale=1">
56
<meta http-equiv="X-UA-Compatible" content="IE=Edge">
67
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
78
<link rel="icon" href="{{ asset("icons/favicon.ico") }}">
89
<title>{% trans %}Sending message{% endtrans %}</title>
910
<link rel="stylesheet" href="{{ asset("css/postSubmit.css") }}">
10-
<script src="{{ asset("js/post.js") }}"></script>
11-
</head>
12-
<body>
11+
{% block preload %}{% endblock %}
12+
</head>
13+
<body>
14+
{% block content %}
15+
<h1 id="page-title" class="mb-2 center hidden">{% trans %}Sending message{% endtrans %}</h1>
1316
<form method="post" action="{{ destination }}">
14-
{#- We need to add this element and call the click method, because calling submit() on the form causes failed
17+
{#- We need to add this element and call the click method, because calling submit() on the form causes failed
1518
submissions if the form has another element with name or id of submit. See:
1619
https://developer.mozilla.org/en/DOM/form.submit#Specification
17-
#}
20+
#}
1821

19-
<input type="submit" id="postLoginSubmitButton">
20-
{%- for name, value in post %}
21-
{%- if value is iterable %}
22-
{%- for index, item in value %}
22+
<input
23+
type="submit"
24+
id="postLoginSubmitButton"
25+
value="{% trans %}Continue{% endtrans %}"
26+
class="js-only"
27+
aria-hidden="true"
28+
tabindex="-1"
29+
>
30+
{%- for name, value in post %}
31+
{%- if value is iterable %}
32+
{%- for index, item in value %}
2333

24-
<input type="hidden" name="{{ name }}[{{ index }}]" value="{{ item }}">
25-
{%- endfor %}
26-
{%- else %}
34+
<input type="hidden" name="{{ name }}[{{ index }}]" value="{{ item }}">
35+
{%- endfor %}
36+
{%- else %}
2737

28-
<input type="hidden" name="{{ name }}" value="{{ value }}">
29-
{%- endif %}
30-
{%- endfor %}
38+
<input type="hidden" name="{{ name }}" value="{{ value }}">
39+
{%- endif %}
40+
{%- endfor %}
3141

32-
<noscript>
33-
<h2>{% trans %}Warning{% endtrans %}</h2>
34-
<p>{% trans %}Since your browser does not support Javascript, you must press the button below to proceed.{%
35-
endtrans %}</p>
36-
<button type="submit">{% trans %}Yes, continue{% endtrans %}</button>
37-
</noscript>
42+
<noscript>
43+
{# Make this css class available only in the case of no script #}
44+
<style>
45+
.js-only {
46+
display: none !important;
47+
}
48+
</style>
49+
<h2>{% trans %}Warning{% endtrans %}</h2>
50+
<p>{% trans %}Since your browser does not support Javascript, you must press the button below to proceed.{%
51+
endtrans %}</p>
52+
<button type="submit">{% trans %}Yes, continue{% endtrans %}</button>
53+
</noscript>
3854
</form>
39-
</body>
55+
56+
<div id="slowPostMessage"
57+
class="js-only slowPostMessage hidden"
58+
role="alert"
59+
aria-live="assertive"
60+
aria-atomic="true"
61+
aria-labelledby="slowpost-title"
62+
aria-describedby="slowpost-desc"
63+
data-slow-post-delay="{{ slow_post_delay_ms|default(30000) }}"
64+
tabindex="-1">
65+
<div class="p-2 mt-1">
66+
<h2 id="slowpost-title" class="mb-2">Connecting to the application…</h2>
67+
<div id="slowpost-desc">
68+
<div class="mb-2">
69+
The application at {{ destination }} may not be responding.
70+
</div>
71+
<div class="mb-0">
72+
Your browser will continue trying to connect. Do not refresh this page or press the back button, as it may cause an error.
73+
If you are unable to connect, return to the original application and try again.
74+
</div>
75+
</div>
76+
</div>
77+
</div>
78+
<span id="loader" class="loader js-only"></span>
79+
{% endblock %}
80+
81+
<script src="{{ asset("js/post.js") }}"></script>
82+
{% block postload %}{% endblock %}
83+
</body>
4084
</html>

‎tests/src/SimpleSAML/Utils/HTTPTest.php‎

Lines changed: 131 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,17 @@
1818
#[CoversClass(Utils\HTTP::class)]
1919
class HTTPTest extends ClearStateTestCase
2020
{
21+
/**
22+
* Set up the test.
23+
*/
24+
protected function setUp(): void
25+
{
26+
parent::setUp();
27+
if (!defined('SIMPLESAMLPHP_TEST_NOEXIT')) {
28+
define('SIMPLESAMLPHP_TEST_NOEXIT', true);
29+
}
30+
}
31+
2132
/**
2233
* Set up the environment ($_SERVER) populating the typical variables from a given URL.
2334
*
@@ -584,7 +595,7 @@ public function testDetectSameSiteNoneBehavior(?string $userAgent, bool $support
584595

585596
public static function detectSameSiteProvider(): array
586597
{
587-
// @codingStandardsIgnoreStart
598+
// phpcs:disable Generic.Files.LineLength
588599
return [
589600
[null, true],
590601
['some-new-browser', true],
@@ -618,6 +629,124 @@ public static function detectSameSiteProvider(): array
618629
// old embedded browser
619630
['Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14_4) AppleWebKit/605.1.15 (KHTML, like Gecko)', false],
620631
];
621-
// @codingStandardsIgnoreEnd
632+
// phpcs:enable Generic.Files.LineLength
633+
}
634+
635+
/**
636+
* submitPOSTData() should throw Error\Exception for an invalid destination URL.
637+
*/
638+
public function testSubmitPOSTDataThrowsOnInvalidURL(): void
639+
{
640+
$httpUtils = new Utils\HTTP();
641+
642+
// Minimal configuration to satisfy internals; won’t be used since we fail early.
643+
Configuration::loadFromArray([
644+
'baseurlpath' => 'https://example.com/simplesaml/',
645+
], '[ARRAY]', 'simplesaml');
646+
647+
$this->expectException(Error\Exception::class);
648+
$this->expectExceptionMessage('Invalid destination URL: not-a-url');
649+
650+
$httpUtils->submitPOSTData('not-a-url', ['a' => 'b']);
651+
}
652+
653+
654+
/**
655+
* When enable.http_post = true, destination is http:// and current request is HTTPS, we must redirect.
656+
* We assert a Location header is sent pointing to a postredirect URL.
657+
*/
658+
#[Depends('testXdebugMode')]
659+
#[RunInSeparateProcess]
660+
public function testSubmitPOSTDataRedirectsFromHttpsToHttp(): void
661+
{
662+
$httpUtils = new Utils\HTTP();
663+
664+
// Configure base URL and allow http post.
665+
Configuration::loadFromArray([
666+
'baseurlpath' => 'https://idp.example.org/simplesaml/',
667+
'enable.http_post' => true,
668+
'secretsalt' => 'abc',
669+
], '[ARRAY]', 'simplesaml');
670+
671+
// Simulate the current request being HTTPS
672+
$this->setupEnvFromURL('https://idp.example.org/simplesaml/module.php/core/someaction?x=1');
673+
674+
// Destination is explicitly http://
675+
$destination = 'http://sp.example.com/acs';
676+
$post = ['SAMLResponse' => 'abc', 'RelayState' => 'xyz'];
677+
678+
try {
679+
$httpUtils->submitPOSTData($destination, $post);
680+
} catch (\Throwable $e) {
681+
}
682+
683+
$headers = function_exists('xdebug_get_headers') ? xdebug_get_headers() : [];
684+
685+
// Find the Location header
686+
$locationHeader = null;
687+
foreach ($headers as $h) {
688+
if (stripos($h, 'Location: ') === 0) {
689+
$locationHeader = substr($h, 10);
690+
break;
691+
}
692+
}
693+
694+
$this->assertNotNull($locationHeader, 'Expected a Location header to be sent');
695+
$this->assertStringStartsWith('http://', $locationHeader, 'Location should be http://');
696+
$this->assertStringContainsString('/core/postredirect', $locationHeader);
697+
$this->assertTrue(
698+
(str_contains($locationHeader, 'RedirInfo=')),
699+
'Location should contain RedirInfo parameter',
700+
);
701+
}
702+
703+
704+
/**
705+
* submitPOSTData() should pass slow_post_delay_ms to the template:
706+
* - default 30000 when config key missing
707+
* - default 30000 when config value < 0
708+
* - exact value when config value is 10000
709+
*/
710+
#[DataProvider('slowPostDelayProvider')]
711+
#[RunInSeparateProcess]
712+
public function testSubmitPOSTDataSlowPostDelay(?int $configured, int $expected): void
713+
{
714+
$httpUtils = new Utils\HTTP();
715+
716+
// Base config
717+
$config = [
718+
'baseurlpath' => 'https://idp.example.org/simplesaml/',
719+
'enable.http_post' => false,
720+
];
721+
if ($configured !== null) {
722+
$config['slow_post_delay_ms'] = $configured;
723+
}
724+
Configuration::loadFromArray($config, '[ARRAY]', 'simplesaml');
725+
726+
// Use https destination to bypass the http-redirect branch entirely
727+
$destination = 'https://sp.example.com/acs';
728+
$post = ['k' => 'v'];
729+
730+
// Capture output
731+
ob_start();
732+
try {
733+
$httpUtils->submitPOSTData($destination, $post);
734+
} catch (\Throwable $e) {
735+
}
736+
$html = ob_get_clean();
737+
738+
// The template writes the delay into data-slow-post-delay attribute
739+
$needle = 'data-slow-post-delay="' . $expected . '"';
740+
$this->assertStringContainsString($needle, $html);
741+
}
742+
743+
public static function slowPostDelayProvider(): array
744+
{
745+
return [
746+
// [configured, expected]
747+
'missing config => default' => [null, 30000],
748+
'negative config => default' => [-5, 30000],
749+
'positive config 10000 => 10000' => [10000, 10000],
750+
];
622751
}
623752
}

0 commit comments

Comments
 (0)