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 @@ -169,14 +169,6 @@ $config = self::getConfig('admin'); $ldap = self::initLDAP($config); - list($_local, $_domain) = explode('@', $user->email, 2); - - $domain = $ldap->find_domain($_domain); - - if (!$domain) { - return false; - } - $entry = [ 'objectclass' => [ 'top', @@ -191,12 +183,9 @@ 'nsroledn' => [] ]; - self::setUserAttributes($user, $entry); - - $base_dn = $ldap->domain_root_dn($_domain); - $dn = "uid={$user->email},ou=People,{$base_dn}"; + if (!self::getUserEntry($ldap, $user->email, $dn) && $dn) { + self::setUserAttributes($user, $entry); - if (!$ldap->get_entry($dn)) { $ldap->add_entry($dn, $entry); } @@ -210,7 +199,7 @@ * * @return void */ - public static function updateDomain($domain) + public static function updateDomain(Domain $domain) { $config = self::getConfig('admin'); $ldap = self::initLDAP($config); @@ -227,7 +216,14 @@ $ldap->close(); } - public static function deleteDomain($domain) + /** + * Delete a domain from LDAP. + * + * @param \App\Domain $domain The domain to update. + * + * @return void + */ + public static function deleteDomain(Domain $domain) { $config = self::getConfig('admin'); $ldap = self::initLDAP($config); @@ -250,31 +246,42 @@ $ldap->close(); } - public static function deleteUser($user) + /** + * Delete a user from LDAP. + * + * @param \App\User $user The user account to update. + * + * @return void + */ + public static function deleteUser(User $user) { $config = self::getConfig('admin'); $ldap = self::initLDAP($config); - list($_local, $_domain) = explode('@', $user->email, 2); - - $domain = $ldap->find_domain($_domain); - - if (!$domain) { - $ldap->close(); - return false; + if (self::getUserEntry($ldap, $user->email, $dn)) { + $ldap->delete_entry($dn); } - $base_dn = $ldap->domain_root_dn($_domain); - $dn = "uid={$user->email},ou=People,{$base_dn}"; + $ldap->close(); + } - if (!$ldap->get_entry($dn)) { - $ldap->close(); - return false; - } + /** + * Get a user data from LDAP. + * + * @param string $email The user email. + * + * @return array|false|null + */ + public static function getUser(string $email) + { + $config = self::getConfig('admin'); + $ldap = self::initLDAP($config); - $ldap->delete_entry($dn); + $user = self::getUserEntry($ldap, $email, $dn, true); $ldap->close(); + + return $user; } /** @@ -282,46 +289,24 @@ * * @param \App\User $user The user account to update. * - * @return bool|void + * @return false|void */ public static function updateUser(User $user) { $config = self::getConfig('admin'); $ldap = self::initLDAP($config); - list($_local, $_domain) = explode('@', $user->email, 2); + $newEntry = $oldEntry = self::getUserEntry($ldap, $user->email, $dn, true); - $domain = $ldap->find_domain($_domain); + if ($oldEntry) { + self::setUserAttributes($user, $newEntry); - if (!$domain) { + $ldap->modify_entry($dn, $oldEntry, $newEntry); $ldap->close(); - return false; - } - - $base_dn = $ldap->domain_root_dn($_domain); - $dn = "uid={$user->email},ou=People,{$base_dn}"; - - $oldEntry = $ldap->get_entry($dn); - - if (!$oldEntry) { + } else { $ldap->close(); return false; } - - if (!array_key_exists('nsroledn', $oldEntry)) { - $roles = $ldap->get_entry_attributes($dn, ['nsroledn']); - if (!empty($roles)) { - $oldEntry['nsroledn'] = (array)$roles['nsroledn']; - } - } - - $newEntry = $oldEntry; - - self::setUserAttributes($user, $newEntry); - - $ldap->modify_entry($dn, $oldEntry, $newEntry); - - $ldap->close(); } /** @@ -388,6 +373,8 @@ $entry['mailquota'] = 0; + $entry['alias'] = $user->aliases->pluck('alias')->toArray(); + $roles = []; foreach ($user->entitlements as $entitlement) { @@ -455,6 +442,43 @@ } /** + * Get user entry from LDAP. + * + * @param \Net_LDAP3 $ldap Ldap connection + * @param string $email User email (uid) + * @param string $dn Reference to user DN + * @param bool $full Get extra attributes, e.g. nsroledn + * + * @return false|null|array User entry, False on error, NULL if not found + */ + protected static function getUserEntry($ldap, $email, &$dn = null, $full = false) + { + list($_local, $_domain) = explode('@', $email, 2); + + $domain = $ldap->find_domain($_domain); + + if (!$domain) { + return false; + } + + $base_dn = $ldap->domain_root_dn($_domain); + $dn = "uid={$email},ou=People,{$base_dn}"; + + $entry = $ldap->get_entry($dn); + + if ($entry && $full) { + if (!array_key_exists('nsroledn', $entry)) { + $roles = $ldap->get_entry_attributes($dn, ['nsroledn']); + if (!empty($roles)) { + $entry['nsroledn'] = (array) $roles['nsroledn']; + } + } + } + + return $entry ?: null; + } + + /** * Logging callback */ public static function logHook($level, $msg): void diff --git a/src/app/Observers/UserAliasObserver.php b/src/app/Observers/UserAliasObserver.php --- a/src/app/Observers/UserAliasObserver.php +++ b/src/app/Observers/UserAliasObserver.php @@ -61,28 +61,4 @@ { \App\Jobs\UserUpdate::dispatch($alias->user); } - - /** - * Handle the user alias "restored" event. - * - * @param \App\UserAlias $alias User email alias - * - * @return void - */ - public function restored(UserAlias $alias) - { - // not used - } - - /** - * Handle the user alias "force deleted" event. - * - * @param \App\UserAlias $alias User email alias - * - * @return void - */ - public function forceDeleted(UserAlias $alias) - { - // not used - } } diff --git a/src/app/Traits/UserAliasesTrait.php b/src/app/Traits/UserAliasesTrait.php --- a/src/app/Traits/UserAliasesTrait.php +++ b/src/app/Traits/UserAliasesTrait.php @@ -2,9 +2,6 @@ namespace App\Traits; -use App\UserAlias; -use Illuminate\Support\Facades\Cache; - trait UserAliasesTrait { /** @@ -13,7 +10,7 @@ * Example Usage: * * ```php - * $user = User::firstOrCreate(['email' => 'some@other.erg']); + * $user = User::firstOrCreate(['email' => 'some@other.org']); * $user->setAliases(['alias1@other.org', 'alias2@other.org']); * ``` * @@ -23,19 +20,21 @@ */ public function setAliases(array $aliases): void { - $existing_aliases = $this->aliases()->get()->map(function ($alias) { - return $alias->alias; - })->toArray(); - $aliases = array_map('strtolower', $aliases); $aliases = array_unique($aliases); - foreach (array_diff($aliases, $existing_aliases) as $alias) { - $this->aliases()->create(['alias' => $alias]); + $existing_aliases = []; + + foreach ($this->aliases()->get() as $alias) { + if (!in_array($alias->alias, $aliases)) { + $alias->delete(); + } else { + $existing_aliases[] = $alias->alias; + } } - foreach (array_diff($existing_aliases, $aliases) as $alias) { - $this->aliases()->where('alias', $alias)->delete(); + foreach (array_diff($aliases, $existing_aliases) as $alias) { + $this->aliases()->create(['alias' => $alias]); } } } diff --git a/src/tests/Feature/Jobs/UserCreateTest.php b/src/tests/Feature/Jobs/UserCreateTest.php --- a/src/tests/Feature/Jobs/UserCreateTest.php +++ b/src/tests/Feature/Jobs/UserCreateTest.php @@ -3,8 +3,6 @@ namespace Tests\Feature\Jobs; use App\Jobs\UserCreate; -use App\User; -use Illuminate\Support\Facades\Mail; use Tests\TestCase; class UserCreateTest extends TestCase diff --git a/src/tests/Feature/Jobs/UserUpdateTest.php b/src/tests/Feature/Jobs/UserUpdateTest.php new file mode 100644 --- /dev/null +++ b/src/tests/Feature/Jobs/UserUpdateTest.php @@ -0,0 +1,83 @@ +deleteTestUser('new-job-user@' . \config('app.domain')); + } + + public function tearDown(): void + { + $this->deleteTestUser('new-job-user@' . \config('app.domain')); + + parent::tearDown(); + } + + /** + * Test job handle + * + * @group ldap + */ + public function testHandle(): void + { + // Ignore any jobs created here (e.g. on setAliases() use) + Queue::fake(); + + $user = $this->getTestUser('new-job-user@' . \config('app.domain')); + + // Create the user in LDAP + $job = new \App\Jobs\UserCreate($user); + $job->handle(); + + // Test setting two aliases + $aliases = [ + 'new-job-user1@' . \config('app.domain'), + 'new-job-user2@' . \config('app.domain'), + ]; + $user->setAliases($aliases); + + $job = new UserUpdate($user->fresh()); + $job->handle(); + + $ldap_user = LDAP::getUser('new-job-user@' . \config('app.domain')); + + $this->assertSame($aliases, $ldap_user['alias']); + + // Test updating aliases list + $aliases = [ + 'new-job-user1@' . \config('app.domain'), + ]; + $user->setAliases($aliases); + + $job = new UserUpdate($user->fresh()); + $job->handle(); + + $ldap_user = LDAP::getUser('new-job-user@' . \config('app.domain')); + + $this->assertSame($aliases, (array) $ldap_user['alias']); + + // Test unsetting aliases list + $aliases = []; + $user->setAliases($aliases); + + $job = new UserUpdate($user->fresh()); + $job->handle(); + + $ldap_user = LDAP::getUser('new-job-user@' . \config('app.domain')); + + $this->assertTrue(empty($ldap_user['alias'])); + } +} diff --git a/src/tests/Feature/UserTest.php b/src/tests/Feature/UserTest.php --- a/src/tests/Feature/UserTest.php +++ b/src/tests/Feature/UserTest.php @@ -292,6 +292,7 @@ public function testSetAliases(): void { Queue::fake(); + Queue::assertNothingPushed(); $user = $this->getTestUser('UserAccountA@UserAccount.com'); @@ -300,6 +301,8 @@ // Add an alias $user->setAliases(['UserAlias1@UserAccount.com']); + Queue::assertPushed(\App\Jobs\UserUpdate::class, 1); + $aliases = $user->aliases()->get(); $this->assertCount(1, $aliases); $this->assertSame('useralias1@useraccount.com', $aliases[0]['alias']); @@ -307,6 +310,8 @@ // Add another alias $user->setAliases(['UserAlias1@UserAccount.com', 'UserAlias2@UserAccount.com']); + Queue::assertPushed(\App\Jobs\UserUpdate::class, 2); + $aliases = $user->aliases()->orderBy('alias')->get(); $this->assertCount(2, $aliases); $this->assertSame('useralias1@useraccount.com', $aliases[0]->alias); @@ -315,6 +320,8 @@ // Remove an alias $user->setAliases(['UserAlias1@UserAccount.com']); + Queue::assertPushed(\App\Jobs\UserUpdate::class, 3); + $aliases = $user->aliases()->get(); $this->assertCount(1, $aliases); $this->assertSame('useralias1@useraccount.com', $aliases[0]['alias']); @@ -322,9 +329,9 @@ // Remove all aliases $user->setAliases([]); - $this->assertCount(0, $user->aliases()->get()); + Queue::assertPushed(\App\Jobs\UserUpdate::class, 4); - // TODO: Test that the changes are propagated to ldap + $this->assertCount(0, $user->aliases()->get()); } /**