PRG

resources/templates/header.php

@@ -2,12 +2,20 @@
 
 use UnityWebPortal\lib\UnitySite;
 
-if ((@$_SESSION["is_admin"] ?? false) == true
-    && $_SERVER["REQUEST_METHOD"] == "POST"
-    && (@$_POST["form_name"] ?? null) == "clearView"
-) {
-    unset($_SESSION["viewUser"]);
-    UnitySite::redirect($CONFIG["site"]["prefix"] . "/admin/user-mgmt.php");
+if ($_SERVER["REQUEST_METHOD"] == "POST") {
+    if ((@$_SESSION["is_admin"] ?? false) == true
+        && (@$_POST["form_name"] ?? null) == "clearView"
+    ) {
+        unset($_SESSION["viewUser"]);
+        UnitySite::redirect($CONFIG["site"]["prefix"] . "/admin/user-mgmt.php");
+    }
+    // Webroot files need to handle their own POSTs before loading the header
+    // so that they can do UnitySite::badRequest before anything else has been printed.
+    // They also must not redirect like standard PRG practice because this
+    // header also needs to handle POST data. So this header does the PRG redirect
+    // for all pages.
+    unset($_POST); // unset ensures that header must not come before POST handling
+    UnitySite::redirect($_SERVER['PHP_SELF']);
 }
 
 if (isset($SSO)) {

test/functional/NewUserTest.php

@@ -14,44 +14,26 @@ private function assertNumberGroupRequests(int $x)
 
     private function requestGroupCreation()
     {
-        $redirectedOrDied = false;
-        try {
-            http_post(
-                __DIR__ . "/../../webroot/panel/new_account.php",
-                ["new_user_sel" => "pi", "eula" => "agree", "confirm_pi" => "agree"]
-            );
-        } catch (\UnityWebPortal\lib\exceptions\PhpUnitNoDieException) {
-            $redirectedOrDied = true;
-        }
-        $this->assertTrue($redirectedOrDied);
+        http_post(
+            __DIR__ . "/../../webroot/panel/new_account.php",
+            ["new_user_sel" => "pi", "eula" => "agree", "confirm_pi" => "agree"]
+        );
     }
 
     private function requestGroupMembership(string $gid)
     {
-        $redirectedOrDied = false;
-        try {
-            http_post(
-                __DIR__ . "/../../webroot/panel/new_account.php",
-                ["new_user_sel" => "not_pi", "eula" => "agree", "pi" => $gid]
-            );
-        } catch (\UnityWebPortal\lib\exceptions\PhpUnitNoDieException) {
-            $redirectedOrDied = true;
-        }
-        $this->assertTrue($redirectedOrDied);
+        http_post(
+            __DIR__ . "/../../webroot/panel/new_account.php",
+            ["new_user_sel" => "not_pi", "eula" => "agree", "pi" => $gid]
+        );
     }
 
     private function cancelAllRequests()
     {
-        $redirectedOrDied = false;
-        try {
-            http_post(
-                __DIR__ . "/../../webroot/panel/new_account.php",
-                ["cancel" => "true"] // value of cancel is arbitrary
-            );
-        } catch (\UnityWebPortal\lib\exceptions\PhpUnitNoDieException) {
-            $redirectedOrDied = true;
-        }
-        $this->assertTrue($redirectedOrDied);
+        http_post(
+            __DIR__ . "/../../webroot/panel/new_account.php",
+            ["cancel" => "true"] // value of cancel is arbitrary
+        );
     }
 
     // delete requests made by that user
@@ -63,15 +45,15 @@ private function ensureUserDoesNotExist()
     {
         global $USER, $SQL, $LDAP;
         $SQL->deleteRequestsByUser($USER->getUID());
-        if ($USER->exists()) {
-            $USER->getLDAPUser()->delete();
-            assert(!$USER->exists());
-        }
         $org = $USER->getOrgGroup();
         if ($org->inOrg($USER)) {
             $org->removeUser($USER);
             assert(!$org->inOrg($USER));
         }
+        if ($USER->exists()) {
+            $USER->getLDAPUser()->delete();
+            assert(!$USER->exists());
+        }
         $all_users_group = $LDAP->getUserGroup();
         $all_member_uids = $all_users_group->getAttribute("memberuid");
         $new_uids = array_diff($all_member_uids, [$USER->getUID()]);

test/functional/ViewAsUserTest.php

@@ -15,15 +15,13 @@ public function _testViewAsUser(array $beforeUser, array $afterUser)
         // $this->assertTrue($USER->isAdmin());
         $beforeUid = $USER->getUID();
         // $this->assertNotEquals($afterUid, $beforeUid);
-        try {
-            http_post(
-                __DIR__ . "/../../webroot/admin/user-mgmt.php",
-                [
-                    "form_name" => "viewAsUser",
-                    "uid" => $afterUid,
-                ],
-            );
-        } catch (PhpUnitNoDieException) {}
+        http_post(
+            __DIR__ . "/../../webroot/admin/user-mgmt.php",
+            [
+                "form_name" => "viewAsUser",
+                "uid" => $afterUid,
+            ],
+        );
         $this->assertArrayHasKey("viewUser", $_SESSION);
         // redirect means that php process dies and user's browser will initiate a new one
         // this makes `require_once autoload.php` run again and init.php changes $USER
@@ -32,12 +30,10 @@ public function _testViewAsUser(array $beforeUser, array $afterUser)
         // now we should be new user
         $this->assertEquals($afterUid, $USER->getUID());
         // $this->assertTrue($_SESSION["user_exists"]);
-        try {
-            http_post(
-                __DIR__ . "/../../resources/templates/header.php",
-                ["form_name" => "clearView"],
-            );
-        } catch (PhpUnitNoDieException) {}
+        http_post(
+            __DIR__ . "/../../resources/templates/header.php",
+            ["form_name" => "clearView"],
+        );
         $this->assertArrayNotHasKey("viewUser", $_SESSION);
         // redirect means that php process dies and user's browser will initiate a new one
         // this makes `require_once autoload.php` run again and init.php changes $USER
@@ -69,15 +65,13 @@ public function testNonAdminViewAsAdmin()
         $adminUid = $USER->getUID();
         $this->assertTrue($USER->isAdmin());
         switchUser(...getNormalUser());
-        try {
-            http_post(
-                __DIR__ . "/../../webroot/admin/user-mgmt.php",
-                [
-                    "form_name" => "viewAsUser",
-                    "uid" => $adminUid,
-                ],
-            );
-        } catch (PhpUnitNoDieException) {}
+        http_post(
+            __DIR__ . "/../../webroot/admin/user-mgmt.php",
+            [
+                "form_name" => "viewAsUser",
+                "uid" => $adminUid,
+            ],
+        );
         $this->assertArrayNotHasKey("viewUser", $_SESSION);
     }
 }

test/phpunit-bootstrap.php

@@ -87,13 +87,18 @@ function http_post(string $phpfile, array $post_data): void
     $_SERVER["PHP_SELF"] = preg_replace("/.*webroot\//", "/", $phpfile);
     $_POST = $post_data;
     ob_start();
+    $post_did_redirect_or_die = false;
     try {
         include $phpfile;
+    } catch (UnityWebPortal\lib\exceptions\PhpUnitNoDieException $e) {
+        $post_did_redirect_or_die = true;
     } finally {
         ob_get_clean(); // discard output
         unset($_POST);
         $_SERVER = $_PREVIOUS_SERVER;
     }
+    // https://en.wikipedia.org/wiki/Post/Redirect/Get
+    assert($post_did_redirect_or_die, "post did not redirect or die!");
 }
 
 function http_get(string $phpfile, array $get_data = array()): void

webroot/index.php

@@ -2,7 +2,7 @@
 
 require_once __DIR__ . "/../resources/autoload.php";
 
-require_once $LOC_HEADER;
+include $LOC_HEADER;
 ?>
 
 

webroot/panel/account.php

@@ -4,8 +4,6 @@
 
 use UnityWebPortal\lib\UnitySite;
 
-require_once $LOC_HEADER;
-
 if ($_SERVER['REQUEST_METHOD'] == "POST") {
     switch (UnitySite::arrayGetOrBadRequest($_POST, "form_type")) {
         case "addKey":
@@ -73,6 +71,8 @@
             break;
     }
 }
+
+include $LOC_HEADER;
 ?>
 
 <h1>Account Settings</h1>

webroot/panel/new_account.php

@@ -39,12 +39,9 @@
                 $pi_group->cancelGroupJoinRequest($user=$USER);
             }
         }
-    } else {
-        UnitySite::badRequest("neither 'new_user_sel' or 'cancel' are set!");
     }
-    UnitySite::redirect($_SERVER['PHP_SELF']);
 }
-require_once $LOC_HEADER;
+include $LOC_HEADER;
 ?>
 
 <h1>Request Account</h1>

webroot/panel/support.php

@@ -2,7 +2,7 @@
 
 require "../../resources/autoload.php";
 
-require_once $LOC_HEADER;
+include $LOC_HEADER;
 ?>
 
 <h1>Support</h1>