From dd09fc5a8ebbf0c12e96fd3b159a3a5b5d0355e6 Mon Sep 17 00:00:00 2001 From: Simon Leary Date: Tue, 3 Jun 2025 10:53:41 -0400 Subject: [PATCH 1/5] fail if request does not exist --- resources/lib/UnityGroup.php | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/resources/lib/UnityGroup.php b/resources/lib/UnityGroup.php index 0a91a9ee..9d96c3dd 100644 --- a/resources/lib/UnityGroup.php +++ b/resources/lib/UnityGroup.php @@ -133,6 +133,12 @@ public function requestGroup($send_mail_to_admins, $send_mail = true) */ public function approveGroup($operator = null, $send_mail = true) { + if (!$this->SQL->requestExists($this->getOwner()->getUID())) { + throw new RuntimeException( + "attempt to approve nonexistent request for group='{$this->getPIUID()}' uid='$new_user'" + ); + } + // check for edge cases... if ($this->exists()) { return; @@ -277,6 +283,12 @@ public function cancelGroupJoinRequest($user, $send_mail = true) */ public function approveUser($new_user, $send_mail = true) { + if (!$this->requestExists($new_user)) { + throw new RuntimeException( + "attempt to approve nonexistent request for group='{$this->getPIUID()}' uid='$new_user'" + ); + } + // check if user exists if (!$new_user->exists()) { $new_user->init(); From c8ab28080c25a6976ce4d70e5c2cc9bd2315ca01 Mon Sep 17 00:00:00 2001 From: Simon Leary Date: Tue, 3 Jun 2025 10:57:33 -0400 Subject: [PATCH 2/5] runtimeexception -> exception --- resources/lib/UnityGroup.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/resources/lib/UnityGroup.php b/resources/lib/UnityGroup.php index 9d96c3dd..a92805e0 100644 --- a/resources/lib/UnityGroup.php +++ b/resources/lib/UnityGroup.php @@ -134,7 +134,7 @@ public function requestGroup($send_mail_to_admins, $send_mail = true) public function approveGroup($operator = null, $send_mail = true) { if (!$this->SQL->requestExists($this->getOwner()->getUID())) { - throw new RuntimeException( + throw new Exception( "attempt to approve nonexistent request for group='{$this->getPIUID()}' uid='$new_user'" ); } @@ -284,7 +284,7 @@ public function cancelGroupJoinRequest($user, $send_mail = true) public function approveUser($new_user, $send_mail = true) { if (!$this->requestExists($new_user)) { - throw new RuntimeException( + throw new Exception( "attempt to approve nonexistent request for group='{$this->getPIUID()}' uid='$new_user'" ); } From f1009ebc06343fe6935b21a359d95213380a5cd0 Mon Sep 17 00:00:00 2001 From: Simon Leary Date: Tue, 3 Jun 2025 11:44:47 -0400 Subject: [PATCH 3/5] implement toString --- resources/lib/UnityGroup.php | 5 +++++ resources/lib/UnityUser.php | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/resources/lib/UnityGroup.php b/resources/lib/UnityGroup.php index a92805e0..2fcafad7 100644 --- a/resources/lib/UnityGroup.php +++ b/resources/lib/UnityGroup.php @@ -47,6 +47,11 @@ public function equals($other_group) return $this->getPIUID() == $other_group->getPIUID(); } + public function __toString() + { + return $this->getPIUID(); + } + /** * Returns this group's PI UID * diff --git a/resources/lib/UnityUser.php b/resources/lib/UnityUser.php index 594e7af6..04c412b9 100644 --- a/resources/lib/UnityUser.php +++ b/resources/lib/UnityUser.php @@ -38,6 +38,11 @@ public function equals($other_user) return $this->getUID() == $other_user->getUID(); } + public function __toString() + { + return $this->uid; + } + /** * This is the method that is run when a new account is created * From 4571266f4ff60a6a3ddc73fbab26b824b6f33ebc Mon Sep 17 00:00:00 2001 From: Simon Leary Date: Tue, 3 Jun 2025 13:21:51 -0400 Subject: [PATCH 4/5] cover weird edge case --- resources/lib/UnityGroup.php | 5 ++++- test/functional/PiMemberApproveTest.php | 7 ++++--- test/functional/PiRemoveUserTest.php | 3 +++ 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/resources/lib/UnityGroup.php b/resources/lib/UnityGroup.php index 2fcafad7..580cd63e 100644 --- a/resources/lib/UnityGroup.php +++ b/resources/lib/UnityGroup.php @@ -399,15 +399,18 @@ public function removeUser($new_user, $send_mail = true) public function newUserRequest($new_user, $send_mail = true) { if ($this->userExists($new_user)) { + UnitySite::errorLog("warning", "user '$new_user' already in group"); return; } if ($this->requestExists($new_user)) { + UnitySite::errorLog("warning", "user '$new_user' already requested group membership"); return; } - // check if account deletion request already exists + // FIXME remove this check or make an error instead if ($this->SQL->accDeletionRequestExists($new_user->getUID())) { + UnitySite::errorLog("warning", "user '$new_user' requested account deletion, nothing doing..."); return; } diff --git a/test/functional/PiMemberApproveTest.php b/test/functional/PiMemberApproveTest.php index dd51b571..0b8b2024 100644 --- a/test/functional/PiMemberApproveTest.php +++ b/test/functional/PiMemberApproveTest.php @@ -64,11 +64,12 @@ public function testApproveNonexistentRequest() $this->assertEmpty($piGroup->getRequests()); try { + $this->expectException(Exception::class); $piGroup->approveUser($notRequestedUser); - $this->assertEquals([$pi->getUID()], $piGroup->getGroupMemberUIDs()); - $this->assertFalse($piGroup->userExists($notRequestedUser)); } finally { - $piGroup->removeUser($notRequestedUser); + if ($piGroup->userExists($notRequestedUser)) { + $piGroup->removeUser($notRequestedUser); + } } } } diff --git a/test/functional/PiRemoveUserTest.php b/test/functional/PiRemoveUserTest.php index 79ecd1ee..c02a8908 100644 --- a/test/functional/PiRemoveUserTest.php +++ b/test/functional/PiRemoveUserTest.php @@ -30,6 +30,9 @@ public function testRemoveUser() foreach ($memberUIDs as $uid) { if ($uid != $piUid) { $memberToDelete = new UnityUser($uid, $LDAP, $SQL, $MAILER, $REDIS, $WEBHOOK); + if ($memberToDelete->hasRequestedAccountDeletion()) { + continue; + } break; } } From 492eb25586983c79206206c84b6bbebc2bc9ea76 Mon Sep 17 00:00:00 2001 From: Simon Leary Date: Tue, 3 Jun 2025 13:23:24 -0400 Subject: [PATCH 5/5] throw exception --- resources/lib/UnityGroup.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/resources/lib/UnityGroup.php b/resources/lib/UnityGroup.php index 580cd63e..f74aad7c 100644 --- a/resources/lib/UnityGroup.php +++ b/resources/lib/UnityGroup.php @@ -408,9 +408,8 @@ public function newUserRequest($new_user, $send_mail = true) return; } - // FIXME remove this check or make an error instead if ($this->SQL->accDeletionRequestExists($new_user->getUID())) { - UnitySite::errorLog("warning", "user '$new_user' requested account deletion, nothing doing..."); + throw new Exception("user '$new_user' requested account deletion"); return; }