From 5916ab7cc541fbabee6322889687cb0b39f6c9d1 Mon Sep 17 00:00:00 2001 From: Simon Leary Date: Fri, 14 Nov 2025 16:39:09 -0500 Subject: [PATCH 1/3] fix redis before append or remove cache array --- resources/lib/UnityGroup.php | 42 +++++++++++++++++++++++++++++------ resources/lib/UnityLDAP.php | 23 +++++++++++++++++++ resources/lib/UnityOrg.php | 17 +++++++++++--- resources/lib/UnityRedis.php | 20 ++++++++++++----- resources/lib/UnityUser.php | 8 ++++++- test/phpunit-bootstrap.php | 16 +++++++++---- workers/update-ldap-cache.php | 30 +++++++------------------ 7 files changed, 113 insertions(+), 43 deletions(-) diff --git a/resources/lib/UnityGroup.php b/resources/lib/UnityGroup.php index a7fac375..b047e243 100644 --- a/resources/lib/UnityGroup.php +++ b/resources/lib/UnityGroup.php @@ -206,10 +206,17 @@ public function cancelGroupJoinRequest(UnityUser $user, bool $send_mail = true): // // now we delete the ldap entry // $this->entry->ensureExists(); // $this->entry->delete(); - // $this->REDIS->removeCacheArray("sorted_groups", "", $this->gid); + // $default_value_getter = [$this->LDAP, "getSortedGroupsForRedis"]; + // $this->REDIS->removeCacheArray("sorted_groups", "", $this->gid, $default_value_getter); // foreach ($users as $user) { - // $this->REDIS->removeCacheArray($user->uid, "groups", $this->gid); + // $this->REDIS->removeCacheArray( + // $user->uid, + // "groups", + // $this->gid, + // fn() => $this->getGroupMemberUIDs(true), + // ); // } + // // FIXME group not removed from user's groups array // // send email to every user of the now deleted PI group // if ($send_mail) { @@ -417,7 +424,8 @@ private function init(): void $this->entry->setAttribute("gidnumber", strval($nextGID)); $this->entry->setAttribute("memberuid", [$owner->uid]); $this->entry->write(); - $this->REDIS->appendCacheArray("sorted_groups", "", $this->gid); + $default_value_getter = [$this->LDAP, "getSortedGroupsForRedis"]; + $this->REDIS->appendCacheArray("sorted_groups", "", $this->gid, $default_value_getter); // TODO if we ever make this project based, // we need to update the cache here with the memberuid } @@ -426,16 +434,36 @@ private function addUserToGroup(UnityUser $new_user): void { $this->entry->appendAttribute("memberuid", $new_user->uid); $this->entry->write(); - $this->REDIS->appendCacheArray($this->gid, "members", $new_user->uid); - $this->REDIS->appendCacheArray($new_user->uid, "groups", $this->gid); + $this->REDIS->appendCacheArray( + $this->gid, + "members", + $new_user->uid, + fn() => $this->getGroupMemberUIDs(true), + ); + $this->REDIS->appendCacheArray( + $new_user->uid, + "groups", + $this->gid, + fn() => $this->LDAP->getPIGroupGIDsWithMemberUID($new_user->uid), + ); } private function removeUserFromGroup(UnityUser $old_user): void { $this->entry->removeAttributeEntryByValue("memberuid", $old_user->uid); $this->entry->write(); - $this->REDIS->removeCacheArray($this->gid, "members", $old_user->uid); - $this->REDIS->removeCacheArray($old_user->uid, "groups", $this->gid); + $this->REDIS->removeCacheArray( + $this->gid, + "members", + $old_user->uid, + fn() => $this->getGroupMemberUIDs(true), + ); + $this->REDIS->removeCacheArray( + $old_user->uid, + "groups", + $this->gid, + fn() => $this->LDAP->getPIGroupGIDsWithMemberUID($old_user->uid), + ); } public function memberExists(UnityUser $user): bool diff --git a/resources/lib/UnityLDAP.php b/resources/lib/UnityLDAP.php index 6e235bc8..f3238db0 100644 --- a/resources/lib/UnityLDAP.php +++ b/resources/lib/UnityLDAP.php @@ -458,4 +458,27 @@ public function getUidFromEmail(string $email): LDAPEntry } throw new exceptions\EntryNotFoundException($email); } + + public function getSortedQualifiedUsersForRedis(): array + { + $qualified_users = $this->getQualifiedUsersUIDs(); + sort($qualified_users); + return $qualified_users; + } + + public function getSortedOrgsForRedis(): array + { + $attributes = $this->getAllOrgGroupsAttributes(["cn"]); + $groups = array_map(fn($x) => $x["cn"][0], $attributes); + sort($groups); + return $groups; + } + + public function getSortedGroupsForRedis(): array + { + $attributes = $this->getAllPIGroupsAttributes(["cn"]); + $groups = array_map(fn($x) => $x["cn"][0], $attributes); + sort($groups); + return $groups; + } } diff --git a/resources/lib/UnityOrg.php b/resources/lib/UnityOrg.php index e121ec37..96cb5f97 100644 --- a/resources/lib/UnityOrg.php +++ b/resources/lib/UnityOrg.php @@ -39,7 +39,8 @@ public function init(): void $this->entry->setAttribute("objectclass", UnityLDAP::POSIX_GROUP_CLASS); $this->entry->setAttribute("gidnumber", strval($nextGID)); $this->entry->write(); - $this->REDIS->appendCacheArray("sorted_orgs", "", $this->gid); + $default_value_getter = [$this->LDAP, "getSortedOrgsForRedis"]; + $this->REDIS->appendCacheArray("sorted_orgs", "", $this->gid, $default_value_getter); } public function exists(): bool @@ -94,13 +95,23 @@ public function addUser(UnityUser $user): void { $this->entry->appendAttribute("memberuid", $user->uid); $this->entry->write(); - $this->REDIS->appendCacheArray($this->gid, "members", $user->uid); + $this->REDIS->appendCacheArray( + $this->gid, + "members", + $user->uid, + fn() => $this->getOrgMemberUIDs(true), + ); } public function removeUser(UnityUser $user): void { $this->entry->removeAttributeEntryByValue("memberuid", $user->uid); $this->entry->write(); - $this->REDIS->removeCacheArray($this->gid, "members", $user->uid); + $this->REDIS->removeCacheArray( + $this->gid, + "members", + $user->uid, + fn() => $this->getOrgMemberUIDs(true), + ); } } diff --git a/resources/lib/UnityRedis.php b/resources/lib/UnityRedis.php index 644f1c39..a6f510d7 100644 --- a/resources/lib/UnityRedis.php +++ b/resources/lib/UnityRedis.php @@ -61,15 +61,19 @@ public function getCache(string $object, string $key): mixed return null; } - public function appendCacheArray(string $object, string $key, mixed $value): void - { + public function appendCacheArray( + string $object, + string $key, + mixed $value, + callable $default_value_getter, + ): void { if (!$this->enabled) { return; } $cached_val = $this->getCache($object, $key); if (is_null($cached_val)) { - $this->setCache($object, $key, [$value]); + $this->setCache($object, $key, $default_value_getter()); } else { if (!is_array($cached_val)) { throw new Exception("This cache value is not an array"); @@ -82,15 +86,19 @@ public function appendCacheArray(string $object, string $key, mixed $value): voi } // TODO return void - public function removeCacheArray(string $object, string $key, mixed $value) - { + public function removeCacheArray( + string $object, + string $key, + mixed $value, + callable $default_value_getter, + ) { if (!$this->enabled) { return null; } $cached_val = $this->getCache($object, $key); if (is_null($cached_val)) { - $this->setCache($object, $key, []); + $this->setCache($object, $key, $default_value_getter()); } else { if (!is_array($cached_val)) { throw new Exception("This cache value is not an array"); diff --git a/resources/lib/UnityUser.php b/resources/lib/UnityUser.php index f98bbcb7..db22a6a4 100644 --- a/resources/lib/UnityUser.php +++ b/resources/lib/UnityUser.php @@ -108,7 +108,13 @@ public function init( $this->LDAP->getQualifiedUserGroup()->appendAttribute("memberuid", $this->uid); $this->LDAP->getQualifiedUserGroup()->write(); - $this->REDIS->appendCacheArray("sorted_qualified_users", "", $this->uid); + $default_value_getter = [$this->LDAP, "getSortedQualifiedUsersForRedis"]; + $this->REDIS->appendCacheArray( + "sorted_qualified_users", + "", + $this->uid, + $default_value_getter, + ); $this->SQL->addLog($this->uid, $_SERVER["REMOTE_ADDR"], "user_added", $this->uid); diff --git a/test/phpunit-bootstrap.php b/test/phpunit-bootstrap.php index f25ae0cd..c4c2af03 100644 --- a/test/phpunit-bootstrap.php +++ b/test/phpunit-bootstrap.php @@ -193,7 +193,8 @@ function ensureUserDoesNotExist() $all_users_group->write(); ensure(!in_array($USER->uid, $all_users_group->getAttribute("memberuid"))); } - $REDIS->removeCacheArray("sorted_qualified_users", "", $USER->uid); + $default_value_getter = [$LDAP, "getSortedQualifiedUsersForRedis"]; + $REDIS->removeCacheArray("sorted_qualified_users", "", $USER->uid, $default_value_getter); } function ensureOrgGroupDoesNotExist() @@ -204,7 +205,8 @@ function ensureOrgGroupDoesNotExist() $org_group->delete(); ensure(!$org_group->exists()); } - $REDIS->removeCacheArray("sorted_orgs", "", $SSO["org"]); + $default_value_getter = [$LDAP, "getSortedOrgsForRedis"]; + $REDIS->removeCacheArray("sorted_orgs", "", $SSO["org"], $default_value_getter); } function ensureUserNotInPIGroup(UnityGroup $pi_group) @@ -214,7 +216,12 @@ function ensureUserNotInPIGroup(UnityGroup $pi_group) $pi_group->removeUser($USER); ensure(!$pi_group->memberExists($USER)); } - $REDIS->removeCacheArray($pi_group->gid, "members", $USER->uid); + $REDIS->removeCacheArray( + $pi_group->gid, + "members", + $USER->uid, + fn() => $pi_group->getGroupMemberUIDs(true), + ); } function ensurePIGroupDoesNotExist() @@ -225,7 +232,8 @@ function ensurePIGroupDoesNotExist() $LDAP->getPIGroupEntry($gid)->delete(); ensure(!$USER->getPIGroup()->exists()); } - $REDIS->removeCacheArray("sorted_groups", "", $gid); + $default_value_getter = [$LDAP, "getSortedGroupsForRedis"]; + $REDIS->removeCacheArray("sorted_groups", "", $gid, $default_value_getter); } function getNormalUser() diff --git a/workers/update-ldap-cache.php b/workers/update-ldap-cache.php index 1dba929a..e72d5110 100755 --- a/workers/update-ldap-cache.php +++ b/workers/update-ldap-cache.php @@ -38,12 +38,11 @@ echo "waiting for LDAP search (users)...\n"; $users = $LDAP->search("objectClass=posixAccount", CONFIG["ldap"]["basedn"], []); echo "response received.\n"; - $user_CNs = $LDAP->getQualifiedUserGroup()->getAttribute("memberuid"); - sort($user_CNs); - $REDIS->setCache("sorted_qualified_users", "", $user_CNs); + $sorted_qualified_users_UIDs = $LDAP->getSortedQualifiedUsersForRedis(); + $REDIS->setCache("sorted_qualified_users", "", $sorted_qualified_users_UIDs); foreach ($users as $user) { $uid = $user->getAttribute("cn")[0]; - if (!in_array($uid, $user_CNs)) { + if (!in_array($uid, $sorted_qualified_users_UIDs)) { continue; } $REDIS->setCache($uid, "firstname", $user->getAttribute("givenname")[0]); @@ -59,13 +58,7 @@ echo "waiting for LDAP search (org groups)...\n"; $org_groups = $org_group_ou->getChildrenArrayStrict(["cn", "memberuid"], true); echo "response received.\n"; - // phpcs:disable - $org_group_CNs = array_map(function ($x) { - return $x["cn"][0]; - }, $org_groups); - // phpcs:enable - sort($org_group_CNs); - $REDIS->setCache("sorted_orgs", "", $org_group_CNs); + $REDIS->setCache("sorted_orgs", "", $LDAP->getSortedOrgsForRedis()); foreach ($org_groups as $org_group) { $gid = $org_group["cn"][0]; $REDIS->setCache($gid, "members", $org_group["memberuid"] ?? []); @@ -75,27 +68,20 @@ echo "waiting for LDAP search (pi groups)...\n"; $pi_groups = $pi_group_ou->getChildrenArrayStrict(["cn", "memberuid"], true); echo "response received.\n"; - // phpcs:disable - $pi_group_CNs = array_map(function ($x) { - return $x["cn"][0]; - }, $pi_groups); - // phpcs:enable - sort($pi_group_CNs); - // FIXME should be sorted_pi_groups - $REDIS->setCache("sorted_groups", "", $pi_group_CNs); + $REDIS->setCache("sorted_groups", "", $LDAP->getSortedGroupsForRedis()); $user_pi_group_member_of = []; - foreach ($user_CNs as $uid) { + foreach ($sorted_qualified_users_UIDs as $uid) { $user_pi_group_member_of[$uid] = []; } foreach ($pi_groups as $pi_group) { $gid = $pi_group["cn"][0]; $members = $pi_group["memberuid"] ?? []; foreach ($members as $uid) { - if (in_array($uid, $user_CNs)) { + if (in_array($uid, $sorted_qualified_users_UIDs)) { array_push($user_pi_group_member_of[$uid], $gid); } else { - echo "warning: group '$gid' has member '$uid' who is not in the users group!\n"; + echo "warning: group '$gid' has member '$uid' who is not qualfied!\n"; } } $REDIS->setCache($gid, "members", $pi_group["memberuid"] ?? []); From e21f0f25e0ccb9c95ca23ad2f2e1be63d9a5698e Mon Sep 17 00:00:00 2001 From: Simon Leary Date: Fri, 14 Nov 2025 16:51:11 -0500 Subject: [PATCH 2/3] rewrite --- resources/lib/UnityRedis.php | 40 +++++++++++++----------------------- 1 file changed, 14 insertions(+), 26 deletions(-) diff --git a/resources/lib/UnityRedis.php b/resources/lib/UnityRedis.php index a6f510d7..9ba68a1c 100644 --- a/resources/lib/UnityRedis.php +++ b/resources/lib/UnityRedis.php @@ -3,7 +3,7 @@ namespace UnityWebPortal\lib; use Redis; -use Exception; +use TypeError; class UnityRedis { @@ -70,22 +70,15 @@ public function appendCacheArray( if (!$this->enabled) { return; } - - $cached_val = $this->getCache($object, $key); - if (is_null($cached_val)) { - $this->setCache($object, $key, $default_value_getter()); - } else { - if (!is_array($cached_val)) { - throw new Exception("This cache value is not an array"); - } - - array_push($cached_val, $value); - sort($cached_val); - $this->setCache($object, $key, $cached_val); + $old_val = $this->getCache($object, $key) ?? $default_value_getter(); + if (!is_array($old_val)) { + throw new TypeError("redis[$object][$key] is not an array!"); } + $new_val = $old_val + [$value]; + sort($new_val); + $this->setCache($object, $key, $new_val); } - // TODO return void public function removeCacheArray( string $object, string $key, @@ -93,20 +86,15 @@ public function removeCacheArray( callable $default_value_getter, ) { if (!$this->enabled) { - return null; + return; } - - $cached_val = $this->getCache($object, $key); - if (is_null($cached_val)) { - $this->setCache($object, $key, $default_value_getter()); - } else { - if (!is_array($cached_val)) { - throw new Exception("This cache value is not an array"); - } - - $cached_val = array_diff($cached_val, [$value]); - $this->setCache($object, $key, $cached_val); + $old_val = $this->getCache($object, $key) ?? $default_value_getter(); + if (!is_array($old_val)) { + throw new TypeError("redis[$object][$key] is not an array!"); } + $new_val = array_diff($old_val, [$value]); + sort($new_val); + $this->setCache($object, $key, $new_val); } public function flushAll(): void From 7046470804e376d2b36f7e227633d4f42c19eb62 Mon Sep 17 00:00:00 2001 From: Simon Leary Date: Fri, 14 Nov 2025 17:58:13 -0500 Subject: [PATCH 3/3] fix bug --- resources/lib/UnityRedis.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/resources/lib/UnityRedis.php b/resources/lib/UnityRedis.php index 9ba68a1c..b24a6094 100644 --- a/resources/lib/UnityRedis.php +++ b/resources/lib/UnityRedis.php @@ -74,7 +74,8 @@ public function appendCacheArray( if (!is_array($old_val)) { throw new TypeError("redis[$object][$key] is not an array!"); } - $new_val = $old_val + [$value]; + $new_val = $old_val; + array_push($new_val, $value); sort($new_val); $this->setCache($object, $key, $new_val); }