diff --git a/src/app/Console/Commands/User/DomainsCommand.php b/src/app/Console/Commands/User/DomainsCommand.php --- a/src/app/Console/Commands/User/DomainsCommand.php +++ b/src/app/Console/Commands/User/DomainsCommand.php @@ -2,40 +2,13 @@ namespace App\Console\Commands\User; -use App\Console\Command; +use App\Console\ObjectRelationListCommand; -class DomainsCommand extends Command +class DomainsCommand extends ObjectRelationListCommand { - /** - * The name and signature of the console command. - * - * @var string - */ - protected $signature = 'user:domains {user}'; - - /** - * The console command description. - * - * @var string - */ - protected $description = "List a user's domains."; - - /** - * Execute the console command. - * - * @return mixed - */ - public function handle() - { - $user = $this->getUser($this->argument('user')); - - if (!$user) { - $this->error("User not found."); - return 1; - } - - foreach ($user->domains() as $domain) { - $this->info($domain->namespace); - } - } + protected $objectClass = \App\User::class; + protected $objectName = 'user'; + protected $objectTitle = 'email'; + protected $objectRelation = 'domains'; + protected $objectRelationArgs = [true, false]; } diff --git a/src/app/Console/Commands/User/GroupsCommand.php b/src/app/Console/Commands/User/GroupsCommand.php new file mode 100644 --- /dev/null +++ b/src/app/Console/Commands/User/GroupsCommand.php @@ -0,0 +1,13 @@ +commandPrefix ? $this->commandPrefix . ":" : "", $this->objectName, - $this->objectRelation, + Str::kebab($this->objectRelation), $this->objectName ); @@ -59,7 +68,7 @@ } if (method_exists($object, $this->objectRelation)) { - $result = call_user_func([$object, $this->objectRelation]); + $result = call_user_func_array([$object, $this->objectRelation], $this->objectRelationArgs); } elseif (property_exists($object, $this->objectRelation)) { $result = $object->{"{$this->objectRelation}"}; } else { @@ -68,7 +77,10 @@ } // Convert query builder into a collection - if ($result instanceof \Illuminate\Database\Eloquent\Relations\Relation) { + if ( + ($result instanceof \Illuminate\Database\Eloquent\Relations\Relation) + || ($result instanceof \Illuminate\Database\Eloquent\Builder) + ) { $result = $result->get(); } diff --git a/src/app/Domain.php b/src/app/Domain.php --- a/src/app/Domain.php +++ b/src/app/Domain.php @@ -440,17 +440,12 @@ return []; } - $entitlements = $wallet->entitlements() + return $wallet->entitlements() ->where('entitleable_type', \App\User::class) - ->where('sku_id', $mailboxSKU->id)->get(); - - $users = []; - - foreach ($entitlements as $entitlement) { - $users[] = $entitlement->entitleable; - } - - return $users; + ->where('sku_id', $mailboxSKU->id) + ->get() + ->pluck('entitleable') + ->all(); } /** diff --git a/src/app/Http/Controllers/API/V4/DomainsController.php b/src/app/Http/Controllers/API/V4/DomainsController.php --- a/src/app/Http/Controllers/API/V4/DomainsController.php +++ b/src/app/Http/Controllers/API/V4/DomainsController.php @@ -25,15 +25,12 @@ { $user = $this->guard()->user(); - $list = \collect($user->domains()) - ->filter(function ($domain) { - return !$domain->isPublic(); - }) + $list = $user->domains(true, false) + ->orderBy('namespace') + ->get() ->map(function ($domain) { return $this->objectToClient($domain); }) - ->sortBy('namespace') - ->values() ->all(); return response()->json($list); diff --git a/src/app/Http/Controllers/API/V4/UsersController.php b/src/app/Http/Controllers/API/V4/UsersController.php --- a/src/app/Http/Controllers/API/V4/UsersController.php +++ b/src/app/Http/Controllers/API/V4/UsersController.php @@ -683,7 +683,7 @@ } // Check if domain exists - $domain = Domain::withEnvTenantContext()->where('namespace', $domain)->first(); + $domain = Domain::withObjectTenantContext($user)->where('namespace', $domain)->first(); if (empty($domain)) { return \trans('validation.domaininvalid'); @@ -700,9 +700,7 @@ } // Check if it is one of domains available to the user - $domains = \collect($user->domains())->pluck('namespace')->all(); - - if (!in_array($domain->namespace, $domains)) { + if (!$user->domains()->where('namespace', $domain->namespace)->exists()) { return \trans('validation.entryexists', ['attribute' => 'domain']); } @@ -760,7 +758,7 @@ } // Check if domain exists - $domain = Domain::withEnvTenantContext()->where('namespace', $domain)->first(); + $domain = Domain::withObjectTenantContext($user)->where('namespace', $domain)->first(); if (empty($domain)) { return \trans('validation.domaininvalid'); @@ -777,9 +775,7 @@ } // Check if it is one of domains available to the user - $domains = \collect($user->domains())->pluck('namespace')->all(); - - if (!in_array($domain->namespace, $domains)) { + if (!$user->domains()->where('namespace', $domain->namespace)->exists()) { return \trans('validation.entryexists', ['attribute' => 'domain']); } diff --git a/src/app/Rules/GroupName.php b/src/app/Rules/GroupName.php --- a/src/app/Rules/GroupName.php +++ b/src/app/Rules/GroupName.php @@ -48,8 +48,8 @@ // Check if the name is unique in the domain // FIXME: Maybe just using the whole groups table would be faster than groups()? $exists = $this->owner->groups() - ->where('groups.name', $name) - ->where('groups.email', 'like', '%@' . $this->domain) + ->where('name', $name) + ->where('email', 'like', '%@' . $this->domain) ->exists(); if ($exists) { diff --git a/src/app/Rules/ResourceName.php b/src/app/Rules/ResourceName.php --- a/src/app/Rules/ResourceName.php +++ b/src/app/Rules/ResourceName.php @@ -53,8 +53,7 @@ } // Check if specified domain belongs to the user - $domains = \collect($this->owner->domains(true, false))->pluck('namespace')->all(); - if (!in_array($this->domain, $domains)) { + if (!$this->owner->domains(true, false)->where('namespace', $this->domain)->exists()) { $this->message = \trans('validation.domaininvalid'); return false; } @@ -62,8 +61,8 @@ // Check if the name is unique in the domain // FIXME: Maybe just using the whole resources table would be faster than resources()? $exists = $this->owner->resources() - ->where('resources.name', $name) - ->where('resources.email', 'like', '%@' . $this->domain) + ->where('name', $name) + ->where('email', 'like', '%@' . $this->domain) ->exists(); if ($exists) { diff --git a/src/app/Rules/SharedFolderName.php b/src/app/Rules/SharedFolderName.php --- a/src/app/Rules/SharedFolderName.php +++ b/src/app/Rules/SharedFolderName.php @@ -53,8 +53,7 @@ } // Check if specified domain belongs to the user - $domains = \collect($this->owner->domains(true, false))->pluck('namespace')->all(); - if (!in_array($this->domain, $domains)) { + if (!$this->owner->domains(true, false)->where('namespace', $this->domain)->exists()) { $this->message = \trans('validation.domaininvalid'); return false; } @@ -62,8 +61,8 @@ // Check if the name is unique in the domain // FIXME: Maybe just using the whole shared_folders table would be faster than sharedFolders()? $exists = $this->owner->sharedFolders() - ->where('shared_folders.name', $name) - ->where('shared_folders.email', 'like', '%@' . $this->domain) + ->where('name', $name) + ->where('email', 'like', '%@' . $this->domain) ->exists(); if ($exists) { diff --git a/src/app/User.php b/src/app/User.php --- a/src/app/User.php +++ b/src/app/User.php @@ -2,9 +2,7 @@ namespace App; -use App\Entitlement; use App\UserAlias; -use App\Sku; use App\Traits\BelongsToTenantTrait; use App\Traits\EntitleableTrait; use App\Traits\UserAliasesTrait; @@ -13,6 +11,7 @@ use App\Traits\SettingsTrait; use App\Wallet; use Illuminate\Database\Eloquent\SoftDeletes; +use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Hash; use Illuminate\Foundation\Auth\User as Authenticatable; use Iatstuti\Database\Support\NullableFields; @@ -265,39 +264,23 @@ * the current user controls but not owns. * @param bool $with_public Include active public domains (for the user tenant). * - * @return Domain[] List of Domain objects + * @return \Illuminate\Database\Eloquent\Builder Query builder */ - public function domains($with_accounts = true, $with_public = true): array + public function domains($with_accounts = true, $with_public = true) { - $domains = []; + $domains = $this->entitleables(Domain::class, $with_accounts); if ($with_public) { - if ($this->tenant_id) { - $domains = Domain::where('tenant_id', $this->tenant_id); - } else { - $domains = Domain::withEnvTenantContext(); - } - - $domains = $domains->whereRaw(sprintf('(type & %s)', Domain::TYPE_PUBLIC)) - ->whereRaw(sprintf('(status & %s)', Domain::STATUS_ACTIVE)) - ->get() - ->all(); - } - - foreach ($this->wallets as $wallet) { - $entitlements = $wallet->entitlements()->where('entitleable_type', Domain::class)->get(); - foreach ($entitlements as $entitlement) { - $domains[] = $entitlement->entitleable; - } - } - - if ($with_accounts) { - foreach ($this->accounts as $wallet) { - $entitlements = $wallet->entitlements()->where('entitleable_type', Domain::class)->get(); - foreach ($entitlements as $entitlement) { - $domains[] = $entitlement->entitleable; + $domains->orWhere(function ($query) { + if (!$this->tenant_id) { + $query->where('tenant_id', $this->tenant_id); + } else { + $query->withEnvTenantContext(); } - } + + $query->whereRaw(sprintf('(domains.type & %s)', Domain::TYPE_PUBLIC)) + ->whereRaw(sprintf('(domains.status & %s)', Domain::STATUS_ACTIVE)); + }); } return $domains; @@ -328,6 +311,36 @@ return false; } + /** + * Return entitleable objects of a specified type controlled by the current user. + * + * @param string $class Object class + * @param bool $with_accounts Include objects assigned to wallets + * the current user controls, but not owns. + * + * @return \Illuminate\Database\Eloquent\Builder Query builder + */ + private function entitleables(string $class, bool $with_accounts = true) + { + $wallets = $this->wallets()->pluck('id')->all(); + + if ($with_accounts) { + $wallets = array_merge($wallets, $this->accounts()->pluck('wallet_id')->all()); + } + + $object = new $class(); + $table = $object->getTable(); + + return $object->select("{$table}.*") + ->whereExists(function ($query) use ($table, $wallets, $class) { + $query->select(DB::raw(1)) + ->from('entitlements') + ->whereColumn('entitleable_id', "{$table}.id") + ->whereIn('entitlements.wallet_id', $wallets) + ->where('entitlements.entitleable_type', $class); + }); + } + /** * Helper to find user by email address, whether it is * main email address, alias or an external email. @@ -374,17 +387,7 @@ */ public function groups($with_accounts = true) { - $wallets = $this->wallets()->pluck('id')->all(); - - if ($with_accounts) { - $wallets = array_merge($wallets, $this->accounts()->pluck('wallet_id')->all()); - } - - return Group::select(['groups.*', 'entitlements.wallet_id']) - ->distinct() - ->join('entitlements', 'entitlements.entitleable_id', '=', 'groups.id') - ->whereIn('entitlements.wallet_id', $wallets) - ->where('entitlements.entitleable_type', Group::class); + return $this->entitleables(Group::class, $with_accounts); } /** @@ -478,17 +481,7 @@ */ public function resources($with_accounts = true) { - $wallets = $this->wallets()->pluck('id')->all(); - - if ($with_accounts) { - $wallets = array_merge($wallets, $this->accounts()->pluck('wallet_id')->all()); - } - - return \App\Resource::select(['resources.*', 'entitlements.wallet_id']) - ->distinct() - ->join('entitlements', 'entitlements.entitleable_id', '=', 'resources.id') - ->whereIn('entitlements.wallet_id', $wallets) - ->where('entitlements.entitleable_type', \App\Resource::class); + return $this->entitleables(\App\Resource::class, $with_accounts); } /** @@ -501,17 +494,7 @@ */ public function sharedFolders($with_accounts = true) { - $wallets = $this->wallets()->pluck('id')->all(); - - if ($with_accounts) { - $wallets = array_merge($wallets, $this->accounts()->pluck('wallet_id')->all()); - } - - return \App\SharedFolder::select(['shared_folders.*', 'entitlements.wallet_id']) - ->distinct() - ->join('entitlements', 'entitlements.entitleable_id', '=', 'shared_folders.id') - ->whereIn('entitlements.wallet_id', $wallets) - ->where('entitlements.entitleable_type', \App\SharedFolder::class); + return $this->entitleables(\App\SharedFolder::class, $with_accounts); } public function senderPolicyFrameworkWhitelist($clientName) @@ -594,17 +577,7 @@ */ public function users($with_accounts = true) { - $wallets = $this->wallets()->pluck('id')->all(); - - if ($with_accounts) { - $wallets = array_merge($wallets, $this->accounts()->pluck('wallet_id')->all()); - } - - return $this->select(['users.*', 'entitlements.wallet_id']) - ->distinct() - ->leftJoin('entitlements', 'entitlements.entitleable_id', '=', 'users.id') - ->whereIn('entitlements.wallet_id', $wallets) - ->where('entitlements.entitleable_type', User::class); + return $this->entitleables(User::class, $with_accounts); } /** diff --git a/src/tests/Feature/Console/User/DomainsTest.php b/src/tests/Feature/Console/User/DomainsTest.php --- a/src/tests/Feature/Console/User/DomainsTest.php +++ b/src/tests/Feature/Console/User/DomainsTest.php @@ -15,13 +15,14 @@ $output = trim(\Artisan::output()); $this->assertSame(1, $code); - $this->assertSame("User not found.", $output); + $this->assertSame("No such user unknown", $output); - $code = \Artisan::call("user:domains john@kolab.org"); + $code = \Artisan::call("user:domains john@kolab.org --attr=namespace"); $output = trim(\Artisan::output()); + $domain = $this->getTestDomain('kolab.org'); + $this->assertSame(0, $code); - $this->assertTrue(strpos($output, "kolab.org") !== false); - $this->assertTrue(strpos($output, \config('app.domain')) !== false); + $this->assertSame("{$domain->id} {$domain->namespace}", $output); } } diff --git a/src/tests/Feature/Console/User/GroupsTest.php b/src/tests/Feature/Console/User/GroupsTest.php new file mode 100644 --- /dev/null +++ b/src/tests/Feature/Console/User/GroupsTest.php @@ -0,0 +1,50 @@ +deleteTestGroup('group-test@kolab.org'); + } + + /** + * {@inheritDoc} + */ + public function tearDown(): void + { + $this->deleteTestGroup('group-test@kolab.org'); + + parent::tearDown(); + } + + /** + * Test command runs + */ + public function testHandle(): void + { + $code = \Artisan::call("user:groups unknown"); + $output = trim(\Artisan::output()); + + $this->assertSame(1, $code); + $this->assertSame("No such user unknown", $output); + + $john = $this->getTestUser('john@kolab.org'); + $group = $this->getTestGroup('group-test@kolab.org'); + $group->assignToWallet($john->wallets->first()); + + $code = \Artisan::call("user:groups john@kolab.org --attr=name"); + $output = trim(\Artisan::output()); + + $this->assertSame(0, $code); + $this->assertSame("{$group->id} {$group->name}", $output); + } +} diff --git a/src/tests/Feature/Console/User/ResourcesTest.php b/src/tests/Feature/Console/User/ResourcesTest.php new file mode 100644 --- /dev/null +++ b/src/tests/Feature/Console/User/ResourcesTest.php @@ -0,0 +1,31 @@ +assertSame(1, $code); + $this->assertSame("No such user unknown", $output); + + $code = \Artisan::call("user:resources john@kolab.org --attr=name"); + $output = trim(\Artisan::output()); + + $resource1 = $this->getTestResource('resource-test1@kolab.org'); + $resource2 = $this->getTestResource('resource-test2@kolab.org'); + + $this->assertSame(0, $code); + $this->assertCount(2, explode("\n", $output)); + $this->assertStringContainsString("{$resource1->id} {$resource1->name}", $output); + $this->assertStringContainsString("{$resource2->id} {$resource2->name}", $output); + } +} diff --git a/src/tests/Feature/Console/User/SharedFoldersTest.php b/src/tests/Feature/Console/User/SharedFoldersTest.php new file mode 100644 --- /dev/null +++ b/src/tests/Feature/Console/User/SharedFoldersTest.php @@ -0,0 +1,31 @@ +assertSame(1, $code); + $this->assertSame("No such user unknown", $output); + + $code = \Artisan::call("user:shared-folders john@kolab.org --attr=name"); + $output = trim(\Artisan::output()); + + $folder1 = $this->getTestSharedFolder('folder-event@kolab.org'); + $folder2 = $this->getTestSharedFolder('folder-contact@kolab.org'); + + $this->assertSame(0, $code); + $this->assertCount(2, explode("\n", $output)); + $this->assertStringContainsString("{$folder1->id} {$folder1->name}", $output); + $this->assertStringContainsString("{$folder2->id} {$folder2->name}", $output); + } +} diff --git a/src/tests/Feature/Console/User/UsersTest.php b/src/tests/Feature/Console/User/UsersTest.php new file mode 100644 --- /dev/null +++ b/src/tests/Feature/Console/User/UsersTest.php @@ -0,0 +1,30 @@ +assertSame(1, $code); + $this->assertSame("No such user unknown", $output); + + $code = \Artisan::call("user:users john@kolab.org --attr=email"); + $output = trim(\Artisan::output()); + + $this->assertSame(0, $code); + $this->assertCount(4, explode("\n", $output)); + $this->assertStringContainsString("john@kolab.org", $output); + $this->assertStringContainsString("ned@kolab.org", $output); + $this->assertStringContainsString("joe@kolab.org", $output); + $this->assertStringContainsString("jack@kolab.org", $output); + } +} 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 @@ -334,7 +334,7 @@ 'type' => Domain::TYPE_PUBLIC, ]); - $domains = collect($user->domains())->pluck('namespace')->all(); + $domains = $user->domains()->pluck('namespace')->all(); $this->assertContains($domain->namespace, $domains); $this->assertContains('kolab.org', $domains); @@ -343,7 +343,7 @@ // include John's domains, kolab.org specifically $user = $this->getTestUser('jack@kolab.org'); - $domains = collect($user->domains())->pluck('namespace')->all(); + $domains = $user->domains()->pluck('namespace')->all(); $this->assertContains($domain->namespace, $domains); $this->assertNotContains('kolab.org', $domains); @@ -353,7 +353,7 @@ $domain->tenant_id = $tenant->id; $domain->save(); - $domains = collect($user->domains())->pluck('namespace')->all(); + $domains = $user->domains()->pluck('namespace')->all(); $this->assertNotContains($domain->namespace, $domains); } @@ -1063,10 +1063,6 @@ $this->assertEquals($joe->id, $users[1]->id); $this->assertEquals($john->id, $users[2]->id); $this->assertEquals($ned->id, $users[3]->id); - $this->assertSame($wallet->id, $users[0]->wallet_id); - $this->assertSame($wallet->id, $users[1]->wallet_id); - $this->assertSame($wallet->id, $users[2]->wallet_id); - $this->assertSame($wallet->id, $users[3]->wallet_id); $users = $jack->users()->orderBy('email')->get();