From bd30167e522cd84dffced7e8b78ebfc3c58944b5 Mon Sep 17 00:00:00 2001 From: Jack Meddick Date: Fri, 5 Dec 2025 15:31:23 -0500 Subject: [PATCH 1/4] removed the constant from UnitySQL --- resources/lib/UnityGroup.php | 10 +++++----- resources/lib/UnitySQL.php | 11 +++++------ test/functional/PIBecomeApproveTest.php | 5 +---- test/functional/PIMemberRequestTest.php | 2 +- test/functional/PiBecomeRequestTest.php | 12 ++++++------ webroot/admin/pi-mgmt.php | 2 +- webroot/panel/account.php | 6 +++--- 7 files changed, 22 insertions(+), 26 deletions(-) diff --git a/resources/lib/UnityGroup.php b/resources/lib/UnityGroup.php index bc057410..b2356552 100644 --- a/resources/lib/UnityGroup.php +++ b/resources/lib/UnityGroup.php @@ -76,7 +76,7 @@ public function requestGroup(bool $send_mail_to_admins, bool $send_mail = true): "name" => $this->getOwner()->getFullName(), "email" => $this->getOwner()->getMail(), ]; - $this->SQL->addRequest($this->getOwner()->uid); + $this->SQL->addRequest($this->getOwner()->uid, "admin"); if ($send_mail) { $this->MAILER->sendMail($this->getOwner()->getMail(), "group_request"); $this->WEBHOOK->sendWebhook("group_request_admin", $context); @@ -93,13 +93,13 @@ public function requestGroup(bool $send_mail_to_admins, bool $send_mail = true): public function approveGroup(?UnityUser $operator = null, bool $send_mail = true): void { $uid = $this->getOwner()->uid; - $request = $this->SQL->getRequest($uid, UnitySQL::REQUEST_BECOME_PI); + $request = $this->SQL->getRequest($uid, "admin"); if ($this->exists()) { return; } \ensure($this->getOwner()->exists()); $this->init(); - $this->SQL->removeRequest($this->getOwner()->uid); + $this->SQL->removeRequest($this->getOwner()->uid, "admin"); $operator = is_null($operator) ? $this->getOwner()->uid : $operator->uid; $this->SQL->addLog( $operator, @@ -137,10 +137,10 @@ public function denyGroup(?UnityUser $operator = null, bool $send_mail = true): public function cancelGroupRequest(bool $send_mail = true): void { - if (!$this->SQL->requestExists($this->getOwner()->uid)) { + if (!$this->SQL->requestExists($this->getOwner()->uid, "admin")) { return; } - $this->SQL->removeRequest($this->getOwner()->uid); + $this->SQL->removeRequest($this->getOwner()->uid, "admin"); if ($send_mail) { $this->MAILER->sendMail("admin", "group_request_cancelled", [ "uid" => $this->getOwner()->uid, diff --git a/resources/lib/UnitySQL.php b/resources/lib/UnitySQL.php index 5d4c7803..f71032fd 100644 --- a/resources/lib/UnitySQL.php +++ b/resources/lib/UnitySQL.php @@ -12,7 +12,6 @@ class UnitySQL private const string TABLE_AUDIT_LOG = "audit_log"; private const string TABLE_ACCOUNT_DELETION_REQUESTS = "account_deletion_requests"; // FIXME this string should be changed to something more intuitive, requires production change - public const string REQUEST_BECOME_PI = "admin"; private $conn; @@ -34,7 +33,7 @@ public function getConn(): PDO // // requests table methods // - public function addRequest(string $requestor, string $dest = self::REQUEST_BECOME_PI): void + public function addRequest(string $requestor, string $dest): void { if ($this->requestExists($requestor, $dest)) { return; @@ -48,7 +47,7 @@ public function addRequest(string $requestor, string $dest = self::REQUEST_BECOM $stmt->execute(); } - public function removeRequest($requestor, string $dest = self::REQUEST_BECOME_PI): void + public function removeRequest($requestor, string $dest): void { if (!$this->requestExists($requestor, $dest)) { return; @@ -63,7 +62,7 @@ public function removeRequest($requestor, string $dest = self::REQUEST_BECOME_PI $stmt->execute(); } - public function removeRequests(string $dest = self::REQUEST_BECOME_PI): void + public function removeRequests(string $dest): void { $stmt = $this->conn->prepare( "DELETE FROM " . self::TABLE_REQS . " WHERE request_for=:request_for", @@ -91,7 +90,7 @@ public function getRequest(string $user, string $dest): array return $result[0]; } - public function requestExists(string $requestor, string $dest = self::REQUEST_BECOME_PI): bool + public function requestExists(string $requestor, string $dest): bool { try { $this->getRequest($requestor, $dest); @@ -109,7 +108,7 @@ public function getAllRequests(): array return $stmt->fetchAll(); } - public function getRequests(string $dest = self::REQUEST_BECOME_PI): array + public function getRequests(string $dest): array { $stmt = $this->conn->prepare( "SELECT * FROM " . self::TABLE_REQS . " WHERE request_for=:request_for", diff --git a/test/functional/PIBecomeApproveTest.php b/test/functional/PIBecomeApproveTest.php index 8ac7b019..551ee876 100644 --- a/test/functional/PIBecomeApproveTest.php +++ b/test/functional/PIBecomeApproveTest.php @@ -9,10 +9,7 @@ class PIBecomeApproveTest extends TestCase private function assertRequestedPIGroup(bool $expected) { global $USER, $SQL; - $this->assertEquals( - $expected, - $SQL->requestExists($USER->uid, UnitySQL::REQUEST_BECOME_PI), - ); + $this->assertEquals($expected, $SQL->requestExists($USER->uid, "admin")); } private function requestGroupCreation() diff --git a/test/functional/PIMemberRequestTest.php b/test/functional/PIMemberRequestTest.php index cea66ae7..b7ca5553 100644 --- a/test/functional/PIMemberRequestTest.php +++ b/test/functional/PIMemberRequestTest.php @@ -36,7 +36,7 @@ public function testRequestMembership() switchUser(...getUserNotPiNotRequestedBecomePi()); $uid = $USER->uid; $this->assertFalse($USER->isPI()); - $this->assertFalse($SQL->requestExists($uid, UnitySQL::REQUEST_BECOME_PI)); + $this->assertFalse($SQL->requestExists($uid, "admin")); $this->assertFalse($pi_group->memberExists($USER)); try { $this->requestMembership($gid); diff --git a/test/functional/PiBecomeRequestTest.php b/test/functional/PiBecomeRequestTest.php index 5714a6b9..4a87d54c 100644 --- a/test/functional/PiBecomeRequestTest.php +++ b/test/functional/PiBecomeRequestTest.php @@ -9,9 +9,9 @@ private function assertNumberPiBecomeRequests(int $x) { global $USER, $SQL; if ($x == 0) { - $this->assertFalse($SQL->requestExists($USER->uid)); + $this->assertFalse($SQL->requestExists($USER->uid, "admin")); } elseif ($x > 0) { - $this->assertTrue($SQL->requestExists($USER->uid)); + $this->assertTrue($SQL->requestExists($USER->uid, "admin")); } else { throw new RuntimeException("x must not be negative"); } @@ -62,8 +62,8 @@ public function testRequestBecomePi() ]); $this->assertNumberPiBecomeRequests(1); } finally { - if ($SQL->requestExists($USER, UnitySQL::REQUEST_BECOME_PI)) { - $SQL->removeRequest($USER->uid, UnitySQL::REQUEST_BECOME_PI); + if ($SQL->requestExists($USER, "admin")) { + $SQL->removeRequest($USER->uid, "admin"); } } } @@ -83,8 +83,8 @@ public function testRequestBecomePiUserRequestedAccountDeletion() ]); $this->assertNumberPiBecomeRequests(0); } finally { - if ($SQL->requestExists($USER, UnitySQL::REQUEST_BECOME_PI)) { - $SQL->removeRequest($USER->uid, UnitySQL::REQUEST_BECOME_PI); + if ($SQL->requestExists($USER, "admin")) { + $SQL->removeRequest($USER->uid, "admin"); } } } diff --git a/webroot/admin/pi-mgmt.php b/webroot/admin/pi-mgmt.php index 3fdd28fc..a547532d 100644 --- a/webroot/admin/pi-mgmt.php +++ b/webroot/admin/pi-mgmt.php @@ -62,7 +62,7 @@ getRequests(); + $requests = $SQL->getRequests("admin"); foreach ($requests as $request) { $uid = $request["uid"]; diff --git a/webroot/panel/account.php b/webroot/panel/account.php index 7167a122..d7be2fa4 100644 --- a/webroot/panel/account.php +++ b/webroot/panel/account.php @@ -70,13 +70,13 @@ if ($USER->isPI()) { UnityHTTPD::badRequest("already a PI"); } - if ($SQL->requestExists($USER->uid)) { + if ($SQL->requestExists($USER->uid, "admin")) { UnityHTTPD::badRequest("already requested to be PI"); } if (!isset($_POST["tos"]) || $_POST["tos"] != "agree") { UnityHTTPD::badRequest("user did not agree to terms of service"); } - $USER->getPIGroup()->requestGroup($SEND_PIMESG_TO_ADMINS); + $USER->getPIGroup()->requestGroup($SEND_PIMESG_TO_ADMINS, "admin"); break; case "cancel_pi_request": $USER->getPIGroup()->cancelGroupRequest(); @@ -165,7 +165,7 @@ "; } else { - if ($SQL->requestExists($USER->uid)) { + if ($SQL->requestExists($USER->uid, "admin")) { $onclick = "return confirm(\"Are you sure you want to cancel this request?\")"; echo ""; echo " From 3fc3eaabad6cbf7e9108cf34f31fdfa7f8f22c57 Mon Sep 17 00:00:00 2001 From: Jack Meddick Date: Fri, 5 Dec 2025 15:44:19 -0500 Subject: [PATCH 2/4] git checks caught a straggler use of the constant --- resources/lib/UnityGroup.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/resources/lib/UnityGroup.php b/resources/lib/UnityGroup.php index b2356552..7e018ef8 100644 --- a/resources/lib/UnityGroup.php +++ b/resources/lib/UnityGroup.php @@ -118,7 +118,7 @@ public function approveGroup(?UnityUser $operator = null, bool $send_mail = true */ public function denyGroup(?UnityUser $operator = null, bool $send_mail = true): void { - $request = $this->SQL->getRequest($this->getOwner()->uid, UnitySQL::REQUEST_BECOME_PI); + $request = $this->SQL->getRequest($this->getOwner()->uid, "admin"); $this->SQL->removeRequest($this->getOwner()->uid); if ($this->exists()) { return; From 03772ee97a34b60530253a17fc7f498d165b25a5 Mon Sep 17 00:00:00 2001 From: Jack Meddick Date: Tue, 9 Dec 2025 13:46:35 -0500 Subject: [PATCH 3/4] added back the constant but now it is explicitly called whenever it is used in a function --- resources/lib/UnityGroup.php | 12 ++++++------ resources/lib/UnitySQL.php | 1 + test/functional/PIBecomeApproveTest.php | 5 ++++- test/functional/PIMemberRequestTest.php | 2 +- test/functional/PiBecomeRequestTest.php | 12 ++++++------ webroot/admin/pi-mgmt.php | 2 +- webroot/panel/account.php | 6 +++--- 7 files changed, 22 insertions(+), 18 deletions(-) diff --git a/resources/lib/UnityGroup.php b/resources/lib/UnityGroup.php index 7e018ef8..688d7106 100644 --- a/resources/lib/UnityGroup.php +++ b/resources/lib/UnityGroup.php @@ -76,7 +76,7 @@ public function requestGroup(bool $send_mail_to_admins, bool $send_mail = true): "name" => $this->getOwner()->getFullName(), "email" => $this->getOwner()->getMail(), ]; - $this->SQL->addRequest($this->getOwner()->uid, "admin"); + $this->SQL->addRequest($this->getOwner()->uid, UnitySQL::REQUEST_BECOME_PI); if ($send_mail) { $this->MAILER->sendMail($this->getOwner()->getMail(), "group_request"); $this->WEBHOOK->sendWebhook("group_request_admin", $context); @@ -93,13 +93,13 @@ public function requestGroup(bool $send_mail_to_admins, bool $send_mail = true): public function approveGroup(?UnityUser $operator = null, bool $send_mail = true): void { $uid = $this->getOwner()->uid; - $request = $this->SQL->getRequest($uid, "admin"); + $request = $this->SQL->getRequest($uid, UnitySQL::REQUEST_BECOME_PI); if ($this->exists()) { return; } \ensure($this->getOwner()->exists()); $this->init(); - $this->SQL->removeRequest($this->getOwner()->uid, "admin"); + $this->SQL->removeRequest($this->getOwner()->uid, UnitySQL::REQUEST_BECOME_PI); $operator = is_null($operator) ? $this->getOwner()->uid : $operator->uid; $this->SQL->addLog( $operator, @@ -118,7 +118,7 @@ public function approveGroup(?UnityUser $operator = null, bool $send_mail = true */ public function denyGroup(?UnityUser $operator = null, bool $send_mail = true): void { - $request = $this->SQL->getRequest($this->getOwner()->uid, "admin"); + $request = $this->SQL->getRequest($this->getOwner()->uid, UnitySQL::REQUEST_BECOME_PI); $this->SQL->removeRequest($this->getOwner()->uid); if ($this->exists()) { return; @@ -137,10 +137,10 @@ public function denyGroup(?UnityUser $operator = null, bool $send_mail = true): public function cancelGroupRequest(bool $send_mail = true): void { - if (!$this->SQL->requestExists($this->getOwner()->uid, "admin")) { + if (!$this->SQL->requestExists($this->getOwner()->uid, UnitySQL::REQUEST_BECOME_PI)) { return; } - $this->SQL->removeRequest($this->getOwner()->uid, "admin"); + $this->SQL->removeRequest($this->getOwner()->uid, UnitySQL::REQUEST_BECOME_PI); if ($send_mail) { $this->MAILER->sendMail("admin", "group_request_cancelled", [ "uid" => $this->getOwner()->uid, diff --git a/resources/lib/UnitySQL.php b/resources/lib/UnitySQL.php index f71032fd..ff9fb3aa 100644 --- a/resources/lib/UnitySQL.php +++ b/resources/lib/UnitySQL.php @@ -12,6 +12,7 @@ class UnitySQL private const string TABLE_AUDIT_LOG = "audit_log"; private const string TABLE_ACCOUNT_DELETION_REQUESTS = "account_deletion_requests"; // FIXME this string should be changed to something more intuitive, requires production change + private const string REQUEST_BECOME_PI = "admin"; private $conn; diff --git a/test/functional/PIBecomeApproveTest.php b/test/functional/PIBecomeApproveTest.php index 551ee876..8ac7b019 100644 --- a/test/functional/PIBecomeApproveTest.php +++ b/test/functional/PIBecomeApproveTest.php @@ -9,7 +9,10 @@ class PIBecomeApproveTest extends TestCase private function assertRequestedPIGroup(bool $expected) { global $USER, $SQL; - $this->assertEquals($expected, $SQL->requestExists($USER->uid, "admin")); + $this->assertEquals( + $expected, + $SQL->requestExists($USER->uid, UnitySQL::REQUEST_BECOME_PI), + ); } private function requestGroupCreation() diff --git a/test/functional/PIMemberRequestTest.php b/test/functional/PIMemberRequestTest.php index b7ca5553..cea66ae7 100644 --- a/test/functional/PIMemberRequestTest.php +++ b/test/functional/PIMemberRequestTest.php @@ -36,7 +36,7 @@ public function testRequestMembership() switchUser(...getUserNotPiNotRequestedBecomePi()); $uid = $USER->uid; $this->assertFalse($USER->isPI()); - $this->assertFalse($SQL->requestExists($uid, "admin")); + $this->assertFalse($SQL->requestExists($uid, UnitySQL::REQUEST_BECOME_PI)); $this->assertFalse($pi_group->memberExists($USER)); try { $this->requestMembership($gid); diff --git a/test/functional/PiBecomeRequestTest.php b/test/functional/PiBecomeRequestTest.php index 4a87d54c..0ac80e3f 100644 --- a/test/functional/PiBecomeRequestTest.php +++ b/test/functional/PiBecomeRequestTest.php @@ -9,9 +9,9 @@ private function assertNumberPiBecomeRequests(int $x) { global $USER, $SQL; if ($x == 0) { - $this->assertFalse($SQL->requestExists($USER->uid, "admin")); + $this->assertFalse($SQL->requestExists($USER->uid, UnitySQL::REQUEST_BECOME_PI)); } elseif ($x > 0) { - $this->assertTrue($SQL->requestExists($USER->uid, "admin")); + $this->assertTrue($SQL->requestExists($USER->uid, UnitySQL::REQUEST_BECOME_PI)); } else { throw new RuntimeException("x must not be negative"); } @@ -62,8 +62,8 @@ public function testRequestBecomePi() ]); $this->assertNumberPiBecomeRequests(1); } finally { - if ($SQL->requestExists($USER, "admin")) { - $SQL->removeRequest($USER->uid, "admin"); + if ($SQL->requestExists($USER, UnitySQL::REQUEST_BECOME_PI)) { + $SQL->removeRequest($USER->uid, UnitySQL::REQUEST_BECOME_PI); } } } @@ -83,8 +83,8 @@ public function testRequestBecomePiUserRequestedAccountDeletion() ]); $this->assertNumberPiBecomeRequests(0); } finally { - if ($SQL->requestExists($USER, "admin")) { - $SQL->removeRequest($USER->uid, "admin"); + if ($SQL->requestExists($USER, UnitySQL::REQUEST_BECOME_PI)) { + $SQL->removeRequest($USER->uid, UnitySQL::REQUEST_BECOME_PI); } } } diff --git a/webroot/admin/pi-mgmt.php b/webroot/admin/pi-mgmt.php index a547532d..416e2309 100644 --- a/webroot/admin/pi-mgmt.php +++ b/webroot/admin/pi-mgmt.php @@ -62,7 +62,7 @@ getRequests("admin"); + $requests = $SQL->getRequests(UnitySQL::REQUEST_BECOME_PI); foreach ($requests as $request) { $uid = $request["uid"]; diff --git a/webroot/panel/account.php b/webroot/panel/account.php index d7be2fa4..c6ae4a28 100644 --- a/webroot/panel/account.php +++ b/webroot/panel/account.php @@ -70,13 +70,13 @@ if ($USER->isPI()) { UnityHTTPD::badRequest("already a PI"); } - if ($SQL->requestExists($USER->uid, "admin")) { + if ($SQL->requestExists($USER->uid, UnitySQL::REQUEST_BECOME_PI)) { UnityHTTPD::badRequest("already requested to be PI"); } if (!isset($_POST["tos"]) || $_POST["tos"] != "agree") { UnityHTTPD::badRequest("user did not agree to terms of service"); } - $USER->getPIGroup()->requestGroup($SEND_PIMESG_TO_ADMINS, "admin"); + $USER->getPIGroup()->requestGroup($SEND_PIMESG_TO_ADMINS, UnitySQL::REQUEST_BECOME_PI); break; case "cancel_pi_request": $USER->getPIGroup()->cancelGroupRequest(); @@ -165,7 +165,7 @@ "; } else { - if ($SQL->requestExists($USER->uid, "admin")) { + if ($SQL->requestExists($USER->uid, UnitySQL::REQUEST_BECOME_PI)) { $onclick = "return confirm(\"Are you sure you want to cancel this request?\")"; echo ""; echo " From 91c2204803b2e58f882b983df7321b807bf9d4b6 Mon Sep 17 00:00:00 2001 From: Jack Meddick Date: Tue, 9 Dec 2025 14:31:41 -0500 Subject: [PATCH 4/4] made the constant accessible --- resources/lib/UnitySQL.php | 2 +- webroot/admin/pi-mgmt.php | 1 + webroot/panel/account.php | 1 + 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/resources/lib/UnitySQL.php b/resources/lib/UnitySQL.php index ff9fb3aa..2d9d8e5b 100644 --- a/resources/lib/UnitySQL.php +++ b/resources/lib/UnitySQL.php @@ -12,7 +12,7 @@ class UnitySQL private const string TABLE_AUDIT_LOG = "audit_log"; private const string TABLE_ACCOUNT_DELETION_REQUESTS = "account_deletion_requests"; // FIXME this string should be changed to something more intuitive, requires production change - private const string REQUEST_BECOME_PI = "admin"; + public const string REQUEST_BECOME_PI = "admin"; private $conn; diff --git a/webroot/admin/pi-mgmt.php b/webroot/admin/pi-mgmt.php index 416e2309..7b161a4b 100644 --- a/webroot/admin/pi-mgmt.php +++ b/webroot/admin/pi-mgmt.php @@ -5,6 +5,7 @@ use UnityWebPortal\lib\UnityUser; use UnityWebPortal\lib\UnityGroup; use UnityWebPortal\lib\UnityHTTPD; +use UnityWebPortal\lib\UnitySQL; if (!$USER->isAdmin()) { UnityHTTPD::forbidden("not an admin"); diff --git a/webroot/panel/account.php b/webroot/panel/account.php index c6ae4a28..eaff0d55 100644 --- a/webroot/panel/account.php +++ b/webroot/panel/account.php @@ -5,6 +5,7 @@ use UnityWebPortal\lib\UnityHTTPD; use UnityWebPortal\lib\exceptions\EncodingUnknownException; use UnityWebPortal\lib\exceptions\EncodingConversionException; +use UnityWebPortal\lib\UnitySQL; $hasGroups = count($USER->getPIGroupGIDs()) > 0;