add tests for approving, denying PI membership requests

.github/workflows/functional.yml

@@ -10,6 +10,8 @@ jobs:
     runs-on: ubuntu-latest
     steps:
     - uses: actions/checkout@v3
+      with:
+        submodules: true
     - name: setup PHP
       uses: shivammathur/setup-php@v2
       with:

.github/workflows/phpunit.yml

@@ -7,6 +7,8 @@ jobs:
     runs-on: ubuntu-latest
     steps:
     - uses: actions/checkout@v3
+      with:
+        submodules: true
     - name: setup PHP
       uses: shivammathur/setup-php@v2
       with:

.github/workflows/pre-commit.yml

@@ -10,6 +10,8 @@ jobs:
     runs-on: ubuntu-latest
     steps:
     - uses: actions/checkout@v3
+      with:
+        submodules: true
     - uses: actions/setup-python@v3
     - name: setup PHP
       uses: shivammathur/setup-php@v2

.gitignore

@@ -9,7 +9,10 @@ resources/custom_user_mappings/*.csv
 **/.rnd
 
 # don't track vendor files from composer
-vendor
+vendor/**
+!vendor/hakasapl/
+!vendor/hakasapl/phpopenldaper/
+!vendor/hakasapl/phpopenldaper/**
 composer.lock
 
 # don't track site configs

.gitmodules

@@ -0,0 +1,3 @@
+[submodule "vendor/hakasapl/phpopenldaper"]
+	path = vendor/hakasapl/phpopenldaper
+	url = [email protected]:hakasapl/phpopenldaper.git

composer.json

@@ -2,8 +2,7 @@
     "require": {
         "psr/log": "1.1.4",
         "phpseclib/phpseclib": "3.0.43",
-        "phpmailer/phpmailer": "6.6.4",
-        "hakasapl/phpopenldaper": "1.0.6"
+        "phpmailer/phpmailer": "6.6.4"
     },
     "require-dev": {
         "phpunit/phpunit": "<12.1"

resources/autoload.php

@@ -6,6 +6,9 @@
 
 // Load Composer Libs
 require_once __DIR__ . "/../vendor/autoload.php";
+// submodule
+require_once __DIR__ . "/../vendor/hakasapl/phpopenldaper/src/PHPOpenLDAPer/LDAPEntry.php";
+require_once __DIR__ . "/../vendor/hakasapl/phpopenldaper/src/PHPOpenLDAPer/LDAPConn.php";
 
 // load libs
 require_once __DIR__ . "/lib/UnityLDAP.php";

resources/lib/UnityGroup.php

@@ -218,10 +218,7 @@ public function denyGroup($operator = null, $send_mail = true)
     //     // now we delete the ldap entry
     //     $ldapPiGroupEntry = $this->getLDAPPiGroup();
     //     if ($ldapPiGroupEntry->exists()) {
-    //         if (!$ldapPiGroupEntry->delete()) {
-    //             throw new Exception("Unable to delete PI ldap group");
-    //         }
-
+    //         $ldapPiGroupEntry->delete();
     //         $this->REDIS->removeCacheArray("sorted_groups", "", $this->getPIUID());
     //         foreach ($users as $user) {
     //             $this->REDIS->removeCacheArray($user->getUID(), "groups", $this->getPIUID());
@@ -487,10 +484,7 @@ private function init()
             $ldapPiGroupEntry->setAttribute("objectclass", UnityLDAP::POSIX_GROUP_CLASS);
             $ldapPiGroupEntry->setAttribute("gidnumber", strval($nextGID));
             $ldapPiGroupEntry->setAttribute("memberuid", array($owner->getUID()));
-
-            if (!$ldapPiGroupEntry->write()) {
-                throw new Exception("Failed to create POSIX group for " . $owner->getUID());  // this shouldn't execute
-            }
+            $ldapPiGroupEntry->write();
         }
 
         $this->REDIS->appendCacheArray("sorted_groups", "", $this->getPIUID());
@@ -503,11 +497,7 @@ private function addUserToGroup($new_user)
         // Add to LDAP Group
         $pi_group = $this->getLDAPPiGroup();
         $pi_group->appendAttribute("memberuid", $new_user->getUID());
-
-        if (!$pi_group->write()) {
-            throw new Exception("Unable to write PI group");
-        }
-
+        $pi_group->write();
         $this->REDIS->appendCacheArray($this->getPIUID(), "members", $new_user->getUID());
         $this->REDIS->appendCacheArray($new_user->getUID(), "groups", $this->getPIUID());
     }
@@ -517,11 +507,7 @@ private function removeUserFromGroup($old_user)
         // Remove from LDAP Group
         $pi_group = $this->getLDAPPiGroup();
         $pi_group->removeAttributeEntryByValue("memberuid", $old_user->getUID());
-
-        if (!$pi_group->write()) {
-            throw new Exception("Unable to write PI group");
-        }
-
+        $pi_group->write();
         $this->REDIS->removeCacheArray($this->getPIUID(), "members", $old_user->getUID());
         $this->REDIS->removeCacheArray($old_user->getUID(), "groups", $this->getPIUID());
     }

resources/lib/UnityLDAP.php

@@ -312,28 +312,24 @@ public function getAllOrgGroups($UnitySQL, $UnityMailer, $UnityRedis, $UnityWebh
     public function getUserEntry($uid)
     {
         $uid = ldap_escape($uid, LDAP_ESCAPE_DN);
-        $ldap_entry = new LDAPEntry($this->getConn(), unityLDAP::RDN . "=$uid," . $this->STR_USEROU);
-        return $ldap_entry;
+        return $this->getEntry(unityLDAP::RDN . "=$uid," . $this->STR_USEROU);
     }
 
     public function getGroupEntry($gid)
     {
         $uid = ldap_escape($gid, LDAP_ESCAPE_DN);
-        $ldap_entry = new LDAPEntry($this->getConn(), unityLDAP::RDN . "=$gid," . $this->STR_GROUPOU);
-        return $ldap_entry;
+        return $this->getEntry(unityLDAP::RDN . "=$gid," . $this->STR_GROUPOU);
     }
 
     public function getPIGroupEntry($gid)
     {
         $uid = ldap_escape($gid, LDAP_ESCAPE_DN);
-        $ldap_entry = new LDAPEntry($this->getConn(), unityLDAP::RDN . "=$gid," . $this->STR_PIGROUPOU);
-        return $ldap_entry;
+        return $this->getEntry(unityLDAP::RDN . "=$gid," . $this->STR_PIGROUPOU);
     }
 
     public function getOrgGroupEntry($gid)
     {
         $uid = ldap_escape($gid, LDAP_ESCAPE_DN);
-        $ldap_entry = new LDAPEntry($this->getConn(), unityLDAP::RDN . "=$gid," . $this->STR_ORGGROUPOU);
-        return $ldap_entry;
+        return $this->getEntry(unityLDAP::RDN . "=$gid," . $this->STR_ORGGROUPOU);
     }
 }

resources/lib/UnityOrg.php

@@ -34,10 +34,7 @@ public function init()
 
             $org_group->setAttribute("objectclass", UnityLDAP::POSIX_GROUP_CLASS);
             $org_group->setAttribute("gidnumber", strval($nextGID));
-
-            if (!$org_group->write()) {
-                throw new Exception("Failed to create POSIX group for " . $this->orgid);  // this shouldn't execute
-            }
+            $org_group->write();
         }
 
         $this->REDIS->appendCacheArray("sorted_orgs", "", $this->getOrgID());
@@ -101,23 +98,15 @@ public function addUser($user)
     {
         $org_group = $this->getLDAPOrgGroup();
         $org_group->appendAttribute("memberuid", $user->getUID());
-
-        if (!$org_group->write()) {
-            throw new Exception("Unable to write to org group");
-        }
-
+        $org_group->write();
         $this->REDIS->appendCacheArray($this->getOrgID(), "members", $user->getUID());
     }
 
     public function removeUser($user)
     {
         $org_group = $this->getLDAPOrgGroup();
         $org_group->removeAttributeEntryByValue("memberuid", $user->getUID());
-
-        if (!$org_group->write()) {
-            throw new Exception("Unable to write to org group");
-        }
-
+        $org_group->write();
         $this->REDIS->removeCacheArray($this->getOrgID(), "members", $user->getUID());
     }
 }

resources/lib/UnityUser.php

@@ -58,10 +58,7 @@ public function init($send_mail = true)
         if (!$ldapGroupEntry->exists()) {
             $ldapGroupEntry->setAttribute("objectclass", UnityLDAP::POSIX_GROUP_CLASS);
             $ldapGroupEntry->setAttribute("gidnumber", strval($id));
-
-            if (!$ldapGroupEntry->write()) {
-                throw new Exception("Failed to create POSIX group for $this->uid");
-            }
+            $ldapGroupEntry->write();
         }
 
         //
@@ -80,11 +77,7 @@ public function init($send_mail = true)
             $ldapUserEntry->setAttribute("loginshell", $this->LDAP->getDefUserShell());
             $ldapUserEntry->setAttribute("uidnumber", strval($id));
             $ldapUserEntry->setAttribute("gidnumber", strval($id));
-
-            if (!$ldapUserEntry->write()) {
-                $ldapGroupEntry->delete();  // Cleanup previous group
-                throw new Exception("Failed to create POSIX user for  $this->uid");
-            }
+            $ldapUserEntry->write();
         }
 
         // update cache
@@ -177,11 +170,7 @@ public function setOrg($org)
     {
         $ldap_user = $this->getLDAPUser();
         $ldap_user->setAttribute("o", $org);
-
-        if (!$ldap_user->write()) {
-            throw new Exception("Error updating LDAP entry $this->uid");
-        }
-
+        $ldap_user->write();
         $this->REDIS->setCache($this->uid, "org", $org);
     }
 
@@ -225,10 +214,7 @@ public function setFirstname($firstname, $operator = null)
             $this->getUID()
         );
 
-        if (!$ldap_user->write()) {
-            throw new Exception("Error updating LDAP entry $this->uid");
-        }
-
+        $ldap_user->write();
         $this->REDIS->setCache($this->uid, "firstname", $firstname);
     }
 
@@ -277,10 +263,7 @@ public function setLastname($lastname, $operator = null)
             $this->getUID()
         );
 
-        if (!$this->getLDAPUser()->write()) {
-            throw new Exception("Error updating LDAP entry $this->uid");
-        }
-
+        $this->getLDAPUser()->write();
         $this->REDIS->setCache($this->uid, "lastname", $lastname);
     }
 
@@ -334,10 +317,7 @@ public function setMail($email, $operator = null)
             $this->getUID()
         );
 
-        if (!$this->getLDAPUser()->write()) {
-            throw new Exception("Error updating LDAP entry $this->uid");
-        }
-
+        $this->getLDAPUser()->write();
         $this->REDIS->setCache($this->uid, "mail", $email);
     }
 
@@ -380,9 +360,7 @@ public function setSSHKeys($keys, $operator = null, $send_mail = true)
         $keys_filt = array_values(array_unique($keys));
         if ($ldapUser->exists()) {
             $ldapUser->setAttribute("sshpublickey", $keys_filt);
-            if (!$ldapUser->write()) {
-                throw new Exception("Failed to modify SSH keys for $this->uid");
-            }
+            $ldapUser->write();
         }
 
         $this->REDIS->setCache($this->uid, "sshkeys", $keys_filt);
@@ -459,9 +437,7 @@ public function setLoginShell($shell, $operator = null, $send_mail = true)
         $ldapUser = $this->getLDAPUser();
         if ($ldapUser->exists()) {
             $ldapUser->setAttribute("loginshell", $shell);
-            if (!$ldapUser->write()) {
-                throw new Exception("Failed to modify login shell for $this->uid");
-            }
+            $ldapUser->write();
         }
 
         $operator = is_null($operator) ? $this->getUID() : $operator->getUID();
@@ -518,10 +494,7 @@ public function setHomeDir($home, $operator = null)
         $ldapUser = $this->getLDAPUser();
         if ($ldapUser->exists()) {
             $ldapUser->setAttribute("homedirectory", $home);
-            if (!$ldapUser->write()) {
-                throw new Exception("Failed to modify home directory for $this->uid");
-            }
-
+            $ldapUser->write();
             $operator = is_null($operator) ? $this->getUID() : $operator->getUID();
 
             $this->SQL->addLog(

test/functional/LoginShellSetTest.php

@@ -23,9 +23,7 @@ public function tearDown(): void
     public static function getShells()
     {
         global $HTTP_HEADER_TEST_INPUTS;
-        // phpcs:disable
         return [["/bin/bash"]] + array_map(function($x){return [$x];}, $HTTP_HEADER_TEST_INPUTS);
-        // phpcs:enable
     }
 
     private function isShellValid(string $shell)

test/functional/PiMemberApproveTest.php

@@ -0,0 +1,74 @@
+<?php
+
+use PHPUnit\Framework\TestCase;
+use PHPUnit\Framework\Attributes\DataProvider;
+use UnityWebPortal\lib\UnityUser;
+
+class PiMemberApproveTest extends TestCase {
+    static $requestUid;
+    static $noRequestUid;
+
+    public static function setUpBeforeClass(): void{
+        global $USER;
+        switchUser(...getNormalUser());
+        self::$requestUid = $USER->getUID();
+        switchUser(...getNormalUser2());
+        self::$noRequestUid = $USER->getUID();
+    }
+
+    private function approveUser(string $uid)
+    {
+        post(
+            __DIR__ . "/../../webroot/panel/pi.php",
+            ["form_type" => "userReq", "action" => "approve", "uid" => $uid]
+        );
+    }
+
+    public function testApproveRequest()
+    {
+        global $USER, $LDAP, $SQL, $MAILER, $REDIS, $WEBHOOK;
+        switchUser(...getUserIsPIHasNoMembersNoMemberRequests());
+        $pi = $USER;
+        $piGroup = $USER->getPIGroup();
+        $this->assertTrue($piGroup->exists());
+        $this->assertEquals([$pi->getUID()], $piGroup->getGroupMemberUIDs());
+        $this->assertEmpty($piGroup->getRequests());
+        $requestedUser = new UnityUser(self::$requestUid, $LDAP, $SQL, $MAILER, $REDIS, $WEBHOOK);
+        try {
+            $piGroup->newUserRequest($requestedUser);
+            $this->assertFalse($piGroup->userExists($requestedUser));
+
+            $piGroup->approveUser($requestedUser);
+            $this->assertEmpty($piGroup->getRequests());
+
+            $this->assertEquals([$pi->getUID(), self::$requestUid], $piGroup->getGroupMemberUIDs());
+            $this->assertTrue($piGroup->userExists($requestedUser));
+        } finally {
+            $piGroup->removeUser($requestedUser);
+            $SQL->removeRequest(self::$requestUid, $piGroup->getPIUID());
+        }
+    }
+
+    public function testApproveNonexistentRequest()
+    {
+        global $USER, $LDAP, $SQL, $MAILER, $REDIS, $WEBHOOK;
+        switchUser(...getUserIsPIHasNoMembersNoMemberRequests());
+        $pi = $USER;
+        $piGroup = $USER->getPIGroup();
+        $this->assertTrue($piGroup->exists());
+        $this->assertEquals([$pi->getUID()], $piGroup->getGroupMemberUIDs());
+        $this->assertEmpty($piGroup->getRequests());
+
+        $notRequestedUser = new UnityUser(self::$noRequestUid, $LDAP, $SQL, $MAILER, $REDIS, $WEBHOOK);
+        $this->assertFalse($piGroup->userExists($notRequestedUser));
+        $this->assertEmpty($piGroup->getRequests());
+
+        try {
+            $piGroup->approveUser($notRequestedUser);
+            $this->assertEquals([$pi->getUID()], $piGroup->getGroupMemberUIDs());
+            $this->assertFalse($piGroup->userExists($notRequestedUser));
+        } finally {
+            $piGroup->removeUser($notRequestedUser);
+        }
+    }
+}