diff --git a/src/app/Http/Controllers/API/SignupController.php b/src/app/Http/Controllers/API/SignupController.php --- a/src/app/Http/Controllers/API/SignupController.php +++ b/src/app/Http/Controllers/API/SignupController.php @@ -327,8 +327,6 @@ return ['domain' => 'validation.domaininvalid']; } - // TODO: DNS registration check - maybe after signup? - // Check if domain is already registered with us if (Domain::where('namespace', $domain)->first()) { return ['domain' => 'validation.domainexists']; @@ -336,7 +334,6 @@ } // Check if user with specified login already exists - // TODO: Aliases $email = $login . '@' . $domain; if (User::findByEmail($email)) { return ['login' => 'validation.loginexists']; 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 @@ -75,18 +75,7 @@ public function info() { $user = $this->guard()->user(); - $response = $user->toArray(); - - // Settings - // TODO: It might be reasonable to limit the list of settings here to these - // that are safe and are used in the UI - $response['settings'] = []; - foreach ($user->settings as $item) { - $response['settings'][$item->key] = $item->value; - } - - // Status info - $response['statusInfo'] = self::statusInfo($user); + $response = $this->userResponse($user); return response()->json($response); } @@ -112,7 +101,6 @@ return response()->json(['status' => 'error', 'errors' => $v->errors()], 422); } - $credentials = $request->only('email', 'password'); if ($token = $this->guard()->attempt($credentials)) { @@ -181,10 +169,12 @@ $user = User::find($id); if (empty($user)) { - return $this->errorResponse(404); + return $this->errorResponse(404); } - return response()->json($user); + $response = $this->userResponse($user); + + return response()->json($response); } /** @@ -216,7 +206,7 @@ $domain = Domain::where('namespace', $domain)->first(); // If that is not a public domain, add domain specific steps - if (!$domain->isPublic()) { + if ($domain && !$domain->isPublic()) { $steps['domain-new'] = true; $steps['domain-ldap-ready'] = 'isLdapReady'; $steps['domain-verified'] = 'isVerified'; @@ -286,6 +276,7 @@ 'billing_address' => 'string|nullable|max:1024', 'country' => 'string|nullable|alpha|size:2', 'currency' => 'string|nullable|alpha|size:3', + 'aliases' => 'array|nullable', ]; if (!empty($request->password) || !empty($request->password_confirmation)) { @@ -299,9 +290,26 @@ return response()->json(['status' => 'error', 'errors' => $v->errors()], 422); } + // Validate aliases input + if (isset($request->aliases)) { + $aliases = []; + + if (!empty($request->aliases)) { + $domains = $user->domains(); + } + + foreach ($request->aliases as $idx => $alias) { + if (is_string($alias) && !empty($alias)) { + // TODO: validation + + $aliases[] = $alias; + } + } + } + // Update user settings $settings = $request->only(array_keys($rules)); - unset($settings['password']); + unset($settings['password'], $settings['aliases']); if (!empty($settings)) { $user->setSettings($settings); @@ -313,6 +321,11 @@ $user->save(); } + // Update aliases + if (!empty($aliases)) { + $user->setAliases($aliases); + } + return response()->json([ 'status' => 'success', 'message' => __('app.user-update-success'), @@ -345,4 +358,35 @@ return $current_user->id == $user_id; } + + /** + * Create a response data array for specified user. + * + * @param \App\User $user User object + * + * @return array Response data + */ + protected function userResponse(User $user): array + { + $response = $user->toArray(); + + // Settings + // TODO: It might be reasonable to limit the list of settings here to these + // that are safe and are used in the UI + $response['settings'] = []; + foreach ($user->settings as $item) { + $response['settings'][$item->key] = $item->value; + } + + // Aliases + $response['aliases'] = []; + foreach ($user->aliases as $item) { + $response['aliases'][] = $item->alias; + } + + // Status info + $response['statusInfo'] = self::statusInfo($user); + + return $response; + } } diff --git a/src/app/Observers/UserAliasObserver.php b/src/app/Observers/UserAliasObserver.php new file mode 100644 --- /dev/null +++ b/src/app/Observers/UserAliasObserver.php @@ -0,0 +1,88 @@ +alias = \strtolower($alias->alias); + + if (User::where('email', $alias->alias)->first()) { + \Log::error("Failed creating alias {$alias->alias}. User exists."); + return false; + } + } + + /** + * Handle the user alias "created" event. + * + * @param \App\UserAlias $alias User email alias + * + * @return void + */ + public function created(UserAlias $alias) + { + \App\Jobs\UserUpdate::dispatch($alias->user); + } + + /** + * Handle the user setting "updated" event. + * + * @param \App\UserAlias $alias User email alias + * + * @return void + */ + public function updated(UserAlias $alias) + { + \App\Jobs\UserUpdate::dispatch($alias->user); + } + + /** + * Handle the user setting "deleted" event. + * + * @param \App\UserAlias $alias User email alias + * + * @return void + */ + public function deleted(UserAlias $alias) + { + \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/Providers/AppServiceProvider.php b/src/app/Providers/AppServiceProvider.php --- a/src/app/Providers/AppServiceProvider.php +++ b/src/app/Providers/AppServiceProvider.php @@ -32,6 +32,7 @@ \App\SignupCode::observe(\App\Observers\SignupCodeObserver::class); \App\Sku::observe(\App\Observers\SkuObserver::class); \App\User::observe(\App\Observers\UserObserver::class); + \App\UserAlias::observe(\App\Observers\UserAliasObserver::class); \App\UserSetting::observe(\App\Observers\UserSettingObserver::class); \App\VerificationCode::observe(\App\Observers\VerificationCodeObserver::class); \App\Wallet::observe(\App\Observers\WalletObserver::class); diff --git a/src/app/Traits/UserAliasesTrait.php b/src/app/Traits/UserAliasesTrait.php new file mode 100644 --- /dev/null +++ b/src/app/Traits/UserAliasesTrait.php @@ -0,0 +1,41 @@ + 'some@other.erg']); + * $user->setAliases(['alias1@other.org', 'alias2@other.org']); + * ``` + * + * @param array $data An array of email addresses + * + * @return void + */ + 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]); + } + + foreach (array_diff($existing_aliases, $aliases) as $alias) { + $this->aliases()->where('alias', $alias)->delete(); + } + } +} diff --git a/src/app/User.php b/src/app/User.php --- a/src/app/User.php +++ b/src/app/User.php @@ -2,24 +2,30 @@ namespace App; +use App\UserAlias; +use App\Traits\UserAliasesTrait; +use App\Traits\UserSettingsTrait; use Illuminate\Notifications\Notifiable; use Illuminate\Contracts\Auth\MustVerifyEmail; use Illuminate\Database\Eloquent\SoftDeletes; use Illuminate\Foundation\Auth\User as Authenticatable; use Iatstuti\Database\Support\NullableFields; use Tymon\JWTAuth\Contracts\JWTSubject; -use App\Traits\UserSettingsTrait; /** * The eloquent definition of a User. * - * @property integer $id - * @property integer $status + * @property string $email + * @property int $id + * @property string $name + * @property string $password + * @property int $status */ class User extends Authenticatable implements JWTSubject { use Notifiable; use NullableFields; + use UserAliasesTrait; use UserSettingsTrait; use SoftDeletes; @@ -95,6 +101,16 @@ ); } + /** + * Email aliases of this user. + * + * @return \Illuminate\Database\Eloquent\Relations\HasMany + */ + public function aliases() + { + return $this->hasMany('App\UserAlias', 'user_id'); + } + /** * Assign a package to a user. The user should not have any existing entitlements. * @@ -216,7 +232,7 @@ * * @param string $email Email address * - * @return \App\User User model object + * @return \App\User User model object if found */ public static function findByEmail(string $email): ?User { @@ -224,11 +240,23 @@ return null; } + $email = \strtolower($email); + $user = self::where('email', $email)->first(); - // TODO: Aliases, External email + if ($user) { + return $user; + } - return $user; + $alias = UserAlias::where('alias', $email)->first(); + + if ($alias) { + return $alias->user; + } + + // TODO: External email + + return null; } public function getJWTIdentifier() diff --git a/src/app/UserAlias.php b/src/app/UserAlias.php new file mode 100644 --- /dev/null +++ b/src/app/UserAlias.php @@ -0,0 +1,33 @@ +belongsTo( + '\App\User', + 'user_id', /* local */ + 'id' /* remote */ + ); + } +} diff --git a/src/database/migrations/2020_02_26_000000_create_user_aliases_table.php b/src/database/migrations/2020_02_26_000000_create_user_aliases_table.php new file mode 100644 --- /dev/null +++ b/src/database/migrations/2020_02_26_000000_create_user_aliases_table.php @@ -0,0 +1,39 @@ +bigIncrements('id'); + $table->bigInteger('user_id'); + $table->string('alias')->unique(); + $table->timestamps(); + + $table->foreign('user_id')->references('id')->on('users') + ->onDelete('cascade')->onUpdate('cascade'); + } + ); + } + + /** + * Reverse the migrations. + * + * @return void + */ + public function down() + { + Schema::dropIfExists('user_aliases'); + } +} 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 @@ -45,6 +45,8 @@ ] ); + $john->setAliases(['john.doe@kolab.org']); + $user_wallets = $john->wallets()->get(); $package_domain = \App\Package::where('title', 'domain-hosting')->first(); @@ -71,6 +73,8 @@ ] ); + $jack->setAliases(['jack.daniels@kolab.org']); + $john->assignPackage($package_kolab, $jack); factory(User::class, 10)->create(); 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 @@ -4,6 +4,47 @@
User account
+
+
+ +
+ +
+
+
+ +
+ +
+
+
+ +
+ +
+
+ +
+ +
+ +
+
+
+ +
+ +
+
+ +
@@ -15,7 +56,7 @@ data() { return { user_id: null, - user: null + user: {} } }, created() { @@ -23,11 +64,31 @@ axios.get('/api/v4/users/' + this.user_id) .then(response => { this.user = response.data + this.user.first_name = response.data.settings.first_name + this.user.last_name = response.data.settings.last_name }) .catch(this.$root.errorHandler) } else { this.$root.errorPage(404) } + }, + mounted() { + $('#first_name').focus() + }, + methods: { + submit() { + this.$root.clearFormValidation($('#user-info form')) + + axios.put('/api/v4/users/' + this.user_id, this.user) + .then(response => { + delete this.user.password + delete this.user.password_confirm + + if (response.data.status == 'success') { + this.$toastr('success', response.data.message) + } + }) + } } } diff --git a/src/tests/Browser/Pages/UserInfo.php b/src/tests/Browser/Pages/UserInfo.php new file mode 100644 --- /dev/null +++ b/src/tests/Browser/Pages/UserInfo.php @@ -0,0 +1,45 @@ +waitFor('@form') + ->assertSeeIn('#user-info .card-title', 'User account'); + } + + /** + * Get the element shortcuts for the page. + * + * @return array + */ + public function elements(): array + { + return [ + '@app' => '#app', + '@form' => '#user-info form', + ]; + } +} diff --git a/src/tests/Browser/Pages/UserList.php b/src/tests/Browser/Pages/UserList.php new file mode 100644 --- /dev/null +++ b/src/tests/Browser/Pages/UserList.php @@ -0,0 +1,46 @@ +assertPathIs($this->url()) + ->waitUntilMissing('@app .app-loader') + ->assertSeeIn('#user-list .card-title', 'User Accounts'); + } + + /** + * Get the element shortcuts for the page. + * + * @return array + */ + public function elements(): array + { + return [ + '@app' => '#app', + '@table' => '#user-list table', + ]; + } +} diff --git a/src/tests/Browser/UsersTest.php b/src/tests/Browser/UsersTest.php new file mode 100644 --- /dev/null +++ b/src/tests/Browser/UsersTest.php @@ -0,0 +1,142 @@ + 'John', + 'last_name' => 'Doe', + ]; + + /** + * {@inheritDoc} + */ + public function setUp(): void + { + parent::setUp(); + + User::where('email', 'john@kolab.org')->first()->setSettings($this->profile); + } + + /** + * {@inheritDoc} + */ + public function tearDown(): void + { + User::where('email', 'john@kolab.org')->first()->setSettings($this->profile); + + parent::tearDown(); + } + + /** + * Test user info page (unauthenticated) + */ + public function testInfoUnauth(): void + { + // Test that the page requires authentication + $this->browse(function (Browser $browser) { + $user = User::where('email', 'john@kolab.org')->first(); + + $browser->visit('/user/' . $user->id)->on(new Home()); + }); + } + + /** + * Test users list page (unauthenticated) + */ + public function testListUnauth(): void + { + // Test that the page requires authentication + $this->browse(function (Browser $browser) { + $browser->visit('/users')->on(new Home()); + }); + } + + /** + * Test users list page + */ + public function testList(): void + { + // Test that the page requires authentication + $this->browse(function (Browser $browser) { + $browser->visit(new Home()) + ->submitLogon('john@kolab.org', 'simple123', true) + ->on(new Dashboard()) + ->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'); + }); + }); + } + + /** + * Test profile page + * + * @depends testList + */ + public function testInfo(): void + { + $this->browse(function (Browser $browser) { + $browser->on(new UserList()) + ->click('@table tr:first-child a') + ->on(new UserInfo()) + ->whenAvailable('@form', function (Browser $browser) { + // Assert form content + $browser->assertFocused('div.row:nth-child(1) input') + ->assertSeeIn('div.row:nth-child(1) label', 'First name') + ->assertValue('div.row:nth-child(1) input[type=text]', $this->profile['first_name']) + ->assertSeeIn('div.row:nth-child(2) label', 'Last name') + ->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') +// ->assertDisabled('div.row:nth-child(3) input') + ->assertSeeIn('div.row:nth-child(4) label', 'Password') + ->assertValue('div.row:nth-child(4) input[type=password]', '') + ->assertSeeIn('div.row:nth-child(5) label', 'Confirm password') + ->assertValue('div.row:nth-child(5) input[type=password]', '') + ->assertSeeIn('button[type=submit]', 'Submit'); + + // Clear some fields and submit + $browser->type('#first_name', '') + ->type('#last_name', '') + ->click('button[type=submit]'); + }) + ->with(new Toast(Toast::TYPE_SUCCESS), function (Browser $browser) { + $browser->assertToastTitle('') + ->assertToastMessage('User data updated successfully') + ->closeToast(); + }); + + // Test error handling + $browser->with('@form', function (Browser $browser) { + $browser->type('#password', 'aaaaaa') + ->type('#password_confirmation', '') + ->click('button[type=submit]') + ->waitFor('#password + .invalid-feedback') + ->assertSeeIn('#password + .invalid-feedback', 'The password confirmation does not match.') + ->assertFocused('#password'); + }) + ->with(new Toast(Toast::TYPE_ERROR), function (Browser $browser) { + $browser->assertToastTitle('Error') + ->assertToastMessage('Form validation error') + ->closeToast(); + }); + + // TODO: Test password change + }); + } +} 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 @@ -55,8 +55,10 @@ $this->assertEquals($user->email, $json['email']); $this->assertEquals(User::STATUS_NEW, $json['status']); $this->assertTrue(is_array($json['statusInfo'])); - $this->assertEquals($user->getSetting('country'), $json['settings']['country']); - $this->assertEquals($user->getSetting('currency'), $json['settings']['currency']); + $this->assertTrue(is_array($json['settings'])); + $this->assertTrue(is_array($json['aliases'])); + + // Note: Details of the content are tested in testUserResponse() } public function testIndex(): void @@ -207,6 +209,29 @@ $this->assertSame('deleted', $result['status']); } + /** + * Test user data response used in show and info actions + */ + public function testUserResponse(): void + { + $user = $this->getTestUser('john@kolab.org'); + + $result = $this->invokeMethod(new UsersController(), 'userResponse', [$user]); + + $this->assertEquals($user->id, $result['id']); + $this->assertEquals($user->email, $result['email']); + $this->assertEquals(1, $result['status']); + $this->assertTrue(is_array($result['statusInfo'])); + + $this->assertTrue(is_array($result['aliases'])); + $this->assertCount(1, $result['aliases']); + $this->assertSame('john.doe@kolab.org', $result['aliases'][0]); + + $this->assertTrue(is_array($result['settings'])); + $this->assertSame('US', $result['settings']['country']); + $this->assertSame('USD', $result['settings']['currency']); + } + /** * Test fetching user data/profile (GET /api/v4/users/) */ @@ -217,8 +242,14 @@ // Test getting profile of self $response = $this->actingAs($userA, 'api')->get("/api/v4/users/{$userA->id}"); + $json = $response->json(); + $response->assertStatus(200); - $response->assertJson(['id' => $userA->id]); + $this->assertEquals($userA->id, $json['id']); + $this->assertEquals($userA->email, $json['email']); + $this->assertTrue(is_array($json['statusInfo'])); + $this->assertTrue(is_array($json['settings'])); + $this->assertTrue(is_array($json['aliases'])); // Test unauthorized access to a profile of other user $user = $this->getTestUser('jack@kolab.org'); @@ -250,7 +281,7 @@ $response = $this->actingAs($userB)->get("/api/v4/users/{$userA->id}", []); $response->assertStatus(403); - // Test updating of self + // Test updating of self (empty request) $response = $this->actingAs($userA)->put("/api/v4/users/{$userA->id}", []); $response->assertStatus(200); @@ -283,6 +314,7 @@ 'billing_address' => 'billing', 'country' => 'CH', 'currency' => 'CHF', + 'aliases' => ['useralias1@userscontroller.com', 'useralias2@userscontroller.com'] ]; $response = $this->actingAs($userA)->put("/api/v4/users/{$userA->id}", $post); @@ -294,10 +326,14 @@ $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']); + unset($post['password'], $post['password_confirmation'], $post['aliases']); foreach ($post as $key => $value) { $this->assertSame($value, $userA->getSetting($key)); } + $aliases = $userA->aliases()->get(); + $this->assertCount(2, $aliases); + $this->assertSame('useralias1@userscontroller.com', $aliases[0]->alias); + $this->assertSame('useralias2@userscontroller.com', $aliases[1]->alias); // Test unsetting values $post = [ @@ -308,6 +344,7 @@ 'billing_address' => '', 'country' => '', 'currency' => '', + 'aliases' => ['useralias2@userscontroller.com'] ]; $response = $this->actingAs($userA)->put("/api/v4/users/{$userA->id}", $post); @@ -318,10 +355,15 @@ $this->assertSame('success', $json['status']); $this->assertSame("User data updated successfully", $json['message']); $this->assertCount(2, $json); + unset($post['aliases']); foreach ($post as $key => $value) { $this->assertNull($userA->getSetting($key)); } + $aliases = $userA->aliases()->get(); + $this->assertCount(1, $aliases); + $this->assertSame('useralias2@userscontroller.com', $aliases[0]->alias); + // 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/UserTest.php b/src/tests/Feature/UserTest.php --- a/src/tests/Feature/UserTest.php +++ b/src/tests/Feature/UserTest.php @@ -28,6 +28,22 @@ parent::tearDown(); } + /** + * Tests for User::assignPackage() + */ + public function testAssignPackage(): void + { + $this->markTestIncomplete(); + } + + /** + * Tests for User::assignPlan() + */ + public function testAssignPlan(): void + { + $this->markTestIncomplete(); + } + /** * Verify user creation process */ @@ -158,7 +174,61 @@ $this->assertInstanceOf(User::class, $result); $this->assertSame($user->id, $result->id); - // TODO: Make sure searching is case-insensitive - // TODO: Alias, eternal email + // Use an alias + $result = User::findByEmail('john.doe@kolab.org'); + $this->assertInstanceOf(User::class, $result); + $this->assertSame($user->id, $result->id); + + // TODO: searching by external email (setting) + $this->markTestIncomplete(); + } + + /** + * Tests for UserAliasesTrait::setAliases() + */ + public function testSetAliases(): void + { + Queue::fake(); + + $user = $this->getTestUser('UserAccountA@UserAccount.com'); + + $this->assertCount(0, $user->aliases->all()); + + // Add an alias + $user->setAliases(['UserAlias@UserAccount.com']); + + $aliases = $user->aliases()->get(); + $this->assertCount(1, $aliases); + $this->assertSame('useralias@useraccount.com', $aliases[0]['alias']); + + // Add another alias + $user->setAliases(['UserAlias@UserAccount.com', 'UserAlias2@UserAccount.com']); + + $aliases = $user->aliases()->orderBy('alias')->get(); + $this->assertCount(2, $aliases); + $this->assertSame('useralias2@useraccount.com', $aliases[0]->alias); + $this->assertSame('useralias@useraccount.com', $aliases[1]->alias); + + // Remove an alias + $user->setAliases(['UserAlias@UserAccount.com']); + + $aliases = $user->aliases()->get(); + $this->assertCount(1, $aliases); + $this->assertSame('useralias@useraccount.com', $aliases[0]['alias']); + + // Remove all aliases + $user->setAliases([]); + + $this->assertCount(0, $user->aliases()->get()); + + // TODO: Test that the changes are propagated to ldap + } + + /** + * Tests for UserSettingsTrait::setSettings() + */ + public function testSetSettings(): 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 @@ -73,4 +73,22 @@ return $property->getValue($object); } + + /** + * Call protected/private method of a class. + * + * @param object $object Instantiated object that we will run method on. + * @param string $methodName Method name to call + * @param array $parameters Array of parameters to pass into method. + * + * @return mixed Method return. + */ + public function invokeMethod($object, $methodName, array $parameters = array()) + { + $reflection = new \ReflectionClass(get_class($object)); + $method = $reflection->getMethod($methodName); + $method->setAccessible(true); + + return $method->invokeArgs($object, $parameters); + } }