Skip to content

Commit 243c6b5

Browse files
simonLeary42simonLeary42
authored
PRG (#233)
* PRG * add PRG into http_post function * let header handle redirect * more similar to original * fix infinite loop * clarity * require_once -> include * remove old file * remove from group before deleting user * allow post to pass to header * catch redirect exception is now in http_post
1 parent 4719bb4 commit 243c6b5

File tree

8 files changed

+58
-72
lines changed

8 files changed

+58
-72
lines changed

‎resources/templates/header.php

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,20 @@
22

33
use UnityWebPortal\lib\UnitySite;
44

5-
if ((@$_SESSION["is_admin"] ?? false) == true
6-
&& $_SERVER["REQUEST_METHOD"] == "POST"
7-
&& (@$_POST["form_name"] ?? null) == "clearView"
8-
) {
9-
unset($_SESSION["viewUser"]);
10-
UnitySite::redirect($CONFIG["site"]["prefix"] . "/admin/user-mgmt.php");
5+
if ($_SERVER["REQUEST_METHOD"] == "POST") {
6+
if ((@$_SESSION["is_admin"] ?? false) == true
7+
&& (@$_POST["form_name"] ?? null) == "clearView"
8+
) {
9+
unset($_SESSION["viewUser"]);
10+
UnitySite::redirect($CONFIG["site"]["prefix"] . "/admin/user-mgmt.php");
11+
}
12+
// Webroot files need to handle their own POSTs before loading the header
13+
// so that they can do UnitySite::badRequest before anything else has been printed.
14+
// They also must not redirect like standard PRG practice because this
15+
// header also needs to handle POST data. So this header does the PRG redirect
16+
// for all pages.
17+
unset($_POST); // unset ensures that header must not come before POST handling
18+
UnitySite::redirect($_SERVER['PHP_SELF']);
1119
}
1220

1321
if (isset($SSO)) {

‎test/functional/NewUserTest.php

Lines changed: 16 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -14,44 +14,26 @@ private function assertNumberGroupRequests(int $x)
1414

1515
private function requestGroupCreation()
1616
{
17-
$redirectedOrDied = false;
18-
try {
19-
http_post(
20-
__DIR__ . "/../../webroot/panel/new_account.php",
21-
["new_user_sel" => "pi", "eula" => "agree", "confirm_pi" => "agree"]
22-
);
23-
} catch (\UnityWebPortal\lib\exceptions\PhpUnitNoDieException) {
24-
$redirectedOrDied = true;
25-
}
26-
$this->assertTrue($redirectedOrDied);
17+
http_post(
18+
__DIR__ . "/../../webroot/panel/new_account.php",
19+
["new_user_sel" => "pi", "eula" => "agree", "confirm_pi" => "agree"]
20+
);
2721
}
2822

2923
private function requestGroupMembership(string $gid)
3024
{
31-
$redirectedOrDied = false;
32-
try {
33-
http_post(
34-
__DIR__ . "/../../webroot/panel/new_account.php",
35-
["new_user_sel" => "not_pi", "eula" => "agree", "pi" => $gid]
36-
);
37-
} catch (\UnityWebPortal\lib\exceptions\PhpUnitNoDieException) {
38-
$redirectedOrDied = true;
39-
}
40-
$this->assertTrue($redirectedOrDied);
25+
http_post(
26+
__DIR__ . "/../../webroot/panel/new_account.php",
27+
["new_user_sel" => "not_pi", "eula" => "agree", "pi" => $gid]
28+
);
4129
}
4230

4331
private function cancelAllRequests()
4432
{
45-
$redirectedOrDied = false;
46-
try {
47-
http_post(
48-
__DIR__ . "/../../webroot/panel/new_account.php",
49-
["cancel" => "true"] // value of cancel is arbitrary
50-
);
51-
} catch (\UnityWebPortal\lib\exceptions\PhpUnitNoDieException) {
52-
$redirectedOrDied = true;
53-
}
54-
$this->assertTrue($redirectedOrDied);
33+
http_post(
34+
__DIR__ . "/../../webroot/panel/new_account.php",
35+
["cancel" => "true"] // value of cancel is arbitrary
36+
);
5537
}
5638

5739
// delete requests made by that user
@@ -63,15 +45,15 @@ private function ensureUserDoesNotExist()
6345
{
6446
global $USER, $SQL, $LDAP;
6547
$SQL->deleteRequestsByUser($USER->getUID());
66-
if ($USER->exists()) {
67-
$USER->getLDAPUser()->delete();
68-
assert(!$USER->exists());
69-
}
7048
$org = $USER->getOrgGroup();
7149
if ($org->inOrg($USER)) {
7250
$org->removeUser($USER);
7351
assert(!$org->inOrg($USER));
7452
}
53+
if ($USER->exists()) {
54+
$USER->getLDAPUser()->delete();
55+
assert(!$USER->exists());
56+
}
7557
$all_users_group = $LDAP->getUserGroup();
7658
$all_member_uids = $all_users_group->getAttribute("memberuid");
7759
$new_uids = array_diff($all_member_uids, [$USER->getUID()]);

‎test/functional/ViewAsUserTest.php

Lines changed: 18 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -15,15 +15,13 @@ public function _testViewAsUser(array $beforeUser, array $afterUser)
1515
// $this->assertTrue($USER->isAdmin());
1616
$beforeUid = $USER->getUID();
1717
// $this->assertNotEquals($afterUid, $beforeUid);
18-
try {
19-
http_post(
20-
__DIR__ . "/../../webroot/admin/user-mgmt.php",
21-
[
22-
"form_name" => "viewAsUser",
23-
"uid" => $afterUid,
24-
],
25-
);
26-
} catch (PhpUnitNoDieException) {}
18+
http_post(
19+
__DIR__ . "/../../webroot/admin/user-mgmt.php",
20+
[
21+
"form_name" => "viewAsUser",
22+
"uid" => $afterUid,
23+
],
24+
);
2725
$this->assertArrayHasKey("viewUser", $_SESSION);
2826
// redirect means that php process dies and user's browser will initiate a new one
2927
// this makes `require_once autoload.php` run again and init.php changes $USER
@@ -32,12 +30,10 @@ public function _testViewAsUser(array $beforeUser, array $afterUser)
3230
// now we should be new user
3331
$this->assertEquals($afterUid, $USER->getUID());
3432
// $this->assertTrue($_SESSION["user_exists"]);
35-
try {
36-
http_post(
37-
__DIR__ . "/../../resources/templates/header.php",
38-
["form_name" => "clearView"],
39-
);
40-
} catch (PhpUnitNoDieException) {}
33+
http_post(
34+
__DIR__ . "/../../resources/templates/header.php",
35+
["form_name" => "clearView"],
36+
);
4137
$this->assertArrayNotHasKey("viewUser", $_SESSION);
4238
// redirect means that php process dies and user's browser will initiate a new one
4339
// this makes `require_once autoload.php` run again and init.php changes $USER
@@ -69,15 +65,13 @@ public function testNonAdminViewAsAdmin()
6965
$adminUid = $USER->getUID();
7066
$this->assertTrue($USER->isAdmin());
7167
switchUser(...getNormalUser());
72-
try {
73-
http_post(
74-
__DIR__ . "/../../webroot/admin/user-mgmt.php",
75-
[
76-
"form_name" => "viewAsUser",
77-
"uid" => $adminUid,
78-
],
79-
);
80-
} catch (PhpUnitNoDieException) {}
68+
http_post(
69+
__DIR__ . "/../../webroot/admin/user-mgmt.php",
70+
[
71+
"form_name" => "viewAsUser",
72+
"uid" => $adminUid,
73+
],
74+
);
8175
$this->assertArrayNotHasKey("viewUser", $_SESSION);
8276
}
8377
}

‎test/phpunit-bootstrap.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,13 +87,18 @@ function http_post(string $phpfile, array $post_data): void
8787
$_SERVER["PHP_SELF"] = preg_replace("/.*webroot\//", "/", $phpfile);
8888
$_POST = $post_data;
8989
ob_start();
90+
$post_did_redirect_or_die = false;
9091
try {
9192
include $phpfile;
93+
} catch (UnityWebPortal\lib\exceptions\PhpUnitNoDieException $e) {
94+
$post_did_redirect_or_die = true;
9295
} finally {
9396
ob_get_clean(); // discard output
9497
unset($_POST);
9598
$_SERVER = $_PREVIOUS_SERVER;
9699
}
100+
// https://en.wikipedia.org/wiki/Post/Redirect/Get
101+
assert($post_did_redirect_or_die, "post did not redirect or die!");
97102
}
98103

99104
function http_get(string $phpfile, array $get_data = array()): void

‎webroot/index.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
require_once __DIR__ . "/../resources/autoload.php";
44

5-
require_once $LOC_HEADER;
5+
include $LOC_HEADER;
66
?>
77

88

‎webroot/panel/account.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,6 @@
44

55
use UnityWebPortal\lib\UnitySite;
66

7-
require_once $LOC_HEADER;
8-
97
if ($_SERVER['REQUEST_METHOD'] == "POST") {
108
switch (UnitySite::arrayGetOrBadRequest($_POST, "form_type")) {
119
case "addKey":
@@ -73,6 +71,8 @@
7371
break;
7472
}
7573
}
74+
75+
include $LOC_HEADER;
7676
?>
7777

7878
<h1>Account Settings</h1>

‎webroot/panel/new_account.php

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,12 +39,9 @@
3939
$pi_group->cancelGroupJoinRequest($user=$USER);
4040
}
4141
}
42-
} else {
43-
UnitySite::badRequest("neither 'new_user_sel' or 'cancel' are set!");
4442
}
45-
UnitySite::redirect($_SERVER['PHP_SELF']);
4643
}
47-
require_once $LOC_HEADER;
44+
include $LOC_HEADER;
4845
?>
4946

5047
<h1>Request Account</h1>

‎webroot/panel/support.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
require "../../resources/autoload.php";
44

5-
require_once $LOC_HEADER;
5+
include $LOC_HEADER;
66
?>
77

88
<h1>Support</h1>

0 commit comments

Comments
 (0)