diff --git a/src/app/Domain.php b/src/app/Domain.php --- a/src/app/Domain.php +++ b/src/app/Domain.php @@ -2,6 +2,7 @@ namespace App; +use App\Wallet; use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\SoftDeletes; @@ -336,4 +337,14 @@ return false; } + + /** + * Returns the wallet by which the domain is controlled + * + * @return \App\Wallet A wallet object + */ + public function wallet(): Wallet + { + return $this->entitlement()->first()->wallet; + } } diff --git a/src/app/Entitlement.php b/src/app/Entitlement.php --- a/src/app/Entitlement.php +++ b/src/app/Entitlement.php @@ -49,6 +49,10 @@ 'description' ]; + protected $casts = [ + 'cost' => 'integer', + ]; + /** * Principally entitleable objects such as 'Domain' or 'User'. * @@ -88,4 +92,12 @@ { return $this->belongsTo('App\Wallet'); } + + /** + * Cost mutator. Make sure cost is integer. + */ + public function setCostAttribute($cost): void + { + $this->attributes['cost'] = round($cost); + } } diff --git a/src/app/Http/Controllers/API/DomainsController.php b/src/app/Http/Controllers/API/DomainsController.php --- a/src/app/Http/Controllers/API/DomainsController.php +++ b/src/app/Http/Controllers/API/DomainsController.php @@ -50,7 +50,7 @@ $domain = Domain::findOrFail($id); // Only owner (or admin) has access to the domain - if (!self::hasAccess($domain)) { + if (!Auth::guard()->user()->canRead($domain)) { return $this->errorResponse(403); } @@ -112,7 +112,7 @@ $domain = Domain::findOrFail($id); // Only owner (or admin) has access to the domain - if (!self::hasAccess($domain)) { + if (!Auth::guard()->user()->canRead($domain)) { return $this->errorResponse(403); } @@ -205,21 +205,4 @@ "@ 3600 TXT \"{$hash_txt}\"", ]; } - - /** - * Check if the current user has access to the domain - * - * @param \App\Domain $domain The domain - * - * @return bool True if current user has access, False otherwise - */ - protected static function hasAccess(Domain $domain): bool - { - $user = Auth::guard()->user(); - $entitlement = $domain->entitlement()->first(); - - // TODO: Admins - - return $entitlement && $entitlement->owner_id == $user->id; - } } diff --git a/src/app/Http/Controllers/API/UsersController.php b/src/app/Http/Controllers/API/UsersController.php --- a/src/app/Http/Controllers/API/UsersController.php +++ b/src/app/Http/Controllers/API/UsersController.php @@ -46,27 +46,45 @@ } /** - * Display a listing of the resources. + * Delete a user. * - * The user themself, and other user entitlements. + * @param int $id User identifier * - * @return \Illuminate\Http\JsonResponse + * @return \Illuminate\Http\JsonResponse The response */ - public function index() + public function destroy($id) { - $user = Auth::user(); + $user = User::find($id); - if (!$user) { - return response()->json(['error' => 'unauthorized'], 401); + if (empty($user)) { + return $this->errorResponse(404); } - $result = [$user]; + // User can't remove himself until he's the controller + if (!$this->guard()->user()->canDelete($user)) { + return $this->errorResponse(403); + } - $user->entitlements()->each( - function ($entitlement) { - $result[] = User::find($entitlement->user_id); - } - ); + $user->delete(); + + return response()->json([ + 'status' => 'success', + 'message' => __('app.user-delete-success'), + ]); + } + + /** + * Listing of users. + * + * The user-entitlements billed to the current user wallet(s) + * + * @return \Illuminate\Http\JsonResponse + */ + public function index() + { + $user = $this->guard()->user(); + + $result = $user->users()->orderBy('email')->get(); return response()->json($result); } @@ -166,16 +184,16 @@ */ public function show($id) { - if (!$this->hasAccess($id)) { - return $this->errorResponse(403); - } - $user = User::find($id); if (empty($user)) { return $this->errorResponse(404); } + if (!$this->guard()->user()->canRead($user)) { + return $this->errorResponse(403); + } + $response = $this->userResponse($user); return response()->json($response); @@ -249,7 +267,9 @@ */ public function store(Request $request) { - if ($this->guard()->user()->controller()->id !== $this->guard()->user()->id) { + $current_user = $this->guard()->user(); + + if ($current_user->wallet()->owner->id != $current_user->id) { return $this->errorResponse(403); } @@ -300,16 +320,17 @@ */ public function update(Request $request, $id) { - if (!$this->hasAccess($id)) { - return $this->errorResponse(403); - } - $user = User::find($id); if (empty($user)) { return $this->errorResponse(404); } + // TODO: Decide what attributes a user can change on his own profile + if (!$this->guard()->user()->canUpdate($user)) { + return $this->errorResponse(403); + } + if ($error_response = $this->validateUserRequest($request, $user, $settings)) { return $error_response; } @@ -349,23 +370,6 @@ return Auth::guard(); } - /** - * Check if the current user has access to the specified user - * - * @param int $user_id User identifier - * - * @return bool True if current user has access, False otherwise - */ - protected function hasAccess($user_id): bool - { - $current_user = $this->guard()->user(); - - // TODO: Admins, other users - // FIXME: This probably should be some kind of middleware/guard - - return $current_user->id == $user_id; - } - /** * Create a response data array for specified user. * @@ -394,6 +398,11 @@ // Status info $response['statusInfo'] = self::statusInfo($user); + // Information about wallets and accounts for access checks + $response['wallets'] = $user->wallets->toArray(); + $response['accounts'] = $user->accounts->toArray(); + $response['wallet'] = $user->wallet()->toArray(); + return $response; } @@ -432,7 +441,7 @@ $errors = $v->errors()->toArray(); } - $controller = $user ? $user->controller() : $this->guard()->user(); + $controller = $user ? $user->wallet()->owner : $this->guard()->user(); // For new user validate email address if (empty($user)) { diff --git a/src/app/Jobs/DomainDelete.php b/src/app/Jobs/DomainDelete.php --- a/src/app/Jobs/DomainDelete.php +++ b/src/app/Jobs/DomainDelete.php @@ -27,13 +27,13 @@ /** * Create a new job instance. * - * @param Domain $domain The domain to delete. + * @param int $domain_id The ID of the domain to delete. * * @return void */ - public function __construct(Domain $domain) + public function __construct(int $domain_id) { - $this->domain = $domain; + $this->domain = Domain::withTrashed()->find($domain_id); } /** @@ -43,6 +43,11 @@ */ public function handle() { - LDAP::deleteDomain($this->domain); + if (!$this->domain->isDeleted()) { + LDAP::deleteDomain($this->domain); + + $this->domain->status |= Domain::STATUS_DELETED; + $this->domain->save(); + } } } diff --git a/src/app/Jobs/UserDelete.php b/src/app/Jobs/UserDelete.php --- a/src/app/Jobs/UserDelete.php +++ b/src/app/Jobs/UserDelete.php @@ -43,6 +43,11 @@ */ public function handle() { - LDAP::deleteUser($this->user); + if (!$this->user->isDeleted()) { + LDAP::deleteUser($this->user); + + $this->user->status |= User::STATUS_DELETED; + $this->user->save(); + } } } diff --git a/src/app/Observers/DomainObserver.php b/src/app/Observers/DomainObserver.php --- a/src/app/Observers/DomainObserver.php +++ b/src/app/Observers/DomainObserver.php @@ -3,6 +3,7 @@ namespace App\Observers; use App\Domain; +use Illuminate\Support\Facades\DB; class DomainObserver { @@ -39,35 +40,47 @@ \App\Jobs\DomainCreate::dispatch($domain); } + /** + * Handle the domain "deleting" event. + * + * @param \App\Domain $domain The domain. + * + * @return void + */ public function deleting(Domain $domain) { - // + // Entitlements do not have referential integrity on the entitled object, so this is our + // way of doing an onDelete('cascade') without the foreign key. + \App\Entitlement::where('entitleable_id', $domain->id) + ->where('entitleable_type', Domain::class) + ->delete(); } /** - * Handle the domain "updated" event. + * Handle the domain "deleted" event. * * @param \App\Domain $domain The domain. * * @return void */ - public function updated(Domain $domain) + public function deleted(Domain $domain) { - // + \App\Jobs\DomainDelete::dispatch($domain->id); } /** - * Handle the domain "deleted" event. + * Handle the domain "updated" event. * * @param \App\Domain $domain The domain. * * @return void */ - public function deleted(Domain $domain) + public function updated(Domain $domain) { - \App\Jobs\DomainDelete::dispatch($domain); + // } + /** * Handle the domain "restored" event. * diff --git a/src/app/Observers/UserObserver.php b/src/app/Observers/UserObserver.php --- a/src/app/Observers/UserObserver.php +++ b/src/app/Observers/UserObserver.php @@ -2,7 +2,10 @@ namespace App\Observers; +use App\Entitlement; +use App\Domain; use App\User; +use Illuminate\Support\Facades\DB; class UserObserver { @@ -79,15 +82,57 @@ */ public function deleting(User $user) { + // TODO: Especially in tests we're doing delete() on a already deleted user. + // Should we escape here - for performance reasons? + // TODO: I think all of this should use database transactions + // Entitlements do not have referential integrity on the entitled object, so this is our // way of doing an onDelete('cascade') without the foreign key. - $entitlements = \App\Entitlement::where('entitleable_id', $user->id) - ->where('entitleable_type', \App\User::class)->get(); + Entitlement::where('entitleable_id', $user->id) + ->where('entitleable_type', User::class) + ->delete(); + + // Remove owned users/domains + $wallets = $user->wallets()->pluck('id')->all(); + $assignments = Entitlement::whereIn('wallet_id', $wallets)->get(); + $users = []; + $domains = []; + $entitlements = []; - foreach ($entitlements as $entitlement) { - $entitlement->delete(); + foreach ($assignments as $entitlement) { + if ($entitlement->entitleable_type == Domain::class) { + $domains[] = $entitlement->entitleable_id; + } elseif ($entitlement->entitleable_type == User::class && $entitlement->entitleable_id != $user->id) { + $users[] = $entitlement->entitleable_id; + } else { + $entitlements[] = $entitlement->id; + } + } + + $users = array_unique($users); + $domains = array_unique($domains); + + // Note: Domains/users need to be deleted one by one to make sure + // events are fired and observers can do the proper cleanup. + // Entitlements have no delete event handlers as for now. + if (!empty($users)) { + foreach (User::whereIn('id', $users)->get() as $_user) { + $_user->delete(); + } } + if (!empty($domains)) { + foreach (Domain::whereIn('id', $domains)->get() as $_domain) { + $_domain->delete(); + } + } + + if (!empty($entitlements)) { + Entitlement::whereIn('id', $entitlements)->delete(); + } + + // FIXME: What do we do with user wallets? + \App\Jobs\UserDelete::dispatch($user->id); } diff --git a/src/app/Providers/AppServiceProvider.php b/src/app/Providers/AppServiceProvider.php --- a/src/app/Providers/AppServiceProvider.php +++ b/src/app/Providers/AppServiceProvider.php @@ -2,7 +2,6 @@ namespace App\Providers; -use Illuminate\Support\Facades\File; use Illuminate\Support\Facades\DB; use Illuminate\Support\ServiceProvider; 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,11 @@ namespace App; +use App\Entitlement; use App\UserAlias; use App\Traits\UserAliasesTrait; use App\Traits\UserSettingsTrait; +use App\Wallet; use Illuminate\Notifications\Notifiable; use Illuminate\Contracts\Auth\MustVerifyEmail; use Illuminate\Database\Eloquent\SoftDeletes; @@ -89,6 +91,8 @@ /** * Any wallets on which this user is a controller. * + * This does not include wallets owned by the user. + * * @return \Illuminate\Database\Eloquent\Relations\BelongsToMany */ public function accounts() @@ -145,26 +149,6 @@ return $user; } - /** - * Returns user controlling the current user (or self when it's the account owner) - * - * @return \App\User A user object - */ - public function controller(): User - { - // FIXME: This is most likely not the best way to do this - $entitlement = \App\Entitlement::where([ - 'entitleable_id' => $this->id, - 'entitleable_type' => User::class - ])->first(); - - if ($entitlement && $entitlement->owner_id != $this->id) { - return $entitlement->owner; - } - - return $this; - } - public function assignPlan($plan, $domain = null) { $this->setSetting('plan_id', $plan->id); @@ -178,6 +162,69 @@ } } + /** + * Check if current user can delete another object. + * + * @param \App\User|\App\Domain $object A user|domain object + * + * @return bool True if he can, False otherwise + */ + public function canDelete($object): bool + { + if (!method_exists($object, 'wallet')) { + return false; + } + + $wallet = $object->wallet(); + + // TODO: For now controller can delete/update the account owner, + // this may change in future, controllers are not 0-regression feature + + return $this->wallets->contains($wallet) || $this->accounts->contains($wallet); + } + + /** + * Check if current user can read data of another object. + * + * @param \App\User|\App\Domain $object A user|domain object + * + * @return bool True if he can, False otherwise + */ + public function canRead($object): bool + { + if (!method_exists($object, 'wallet')) { + return false; + } + + if ($object instanceof User && $this->id == $object->id) { + return true; + } + + $wallet = $object->wallet(); + + return $this->wallets->contains($wallet) || $this->accounts->contains($wallet); + } + + /** + * Check if current user can update data of another object. + * + * @param \App\User|\App\Domain $object A user|domain object + * + * @return bool True if he can, False otherwise + */ + public function canUpdate($object): bool + { + if (!method_exists($object, 'wallet')) { + return false; + } + + if ($object instanceof User && $this->id == $object->id) { + return true; + } + + return $this->canDelete($object); + } + /** * List the domains to which this user is entitled. * @@ -361,6 +408,27 @@ return $this->hasMany('App\UserSetting', 'user_id'); } + /** + * Return users controlled by the current user. + * + * Users assigned to wallets the current user controls or owns. + * + * @return \Illuminate\Database\Eloquent\Builder Query builder + */ + public function users() + { + $wallets = array_merge( + $this->wallets()->pluck('id')->all(), + $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', 'App\User'); + } + /** * Verification codes for this user. * @@ -371,6 +439,20 @@ return $this->hasMany('App\VerificationCode', 'user_id', 'id'); } + /** + * Returns the wallet by which the user is controlled + * + * @return \App\Wallet A wallet object + */ + public function wallet(): Wallet + { + $entitlement = $this->entitlement()->first(); + + // TODO: No entitlement should not happen, but in tests we have + // such cases, so we fallback to the user's wallet in this case + return $entitlement ? $entitlement->wallet : $this->wallets()->first(); + } + /** * Wallets this user owns. * diff --git a/src/composer.json b/src/composer.json --- a/src/composer.json +++ b/src/composer.json @@ -22,6 +22,7 @@ "kolab/net_ldap3": "dev-master", "laravel/framework": "6.*", "laravel/tinker": "^1.0", + "morrislaptop/laravel-queue-clear": "^1.2", "silviolleite/laravelpwa": "^1.0", "spatie/laravel-translatable": "^4.2", "swooletw/laravel-swoole": "^2.6", diff --git a/src/database/seeds/UserSeeder.php b/src/database/seeds/UserSeeder.php --- a/src/database/seeds/UserSeeder.php +++ b/src/database/seeds/UserSeeder.php @@ -6,6 +6,7 @@ use App\Sku; use Carbon\Carbon; use Illuminate\Database\Seeder; +use App\Wallet; // phpcs:ignore class UserSeeder extends Seeder @@ -87,6 +88,29 @@ $entitlement->save(); } + $ned = User::create( + [ + 'name' => 'Edward Flanders', + 'email' => 'ned@kolab.org', + 'password' => 'simple123', + 'email_verified_at' => now() + ] + ); + + $ned->setSettings( + [ + 'first_name' => 'Edward', + 'last_name' => 'Flanders', + 'currency' => 'USD', + 'country' => 'US' + ] + ); + + $john->assignPackage($package_kolab, $ned); + + // Ned is a controller on Jack's wallet + $john->wallets()->first()->addController($ned); + factory(User::class, 10)->create(); } } diff --git a/src/phpunit.xml b/src/phpunit.xml --- a/src/phpunit.xml +++ b/src/phpunit.xml @@ -9,10 +9,6 @@ processIsolation="false" stopOnFailure="false"> - - tests/Browser - - tests/Unit @@ -20,6 +16,10 @@ tests/Feature + + + tests/Browser + diff --git a/src/resources/js/app.js b/src/resources/js/app.js --- a/src/resources/js/app.js +++ b/src/resources/js/app.js @@ -117,6 +117,23 @@ $(form).find('.is-invalid').removeClass('is-invalid') $(form).find('.invalid-feedback').remove() }, + isController(wallet_id) { + if (wallet_id && store.state.authInfo) { + let i + for (i = 0; i < store.state.authInfo.wallets.length; i++) { + if (wallet_id == store.state.authInfo.wallets[i].id) { + return true + } + } + for (i = 0; i < store.state.authInfo.accounts.length; i++) { + if (wallet_id == store.state.authInfo.accounts[i].id) { + return true + } + } + } + + return false + }, // Set user state to "logged in" loginUser(token, dashboard) { store.commit('logoutUser') // destroy old state data diff --git a/src/resources/lang/en/app.php b/src/resources/lang/en/app.php --- a/src/resources/lang/en/app.php +++ b/src/resources/lang/en/app.php @@ -20,7 +20,8 @@ 'process-domain-verified' => 'Custom domain verified', 'process-domain-confirmed' => 'Custom domain ownership verified', - 'domain-verify-success' => 'Domain verified successfully', - 'user-update-success' => 'User data updated successfully', - 'user-create-success' => 'User created successfully', + 'domain-verify-success' => 'Domain verified successfully.', + 'user-update-success' => 'User data updated successfully.', + 'user-create-success' => 'User created successfully.', + 'user-delete-success' => 'User deleted successfully.', ]; diff --git a/src/resources/lang/en/auth.php b/src/resources/lang/en/auth.php --- a/src/resources/lang/en/auth.php +++ b/src/resources/lang/en/auth.php @@ -14,6 +14,6 @@ */ 'failed' => 'Invalid username or password.', - 'throttle' => 'Too many login attempts. Please try again in :seconds seconds', - 'logoutsuccess' => 'Successfully logged out', + 'throttle' => 'Too many login attempts. Please try again in :seconds seconds.', + 'logoutsuccess' => 'Successfully logged out.', ]; diff --git a/src/resources/vue/components/User/Info.vue b/src/resources/vue/components/User/Info.vue --- a/src/resources/vue/components/User/Info.vue +++ b/src/resources/vue/components/User/Info.vue @@ -108,7 +108,7 @@ // on new user redirect to users list if (this.user_id === 'new') { - this.$route.push({ name: 'users' }) + this.$router.push({ name: 'users' }) } }) } diff --git a/src/resources/vue/components/User/List.vue b/src/resources/vue/components/User/List.vue --- a/src/resources/vue/components/User/List.vue +++ b/src/resources/vue/components/User/List.vue @@ -15,15 +15,42 @@ - - {{ user.email }} - + + + {{ user.email }} + + + + + + @@ -31,7 +58,8 @@ export default { data() { return { - users: [] + users: [], + current_user: null } }, created() { @@ -40,6 +68,47 @@ this.users = response.data }) .catch(this.$root.errorHandler) + }, + methods: { + deleteUser(id) { + let dialog = $('#delete-warning').modal('hide') + + // Delete the user from the confirm dialog + if (!id && this.current_user) { + id = this.current_user.id + axios.delete('/api/v4/users/' + id) + .then(response => { + if (response.data.status == 'success') { + this.$toastr('success', response.data.message) + $('#user' + id).remove() + } + }) + + return + } + + + // Deleting self, redirect to /profile/delete page + if (id == this.$store.state.authInfo.id) { + this.$router.push({ name: 'profile-delete' }) + return + } + + // Display the warning + if (this.current_user = this.getUser(id)) { + dialog.find('.modal-title').text('Delete ' + this.current_user.email) + dialog.on('shown.bs.modal', () => { + dialog.find('button.modal-cancel').focus() + }).modal() + } + }, + getUser(id) { + for (let i = 0; i < this.users.length; i++) { + if (this.users[i].id == id) { + return this.users[i] + } + } + } } } diff --git a/src/resources/vue/components/User/Profile.vue b/src/resources/vue/components/User/Profile.vue --- a/src/resources/vue/components/User/Profile.vue +++ b/src/resources/vue/components/User/Profile.vue @@ -56,7 +56,9 @@ - + + Delete account @@ -69,10 +71,12 @@ data() { return { profile: {}, + wallet_id: null, countries: window.config.countries } }, created() { + this.wallet_id = this.$store.state.authInfo.wallet.id this.profile = this.$store.state.authInfo.settings }, mounted() { diff --git a/src/resources/vue/components/User/ProfileDelete.vue b/src/resources/vue/components/User/ProfileDelete.vue new file mode 100644 --- /dev/null +++ b/src/resources/vue/components/User/ProfileDelete.vue @@ -0,0 +1,47 @@ + + + diff --git a/src/resources/vue/js/routes.js b/src/resources/vue/js/routes.js --- a/src/resources/vue/js/routes.js +++ b/src/resources/vue/js/routes.js @@ -14,6 +14,7 @@ import UserInfoComponent from '../components/User/Info' import UserListComponent from '../components/User/List' import UserProfileComponent from '../components/User/Profile' +import UserProfileDeleteComponent from '../components/User/ProfileDelete' import store from './store' @@ -61,6 +62,12 @@ component: UserProfileComponent, meta: { requiresAuth: true } }, + { + path: '/profile/delete', + name: 'profile-delete', + component: UserProfileDeleteComponent, + meta: { requiresAuth: true } + }, { path: '/signup/:param?', name: 'signup', diff --git a/src/tests/Browser.php b/src/tests/Browser.php --- a/src/tests/Browser.php +++ b/src/tests/Browser.php @@ -4,7 +4,7 @@ use Facebook\WebDriver\WebDriverKeys; use PHPUnit\Framework\Assert; -use Tests\Browser\Components; +use Tests\Browser\Components\Error; /** * Laravel Dusk Browser extensions @@ -32,6 +32,18 @@ return $this; } + /** + * Assert specified error page is displayed. + */ + public function assertErrorPage(int $error_code) + { + $this->with(new Error($error_code), function ($browser) { + // empty, assertions will be made by the Error component itself + }); + + return $this; + } + /** * Assert that the given element has specified class assigned. */ @@ -45,6 +57,16 @@ return $this; } + /** + * Remove all toast messages + */ + public function clearToasts() + { + $this->script("jQuery('.toast-container > *').remove()"); + + return $this; + } + /** * Check if in Phone mode */ diff --git a/src/tests/Browser/Components/Error.php b/src/tests/Browser/Components/Dialog.php copy from src/tests/Browser/Components/Error.php copy to src/tests/Browser/Components/Dialog.php --- a/src/tests/Browser/Components/Error.php +++ b/src/tests/Browser/Components/Dialog.php @@ -5,18 +5,14 @@ use Laravel\Dusk\Component as BaseComponent; use PHPUnit\Framework\Assert as PHPUnit; -class Error extends BaseComponent +class Dialog extends BaseComponent { - protected $code; - protected $message; - protected $messages_map = [ - 404 => 'Not Found' - ]; + protected $selector; - public function __construct($code) + + public function __construct($selector) { - $this->code = $code; - $this->message = $this->messages_map[$code]; + $this->selector = trim($selector); } /** @@ -26,7 +22,7 @@ */ public function selector() { - return '#error-page'; + return $this->selector; } /** @@ -38,9 +34,7 @@ */ public function assert($browser) { - $browser->waitFor($this->selector()) - ->assertSeeIn('@code', $this->code) - ->assertSeeIn('@message', $this->message); + $browser->waitFor($this->selector() . '.modal.show'); } /** @@ -50,11 +44,11 @@ */ public function elements() { - $selector = $this->selector(); - return [ - '@code' => "$selector .code", - '@message' => "$selector .message", + '@title' => '.modal-header .modal-title', + '@body' => '.modal-body', + '@button-action' => '.modal-footer button.modal-action', + '@button-cancel' => '.modal-footer button.modal-cancel', ]; } } diff --git a/src/tests/Browser/Components/Error.php b/src/tests/Browser/Components/Error.php --- a/src/tests/Browser/Components/Error.php +++ b/src/tests/Browser/Components/Error.php @@ -10,7 +10,12 @@ protected $code; protected $message; protected $messages_map = [ - 404 => 'Not Found' + 400 => "Bad request", + 401 => "Unauthorized", + 403 => "Access denied", + 404 => "Not Found", + 405 => "Method not allowed", + 500 => "Internal server error", ]; public function __construct($code) diff --git a/src/tests/Browser/DomainTest.php b/src/tests/Browser/DomainTest.php --- a/src/tests/Browser/DomainTest.php +++ b/src/tests/Browser/DomainTest.php @@ -5,7 +5,6 @@ use App\Domain; use App\User; use Tests\Browser; -use Tests\Browser\Components\Error; use Tests\Browser\Components\Toast; use Tests\Browser\Pages\Dashboard; use Tests\Browser\Pages\DomainInfo; @@ -41,11 +40,7 @@ $browser->visit('/domain/123') ->on(new Home()) ->submitLogon('john@kolab.org', 'simple123') - // TODO: the check below could look simpler, but we can't - // just remove the callback argument. We'll create - // Browser wrapper in future, then we could create expectError() method - ->with(new Error('404'), function ($browser) { - }); + ->assertErrorPage(404); }); } @@ -131,5 +126,7 @@ $browser->assertSeeIn('pre', 'kolab.org'); }); }); + + // TODO: Test domains list acting as Ned (John's "delegatee") } } diff --git a/src/tests/Browser/Pages/Home.php b/src/tests/Browser/Pages/Home.php --- a/src/tests/Browser/Pages/Home.php +++ b/src/tests/Browser/Pages/Home.php @@ -25,7 +25,7 @@ */ public function assert($browser) { - $browser->assertPathIs($this->url()) + $browser->waitForLocation($this->url()) ->assertVisible('form.form-signin'); } diff --git a/src/tests/Browser/PasswordResetTest.php b/src/tests/Browser/PasswordResetTest.php --- a/src/tests/Browser/PasswordResetTest.php +++ b/src/tests/Browser/PasswordResetTest.php @@ -266,8 +266,7 @@ $browser->waitUntilMissing('@step3'); // At this point we should be auto-logged-in to dashboard - $dashboard = new Dashboard(); - $dashboard->assert($browser); + $browser->on(new Dashboard()); // FIXME: Is it enough to be sure user is logged in? }); diff --git a/src/tests/Browser/SignupTest.php b/src/tests/Browser/SignupTest.php --- a/src/tests/Browser/SignupTest.php +++ b/src/tests/Browser/SignupTest.php @@ -485,7 +485,7 @@ }); // Submit invalid domain - $browser->with('@step3', function ($step) use ($browser) { + $browser->with('@step3', function ($step) { $step->type('#signup_domain', 'user-domain-signup.com') ->click('[type=submit]'); }); diff --git a/src/tests/Browser/UserProfileTest.php b/src/tests/Browser/UserProfileTest.php --- a/src/tests/Browser/UserProfileTest.php +++ b/src/tests/Browser/UserProfileTest.php @@ -31,6 +31,7 @@ parent::setUp(); User::where('email', 'john@kolab.org')->first()->setSettings($this->profile); + $this->deleteTestUser('profile-delete@kolabnow.com'); } /** @@ -39,6 +40,7 @@ public function tearDown(): void { User::where('email', 'john@kolab.org')->first()->setSettings($this->profile); + $this->deleteTestUser('profile-delete@kolabnow.com'); parent::tearDown(); } @@ -66,6 +68,7 @@ ->assertSeeIn('@links .link-profile', 'Your profile') ->click('@links .link-profile') ->on(new UserProfile()) + ->assertSeeIn('#user-profile .button-delete', 'Delete account') ->whenAvailable('@form', function (Browser $browser) { // Assert form content $browser->assertFocused('div.row:nth-child(1) input') @@ -103,7 +106,6 @@ ->closeToast(); }); - // Test error handling $browser->with('@form', function (Browser $browser) { $browser->type('#phone', 'aaaaaa') @@ -124,4 +126,74 @@ }); }); } + + /** + * Test profile of non-controller user + */ + public function testProfileNonController(): void + { + // Test acting as non-controller + $this->browse(function (Browser $browser) { + $browser->visit('/logout') + ->visit(new Home()) + ->submitLogon('jack@kolab.org', 'simple123', true) + ->on(new Dashboard()) + ->assertSeeIn('@links .link-profile', 'Your profile') + ->click('@links .link-profile') + ->on(new UserProfile()) + ->assertMissing('#user-profile .button-delete') + ->whenAvailable('@form', function (Browser $browser) { + // TODO: decide on what fields the non-controller user should be able + // to see/change + }); + + // Test that /profile/delete page is not accessible + $browser->visit('/profile/delete') + ->assertErrorPage(403); + }); + } + + /** + * Test profile delete page + */ + public function testProfileDelete(): void + { + $user = $this->getTestUser('profile-delete@kolabnow.com', ['password' => 'simple123']); + + $this->browse(function (Browser $browser) use ($user) { + $browser->visit('/logout') + ->on(new Home()) + ->submitLogon('profile-delete@kolabnow.com', 'simple123', true) + ->on(new Dashboard()) + ->clearToasts() + ->assertSeeIn('@links .link-profile', 'Your profile') + ->click('@links .link-profile') + ->on(new UserProfile()) + ->click('#user-profile .button-delete') + ->waitForLocation('/profile/delete') + ->assertSeeIn('#user-delete .card-title', 'Delete this account?') + ->assertSeeIn('#user-delete .button-cancel', 'Cancel') + ->assertSeeIn('#user-delete .card-text', 'This operation is irreversible') + ->assertFocused('#user-delete .button-cancel') + ->click('#user-delete .button-cancel') + ->waitForLocation('/profile') + ->on(new UserProfile()); + + // Test deleting the user + $browser->click('#user-profile .button-delete') + ->waitForLocation('/profile/delete') + ->click('#user-delete .button-delete') + ->waitForLocation('/login') + ->with(new Toast(Toast::TYPE_SUCCESS), function (Browser $browser) { + $browser->assertToastTitle('') + ->assertToastMessage('User deleted successfully.') + ->closeToast(); + }); + + $this->assertTrue($user->fresh()->trashed()); + }); + } + + // TODO: Test that Ned (John's "delegatee") can delete himself + // TODO: Test that Ned (John's "delegatee") can/can't delete John ? } diff --git a/src/tests/Browser/UsersTest.php b/src/tests/Browser/UsersTest.php --- a/src/tests/Browser/UsersTest.php +++ b/src/tests/Browser/UsersTest.php @@ -5,6 +5,7 @@ use App\User; use App\UserAlias; use Tests\Browser; +use Tests\Browser\Components\Dialog; use Tests\Browser\Components\ListInput; use Tests\Browser\Components\Toast; use Tests\Browser\Pages\Dashboard; @@ -28,8 +29,7 @@ { parent::setUp(); - // TODO: Use TestCase::deleteTestUser() - User::withTrashed()->where('email', 'john.rambo@kolab.org')->forceDelete(); + $this->deleteTestUser('julia.roberts@kolab.org'); $john = User::where('email', 'john@kolab.org')->first(); $john->setSettings($this->profile); @@ -42,8 +42,7 @@ */ public function tearDown(): void { - // TODO: Use TestCase::deleteTestUser() - User::withTrashed()->where('email', 'john.rambo@kolab.org')->forceDelete(); + $this->deleteTestUser('julia.roberts@kolab.org'); $john = User::where('email', 'john@kolab.org')->first(); $john->setSettings($this->profile); @@ -75,8 +74,6 @@ $this->browse(function (Browser $browser) { $browser->visit('/users')->on(new Home()); }); - - // TODO: Test that jack@kolab.org can't access this page } /** @@ -92,9 +89,14 @@ ->assertSeeIn('@links .link-users', 'User accounts') ->click('@links .link-users') ->on(new UserList()) - ->whenAvailable('@table', function ($browser) { - $this->assertCount(1, $browser->elements('tbody tr')); - $browser->assertSeeIn('tbody tr td a', 'john@kolab.org'); + ->whenAvailable('@table', function (Browser $browser) { + $browser->assertElementsCount('tbody tr', 3) + ->assertSeeIn('tbody tr:nth-child(1) a', 'jack@kolab.org') + ->assertSeeIn('tbody tr:nth-child(2) a', 'john@kolab.org') + ->assertSeeIn('tbody tr:nth-child(3) a', 'ned@kolab.org') + ->assertVisible('tbody tr:nth-child(1) button.button-delete') + ->assertVisible('tbody tr:nth-child(2) button.button-delete') + ->assertVisible('tbody tr:nth-child(3) button.button-delete'); }); }); } @@ -108,7 +110,7 @@ { $this->browse(function (Browser $browser) { $browser->on(new UserList()) - ->click('@table tr:first-child a') + ->click('@table tr:nth-child(2) a') ->on(new UserInfo()) ->assertSeeIn('#user-info .card-title', 'User account') ->with('@form', function (Browser $browser) { @@ -120,7 +122,7 @@ ->assertValue('div.row:nth-child(2) input[type=text]', $this->profile['last_name']) ->assertSeeIn('div.row:nth-child(3) label', 'Email') ->assertValue('div.row:nth-child(3) input[type=text]', 'john@kolab.org') -//TODO ->assertDisabled('div.row:nth-child(3) input') + ->assertDisabled('div.row:nth-child(3) input[type=text]') ->assertSeeIn('div.row:nth-child(4) label', 'Email aliases') ->assertVisible('div.row:nth-child(4) .listinput-widget') ->with(new ListInput('#aliases'), function (Browser $browser) { @@ -226,7 +228,7 @@ ->assertValue('div.row:nth-child(2) input[type=text]', '') ->assertSeeIn('div.row:nth-child(3) label', 'Email') ->assertValue('div.row:nth-child(3) input[type=text]', '') - ->assertEnabled('div.row:nth-child(3) input') + ->assertEnabled('div.row:nth-child(3) input[type=text]') ->assertSeeIn('div.row:nth-child(4) label', 'Email aliases') ->assertVisible('div.row:nth-child(4) .listinput-widget') ->with(new ListInput('#aliases'), function (Browser $browser) { @@ -263,7 +265,7 @@ // Test form error handling (aliases) $browser->with('@form', function (Browser $browser) { - $browser->type('#email', 'john.rambo@kolab.org') + $browser->type('#email', 'julia.roberts@kolab.org') ->type('#password_confirmation', 'simple123') ->with(new ListInput('#aliases'), function (Browser $browser) { $browser->addListEntry('invalid address'); @@ -285,7 +287,7 @@ $browser->with('@form', function (Browser $browser) { $browser->with(new ListInput('#aliases'), function (Browser $browser) { $browser->removeListEntry(1) - ->addListEntry('john.rambo2@kolab.org'); + ->addListEntry('julia.roberts2@kolab.org'); }) ->click('button[type=submit]'); }) @@ -293,13 +295,105 @@ $browser->assertToastTitle('') ->assertToastMessage('User created successfully') ->closeToast(); - }); - - // TODO: assert redirect to users list + }) + // check redirection to users list + ->waitForLocation('/users') + ->on(new UserList()) + ->whenAvailable('@table', function (Browser $browser) { +// TODO: This will not work until we handle entitlements on user creation +// $browser->assertElementsCount('tbody tr', 3) +// ->assertSeeIn('tbody tr:nth-child(3) a', 'julia.roberts@kolab.org'); + }); - $john = User::where('email', 'john.rambo@kolab.org')->first(); - $alias = UserAlias::where('user_id', $john->id)->where('alias', 'john.rambo2@kolab.org')->first(); + $julia = User::where('email', 'julia.roberts@kolab.org')->first(); + $alias = UserAlias::where('user_id', $julia->id)->where('alias', 'julia.roberts2@kolab.org')->first(); $this->assertTrue(!empty($alias)); }); } + + /** + * Test user delete + * + * @depends testNewUser + */ + public function testDeleteUser(): void + { + // First create a new user + $john = $this->getTestUser('john@kolab.org'); + $julia = $this->getTestUser('julia.roberts@kolab.org'); + $package_kolab = \App\Package::where('title', 'kolab')->first(); + $john->assignPackage($package_kolab, $julia); + + // Test deleting non-controller user + $this->browse(function (Browser $browser) { + $browser->visit(new UserList()) + ->whenAvailable('@table', function (Browser $browser) { + $browser->assertElementsCount('tbody tr', 4) + ->assertSeeIn('tbody tr:nth-child(3) a', 'julia.roberts@kolab.org') + ->click('tbody tr:nth-child(3) button.button-delete'); + }) + ->with(new Dialog('#delete-warning'), function (Browser $browser) { + $browser->assertSeeIn('@title', 'Delete julia.roberts@kolab.org') + ->assertFocused('@button-cancel') + ->assertSeeIn('@button-cancel', 'Cancel') + ->assertSeeIn('@button-action', 'Delete') + ->click('@button-cancel'); + }) + ->whenAvailable('@table', function (Browser $browser) { + $browser->click('tbody tr:nth-child(3) button.button-delete'); + }) + ->with(new Dialog('#delete-warning'), function (Browser $browser) { + $browser->click('@button-action'); + }) + ->with(new Toast(Toast::TYPE_SUCCESS), function (Browser $browser) { + $browser->assertToastTitle('') + ->assertToastMessage('User deleted successfully') + ->closeToast(); + }) + ->with('@table', function (Browser $browser) { + $browser->assertElementsCount('tbody tr', 3) + ->assertSeeIn('tbody tr:nth-child(1) a', 'jack@kolab.org') + ->assertSeeIn('tbody tr:nth-child(2) a', 'john@kolab.org') + ->assertSeeIn('tbody tr:nth-child(3) a', 'ned@kolab.org'); + }); + + $julia = User::where('email', 'julia.roberts@kolab.org')->first(); + $this->assertTrue(empty($julia)); + + // Test clicking Delete on the controller record redirects to /profile/delete + $browser + ->with('@table', function (Browser $browser) { + $browser->click('tbody tr:nth-child(2) button.button-delete'); + }) + ->waitForLocation('/profile/delete'); + }); + + // Test that non-controller user cannot see/delete himself on the users list + // Note: Access to /profile/delete page is tested in UserProfileTest.php + $this->browse(function (Browser $browser) { + $browser->visit('/logout') + ->on(new Home()) + ->submitLogon('jack@kolab.org', 'simple123', true) + ->visit(new UserList()) + ->whenAvailable('@table', function (Browser $browser) { + $browser->assertElementsCount('tbody tr', 0); + }); + }); + + // Test that controller user (Ned) can see/delete all the users ??? + $this->browse(function (Browser $browser) { + $browser->visit('/logout') + ->on(new Home()) + ->submitLogon('ned@kolab.org', 'simple123', true) + ->visit(new UserList()) + ->whenAvailable('@table', function (Browser $browser) { + $browser->assertElementsCount('tbody tr', 3) + ->assertElementsCount('tbody button.button-delete', 3); + }); + + // TODO: Test the delete action in details + }); + + // TODO: Test what happens with the logged in user session after he's been deleted by another user + } } diff --git a/src/tests/DuskTestCase.php b/src/tests/DuskTestCase.php --- a/src/tests/DuskTestCase.php +++ b/src/tests/DuskTestCase.php @@ -22,7 +22,7 @@ return; } - $job = new \App\Jobs\DomainDelete($domain); + $job = new \App\Jobs\DomainDelete($domain->id); $job->handle(); $domain->forceDelete(); diff --git a/src/tests/Feature/Controller/DomainsTest.php b/src/tests/Feature/Controller/DomainsTest.php --- a/src/tests/Feature/Controller/DomainsTest.php +++ b/src/tests/Feature/Controller/DomainsTest.php @@ -38,16 +38,14 @@ public function testConfirm(): void { $sku_domain = Sku::where('title', 'domain')->first(); + $john = $this->getTestUser('john@kolab.org'); + $ned = $this->getTestUser('ned@kolab.org'); $user = $this->getTestUser('test1@domainscontroller.com'); $domain = $this->getTestDomain('domainscontroller.com', [ 'status' => Domain::STATUS_NEW, 'type' => Domain::TYPE_EXTERNAL, ]); - // No entitlement (user has no access to this domain), expect 403 - $response = $this->actingAs($user)->get("api/v4/domains/{$domain->id}/confirm"); - $response->assertStatus(403); - Entitlement::create([ 'owner_id' => $user->id, 'wallet_id' => $user->wallets()->first()->id, @@ -72,7 +70,16 @@ $json = $response->json(); $this->assertEquals('success', $json['status']); - $this->assertEquals('Domain verified successfully', $json['message']); + $this->assertEquals('Domain verified successfully.', $json['message']); + + // Not authorized access + $response = $this->actingAs($john)->get("api/v4/domains/{$domain->id}/confirm"); + $response->assertStatus(403); + + // Authorized access by additional account controller + $domain = $this->getTestDomain('kolab.org'); + $response = $this->actingAs($ned)->get("api/v4/domains/{$domain->id}/confirm"); + $response->assertStatus(200); } /** @@ -90,9 +97,18 @@ $this->assertSame([], $json); // User with custom domain(s) - $user = $this->getTestUser('john@kolab.org'); + $john = $this->getTestUser('john@kolab.org'); + $ned = $this->getTestUser('ned@kolab.org'); - $response = $this->actingAs($user)->get("api/v4/domains"); + $response = $this->actingAs($john)->get("api/v4/domains"); + $response->assertStatus(200); + + $json = $response->json(); + + $this->assertCount(1, $json); + $this->assertSame('kolab.org', $json[0]['namespace']); + + $response = $this->actingAs($ned)->get("api/v4/domains"); $response->assertStatus(200); $json = $response->json(); @@ -113,10 +129,6 @@ 'type' => Domain::TYPE_EXTERNAL, ]); - // No entitlement (user has no access to this domain), expect 403 - $response = $this->actingAs($user)->get("api/v4/domains/{$domain->id}"); - $response->assertStatus(403); - Entitlement::create([ 'owner_id' => $user->id, 'wallet_id' => $user->wallets()->first()->id, @@ -143,5 +155,23 @@ $this->assertCount(8, $json['dns']); $this->assertTrue(strpos(implode("\n", $json['dns']), $domain->namespace) !== false); $this->assertTrue(strpos(implode("\n", $json['dns']), $domain->hash()) !== false); + + $john = $this->getTestUser('john@kolab.org'); + $ned = $this->getTestUser('ned@kolab.org'); + $jack = $this->getTestUser('jack@kolab.org'); + + // Not authorized - Other account domain + $response = $this->actingAs($john)->get("api/v4/domains/{$domain->id}"); + $response->assertStatus(403); + + $domain = $this->getTestDomain('kolab.org'); + + // Ned is an additional controller on kolab.org's wallet + $response = $this->actingAs($ned)->get("api/v4/domains/{$domain->id}"); + $response->assertStatus(200); + + // Jack has no entitlement/control over kolab.org + $response = $this->actingAs($jack)->get("api/v4/domains/{$domain->id}"); + $response->assertStatus(403); } } diff --git a/src/tests/Feature/Controller/UsersTest.php b/src/tests/Feature/Controller/UsersTest.php --- a/src/tests/Feature/Controller/UsersTest.php +++ b/src/tests/Feature/Controller/UsersTest.php @@ -2,8 +2,8 @@ namespace Tests\Feature\Controller; -use App\Http\Controllers\API\UsersController; use App\Domain; +use App\Http\Controllers\API\UsersController; use App\User; use Illuminate\Support\Facades\Queue; use Illuminate\Support\Str; @@ -19,6 +19,8 @@ parent::setUp(); $this->deleteTestUser('UsersControllerTest1@userscontroller.com'); + $this->deleteTestUser('UsersControllerTest2@userscontroller.com'); + $this->deleteTestUser('UsersControllerTest3@userscontroller.com'); $this->deleteTestUser('UserEntitlement2A@UserEntitlement.com'); $this->deleteTestUser('john2.doe2@kolab.org'); $this->deleteTestDomain('userscontroller.com'); @@ -30,6 +32,8 @@ public function tearDown(): void { $this->deleteTestUser('UsersControllerTest1@userscontroller.com'); + $this->deleteTestUser('UsersControllerTest2@userscontroller.com'); + $this->deleteTestUser('UsersControllerTest3@userscontroller.com'); $this->deleteTestUser('UserEntitlement2A@UserEntitlement.com'); $this->deleteTestUser('john2.doe2@kolab.org'); $this->deleteTestDomain('userscontroller.com'); @@ -63,10 +67,149 @@ // Note: Details of the content are tested in testUserResponse() } + /** + * Test user deleting (DELETE /api/v4/users/) + */ + public function testDestroy(): void + { + // First create some users/accounts to delete + $package_kolab = \App\Package::where('title', 'kolab')->first(); + $package_domain = \App\Package::where('title', 'domain-hosting')->first(); + + $john = $this->getTestUser('john@kolab.org'); + $user1 = $this->getTestUser('UsersControllerTest1@userscontroller.com'); + $user2 = $this->getTestUser('UsersControllerTest2@userscontroller.com'); + $user3 = $this->getTestUser('UsersControllerTest3@userscontroller.com'); + $domain = $this->getTestDomain('userscontroller.com', [ + 'status' => Domain::STATUS_NEW, + 'type' => Domain::TYPE_PUBLIC, + ]); + $user1->assignPackage($package_kolab); + $domain->assignPackage($package_domain, $user1); + $user1->assignPackage($package_kolab, $user2); + $user1->assignPackage($package_kolab, $user3); + + // Test unauth access + $response = $this->delete("api/v4/users/{$user2->id}"); + $response->assertStatus(401); + + // Test access to other user/account + $response = $this->actingAs($john)->delete("api/v4/users/{$user2->id}"); + $response->assertStatus(403); + $response = $this->actingAs($john)->delete("api/v4/users/{$user1->id}"); + $response->assertStatus(403); + $json = $response->json(); + + $this->assertSame('error', $json['status']); + $this->assertSame("Access denied", $json['message']); + $this->assertCount(2, $json); + + // Test that non-controller cannot remove himself + $response = $this->actingAs($user3)->delete("api/v4/users/{$user3->id}"); + $response->assertStatus(403); + + // Test removing a non-controller user + $response = $this->actingAs($user1)->delete("api/v4/users/{$user3->id}"); + $response->assertStatus(200); + + $json = $response->json(); + + $this->assertEquals('success', $json['status']); + $this->assertEquals('User deleted successfully.', $json['message']); + + // Test removing self (an account with users) + $response = $this->actingAs($user1)->delete("api/v4/users/{$user1->id}"); + $response->assertStatus(200); + + $json = $response->json(); + + $this->assertEquals('success', $json['status']); + $this->assertEquals('User deleted successfully.', $json['message']); + } + + /** + * Test user deleting (DELETE /api/v4/users/) + */ + public function testDestroyByController(): void + { + // Create an account with additional controller - $user2 + $package_kolab = \App\Package::where('title', 'kolab')->first(); + $package_domain = \App\Package::where('title', 'domain-hosting')->first(); + $user1 = $this->getTestUser('UsersControllerTest1@userscontroller.com'); + $user2 = $this->getTestUser('UsersControllerTest2@userscontroller.com'); + $user3 = $this->getTestUser('UsersControllerTest3@userscontroller.com'); + $domain = $this->getTestDomain('userscontroller.com', [ + 'status' => Domain::STATUS_NEW, + 'type' => Domain::TYPE_PUBLIC, + ]); + $user1->assignPackage($package_kolab); + $domain->assignPackage($package_domain, $user1); + $user1->assignPackage($package_kolab, $user2); + $user1->assignPackage($package_kolab, $user3); + $user1->wallets()->first()->addController($user2); + + // TODO/FIXME: + // For now controller can delete himself, as well as + // the whole account he has control to, including the owner + // Probably he should not be able to do either of those + // However, this is not 0-regression scenario as we + // do not fully support additional controllers. + + //$response = $this->actingAs($user2)->delete("api/v4/users/{$user2->id}"); + //$response->assertStatus(403); + + $response = $this->actingAs($user2)->delete("api/v4/users/{$user3->id}"); + $response->assertStatus(200); + + $response = $this->actingAs($user2)->delete("api/v4/users/{$user1->id}"); + $response->assertStatus(200); + + // Note: More detailed assertions in testDestroy() above + + $this->assertTrue($user1->fresh()->trashed()); + $this->assertTrue($user2->fresh()->trashed()); + $this->assertTrue($user3->fresh()->trashed()); + } + + /** + * Test user listing (GET /api/v4/users) + */ public function testIndex(): void { - // TODO - $this->markTestIncomplete(); + // Test unauth access + $response = $this->get("api/v4/users"); + $response->assertStatus(401); + + $jack = $this->getTestUser('jack@kolab.org'); + $john = $this->getTestUser('john@kolab.org'); + $ned = $this->getTestUser('ned@kolab.org'); + + $response = $this->actingAs($jack)->get("/api/v4/users"); + $response->assertStatus(200); + + $json = $response->json(); + + $this->assertCount(0, $json); + + $response = $this->actingAs($john)->get("/api/v4/users"); + $response->assertStatus(200); + + $json = $response->json(); + + $this->assertCount(3, $json); + $this->assertSame($jack->email, $json[0]['email']); + $this->assertSame($john->email, $json[1]['email']); + $this->assertSame($ned->email, $json[2]['email']); + + $response = $this->actingAs($ned)->get("/api/v4/users"); + $response->assertStatus(200); + + $json = $response->json(); + + $this->assertCount(3, $json); + $this->assertSame($jack->email, $json[0]['email']); + $this->assertSame($john->email, $json[1]['email']); + $this->assertSame($ned->email, $json[2]['email']); } /** @@ -129,7 +272,7 @@ $json = $response->json(); $this->assertEquals('success', $json['status']); - $this->assertEquals('Successfully logged out', $json['message']); + $this->assertEquals('Successfully logged out.', $json['message']); // Check if it really destroyed the token? $response = $this->withHeaders(['Authorization' => 'Bearer ' . $token])->get("api/auth/info"); @@ -217,7 +360,7 @@ public function testUserResponse(): void { $user = $this->getTestUser('john@kolab.org'); - + $wallet = $user->wallets()->first(); $result = $this->invokeMethod(new UsersController(), 'userResponse', [$user]); $this->assertEquals($user->id, $result['id']); @@ -232,6 +375,26 @@ $this->assertTrue(is_array($result['settings'])); $this->assertSame('US', $result['settings']['country']); $this->assertSame('USD', $result['settings']['currency']); + + $this->assertTrue(is_array($result['accounts'])); + $this->assertTrue(is_array($result['wallets'])); + $this->assertCount(0, $result['accounts']); + $this->assertCount(1, $result['wallets']); + $this->assertSame($wallet->id, $result['wallet']['id']); + + $ned = $this->getTestUser('ned@kolab.org'); + $ned_wallet = $ned->wallets()->first(); + $result = $this->invokeMethod(new UsersController(), 'userResponse', [$ned]); + + $this->assertEquals($ned->id, $result['id']); + $this->assertEquals($ned->email, $result['email']); + $this->assertTrue(is_array($result['accounts'])); + $this->assertTrue(is_array($result['wallets'])); + $this->assertCount(1, $result['accounts']); + $this->assertCount(1, $result['wallets']); + $this->assertSame($wallet->id, $result['wallet']['id']); + $this->assertSame($wallet->id, $result['accounts'][0]['id']); + $this->assertSame($ned_wallet->id, $result['wallets'][0]['id']); } /** @@ -253,13 +416,26 @@ $this->assertTrue(is_array($json['settings'])); $this->assertTrue(is_array($json['aliases'])); + $john = $this->getTestUser('john@kolab.org'); + $jack = $this->getTestUser('jack@kolab.org'); + $ned = $this->getTestUser('ned@kolab.org'); + // Test unauthorized access to a profile of other user - $user = $this->getTestUser('jack@kolab.org'); - $response = $this->actingAs($user)->get("/api/v4/users/{$userA->id}"); + $response = $this->actingAs($jack)->get("/api/v4/users/{$userA->id}"); $response->assertStatus(403); - // TODO: Test authorized access to a profile of other user - $this->markTestIncomplete(); + // Test authorized access to a profile of other user + // Ned: Additional account controller + $response = $this->actingAs($ned)->get("/api/v4/users/{$john->id}"); + $response->assertStatus(200); + $response = $this->actingAs($ned)->get("/api/v4/users/{$jack->id}"); + $response->assertStatus(200); + + // John: Account owner + $response = $this->actingAs($john)->get("/api/v4/users/{$jack->id}"); + $response->assertStatus(200); + $response = $this->actingAs($john)->get("/api/v4/users/{$ned->id}"); + $response->assertStatus(200); } /** @@ -338,7 +514,7 @@ $response->assertStatus(200); $this->assertSame('success', $json['status']); - $this->assertSame("User created successfully", $json['message']); + $this->assertSame("User created successfully.", $json['message']); $this->assertCount(2, $json); $user = User::where('email', 'john2.doe2@kolab.org')->first(); @@ -351,6 +527,22 @@ $this->assertSame('useralias2@kolab.org', $aliases[1]->alias); // TODO: Test assigning a package to new user + // TODO: Test the wallet to which the new user should be assigned to + + // Test acting as account controller (not owner) + /* + // FIXME: How do we know to which wallet the new user should be assigned to? + + $this->deleteTestUser('john2.doe2@kolab.org'); + $response = $this->actingAs($ned)->post("/api/v4/users", $post); + $json = $response->json(); + + $response->assertStatus(200); + + $this->assertSame('success', $json['status']); + */ + + $this->markTestIncomplete(); } /** @@ -360,6 +552,8 @@ { $userA = $this->getTestUser('UsersControllerTest1@userscontroller.com'); $jack = $this->getTestUser('jack@kolab.org'); + $john = $this->getTestUser('john@kolab.org'); + $ned = $this->getTestUser('ned@kolab.org'); $domain = $this->getTestDomain( 'userscontroller.com', ['status' => Domain::STATUS_NEW, 'type' => Domain::TYPE_EXTERNAL] @@ -369,6 +563,10 @@ $response = $this->actingAs($jack)->get("/api/v4/users/{$userA->id}", []); $response->assertStatus(403); + // Test authorized update of account owner by account controller + $response = $this->actingAs($ned)->get("/api/v4/users/{$john->id}", []); + $response->assertStatus(200); + // Test updating of self (empty request) $response = $this->actingAs($userA)->put("/api/v4/users/{$userA->id}", []); $response->assertStatus(200); @@ -376,7 +574,7 @@ $json = $response->json(); $this->assertSame('success', $json['status']); - $this->assertSame("User data updated successfully", $json['message']); + $this->assertSame("User data updated successfully.", $json['message']); $this->assertCount(2, $json); // Test some invalid data @@ -411,7 +609,7 @@ $response->assertStatus(200); $this->assertSame('success', $json['status']); - $this->assertSame("User data updated successfully", $json['message']); + $this->assertSame("User data updated successfully.", $json['message']); $this->assertCount(2, $json); $this->assertTrue($userA->password != $userA->fresh()->password); unset($post['password'], $post['password_confirmation'], $post['aliases']); @@ -441,7 +639,7 @@ $response->assertStatus(200); $this->assertSame('success', $json['status']); - $this->assertSame("User data updated successfully", $json['message']); + $this->assertSame("User data updated successfully.", $json['message']); $this->assertCount(2, $json); unset($post['aliases']); foreach ($post as $key => $value) { @@ -469,9 +667,11 @@ $this->assertSame("The specified domain is not available.", $json['errors']['aliases'][1]); $this->assertSame("The password confirmation does not match.", $json['errors']['password'][0]); + // Test authorized update of other user + $response = $this->actingAs($ned)->get("/api/v4/users/{$jack->id}", []); + $response->assertStatus(200); + // TODO: Test error on aliases with invalid/non-existing/other-user's domain - // TODO: Test authorized update of other user - $this->markTestIncomplete(); } /** diff --git a/src/tests/Feature/DomainTest.php b/src/tests/Feature/DomainTest.php --- a/src/tests/Feature/DomainTest.php +++ b/src/tests/Feature/DomainTest.php @@ -163,4 +163,32 @@ $this->assertTrue($domain->confirm()); $this->assertTrue($domain->isConfirmed()); } + + /** + * Test domain deletion + */ + public function testDelete(): void + { + Queue::fake(); + + $domain = $this->getTestDomain('gmail.com', [ + 'status' => Domain::STATUS_NEW, + 'type' => Domain::TYPE_PUBLIC, + ]); + + $domain->delete(); + + $this->assertTrue($domain->fresh()->trashed()); + $this->assertFalse($domain->fresh()->isDeleted()); + + // Delete the domain for real + $job = new \App\Jobs\DomainDelete($domain->id); + $job->handle(); + + $this->assertTrue(Domain::withTrashed()->where('id', $domain->id)->first()->isDeleted()); + + $domain->forceDelete(); + + $this->assertCount(0, Domain::withTrashed()->where('id', $domain->id)->get()); + } } diff --git a/src/tests/Feature/EntitlementTest.php b/src/tests/Feature/EntitlementTest.php --- a/src/tests/Feature/EntitlementTest.php +++ b/src/tests/Feature/EntitlementTest.php @@ -19,12 +19,14 @@ $this->deleteTestUser('entitlement-test@kolabnow.com'); $this->deleteTestUser('entitled-user@custom-domain.com'); + $this->deleteTestDomain('custom-domain.com'); } public function tearDown(): void { $this->deleteTestUser('entitlement-test@kolabnow.com'); $this->deleteTestUser('entitled-user@custom-domain.com'); + $this->deleteTestDomain('custom-domain.com'); parent::tearDown(); } diff --git a/src/tests/Feature/SkuTest.php b/src/tests/Feature/SkuTest.php --- a/src/tests/Feature/SkuTest.php +++ b/src/tests/Feature/SkuTest.php @@ -48,7 +48,7 @@ public function testSkuEntitlements(): void { - $this->assertCount(2, Sku::where('title', 'mailbox')->first()->entitlements); + $this->assertCount(3, Sku::where('title', 'mailbox')->first()->entitlements); } public function testSkuPackages(): void 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 @@ -2,6 +2,7 @@ namespace Tests\Feature; +use App\Domain; use App\User; use Illuminate\Support\Facades\Queue; use Tests\TestCase; @@ -13,17 +14,21 @@ parent::setUp(); $this->deleteTestUser('user-create-test@' . \config('app.domain')); + $this->deleteTestUser('userdeletejob@kolabnow.com'); $this->deleteTestUser('UserAccountA@UserAccount.com'); $this->deleteTestUser('UserAccountB@UserAccount.com'); - $this->deleteTestUser('userdeletejob@kolabnow.com'); + $this->deleteTestUser('UserAccountC@UserAccount.com'); + $this->deleteTestDomain('UserAccount.com'); } public function tearDown(): void { $this->deleteTestUser('user-create-test@' . \config('app.domain')); + $this->deleteTestUser('userdeletejob@kolabnow.com'); $this->deleteTestUser('UserAccountA@UserAccount.com'); $this->deleteTestUser('UserAccountB@UserAccount.com'); - $this->deleteTestUser('userdeletejob@kolabnow.com'); + $this->deleteTestUser('UserAccountC@UserAccount.com'); + $this->deleteTestDomain('UserAccount.com'); parent::tearDown(); } @@ -103,18 +108,24 @@ $this->assertTrue($userB->accounts()->get()[0]->id === $userA->wallets()->get()[0]->id); } - /** - * Tests for User::controller() - */ - public function testController(): void + public function testAccounts(): void { - $john = $this->getTestUser('john@kolab.org'); + $this->markTestIncomplete(); + } - $this->assertSame($john->id, $john->controller()->id); + public function testCanDelete(): void + { + $this->markTestIncomplete(); + } - $jack = $this->getTestUser('jack@kolab.org'); + public function testCanRead(): void + { + $this->markTestIncomplete(); + } - $this->assertSame($john->id, $jack->controller()->id); + public function testCanUpdate(): void + { + $this->markTestIncomplete(); } /** @@ -147,6 +158,10 @@ public function testUserQuota(): void { + // TODO: This test does not test much, probably could be removed + // or moved to somewhere else, or extended with + // other entitlements() related cases. + $user = $this->getTestUser('john@kolab.org'); $storage_sku = \App\Sku::where('title', 'storage')->first(); @@ -164,26 +179,79 @@ /** * Test user deletion */ - public function testUserDelete(): void + public function testDelete(): void { - $user = $this->getTestUser('userdeletejob@kolabnow.com'); + Queue::fake(); + $user = $this->getTestUser('userdeletejob@kolabnow.com'); $package = \App\Package::where('title', 'kolab')->first(); - $user->assignPackage($package); $id = $user->id; + $entitlements = \App\Entitlement::where('owner_id', $id)->get(); + $this->assertCount(4, $entitlements); + $user->delete(); + $entitlements = \App\Entitlement::where('owner_id', $id)->get(); + $this->assertCount(0, $entitlements); + $this->assertTrue($user->fresh()->trashed()); + $this->assertFalse($user->fresh()->isDeleted()); + + // Delete the user for real $job = new \App\Jobs\UserDelete($id); $job->handle(); + $this->assertTrue(User::withTrashed()->where('id', $id)->first()->isDeleted()); + $user->forceDelete(); - $entitlements = \App\Entitlement::where('owner_id', 'id')->get(); + $this->assertCount(0, User::withTrashed()->where('id', $id)->get()); - $this->assertCount(0, $entitlements); + // Test an account with users + $userA = $this->getTestUser('UserAccountA@UserAccount.com'); + $userB = $this->getTestUser('UserAccountB@UserAccount.com'); + $userC = $this->getTestUser('UserAccountC@UserAccount.com'); + $package_kolab = \App\Package::where('title', 'kolab')->first(); + $package_domain = \App\Package::where('title', 'domain-hosting')->first(); + $domain = $this->getTestDomain('UserAccount.com', [ + 'status' => Domain::STATUS_NEW, + 'type' => Domain::TYPE_HOSTED, + ]); + $userA->assignPackage($package_kolab); + $domain->assignPackage($package_domain, $userA); + $userA->assignPackage($package_kolab, $userB); + $userA->assignPackage($package_kolab, $userC); + + $entitlementsA = \App\Entitlement::where('entitleable_id', $userA->id); + $entitlementsB = \App\Entitlement::where('entitleable_id', $userB->id); + $entitlementsC = \App\Entitlement::where('entitleable_id', $userC->id); + $entitlementsDomain = \App\Entitlement::where('entitleable_id', $domain->id); + $this->assertSame(4, $entitlementsA->count()); + $this->assertSame(4, $entitlementsB->count()); + $this->assertSame(4, $entitlementsC->count()); + $this->assertSame(1, $entitlementsDomain->count()); + + // Delete non-controller user + $userC->delete(); + + $this->assertTrue($userC->fresh()->trashed()); + $this->assertFalse($userC->fresh()->isDeleted()); + $this->assertSame(0, $entitlementsC->count()); + + // Delete the controller (and expect "sub"-users to be deleted too) + $userA->delete(); + + $this->assertSame(0, $entitlementsA->count()); + $this->assertSame(0, $entitlementsB->count()); + $this->assertSame(0, $entitlementsDomain->count()); + $this->assertTrue($userA->fresh()->trashed()); + $this->assertTrue($userB->fresh()->trashed()); + $this->assertTrue($domain->fresh()->trashed()); + $this->assertFalse($userA->isDeleted()); + $this->assertFalse($userB->isDeleted()); + $this->assertFalse($domain->isDeleted()); } /** @@ -260,4 +328,38 @@ { $this->markTestIncomplete(); } + + /** + * Tests for User::users() + */ + public function testUsers(): void + { + $john = $this->getTestUser('john@kolab.org'); + $jack = $this->getTestUser('jack@kolab.org'); + $ned = $this->getTestUser('ned@kolab.org'); + $wallet = $john->wallets()->first(); + + $users = $john->users()->orderBy('email')->get(); + + $this->assertCount(3, $users); + $this->assertEquals($jack->id, $users[0]->id); + $this->assertEquals($john->id, $users[1]->id); + $this->assertEquals($ned->id, $users[2]->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); + + $users = $jack->users()->orderBy('email')->get(); + + $this->assertCount(0, $users); + + $users = $ned->users()->orderBy('email')->get(); + + $this->assertCount(3, $users); + } + + public function testWallets(): void + { + $this->markTestIncomplete(); + } } diff --git a/src/tests/TestCase.php b/src/tests/TestCase.php --- a/src/tests/TestCase.php +++ b/src/tests/TestCase.php @@ -30,7 +30,7 @@ return; } - $job = new \App\Jobs\DomainDelete($domain); + $job = new \App\Jobs\DomainDelete($domain->id); $job->handle(); $domain->forceDelete();