From 29d74a3689f1a2645dfb589ee203393e6ceef15f Mon Sep 17 00:00:00 2001 From: Simon Leary Date: Sun, 6 Jul 2025 20:45:39 -0400 Subject: [PATCH 1/6] cleanup2 --- resources/lib/UnityGroup.php | 91 ++++++++----------- resources/lib/UnityUser.php | 4 +- test/functional/NewUserTest.php | 14 +-- test/functional/PIMemberRequestTest.php | 2 +- test/functional/PiMemberApproveTest.php | 27 +++--- test/functional/PiMemberDenyTest.php | 8 +- webroot/admin/ajax/get_group_members.php | 14 +-- webroot/admin/pi-mgmt.php | 14 +-- webroot/admin/user-mgmt.php | 2 +- webroot/js/tables.js | 4 +- webroot/panel/ajax/get_group_members.php | 4 +- webroot/panel/groups.php | 24 ++--- webroot/panel/modal/pi_search.php | 2 +- webroot/panel/new_account.php | 56 ++++++------ workers/group_user_request_owner_reminder.php | 6 +- 15 files changed, 132 insertions(+), 140 deletions(-) diff --git a/resources/lib/UnityGroup.php b/resources/lib/UnityGroup.php index 9fac0191..ba6be5d4 100644 --- a/resources/lib/UnityGroup.php +++ b/resources/lib/UnityGroup.php @@ -11,7 +11,7 @@ class UnityGroup { public const PI_PREFIX = "pi_"; - private $pi_uid; + public $gid; // Services private $LDAP; @@ -23,13 +23,13 @@ class UnityGroup /** * Constructor for the object * - * @param string $pi_uid PI UID in the format + * @param string $gid PI UID in the format * @param LDAP $LDAP LDAP Connection * @param SQL $SQL SQL Connection */ - public function __construct($pi_uid, $LDAP, $SQL, $MAILER, $REDIS, $WEBHOOK) + public function __construct($gid, $LDAP, $SQL, $MAILER, $REDIS, $WEBHOOK) { - $this->pi_uid = $pi_uid; + $this->gid = $gid; $this->LDAP = $LDAP; $this->SQL = $SQL; @@ -44,22 +44,12 @@ public function equals($other_group) throw new Exception("Unable to check equality because the parameter is not a " . self::class . " object"); } - return $this->getPIUID() == $other_group->getPIUID(); + return $this->gid == $other_group->gid; } public function __toString() { - return $this->getPIUID(); - } - - /** - * Returns this group's PI UID - * - * @return string PI UID of the group - */ - public function getPIUID() - { - return $this->pi_uid; + return $this->gid; } /** @@ -235,7 +225,7 @@ public function cancelGroupJoinRequest($user, $send_mail = true) return; } - $this->SQL->removeRequest($user->getUID(), $this->pi_uid); + $this->SQL->removeRequest($user->getUID(), $this->gid); if ($send_mail) { // send email to requestor @@ -254,7 +244,7 @@ public function cancelGroupJoinRequest($user, $send_mail = true) // { // // remove any pending requests // // this will silently fail if the request doesn't exist (which is what we want) - // $this->SQL->removeRequests($this->pi_uid); + // $this->SQL->removeRequests($this->gid); // // we don't need to do anything extra if the group is already deleted // if (!$this->exists()) { @@ -268,9 +258,9 @@ public function cancelGroupJoinRequest($user, $send_mail = true) // $ldapPiGroupEntry = $this->getLDAPPiGroup(); // if ($ldapPiGroupEntry->exists()) { // $ldapPiGroupEntry->delete(); - // $this->REDIS->removeCacheArray("sorted_groups", "", $this->getPIUID()); + // $this->REDIS->removeCacheArray("sorted_groups", "", $this->gid); // foreach ($users as $user) { - // $this->REDIS->removeCacheArray($user->getUID(), "groups", $this->getPIUID()); + // $this->REDIS->removeCacheArray($user->getUID(), "groups", $this->gid); // } // } @@ -280,7 +270,7 @@ public function cancelGroupJoinRequest($user, $send_mail = true) // $this->MAILER->sendMail( // $user->getMail(), // "group_disband", - // array("group_name" => $this->pi_uid) + // array("group_name" => $this->gid) // ); // } // } @@ -292,8 +282,7 @@ public function cancelGroupJoinRequest($user, $send_mail = true) public function approveUser($new_user, $send_mail = true) { $uid = $new_user->getUID(); - $gid = $this->getPIUID(); - $request = $this->SQL->getRequest($uid, $gid); + $request = $this->SQL->getRequest($uid, $this->gid); // check if user exists if (!$new_user->exists()) { $new_user->init( @@ -307,7 +296,7 @@ public function approveUser($new_user, $send_mail = true) // add user to the LDAP object $this->addUserToGroup($new_user); - $this->SQL->removeRequest($new_user->getUID(), $this->pi_uid); + $this->SQL->removeRequest($new_user->getUID(), $this->gid); // send email to the requestor if ($send_mail) { @@ -315,14 +304,14 @@ public function approveUser($new_user, $send_mail = true) $this->MAILER->sendMail( $new_user->getMail(), "group_user_added", - array("group" => $this->pi_uid) + array("group" => $this->gid) ); // send email to the PI $this->MAILER->sendMail( $this->getOwner()->getMail(), "group_user_added_owner", array( - "group" => $this->pi_uid, + "group" => $this->gid, "user" => $new_user->getUID(), "name" => $request["firstname"] . " " . $request["lastname"], "email" => $request["email"], @@ -335,18 +324,17 @@ public function approveUser($new_user, $send_mail = true) public function denyUser($new_user, $send_mail = true) { $uid = $new_user->getUID(); - $gid = $this->getPIUID(); - $request = $this->SQL->getRequest($uid, $gid); + $request = $this->SQL->getRequest($uid, $this->gid); // remove request, this will fail silently if the request doesn't exist - $this->SQL->removeRequest($new_user->getUID(), $this->pi_uid); + $this->SQL->removeRequest($new_user->getUID(), $this->gid); if ($send_mail) { // send email to the user $this->MAILER->sendMail( $new_user->getMail(), "group_user_denied", - array("group" => $this->pi_uid) + array("group" => $this->gid) ); // send email to the PI @@ -354,7 +342,7 @@ public function denyUser($new_user, $send_mail = true) $this->getOwner()->getMail(), "group_user_denied_owner", array( - "group" => $this->pi_uid, + "group" => $this->gid, "user" => $new_user->getUID(), "name" => $new_user->getFullName(), "email" => $new_user->getMail(), @@ -382,7 +370,7 @@ public function removeUser($new_user, $send_mail = true) $this->MAILER->sendMail( $new_user->getMail(), "group_user_removed", - array("group" => $this->pi_uid) + array("group" => $this->gid) ); // send email to the PI @@ -390,7 +378,7 @@ public function removeUser($new_user, $send_mail = true) $this->getOwner()->getMail(), "group_user_removed_owner", array( - "group" => $this->pi_uid, + "group" => $this->gid, "user" => $new_user->getUID(), "name" => $new_user->getFullName(), "email" => $new_user->getMail(), @@ -424,7 +412,7 @@ public function newUserRequest($new_user, $firstname, $lastname, $email, $org, $ $this->MAILER->sendMail( $email, "group_user_request", - array("group" => $this->pi_uid) + array("group" => $this->gid) ); // send email to PI @@ -432,7 +420,7 @@ public function newUserRequest($new_user, $firstname, $lastname, $email, $org, $ $this->getOwner()->getMail(), "group_user_request_owner", array( - "group" => $this->pi_uid, + "group" => $this->gid, "user" => $new_user->getUID(), "name" => "$firstname $lastname", "email" => $email, @@ -444,7 +432,7 @@ public function newUserRequest($new_user, $firstname, $lastname, $email, $org, $ public function getRequests() { - $requests = $this->SQL->getRequests($this->pi_uid); + $requests = $this->SQL->getRequests($this->gid); $out = array(); foreach ($requests as $request) { @@ -494,7 +482,7 @@ public function getGroupMembers($ignorecache = false) public function getGroupMemberUIDs($ignorecache = false) { if (!$ignorecache) { - $cached_val = $this->REDIS->getCache($this->getPIUID(), "members"); + $cached_val = $this->REDIS->getCache($this->gid, "members"); if (!is_null($cached_val)) { $members = $cached_val; } @@ -507,7 +495,7 @@ public function getGroupMemberUIDs($ignorecache = false) } if (!$ignorecache && $updatecache) { sort($members); - $this->REDIS->setCache($this->getPIUID(), "members", $members); + $this->REDIS->setCache($this->gid, "members", $members); } return $members; } @@ -547,7 +535,7 @@ private function init() $ldapPiGroupEntry->write(); } - $this->REDIS->appendCacheArray("sorted_groups", "", $this->getPIUID()); + $this->REDIS->appendCacheArray("sorted_groups", "", $this->gid); // TODO if we ever make this project based, we need to update the cache here with the memberuid } @@ -558,8 +546,8 @@ private function addUserToGroup($new_user) $pi_group = $this->getLDAPPiGroup(); $pi_group->appendAttribute("memberuid", $new_user->getUID()); $pi_group->write(); - $this->REDIS->appendCacheArray($this->getPIUID(), "members", $new_user->getUID()); - $this->REDIS->appendCacheArray($new_user->getUID(), "groups", $this->getPIUID()); + $this->REDIS->appendCacheArray($this->gid, "members", $new_user->getUID()); + $this->REDIS->appendCacheArray($new_user->getUID(), "groups", $this->gid); } private function removeUserFromGroup($old_user) @@ -568,8 +556,8 @@ private function removeUserFromGroup($old_user) $pi_group = $this->getLDAPPiGroup(); $pi_group->removeAttributeEntryByValue("memberuid", $old_user->getUID()); $pi_group->write(); - $this->REDIS->removeCacheArray($this->getPIUID(), "members", $old_user->getUID()); - $this->REDIS->removeCacheArray($old_user->getUID(), "groups", $this->getPIUID()); + $this->REDIS->removeCacheArray($this->gid, "members", $old_user->getUID()); + $this->REDIS->removeCacheArray($old_user->getUID(), "groups", $this->gid); } public function userExists($user) @@ -579,7 +567,7 @@ public function userExists($user) private function addRequest($uid, $firstname, $lastname, $email, $org) { - $this->SQL->addRequest($uid, $firstname, $lastname, $email, $org, $this->pi_uid); + $this->SQL->addRequest($uid, $firstname, $lastname, $email, $org, $this->gid); } // @@ -589,7 +577,7 @@ private function addRequest($uid, $firstname, $lastname, $email, $org) public function getOwner() { return new UnityUser( - self::getUIDfromPIUID($this->pi_uid), + self::GID2OwnerUID($this->gid), $this->LDAP, $this->SQL, $this->MAILER, @@ -600,20 +588,19 @@ public function getOwner() public function getLDAPPiGroup() { - return $this->LDAP->getPIGroupEntry($this->pi_uid); + return $this->LDAP->getPIGroupEntry($this->gid); } - public static function getPIUIDfromUID($uid) + public static function ownerUID2GID($uid) { return self::PI_PREFIX . $uid; } - public static function getUIDfromPIUID($pi_uid) + public static function GID2OwnerUID($gid) { - if (substr($pi_uid, 0, strlen(self::PI_PREFIX)) == self::PI_PREFIX) { - return substr($pi_uid, strlen(self::PI_PREFIX)); - } else { - throw new Exception("PI netid doesn't have the correct prefix."); + if (substr($gid, 0, strlen(self::PI_PREFIX)) != self::PI_PREFIX) { + throw new Exception("PI group GID doesn't have the correct prefix."); } + return substr($gid, strlen(self::PI_PREFIX)); } } diff --git a/resources/lib/UnityUser.php b/resources/lib/UnityUser.php index 983e2a5f..cedc9061 100644 --- a/resources/lib/UnityUser.php +++ b/resources/lib/UnityUser.php @@ -583,7 +583,7 @@ public function isPI() public function getPIGroup() { return new UnityGroup( - UnityGroup::getPIUIDfromUID($this->uid), + UnityGroup::ownerUID2GID($this->uid), $this->LDAP, $this->SQL, $this->MAILER, @@ -639,7 +639,7 @@ public function getGroups($ignorecache = false) foreach ($all_pi_groups as $pi_group) { if (in_array($this->getUID(), $pi_group->getGroupMemberUIDs())) { array_push($out, $pi_group); - array_push($cache_arr, $pi_group->getPIUID()); + array_push($cache_arr, $pi_group->gid); } } diff --git a/test/functional/NewUserTest.php b/test/functional/NewUserTest.php index b0f0fb23..43ece84b 100644 --- a/test/functional/NewUserTest.php +++ b/test/functional/NewUserTest.php @@ -114,7 +114,7 @@ public function testCreateUserByJoinGoup() global $USER, $SSO, $LDAP, $SQL, $MAILER, $REDIS, $WEBHOOK; switchUser(...getUserIsPIHasNoMembersNoMemberRequests()); $pi_group = $USER->getPIGroup(); - $gid = $pi_group->getPIUID(); + $gid = $pi_group->gid; switchUser(...getNonExistentUser()); $this->assertTrue(!$USER->exists()); $newOrg = new UnityOrg($SSO["org"], $LDAP, $SQL, $MAILER, $REDIS, $WEBHOOK); @@ -123,12 +123,12 @@ public function testCreateUserByJoinGoup() $this->assertTrue(!$pi_group->userExists($USER)); $this->assertRequestedMembership(false, $gid); try { - $this->requestGroupMembership($pi_group->getPIUID()); + $this->requestGroupMembership($pi_group->gid); $this->assertRequestedMembership(true, $gid); // $second_request_failed = false; // try { - $this->requestGroupMembership($pi_group->getPIUID()); + $this->requestGroupMembership($pi_group->gid); // } catch(Exception) { // $second_request_failed = true; // } @@ -138,7 +138,7 @@ public function testCreateUserByJoinGoup() $this->cancelAllRequests(); $this->assertRequestedMembership(false, $gid); - $this->requestGroupMembership($pi_group->getPIUID()); + $this->requestGroupMembership($pi_group->gid); $this->assertTrue($pi_group->requestExists($USER)); $this->assertRequestedMembership(true, $gid); @@ -153,7 +153,7 @@ public function testCreateUserByJoinGoup() // $third_request_failed = false; // try { - $this->requestGroupMembership($pi_group->getPIUID()); + $this->requestGroupMembership($pi_group->gid); // } catch(Exception) { // $third_request_failed = true; // } @@ -182,7 +182,7 @@ public function testCreateUserByCreateGroup() // $second_request_failed = false; // try { - $this->requestGroupCreation(); + $this->requestGroupCreation(); // } catch(Exception) { // $second_request_failed = true; // } @@ -205,7 +205,7 @@ public function testCreateUserByCreateGroup() // $third_request_failed = false; // try { - $this->requestGroupCreation(); + $this->requestGroupCreation(); // } catch(Exception) { // $third_request_failed = true; // } diff --git a/test/functional/PIMemberRequestTest.php b/test/functional/PIMemberRequestTest.php index ec4cff71..37e29082 100644 --- a/test/functional/PIMemberRequestTest.php +++ b/test/functional/PIMemberRequestTest.php @@ -28,7 +28,7 @@ public function testRequestMembership() switchUser(...getUserIsPIHasNoMembersNoMemberRequests()); $pi = $USER; $pi_group = $USER->getPIGroup(); - $gid = $pi_group->getPIUID(); + $gid = $pi_group->gid; $this->assertTrue($USER->isPI()); $this->assertTrue($pi_group->exists()); $this->assertTrue(arraysAreEqualUnOrdered([$pi], $pi_group->getGroupMembers())); diff --git a/test/functional/PiMemberApproveTest.php b/test/functional/PiMemberApproveTest.php index 94ee6af8..b283a5fc 100644 --- a/test/functional/PiMemberApproveTest.php +++ b/test/functional/PiMemberApproveTest.php @@ -6,18 +6,19 @@ use UnityWebPortal\lib\UnityGroup; use UnityWebPortal\lib\UnitySSO; -class PiMemberApproveTest extends TestCase { - static $userWithRequestSwitchArgs; - static $userWithoutRequestSwitchArgs; - static $piSwitchArgs; - static $pi; - static $userWithRequestUID; - static $userWithoutRequestUID; - static $piUID; - static $userWithRequest; - static $userWithoutRequest; - static $piGroup; - static $piGroupGID; +class PiMemberApproveTest extends TestCase +{ + static $userWithRequestSwitchArgs; + static $userWithoutRequestSwitchArgs; + static $piSwitchArgs; + static $pi; + static $userWithRequestUID; + static $userWithoutRequestUID; + static $piUID; + static $userWithRequest; + static $userWithoutRequest; + static $piGroup; + static $piGroupGID; private function approveUser(string $uid) { @@ -62,7 +63,7 @@ public function testApproveRequest() $this->assertEmpty($piGroup->getRequests()); try { switchUser(...$userSwitchArgs); - $this->requestJoinPI($piGroup->getPIUID()); + $this->requestJoinPI($piGroup->gid); $this->assertFalse($piGroup->userExists($user)); switchUser(...$piSwitchArgs); diff --git a/test/functional/PiMemberDenyTest.php b/test/functional/PiMemberDenyTest.php index 4278a1d5..454b9bd8 100644 --- a/test/functional/PiMemberDenyTest.php +++ b/test/functional/PiMemberDenyTest.php @@ -4,10 +4,12 @@ use PHPUnit\Framework\Attributes\DataProvider; use UnityWebPortal\lib\UnityUser; -class PiMemberDenyTest extends TestCase { +class PiMemberDenyTest extends TestCase +{ static $requestUid; - public static function setUpBeforeClass(): void{ + public static function setUpBeforeClass(): void + { global $USER; switchUser(...getNormalUser()); self::$requestUid = $USER->getUID(); @@ -56,7 +58,7 @@ public function testDenyRequest() ); $this->assertFalse($piGroup->userExists($requestedUser)); } finally { - $SQL->removeRequest(self::$requestUid, $piGroup->getPIUID()); + $SQL->removeRequest(self::$requestUid, $piGroup->gid); } } } diff --git a/webroot/admin/ajax/get_group_members.php b/webroot/admin/ajax/get_group_members.php index e3e56759..be2f2c31 100644 --- a/webroot/admin/ajax/get_group_members.php +++ b/webroot/admin/ajax/get_group_members.php @@ -9,11 +9,11 @@ UnitySite::forbidden("not an admin"); } -if (!isset($_GET["pi_uid"])) { +if (!isset($_GET["gid"])) { UnitySite::badRequest("PI UID not set"); } -$group = new UnityGroup($_GET["pi_uid"], $LDAP, $SQL, $MAILER, $REDIS, $WEBHOOK); +$group = new UnityGroup($_GET["gid"], $LDAP, $SQL, $MAILER, $REDIS, $WEBHOOK); $members = $group->getGroupMembers(); $requests = $group->getRequests(); @@ -35,11 +35,11 @@ echo "" . $member->getMail() . ""; echo ""; echo - "
+ " - +
"; echo ""; @@ -60,11 +60,11 @@ echo "" . $email . ""; echo ""; echo - "
- +
"; echo ""; diff --git a/webroot/admin/pi-mgmt.php b/webroot/admin/pi-mgmt.php index c546704b..2826bc3c 100644 --- a/webroot/admin/pi-mgmt.php +++ b/webroot/admin/pi-mgmt.php @@ -83,7 +83,7 @@ echo "" . date("jS F, Y", strtotime($request['timestamp'])) . ""; echo ""; echo - "
+ " Actions -getAllPIGroups($SQL, $MAILER, $REDIS, $WEBHOOK); usort($accounts, function ($a, $b) { - return strcmp($a->getPIUID(), $b->getPIUID()); + return strcmp($a->gid, $b->gid); }); foreach ($accounts as $pi_group) { @@ -121,8 +121,8 @@ echo ""; echo "" . $pi_user->getFirstname() . - " " . $pi_user->getLastname() . ""; - echo "" . $pi_group->getPIUID() . ""; + " " . $pi_user->getLastname() . ""; + echo "" . $pi_group->gid . ""; echo "" . $pi_user->getMail() . ""; echo ""; } @@ -132,7 +132,7 @@ "; $cur_user_groups = $user->getGroups(); foreach ($cur_user_groups as $cur_group) { - echo "" . $cur_group->getPIUID() . ""; + echo "" . $cur_group->gid . ""; if ($cur_group !== array_key_last($cur_user_groups)) { echo '
'; } diff --git a/webroot/js/tables.js b/webroot/js/tables.js index ed0de340..a749b43e 100644 --- a/webroot/js/tables.js +++ b/webroot/js/tables.js @@ -13,7 +13,7 @@ $("button.btnExpand").click(function() { var pi_wrapper = $(this).parent(); // parent column (td) var piRow = pi_wrapper.parent(); // parent row (tr) var piTree = piRow.parent(); // parent table (table) - var pi_uid = pi_wrapper.next().html(); + var gid = pi_wrapper.next().html(); if ($(this).hasClass("btnExpanded")) { // already expanded @@ -32,7 +32,7 @@ $("button.btnExpand").click(function() { $("button.btnExpanded").trigger("click"); // not expanded $.ajax({ - url: ajax_url + pi_uid, + url: ajax_url + gid, success: function(result) { console.log(result); piRow.after(result); diff --git a/webroot/panel/ajax/get_group_members.php b/webroot/panel/ajax/get_group_members.php index 222c2e25..3d1e9e69 100644 --- a/webroot/panel/ajax/get_group_members.php +++ b/webroot/panel/ajax/get_group_members.php @@ -5,11 +5,11 @@ use UnityWebPortal\lib\UnityGroup; use UnityWebPortal\lib\UnitySite; -if (!isset($_GET["pi_uid"])) { +if (!isset($_GET["gid"])) { UnitySite::badRequest("PI UID not set"); } -$group = new UnityGroup($_GET["pi_uid"], $LDAP, $SQL, $MAILER, $REDIS, $WEBHOOK); +$group = new UnityGroup($_GET["gid"], $LDAP, $SQL, $MAILER, $REDIS, $WEBHOOK); if (!$group->userExists($USER)) { UnitySite::forbidden("not a group member"); } diff --git a/webroot/panel/groups.php b/webroot/panel/groups.php index ed5b8ba4..cfd88523 100644 --- a/webroot/panel/groups.php +++ b/webroot/panel/groups.php @@ -35,7 +35,7 @@ $sso_user = $SSO["user"]; UnitySite::badRequest( "cannot request due to uid mismatch: " . - "USER='{$USER->getUID()}' SSO[user]='$sso_user'" + "USER='{$USER->getUID()}' SSO[user]='$sso_user'" ); } @@ -83,12 +83,12 @@ $requested_owner = $requested_account->getOwner(); echo ""; echo "" . $requested_owner->getFirstname() . " " . $requested_owner->getLastname() . ""; - echo "" . $requested_account->getPIUID() . ""; + echo "" . $requested_account->gid . ""; echo "" . $requested_owner->getMail() . ""; echo "" . date("jS F, Y", strtotime($request['timestamp'])) . ""; echo ""; echo " - +
"; @@ -112,7 +112,7 @@ if (count($groups) == 0) { echo "You are not a member of any groups. Request to join a PI using the button below, or request your own PI account on the account settings page"; + "/panel/account.php'>account settings page"; } echo ""; @@ -126,16 +126,16 @@ echo ""; echo - ""; - echo ""; + echo ""; echo ""; echo - ""; @@ -161,7 +161,7 @@