diff --git a/src/app/Backends/LDAP.php b/src/app/Backends/LDAP.php --- a/src/app/Backends/LDAP.php +++ b/src/app/Backends/LDAP.php @@ -22,6 +22,8 @@ * Starts a new LDAP connection that will be used by all methods * until you call self::disconnect() explicitely. Normally every * method uses a separate connection. + * + * @throws \Exception */ public static function connect(): void { @@ -47,9 +49,9 @@ * * @param \App\Domain $domain The domain to create. * - * @return void + * @throws \Exception */ - public static function createDomain(Domain $domain) + public static function createDomain(Domain $domain): void { $config = self::getConfig('admin'); $ldap = self::initLDAP($config); @@ -90,8 +92,14 @@ $dn = "associateddomain={$domain->namespace},{$config['domain_base_dn']}"; + self::setDomainAttributes($domain, $entry); + if (!$ldap->get_entry($dn)) { - $ldap->add_entry($dn, $entry); + $result = $ldap->add_entry($dn, $entry); + + if (!$result) { + self::throwException($ldap, "Failed to create a domain in LDAP"); + } } // create ou, roles, ous @@ -155,7 +163,7 @@ } } - foreach (['kolab-admin', 'billing-user'] as $item) { + foreach (['kolab-admin'] as $item) { if (!$ldap->get_entry("cn={$item},{$domainBaseDN}")) { $ldap->add_entry( "cn={$item},{$domainBaseDN}", @@ -174,6 +182,8 @@ } } + // TODO: Assign kolab-admin role to the owner? + if (empty(self::$ldap)) { $ldap->close(); } @@ -199,9 +209,9 @@ * * @param \App\User $user The user account to create. * - * @return bool|void + * @throws \Exception */ - public static function createUser(User $user) + public static function createUser(User $user): void { $config = self::getConfig('admin'); $ldap = self::initLDAP($config); @@ -223,7 +233,11 @@ if (!self::getUserEntry($ldap, $user->email, $dn) && $dn) { self::setUserAttributes($user, $entry); - $ldap->add_entry($dn, $entry); + $result = $ldap->add_entry($dn, $entry); + + if (!$result) { + self::throwException($ldap, "Failed to create a user in LDAP"); + } } if (empty(self::$ldap)) { @@ -232,25 +246,39 @@ } /** - * Update a domain in LDAP. + * Delete a domain from LDAP. * * @param \App\Domain $domain The domain to update. * - * @return void + * @throws \Exception */ - public static function updateDomain(Domain $domain) + public static function deleteDomain(Domain $domain): void { $config = self::getConfig('admin'); $ldap = self::initLDAP($config); - $ldapDomain = $ldap->find_domain($domain->namespace); + $hostedRootDN = \config('ldap.hosted.root_dn'); + $mgmtRootDN = \config('ldap.admin.root_dn'); - $oldEntry = $ldap->get_entry($ldapDomain['dn']); - $newEntry = $oldEntry; + $domainBaseDN = "ou={$domain->namespace},{$hostedRootDN}"; - self::setDomainAttributes($domain, $newEntry); + if ($ldap->get_entry($domainBaseDN)) { + $result = $ldap->delete_entry_recursive($domainBaseDN); + + if (!$result) { + self::throwException($ldap, "Failed to delete a domain from LDAP"); + } + } + + if ($ldap_domain = $ldap->find_domain($domain->namespace)) { + if ($ldap->get_entry($ldap_domain['dn'])) { + $result = $ldap->delete_entry($ldap_domain['dn']); - $ldap->modify_entry($ldapDomain['dn'], $oldEntry, $newEntry); + if (!$result) { + self::throwException($ldap, "Failed to delete a domain from LDAP"); + } + } + } if (empty(self::$ldap)) { $ldap->close(); @@ -258,29 +286,22 @@ } /** - * Delete a domain from LDAP. + * Delete a user from LDAP. * - * @param \App\Domain $domain The domain to update. + * @param \App\User $user The user account to update. * - * @return void + * @throws \Exception */ - public static function deleteDomain(Domain $domain) + public static function deleteUser(User $user): void { $config = self::getConfig('admin'); $ldap = self::initLDAP($config); - $hostedRootDN = \config('ldap.hosted.root_dn'); - $mgmtRootDN = \config('ldap.admin.root_dn'); - - $domainBaseDN = "ou={$domain->namespace},{$hostedRootDN}"; - - if ($ldap->get_entry($domainBaseDN)) { - $ldap->delete_entry_recursive($domainBaseDN); - } + if (self::getUserEntry($ldap, $user->email, $dn)) { + $result = $ldap->delete_entry($dn); - if ($ldap_domain = $ldap->find_domain($domain->namespace)) { - if ($ldap->get_entry($ldap_domain['dn'])) { - $ldap->delete_entry($ldap_domain['dn']); + if (!$result) { + self::throwException($ldap, "Failed to delete a user from LDAP"); } } @@ -290,24 +311,29 @@ } /** - * Delete a user from LDAP. + * Get a domain data from LDAP. * - * @param \App\User $user The user account to update. + * @param string $namespace The domain name * - * @return void + * @return array|false|null + * @throws \Exception */ - public static function deleteUser(User $user) + public static function getDomain(string $namespace) { $config = self::getConfig('admin'); $ldap = self::initLDAP($config); - if (self::getUserEntry($ldap, $user->email, $dn)) { - $ldap->delete_entry($dn); + $ldapDomain = $ldap->find_domain($namespace); + + if ($ldapDomain) { + $domain = $ldap->get_entry($ldapDomain['dn']); } if (empty(self::$ldap)) { $ldap->close(); } + + return $domain ?? null; } /** @@ -316,6 +342,7 @@ * @param string $email The user email. * * @return array|false|null + * @throws \Exception */ public static function getUser(string $email) { @@ -332,13 +359,47 @@ } /** + * Update a domain in LDAP. + * + * @param \App\Domain $domain The domain to update. + * + * @throws \Exception + */ + public static function updateDomain(Domain $domain): void + { + $config = self::getConfig('admin'); + $ldap = self::initLDAP($config); + + $ldapDomain = $ldap->find_domain($domain->namespace); + + if (!$ldapDomain) { + self::throwException($ldap, "Failed to update a domain in LDAP (domain not found)"); + } + + $oldEntry = $ldap->get_entry($ldapDomain['dn']); + $newEntry = $oldEntry; + + self::setDomainAttributes($domain, $newEntry); + + $result = $ldap->modify_entry($ldapDomain['dn'], $oldEntry, $newEntry); + + if (!is_array($result)) { + self::throwException($ldap, "Failed to update a domain in LDAP"); + } + + if (empty(self::$ldap)) { + $ldap->close(); + } + } + + /** * Update a user in LDAP. * * @param \App\User $user The user account to update. * - * @return false|void + * @throws \Exception */ - public static function updateUser(User $user) + public static function updateUser(User $user): void { $config = self::getConfig('admin'); $ldap = self::initLDAP($config); @@ -346,12 +407,16 @@ $newEntry = $oldEntry = self::getUserEntry($ldap, $user->email, $dn, true); if (!$oldEntry) { - return false; + self::throwException($ldap, "Failed to update a user in LDAP (user not found)"); } self::setUserAttributes($user, $newEntry); - $ldap->modify_entry($dn, $oldEntry, $newEntry); + $result = $ldap->modify_entry($dn, $oldEntry, $newEntry); + + if (!is_array($result)) { + self::throwException($ldap, "Failed to update a user in LDAP"); + } if (empty(self::$ldap)) { $ldap->close(); @@ -369,11 +434,17 @@ $ldap = new \Net_LDAP3($config); - $ldap->connect(); + $connected = $ldap->connect(); + + if (!$connected) { + throw new \Exception("Failed to connect to LDAP"); + } - $ldap->bind(\config("ldap.{$privilege}.bind_dn"), \config("ldap.{$privilege}.bind_pw")); + $bound = $ldap->bind(\config("ldap.{$privilege}.bind_dn"), \config("ldap.{$privilege}.bind_pw")); - // TODO: error handling + if (!$bound) { + throw new \Exception("Failed to bind to LDAP"); + } return $ldap; } @@ -502,14 +573,14 @@ * * @return false|null|array User entry, False on error, NULL if not found */ - protected static function getUserEntry($ldap, $email, &$dn = null, $full = false) + private static function getUserEntry($ldap, $email, &$dn = null, $full = false) { list($_local, $_domain) = explode('@', $email, 2); $domain = $ldap->find_domain($_domain); if (!$domain) { - return false; + return $domain; } $base_dn = $ldap->domain_root_dn($_domain); @@ -582,4 +653,21 @@ \Log::{$function}($msg); } + + /** + * Throw exception and close the connection when needed + * + * @param \Net_LDAP3 $ldap Ldap connection + * @param string $message Exception message + * + * @throws \Exception + */ + private static function throwException($ldap, string $message): void + { + if (empty(self::$ldap) && !empty($ldap)) { + $ldap->close(); + } + + throw new \Exception($message); + } } diff --git a/src/tests/Feature/Backends/LDAPTest.php b/src/tests/Feature/Backends/LDAPTest.php --- a/src/tests/Feature/Backends/LDAPTest.php +++ b/src/tests/Feature/Backends/LDAPTest.php @@ -11,6 +11,8 @@ class LDAPTest extends TestCase { + private $ldap_config = []; + /** * {@inheritDoc} */ @@ -18,7 +20,12 @@ { parent::setUp(); + $this->ldap_config = [ + 'ldap.hosts' => \config('ldap.hosts'), + ]; + $this->deleteTestUser('user-ldap-test@' . \config('app.domain')); + $this->deleteTestDomain('testldap.com'); } /** @@ -26,19 +33,80 @@ */ public function tearDown(): void { + \config($this->ldap_config); + $this->deleteTestUser('user-ldap-test@' . \config('app.domain')); + $this->deleteTestDomain('testldap.com'); parent::tearDown(); } /** + * Test handling connection errors + * + * @group ldap + */ + public function testConnectException(): void + { + \config(['ldap.hosts' => 'non-existing.host']); + + $this->expectException(\Exception::class); + + LDAP::connect(); + } + + /** * Test creating/updating/deleting a domain record * * @group ldap */ public function testDomain(): void { - $this->markTestIncomplete(); + Queue::fake(); + + $domain = $this->getTestDomain('testldap.com', [ + 'type' => Domain::TYPE_EXTERNAL, + 'status' => Domain::STATUS_NEW | Domain::STATUS_ACTIVE, + ]); + + // Create the domain + LDAP::createDomain($domain); + + $ldap_domain = LDAP::getDomain($domain->namespace); + + $expected = [ + 'associateddomain' => $domain->namespace, + 'inetdomainstatus' => $domain->status, + 'objectclass' => [ + 'top', + 'domainrelatedobject', + 'inetdomain' + ], + ]; + + foreach ($expected as $attr => $value) { + $this->assertEquals($value, isset($ldap_domain[$attr]) ? $ldap_domain[$attr] : null); + } + + // TODO: Test other attributes, aci, roles/ous + + // Update the domain + $domain->status |= User::STATUS_LDAP_READY; + + LDAP::updateDomain($domain); + + $expected['inetdomainstatus'] = $domain->status; + + $ldap_domain = LDAP::getDomain($domain->namespace); + + foreach ($expected as $attr => $value) { + $this->assertEquals($value, isset($ldap_domain[$attr]) ? $ldap_domain[$attr] : null); + } + + // Delete the domain + LDAP::deleteDomain($domain); + + $this->assertSame(null, LDAP::getDomain($domain->namespace)); } /** @@ -150,4 +218,41 @@ $this->assertSame(null, LDAP::getUser($user->email)); } + + /** + * Test handling update of a non-existing domain + * + * @group ldap + */ + public function testUpdateDomainException(): void + { + $this->expectException(\Exception::class); + $this->expectExceptionMessageMatches('/domain not found/'); + + $domain = new Domain([ + 'namespace' => 'testldap.com', + 'type' => Domain::TYPE_EXTERNAL, + 'status' => Domain::STATUS_NEW | Domain::STATUS_ACTIVE, + ]); + + LDAP::updateDomain($domain); + } + + /** + * Test handling update of a non-existing user + * + * @group ldap + */ + public function testUpdateUserException(): void + { + $this->expectException(\Exception::class); + $this->expectExceptionMessageMatches('/user not found/'); + + $user = new User([ + 'email' => 'test-non-existing-ldap@kolab.org', + 'status' => User::STATUS_ACTIVE, + ]); + + LDAP::updateUser($user); + } }