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 @@ -7,6 +7,9 @@ use App\Jobs\SignupVerificationSMS; use App\Domain; use App\Plan; +use App\Rules\ExternalEmail; +use App\Rules\UserEmailDomain; +use App\Rules\UserEmailLocal; use App\SignupCode; use App\User; use Illuminate\Http\Request; @@ -77,7 +80,7 @@ // Validate user email (or phone) if ($error = $this->validatePhoneOrEmail($request->email, $is_phone)) { - return response()->json(['status' => 'error', 'errors' => ['email' => __($error)]], 422); + return response()->json(['status' => 'error', 'errors' => ['email' => $error]], 422); } // Generate the verification code @@ -187,8 +190,7 @@ $domain = $request->domain; // Validate login - if ($errors = $this->validateLogin($login, $domain, $is_domain)) { - $errors = $this->resolveErrors($errors); + if ($errors = self::validateLogin($login, $domain, $is_domain)) { return response()->json(['status' => 'error', 'errors' => $errors], 422); } @@ -234,56 +236,62 @@ } /** - * Checks if the input string is a valid email address or a phone number - * - * @param string $input Email address or phone number - * @param bool $is_phone Will have been set to True if the string is valid phone number + * Returns plan for the signup process * - * @return string Error message label on validation error + * @returns \App\Plan Plan object selected for current signup process */ - protected function validatePhoneOrEmail($input, &$is_phone = false) + protected function getPlan() { - $is_phone = false; - - return $this->validateEmail($input); - - // TODO: Phone number support -/* - if (strpos($input, '@')) { - return $this->validateEmail($input); - } + if (!$this->plan) { + // Get the plan if specified and exists... + if ($this->code && $this->code->data['plan']) { + $plan = Plan::where('title', $this->code->data['plan'])->first(); + } - $input = str_replace(array('-', ' '), '', $input); + // ...otherwise use the default plan + if (empty($plan)) { + // TODO: Get default plan title from config + $plan = Plan::where('title', 'individual')->first(); + } - if (!preg_match('/^\+?[0-9]{9,12}$/', $input)) { - return 'validation.noemailorphone'; + $this->plan = $plan; } - $is_phone = true; -*/ + return $this->plan; } /** - * Email address validation + * Checks if the input string is a valid email address or a phone number * - * @param string $email Email address + * @param string $input Email address or phone number + * @param bool $is_phone Will have been set to True if the string is valid phone number * - * @return string Error message label on validation error + * @return string Error message on validation error */ - protected function validateEmail($email) + protected static function validatePhoneOrEmail($input, &$is_phone = false): ?string { - $v = Validator::make(['email' => $email], ['email' => 'required|email']); + $is_phone = false; + + $v = Validator::make( + ['email' => $input], + ['email' => ['required', 'string', new ExternalEmail()]] + ); if ($v->fails()) { - return 'validation.emailinvalid'; + return $v->errors()->toArray()['email'][0]; } - list($local, $domain) = explode('@', $email); + // TODO: Phone number support +/* + $input = str_replace(array('-', ' '), '', $input); - // don't allow @localhost and other no-fqdn - if (strpos($domain, '.') === false) { - return 'validation.emailinvalid'; + if (!preg_match('/^\+?[0-9]{9,12}$/', $input)) { + return \trans('validation.noemailorphone'); } + + $is_phone = true; +*/ + return null; } /** @@ -295,90 +303,45 @@ * * @return array Error messages on validation error */ - protected function validateLogin($login, $domain, $external = false) + protected static function validateLogin($login, $domain, $external = false): ?array { - // don't allow @localhost and other no-fqdn - if (empty($domain) || strpos($domain, '.') === false || stripos($domain, 'www.') === 0) { - return ['domain' => 'validation.domaininvalid']; - } + // Validate login part alone + $v = Validator::make( + ['login' => $login], + ['login' => ['required', 'string', new UserEmailLocal($external)]] + ); - // Local part validation - if (!preg_match('/^[A-Za-z0-9_.-]+$/', $login)) { - return ['login' => 'validation.logininvalid']; + if ($v->fails()) { + return ['login' => $v->errors()->toArray()['login'][0]]; } - $domain = Str::lower($domain); + $domains = $external ? null : Domain::getPublicDomains(); - if (!$external) { - // Check if the local part is not one of exceptions - $exceptions = '/^(admin|administrator|sales|root)$/i'; - if (preg_match($exceptions, $login)) { - return ['login' => 'validation.loginexists']; - } + // Validate the domain + $v = Validator::make( + ['domain' => $domain], + ['domain' => ['required', 'string', new UserEmailDomain($domains)]] + ); - // Check if specified domain is allowed for signup - if (!in_array($domain, Domain::getPublicDomains())) { - return ['domain' => 'validation.domaininvalid']; - } - } else { - // Use email validator to validate the domain part - $v = Validator::make(['email' => 'user@' . $domain], ['email' => 'required|email']); - if ($v->fails()) { - return ['domain' => 'validation.domaininvalid']; - } + if ($v->fails()) { + return ['domain' => $v->errors()->toArray()['domain'][0]]; + } - // TODO: DNS registration check - maybe after signup? + $domain = Str::lower($domain); - // Check if domain is already registered with us + // Check if domain is already registered with us + if ($external) { if (Domain::where('namespace', $domain)->first()) { - return ['domain' => 'validation.domainexists']; + return ['domain' => \trans('validation.domainexists')]; } } // Check if user with specified login already exists - // TODO: Aliases $email = $login . '@' . $domain; if (User::findByEmail($email)) { - return ['login' => 'validation.loginexists']; - } - } - - /** - * Returns plan for the signup process - * - * @returns \App\Plan Plan object selected for current signup process - */ - protected function getPlan() - { - if (!$this->plan) { - // Get the plan if specified and exists... - if ($this->code && $this->code->data['plan']) { - $plan = Plan::where('title', $this->code->data['plan'])->first(); - } - - // ...otherwise use the default plan - if (empty($plan)) { - // TODO: Get default plan title from config - $plan = Plan::where('title', 'individual')->first(); - } - - $this->plan = $plan; - } - - return $this->plan; - } - - /** - * Convert error labels to actual (localized) text - */ - protected function resolveErrors(array $errors): array - { - $result = []; - - foreach ($errors as $idx => $label) { - $result[$idx] = __($label); + return ['login' => \trans('validation.loginexists')]; } - return $result; + return null; } } 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 @@ -4,10 +4,14 @@ use App\Http\Controllers\Controller; use App\Domain; +use App\Rules\UserEmailDomain; +use App\Rules\UserEmailLocal; use App\User; use Illuminate\Http\Request; use Illuminate\Support\Facades\Auth; +use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Validator; +use Illuminate\Support\Str; class UsersController extends Controller { @@ -75,18 +79,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 +105,6 @@ return response()->json(['status' => 'error', 'errors' => $v->errors()], 422); } - $credentials = $request->only('email', 'password'); if ($token = $this->guard()->attempt($credentials)) { @@ -181,10 +173,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 +210,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'; @@ -255,7 +249,45 @@ */ public function store(Request $request) { - // TODO + if ($this->guard()->user()->controller()->id !== $this->guard()->user()->id) { + return $this->errorResponse(403); + } + + if ($error_response = $this->validateUserRequest($request, null, $settings)) { + return $error_response; + } + + $user_name = !empty($settings['first_name']) ? $settings['first_name'] : ''; + if (!empty($settings['last_name'])) { + $user_name .= ' ' . $settings['last_name']; + } + + DB::beginTransaction(); + + // Create user record + $user = User::create([ + 'name' => $user_name, + 'email' => $request->email, + 'password' => $request->password, + ]); + + if (!empty($settings)) { + $user->setSettings($settings); + } + + // TODO: Assign package + + // Add aliases + if (!empty($request->aliases)) { + $user->setAliases($request->aliases); + } + + DB::commit(); + + return response()->json([ + 'status' => 'success', + 'message' => __('app.user-create-success'), + ]); } /** @@ -278,41 +310,29 @@ return $this->errorResponse(404); } - $rules = [ - 'external_email' => 'nullable|email', - 'phone' => 'string|nullable|max:64|regex:/^[0-9+() -]+$/', - 'first_name' => 'string|nullable|max:512', - 'last_name' => 'string|nullable|max:512', - 'billing_address' => 'string|nullable|max:1024', - 'country' => 'string|nullable|alpha|size:2', - 'currency' => 'string|nullable|alpha|size:3', - ]; - - if (!empty($request->password) || !empty($request->password_confirmation)) { - $rules['password'] = 'required|min:4|max:2048|confirmed'; + if ($error_response = $this->validateUserRequest($request, $user, $settings)) { + return $error_response; } - // Validate input - $v = Validator::make($request->all(), $rules); - - if ($v->fails()) { - return response()->json(['status' => 'error', 'errors' => $v->errors()], 422); - } - - // Update user settings - $settings = $request->only(array_keys($rules)); - unset($settings['password']); + DB::beginTransaction(); if (!empty($settings)) { $user->setSettings($settings); } // Update user password - if (!empty($rules['password'])) { + if (!empty($request->password)) { $user->password = $request->password; $user->save(); } + // Update aliases + if (isset($request->aliases)) { + $user->setAliases($request->aliases); + } + + DB::commit(); + return response()->json([ 'status' => 'success', 'message' => __('app.user-update-success'), @@ -345,4 +365,181 @@ 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; + } + + /** + * Validate user input + * + * @param \Illuminate\Http\Request $request The API request. + * @param \App\User|null $user User identifier + * @param array $settings User settings (from the request) + * + * @return \Illuminate\Http\JsonResponse The response on error + */ + protected function validateUserRequest(Request $request, $user, &$settings = []) + { + $rules = [ + 'external_email' => 'nullable|email', + 'phone' => 'string|nullable|max:64|regex:/^[0-9+() -]+$/', + 'first_name' => 'string|nullable|max:512', + 'last_name' => 'string|nullable|max:512', + 'billing_address' => 'string|nullable|max:1024', + 'country' => 'string|nullable|alpha|size:2', + 'currency' => 'string|nullable|alpha|size:3', + 'aliases' => 'array|nullable', + ]; + + if (empty($user) || !empty($request->password) || !empty($request->password_confirmation)) { + $rules['password'] = 'required|min:4|max:2048|confirmed'; + } + + $errors = []; + + // Validate input + $v = Validator::make($request->all(), $rules); + + if ($v->fails()) { + $errors = $v->errors()->toArray(); + } + + $controller = $user ? $user->controller() : $this->guard()->user(); + + // For new user validate email address + if (empty($user)) { + $email = $request->email; + + if (empty($email)) { + $errors['email'] = \trans('validation.required', ['attribute' => 'email']); + } elseif ($error = self::validateEmail($email, $controller, false)) { + $errors['email'] = $error; + } + } + + // Validate aliases input + if (isset($request->aliases)) { + $aliases = []; + $existing_aliases = $user ? $user->aliases()->get()->pluck('alias')->toArray() : []; + + foreach ($request->aliases as $idx => $alias) { + if (is_string($alias) && !empty($alias)) { + // Alias cannot be the same as the email address (new user) + if (!empty($email) && Str::lower($alias) == Str::lower($email)) { + continue; + } + + // validate new aliases + if ( + !in_array($alias, $existing_aliases) + && ($error = self::validateEmail($alias, $controller, true)) + ) { + if (!isset($errors['aliases'])) { + $errors['aliases'] = []; + } + $errors['aliases'][$idx] = $error; + continue; + } + + $aliases[] = $alias; + } + } + + $request->aliases = $aliases; + } + + if (!empty($errors)) { + return response()->json(['status' => 'error', 'errors' => $errors], 422); + } + + // Update user settings + $settings = $request->only(array_keys($rules)); + unset($settings['password'], $settings['aliases'], $settings['email']); + } + + /** + * Email address (login or alias) validation + * + * @param string $email Email address + * @param \App\User $user The account owner + * @param bool $is_alias The email is an alias + * + * @return string Error message on validation error + */ + protected static function validateEmail(string $email, User $user, bool $is_alias = false): ?string + { + $attribute = $is_alias ? 'alias' : 'email'; + + if (strpos($email, '@') === false) { + return \trans('validation.entryinvalid', ['attribute' => $attribute]); + } + + list($login, $domain) = explode('@', $email); + + // Check if domain exists + $domain = Domain::where('namespace', Str::lower($domain))->first(); + + if (empty($domain)) { + return \trans('validation.domaininvalid'); + } + + // Validate login part alone + $v = Validator::make( + [$attribute => $login], + [$attribute => ['required', new UserEmailLocal(!$domain->isPublic())]] + ); + + if ($v->fails()) { + return $v->errors()->toArray()[$attribute][0]; + } + + // Check if it is one of domains available to the user + // TODO: We should have a helper that returns "flat" array with domain names + // I guess we could use pluck() somehow + $domains = array_map( + function ($domain) { + return $domain->namespace; + }, + $user->domains() + ); + + if (!in_array($domain->namespace, $domains)) { + return \trans('validation.entryexists', ['attribute' => 'domain']); + } + + // Check if user with specified address already exists + if (User::findByEmail($email)) { + return \trans('validation.entryexists', ['attribute' => $attribute]); + } + + return null; + } } 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/Rules/ExternalEmail.php b/src/app/Rules/ExternalEmail.php new file mode 100644 --- /dev/null +++ b/src/app/Rules/ExternalEmail.php @@ -0,0 +1,52 @@ + $email], ['email' => 'required|email']); + + if ($v->fails()) { + $this->message = \trans('validation.emailinvalid'); + return false; + } + + list($local, $domain) = explode('@', $email); + + // don't allow @localhost and other no-fqdn + if (strpos($domain, '.') === false) { + $this->message = \trans('validation.emailinvalid'); + return false; + } + + return true; + } + + /** + * Get the validation error message. + * + * @return string + */ + public function message(): ?string + { + return $this->message; + } +} diff --git a/src/app/Rules/UserEmailDomain.php b/src/app/Rules/UserEmailDomain.php new file mode 100644 --- /dev/null +++ b/src/app/Rules/UserEmailDomain.php @@ -0,0 +1,70 @@ +domains = $domains; + } + + /** + * Determine if the validation rule passes. + * + * Validation of local part of an email address that's + * going to be user's login. + * + * @param string $attribute Attribute name + * @param mixed $domain Domain part of email address + * + * @return bool + */ + public function passes($attribute, $domain): bool + { + // don't allow @localhost and other no-fqdn + if (empty($domain) || strpos($domain, '.') === false || stripos($domain, 'www.') === 0) { + $this->message = \trans('validation.domaininvalid'); + return false; + } + + $domain = Str::lower($domain); + + // Use email validator to validate the domain part + $v = Validator::make(['email' => 'user@' . $domain], ['email' => 'required|email']); + if ($v->fails()) { + $this->message = \trans('validation.domaininvalid'); + return false; + } + + // Check if specified domain is allowed for signup + if (is_array($this->domains) && !in_array($domain, $this->domains)) { + $this->message = \trans('validation.domaininvalid'); + return false; + } + + return true; + } + + /** + * Get the validation error message. + * + * @return string + */ + public function message(): ?string + { + return $this->message; + } +} diff --git a/src/app/Rules/UserEmailLocal.php b/src/app/Rules/UserEmailLocal.php new file mode 100644 --- /dev/null +++ b/src/app/Rules/UserEmailLocal.php @@ -0,0 +1,72 @@ +external = $external; + } + + /** + * Determine if the validation rule passes. + * + * Validation of local part of an email address that's + * going to be user's login. + * + * @param string $attribute Attribute name + * @param mixed $login Local part of email address + * + * @return bool + */ + public function passes($attribute, $login): bool + { + // Strict validation + if (!preg_match('/^[A-Za-z0-9_.-]+$/', $login)) { + $this->message = \trans('validation.entryinvalid', ['attribute' => $attribute]); + return false; + } + + // Standard email address validation + $v = Validator::make([$attribute => $login . '@test.com'], [$attribute => 'required|email']); + if ($v->fails()) { + $this->message = \trans('validation.entryinvalid', ['attribute' => $attribute]); + return false; + } + + // Check if the local part is not one of exceptions + // (when signing up for an account in public domain + if (!$this->external) { + $exceptions = '/^(admin|administrator|sales|root)$/i'; + + if (preg_match($exceptions, $login)) { + $this->message = \trans('validation.entryexists', ['attribute' => $attribute]); + return false; + } + } + + return true; + } + + /** + * Get the validation error message. + * + * @return string + */ + public function message(): ?string + { + return $this->message; + } +} 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 $aliases 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; @@ -96,6 +102,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. * * @param \App\Package $package The package to assign. @@ -128,6 +144,26 @@ 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); @@ -216,7 +252,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 +260,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/js/app.js b/src/resources/js/app.js --- a/src/resources/js/app.js +++ b/src/resources/js/app.js @@ -35,17 +35,46 @@ const input = $('#' + input_name) if (input.length) { - input.addClass('is-invalid') - input.parent().find('.invalid-feedback').remove() - input.parent().append($('
') - .text($.type(msg) === 'string' ? msg : msg.join(' '))) + // Create an error message\ + // API responses can use a string, array or object + let msg_text = '' + if ($.type(msg) !== 'string') { + $.each(msg, (index, str) => { + msg_text += str + ' ' + }) + } + else { + msg_text = msg + } + + let feedback = $('
').text(msg_text) + + if (input.is('.listinput')) { + // List input widget + let list = input.next('.listinput-widget') + + list.children(':not(:first-child)').each((index, element) => { + if (msg[index]) { + $(element).find('input').addClass('is-invalid') + } + }) + + list.addClass('is-invalid').next('.invalid-feedback').remove() + list.after(feedback) + } + else { + // Standard form element + input.addClass('is-invalid') + input.parent().find('.invalid-feedback').remove() + input.parent().append(feedback) + } return false } }); }) - $('form .is-invalid').first().focus() + $('form .is-invalid:not(.listinput-widget)').first().focus() } else if (error.response && error.response.data) { error_msg = error.response.data.message 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 @@ -22,4 +22,5 @@ 'domain-verify-success' => 'Domain verified successfully', 'user-update-success' => 'User data updated successfully', + 'user-create-success' => 'User created successfully', ]; diff --git a/src/resources/lang/en/validation.php b/src/resources/lang/en/validation.php --- a/src/resources/lang/en/validation.php +++ b/src/resources/lang/en/validation.php @@ -117,14 +117,16 @@ 'url' => 'The :attribute format is invalid.', 'uuid' => 'The :attribute must be a valid UUID.', - 'emailinvalid' => 'The specified email address is invalid', - 'domaininvalid' => 'The specified domain is invalid', - 'logininvalid' => 'The specified login contains forbidden characters', - 'loginexists' => 'The specified login is not available for signup', - 'domainexists' => 'The specified domain is not available for signup', - 'noemailorphone' => 'The specified text is neither a valid email address nor a phone number', - 'usernotexists' => 'Unable to find user', - 'noextemail' => 'This user has no external email address', + 'emailinvalid' => 'The specified email address is invalid.', + 'domaininvalid' => 'The specified domain is invalid.', + 'logininvalid' => 'The specified login is invalid.', + 'loginexists' => 'The specified login is not available.', + 'domainexists' => 'The specified domain is not available.', + 'noemailorphone' => 'The specified text is neither a valid email address nor a phone number.', + 'usernotexists' => 'Unable to find user.', + 'noextemail' => 'This user has no external email address.', + 'entryinvalid' => 'The specified :attribute is invalid.', + 'entryexists' => 'The specified :attribute is not available.', /* |-------------------------------------------------------------------------- @@ -155,5 +157,4 @@ */ 'attributes' => [], - ]; diff --git a/src/resources/sass/app.scss b/src/resources/sass/app.scss --- a/src/resources/sass/app.scss +++ b/src/resources/sass/app.scss @@ -78,3 +78,27 @@ font-size: 1.2rem; font-weight: bold; } + +.listinput { + display: none; +} + +.listinput-widget { + & > div { + &:not(:last-child) { + margin-bottom: -1px; + + input, a.btn { + border-bottom-right-radius: 0; + border-bottom-left-radius: 0; + } + } + + &:not(:first-child) { + input, a.btn { + border-top-right-radius: 0; + border-top-left-radius: 0; + } + } + } +} 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 @@ -2,8 +2,48 @@
-
User account
+
User account
+
New user account
+
+
+ +
+ +
+
+
+ +
+ +
+
+
+ +
+ +
+
+
+ +
+ +
+
+
+ +
+ +
+
+
+ +
+ +
+
+ +
@@ -15,19 +55,135 @@ data() { return { user_id: null, - user: null + user: {} } }, created() { - if (this.user_id = this.$route.params.user) { + this.user_id = this.$route.params.user + + if (this.user_id === 'new') { + // do nothing (for now) + } + else { 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 + $('#aliases').val(response.data.aliases.join("\n")) + listinput('#aliases') }) .catch(this.$root.errorHandler) - } else { - this.$root.errorPage(404) + } + }, + mounted() { + if (this.user_id === 'new') { + listinput('#aliases') + } + + $('#first_name').focus() + }, + methods: { + submit() { + this.$root.clearFormValidation($('#user-info form')) + + this.user.aliases = $('#aliases').val().split("\n") + + let method = 'post' + let location = '/api/v4/users' + + if (this.user_id !== 'new') { + method = 'put' + location += '/' + this.user_id + } + + axios[method](location, this.user) + .then(response => { + delete this.user.password + delete this.user.password_confirm + + if (response.data.status == 'success') { + this.$toastr('success', response.data.message) + } + + // on new user redirect to users list + if (this.user_id === 'new') { + this.$route.push({ name: 'users' }) + } + }) } } } + + // List widget + // TODO: move it to a separate component file when needed + function listinput(elem) + { + elem = $(elem).addClass('listinput'); + + let widget = $('
') + let main_row = $('
') + let wrap = $('
') + let input = $('') + let add_btn = $('').text('Add') + + let update = () => { + let value = [] + + widget.find('input:not(.main-input)').each((index, input) => { + if (input.value) { + value.push(input.value) + } + }) + + elem.val(value.join("\n")) + } + + let add_func = (value) => { + let row = $('
') + let rinput = $('').val(value) + let rwrap = $('
') + let del_btn = $('') + .text('Remove') + .on('click', e => { + row.remove() + input.focus() + update() + }) + + widget.append(row.append(rinput).append(rwrap.append(del_btn))) + } + + // Create the widget and add to DOM + widget.append(main_row.append(input).append(wrap.append(add_btn))) + .insertAfter(elem) + + // Add rows for every line in the original textarea + let value = $.trim(elem.val()) + if (value.length) { + value.split("\n").forEach(add_func) + } + + // Click handler on the Add button + add_btn.on('click', e => { + let value = input.val() + + if (!value) { + return; + } + + input.val('').focus(); + add_func(value) + update() + }) + + // Enter key handler on main input + input.on('keydown', function(e) { + if (e.which == 13 && this.value) { + add_btn.click() + return false + } + }) + } + 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 @@ -4,6 +4,7 @@
User Accounts
+ Create user diff --git a/src/tests/Browser/Components/ListInput.php b/src/tests/Browser/Components/ListInput.php new file mode 100644 --- /dev/null +++ b/src/tests/Browser/Components/ListInput.php @@ -0,0 +1,109 @@ +selector = $selector; + } + + /** + * Get the root selector for the component. + * + * @return string + */ + public function selector() + { + return $this->selector . ' + .listinput-widget'; + } + + /** + * Assert that the browser page contains the component. + * + * @param Browser $browser + * + * @return void + */ + public function assert(Browser $browser) + { +// $list = explode("\n", $browser->value($this->selector)); + + $browser->waitFor($this->selector()) + ->assertMissing($this->selector) + ->assertVisible('@input') + ->assertVisible('@add-btn'); +// ->assertListInputValue($list); + } + + /** + * Get the element shortcuts for the component. + * + * @return array + */ + public function elements() + { + return [ + '@input' => '.input-group:first-child input', + '@add-btn' => '.input-group:first-child a.btn', + ]; + } + + /** + * Assert list input content + */ + public function assertListInputValue(Browser $browser, array $list) + { + if (empty($list)) { + $browser->assertMissing('.input-group:not(:first-child)'); + return; + } + + foreach ($list as $idx => $value) { + $selector = '.input-group:nth-child(' . ($idx + 2) . ') input'; + $browser->assertVisible($selector)->assertValue($selector, $value); + } + } + + /** + * Add list entry + */ + public function addListEntry(Browser $browser, string $value) + { + $browser->type('@input', $value) + ->click('@add-btn') + ->assertValue('.input-group:last-child input', $value); + } + + /** + * Remove list entry + */ + public function removeListEntry(Browser $browser, int $num) + { + $selector = '.input-group:nth-child(' . ($num + 1) . ') a.btn'; + $browser->click($selector)->assertMissing($selector); + } + + /** + * Assert an error message on the widget + */ + public function assertFormError(Browser $browser, int $num, string $msg, bool $focused = false) + { + $selector = '.input-group:nth-child(' . ($num + 1) . ') input.is-invalid'; + + $browser->assertVisible($selector) + ->assertSeeIn(' + .invalid-feedback', $msg); + + if ($focused) { + $browser->assertFocused($selector); + } + } +} diff --git a/src/tests/Browser/Components/Toast.php b/src/tests/Browser/Components/Toast.php --- a/src/tests/Browser/Components/Toast.php +++ b/src/tests/Browser/Components/Toast.php @@ -14,6 +14,7 @@ public const TYPE_INFO = 'info'; protected $type; + protected $element; public function __construct($type) @@ -41,6 +42,7 @@ public function assert(Browser $browser) { $browser->waitFor($this->selector()); + $this->element = $browser->element($this->selector()); } /** @@ -81,6 +83,6 @@ */ public function closeToast(Browser $browser) { - $browser->click(); + $this->element->click(); } } 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,44 @@ +waitFor('@form'); + } + + /** + * 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,305 @@ + 'John', + 'last_name' => 'Doe', + ]; + + /** + * {@inheritDoc} + */ + public function setUp(): void + { + parent::setUp(); + + // TODO: Use TestCase::deleteTestUser() + User::withTrashed()->where('email', 'john.rambo@kolab.org')->forceDelete(); + + $john = User::where('email', 'john@kolab.org')->first(); + $john->setSettings($this->profile); + UserAlias::where('user_id', $john->id) + ->where('alias', 'john.test@kolab.org')->delete(); + } + + /** + * {@inheritDoc} + */ + public function tearDown(): void + { + // TODO: Use TestCase::deleteTestUser() + User::withTrashed()->where('email', 'john.rambo@kolab.org')->forceDelete(); + + $john = User::where('email', 'john@kolab.org')->first(); + $john->setSettings($this->profile); + UserAlias::where('user_id', $john->id) + ->where('alias', 'john.test@kolab.org')->delete(); + + 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()); + }); + + // TODO: Test that jack@kolab.org can't access this page + } + + /** + * 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 user account editing page (not 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()) + ->assertSeeIn('#user-info .card-title', 'User account') + ->with('@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') +//TODO ->assertDisabled('div.row:nth-child(3) input') + ->assertSeeIn('div.row:nth-child(4) label', 'Email aliases') + ->assertVisible('div.row:nth-child(4) .listinput-widget') + ->with(new ListInput('#aliases'), function (Browser $browser) { + $browser->assertListInputValue(['john.doe@kolab.org']) + ->assertValue('@input', ''); + }) + ->assertSeeIn('div.row:nth-child(5) label', 'Password') + ->assertValue('div.row:nth-child(5) input[type=password]', '') + ->assertSeeIn('div.row:nth-child(6) label', 'Confirm password') + ->assertValue('div.row:nth-child(6) 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 (password) + $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 + + // Test form error handling (aliases) + $browser->with('@form', function (Browser $browser) { + // TODO: For some reason, clearing the input value + // with ->type('#password', '') does not work, maybe some dusk/vue intricacy + // For now we just use the default password + $browser->type('#password', 'simple123') + ->type('#password_confirmation', 'simple123') + ->with(new ListInput('#aliases'), function (Browser $browser) { + $browser->addListEntry('invalid address'); + }) + ->click('button[type=submit]'); + }) + ->with(new Toast(Toast::TYPE_ERROR), function (Browser $browser) { + $browser->assertToastTitle('Error') + ->assertToastMessage('Form validation error') + ->closeToast(); + }) + ->with('@form', function (Browser $browser) { + $browser->with(new ListInput('#aliases'), function (Browser $browser) { + $browser->assertFormError(2, 'The specified alias is invalid.', false); + }); + }); + + // Test adding aliases + $browser->with('@form', function (Browser $browser) { + $browser->with(new ListInput('#aliases'), function (Browser $browser) { + $browser->removeListEntry(2) + ->addListEntry('john.test@kolab.org'); + }) + ->click('button[type=submit]'); + }) + ->with(new Toast(Toast::TYPE_SUCCESS), function (Browser $browser) { + $browser->assertToastTitle('') + ->assertToastMessage('User data updated successfully') + ->closeToast(); + }); + + $john = User::where('email', 'john@kolab.org')->first(); + $alias = UserAlias::where('user_id', $john->id)->where('alias', 'john.test@kolab.org')->first(); + $this->assertTrue(!empty($alias)); + }); + } + + /** + * Test user adding page + * + * @depends testList + */ + public function testNewUser(): void + { + $this->browse(function (Browser $browser) { + $browser->visit(new UserList()) + ->assertSeeIn('button.create-user', 'Create user') + ->click('button.create-user') + ->on(new UserInfo()) + ->assertSeeIn('#user-info .card-title', 'New user account') + ->with('@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]', '') + ->assertSeeIn('div.row:nth-child(2) label', 'Last name') + ->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') + ->assertSeeIn('div.row:nth-child(4) label', 'Email aliases') + ->assertVisible('div.row:nth-child(4) .listinput-widget') + ->with(new ListInput('#aliases'), function (Browser $browser) { + $browser->assertListInputValue([]) + ->assertValue('@input', ''); + }) + ->assertSeeIn('div.row:nth-child(5) label', 'Password') + ->assertValue('div.row:nth-child(5) input[type=password]', '') + ->assertSeeIn('div.row:nth-child(6) label', 'Confirm password') + ->assertValue('div.row:nth-child(6) input[type=password]', '') + ->assertSeeIn('button[type=submit]', 'Submit'); + + // Test browser-side required fields and error handling + $browser->click('button[type=submit]') + ->assertFocused('#email') + ->type('#email', 'invalid email') + ->click('button[type=submit]') + ->assertFocused('#password') + ->type('#password', 'simple123') + ->click('button[type=submit]') + ->assertFocused('#password_confirmation') + ->type('#password_confirmation', 'simple') + ->click('button[type=submit]'); + }) + ->with(new Toast(Toast::TYPE_ERROR), function (Browser $browser) { + $browser->assertToastTitle('Error') + ->assertToastMessage('Form validation error') + ->closeToast(); + }) + ->with('@form', function (Browser $browser) { + $browser->assertSeeIn('#email + .invalid-feedback', 'The specified email is invalid.') + ->assertSeeIn('#password + .invalid-feedback', 'The password confirmation does not match.'); + }); + + // Test form error handling (aliases) + $browser->with('@form', function (Browser $browser) { + $browser->type('#email', 'john.rambo@kolab.org') + ->type('#password_confirmation', 'simple123') + ->with(new ListInput('#aliases'), function (Browser $browser) { + $browser->addListEntry('invalid address'); + }) + ->click('button[type=submit]'); + }) + ->with(new Toast(Toast::TYPE_ERROR), function (Browser $browser) { + $browser->assertToastTitle('Error') + ->assertToastMessage('Form validation error') + ->closeToast(); + }) + ->with('@form', function (Browser $browser) { + $browser->with(new ListInput('#aliases'), function (Browser $browser) { + $browser->assertFormError(1, 'The specified alias is invalid.', false); + }); + }); + + // Successful account creation + $browser->with('@form', function (Browser $browser) { + $browser->with(new ListInput('#aliases'), function (Browser $browser) { + $browser->removeListEntry(1) + ->addListEntry('john.rambo2@kolab.org'); + }) + ->click('button[type=submit]'); + }) + ->with(new Toast(Toast::TYPE_SUCCESS), function (Browser $browser) { + $browser->assertToastTitle('') + ->assertToastMessage('User created successfully') + ->closeToast(); + }); + + // TODO: assert redirect to users list + + $john = User::where('email', 'john.rambo@kolab.org')->first(); + $alias = UserAlias::where('user_id', $john->id)->where('alias', 'john.rambo2@kolab.org')->first(); + $this->assertTrue(!empty($alias)); + }); + } +} diff --git a/src/tests/Feature/Controller/SignupTest.php b/src/tests/Feature/Controller/SignupTest.php --- a/src/tests/Feature/Controller/SignupTest.php +++ b/src/tests/Feature/Controller/SignupTest.php @@ -550,92 +550,52 @@ } /** - * List of email address validation cases for testValidateEmail() - * - * @return array Arguments for testValidateEmail() - */ - public function dataValidateEmail() - { - return [ - // invalid - ['', 'validation.emailinvalid'], - ['example.org', 'validation.emailinvalid'], - ['@example.org', 'validation.emailinvalid'], - ['test@localhost', 'validation.emailinvalid'], - // valid - ['test@domain.tld', null], - ['&@example.org', null], - ]; - } - - /** - * Signup email validation. - * - * Note: Technicly these are unit tests, but let's keep it here for now. - * FIXME: Shall we do a http request for each case? - * - * @dataProvider dataValidateEmail - */ - public function testValidateEmail($email, $expected_result) - { - $method = new \ReflectionMethod('App\Http\Controllers\API\SignupController', 'validateEmail'); - $method->setAccessible(true); - - $result = $method->invoke(new SignupController(), $email); - - $this->assertSame($expected_result, $result); - } - - /** * List of login/domain validation cases for testValidateLogin() * * @return array Arguments for testValidateLogin() */ - public function dataValidateLogin() + public function dataValidateLogin(): array { $domain = $this->getPublicDomain(); return [ // Individual account - ['', $domain, false, ['login' => 'validation.logininvalid']], - ['test123456', 'localhost', false, ['domain' => 'validation.domaininvalid']], - ['test123456', 'unknown-domain.org', false, ['domain' => 'validation.domaininvalid']], + ['', $domain, false, ['login' => 'The login field is required.']], + ['test123456', 'localhost', false, ['domain' => 'The specified domain is invalid.']], + ['test123456', 'unknown-domain.org', false, ['domain' => 'The specified domain is invalid.']], ['test.test', $domain, false, null], ['test_test', $domain, false, null], ['test-test', $domain, false, null], - ['admin', $domain, false, ['login' => 'validation.loginexists']], - ['administrator', $domain, false, ['login' => 'validation.loginexists']], - ['sales', $domain, false, ['login' => 'validation.loginexists']], - ['root', $domain, false, ['login' => 'validation.loginexists']], + ['admin', $domain, false, ['login' => 'The specified login is not available.']], + ['administrator', $domain, false, ['login' => 'The specified login is not available.']], + ['sales', $domain, false, ['login' => 'The specified login is not available.']], + ['root', $domain, false, ['login' => 'The specified login is not available.']], - // existing user - ['jack', 'kolab.org', true, ['domain' => 'validation.domainexists']], + // TODO existing (public domain) user + // ['signuplogin', $domain, false, ['login' => 'The specified login is not available.']], // Domain account ['admin', 'kolabsys.com', true, null], - ['testnonsystemdomain', 'invalid', true, ['domain' => 'validation.domaininvalid']], - ['testnonsystemdomain', '.com', true, ['domain' => 'validation.domaininvalid']], + ['testnonsystemdomain', 'invalid', true, ['domain' => 'The specified domain is invalid.']], + ['testnonsystemdomain', '.com', true, ['domain' => 'The specified domain is invalid.']], - // existing user - ['john', 'kolab.org', true, ['domain' => 'validation.domainexists']], + // existing custom domain + ['jack', 'kolab.org', true, ['domain' => 'The specified domain is not available.']], ]; } /** * Signup login/domain validation. * - * Note: Technicly these include unit tests, but let's keep it here for now. + * Note: Technically these include unit tests, but let's keep it here for now. * FIXME: Shall we do a http request for each case? * * @dataProvider dataValidateLogin */ - public function testValidateLogin($login, $domain, $external, $expected_result) + public function testValidateLogin($login, $domain, $external, $expected_result): void { - $method = new \ReflectionMethod('App\Http\Controllers\API\SignupController', 'validateLogin'); - $method->setAccessible(true); + $result = $this->invokeMethod(new SignupController(), 'validateLogin', [$login, $domain, $external]); - $result = $method->invoke(new SignupController(), $login, $domain, $external); - - $this->assertSame($expected_result, $result, var_export(func_get_args(), true)); + $this->assertSame($expected_result, $result); } } 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 @@ -20,6 +20,7 @@ $this->deleteTestUser('UsersControllerTest1@userscontroller.com'); $this->deleteTestUser('UserEntitlement2A@UserEntitlement.com'); + $this->deleteTestUser('john2.doe2@kolab.org'); $this->deleteTestDomain('userscontroller.com'); } @@ -30,6 +31,7 @@ { $this->deleteTestUser('UsersControllerTest1@userscontroller.com'); $this->deleteTestUser('UserEntitlement2A@UserEntitlement.com'); + $this->deleteTestUser('john2.doe2@kolab.org'); $this->deleteTestDomain('userscontroller.com'); parent::tearDown(); @@ -55,8 +57,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 @@ -208,6 +212,29 @@ } /** + * 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($user->status, $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/) */ public function testShow(): void @@ -217,8 +244,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'); @@ -234,8 +267,90 @@ */ public function testStore(): void { - // TODO - $this->markTestIncomplete(); + $jack = $this->getTestUser('jack@kolab.org'); + $john = $this->getTestUser('john@kolab.org'); + + // Test empty request + $response = $this->actingAs($john)->post("/api/v4/users", []); + $response->assertStatus(422); + + $json = $response->json(); + + $this->assertSame('error', $json['status']); + $this->assertSame("The email field is required.", $json['errors']['email']); + $this->assertSame("The password field is required.", $json['errors']['password'][0]); + $this->assertCount(2, $json); + + // Test access by user not being a wallet controller + $post = ['first_name' => 'Test']; + $response = $this->actingAs($jack)->post("/api/v4/users", $post); + $json = $response->json(); + + $response->assertStatus(403); + + $this->assertSame('error', $json['status']); + $this->assertSame("Access denied", $json['message']); + $this->assertCount(2, $json); + + // Test some invalid data + $post = ['password' => '12345678', 'email' => 'invalid']; + $response = $this->actingAs($john)->post("/api/v4/users", $post); + $response->assertStatus(422); + + $json = $response->json(); + + $this->assertSame('error', $json['status']); + $this->assertCount(2, $json); + $this->assertSame('The password confirmation does not match.', $json['errors']['password'][0]); + $this->assertSame('The specified email is invalid.', $json['errors']['email']); + + // Test existing user email + $post = [ + 'password' => 'simple', + 'password_confirmation' => 'simple', + 'first_name' => 'John2', + 'last_name' => 'Doe2', + 'email' => 'jack.daniels@kolab.org', + ]; + + $response = $this->actingAs($john)->post("/api/v4/users", $post); + $response->assertStatus(422); + + $json = $response->json(); + + $this->assertSame('error', $json['status']); + $this->assertCount(2, $json); + $this->assertSame('The specified email is not available.', $json['errors']['email']); + + // Test full user data + $post = [ + 'password' => 'simple', + 'password_confirmation' => 'simple', + 'first_name' => 'John2', + 'last_name' => 'Doe2', + 'email' => 'john2.doe2@kolab.org', + 'aliases' => ['useralias1@kolab.org', 'useralias2@kolab.org'] + ]; + + $response = $this->actingAs($john)->post("/api/v4/users", $post); + $json = $response->json(); + + $response->assertStatus(200); + + $this->assertSame('success', $json['status']); + $this->assertSame("User created successfully", $json['message']); + $this->assertCount(2, $json); + + $user = User::where('email', 'john2.doe2@kolab.org')->first(); + $this->assertInstanceOf(User::class, $user); + $this->assertSame('John2', $user->getSetting('first_name')); + $this->assertSame('Doe2', $user->getSetting('last_name')); + $aliases = $user->aliases()->orderBy('alias')->get(); + $this->assertCount(2, $aliases); + $this->assertSame('useralias1@kolab.org', $aliases[0]->alias); + $this->assertSame('useralias2@kolab.org', $aliases[1]->alias); + + // TODO: Test assigning a package to new user } /** @@ -244,13 +359,17 @@ public function testUpdate(): void { $userA = $this->getTestUser('UsersControllerTest1@userscontroller.com'); - $userB = $this->getTestUser('jack@kolab.org'); + $jack = $this->getTestUser('jack@kolab.org'); + $domain = $this->getTestDomain( + 'userscontroller.com', + ['status' => Domain::STATUS_NEW, 'type' => Domain::TYPE_EXTERNAL] + ); // Test unauthorized update of other user profile - $response = $this->actingAs($userB)->get("/api/v4/users/{$userA->id}", []); + $response = $this->actingAs($jack)->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 +402,7 @@ 'billing_address' => 'billing', 'country' => 'CH', 'currency' => 'CHF', + 'aliases' => ['useralias1@' . \config('app.domain'), 'useralias2@' . \config('app.domain')] ]; $response = $this->actingAs($userA)->put("/api/v4/users/{$userA->id}", $post); @@ -294,10 +414,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()->orderBy('alias')->get(); + $this->assertCount(2, $aliases); + $this->assertSame('useralias1@' . \config('app.domain'), $aliases[0]->alias); + $this->assertSame('useralias2@' . \config('app.domain'), $aliases[1]->alias); // Test unsetting values $post = [ @@ -308,6 +432,7 @@ 'billing_address' => '', 'country' => '', 'currency' => '', + 'aliases' => ['useralias2@' . \config('app.domain')] ]; $response = $this->actingAs($userA)->put("/api/v4/users/{$userA->id}", $post); @@ -318,11 +443,95 @@ $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@' . \config('app.domain'), $aliases[0]->alias); + + // Test error on setting an alias to other user's domain + // and missing password confirmation + $post = [ + 'password' => 'simple123', + 'aliases' => ['useralias2@' . \config('app.domain'), 'useralias1@kolab.org'] + ]; + + $response = $this->actingAs($userA)->put("/api/v4/users/{$userA->id}", $post); + $json = $response->json(); + + $response->assertStatus(422); + + $this->assertSame('error', $json['status']); + $this->assertCount(2, $json['errors']); + $this->assertCount(1, $json['errors']['aliases']); + $this->assertSame("The specified domain is not available.", $json['errors']['aliases'][1]); + $this->assertSame("The password confirmation does not match.", $json['errors']['password'][0]); + // TODO: Test error on aliases with invalid/non-existing/other-user's domain // TODO: Test authorized update of other user $this->markTestIncomplete(); } + + /** + * List of alias validation cases for testValidateEmail() + * + * @return array Arguments for testValidateEmail() + */ + public function dataValidateEmail(): array + { + $this->refreshApplication(); + $public_domains = Domain::getPublicDomains(); + $domain = reset($public_domains); + + $john = $this->getTestUser('john@kolab.org'); + $jack = $this->getTestUser('jack@kolab.org'); + $user = $this->getTestUser('UsersControllerTest1@userscontroller.com'); + + return [ + // Invalid format + ["$domain", $john, true, 'The specified alias is invalid.'], + [".@$domain", $john, true, 'The specified alias is invalid.'], + ["test123456@localhost", $john, true, 'The specified domain is invalid.'], + ["test123456@unknown-domain.org", $john, true, 'The specified domain is invalid.'], + + ["$domain", $john, false, 'The specified email is invalid.'], + [".@$domain", $john, false, 'The specified email is invalid.'], + + // forbidden local part on public domains + ["admin@$domain", $john, true, 'The specified alias is not available.'], + ["administrator@$domain", $john, true, 'The specified alias is not available.'], + + // forbidden (other user's domain) + ["testtest@kolab.org", $user, true, 'The specified domain is not available.'], + + // existing alias of other user + ["jack.daniels@kolab.org", $john, true, 'The specified alias is not available.'], + + // existing user + ["jack@kolab.org", $john, true, 'The specified alias is not available.'], + + // valid (user domain) + ["admin@kolab.org", $john, true, null], + + // valid (public domain) + ["test.test@$domain", $john, true, null], + ]; + } + + /** + * User email/alias validation. + * + * Note: Technically these include unit tests, but let's keep it here for now. + * FIXME: Shall we do a http request for each case? + * + * @dataProvider dataValidateEmail + */ + public function testValidateEmail($alias, $user, $is_alias, $expected_result): void + { + $result = $this->invokeMethod(new UsersController(), 'validateEmail', [$alias, $user, $is_alias]); + + $this->assertSame($expected_result, $result); + } } 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 @@ -29,6 +29,22 @@ } /** + * Tests for User::assignPackage() + */ + public function testAssignPackage(): void + { + $this->markTestIncomplete(); + } + + /** + * Tests for User::assignPlan() + */ + public function testAssignPlan(): void + { + $this->markTestIncomplete(); + } + + /** * Verify user creation process */ public function testUserCreateJob(): void @@ -87,7 +103,24 @@ $this->assertTrue($userB->accounts()->get()[0]->id === $userA->wallets()->get()[0]->id); } - public function testUserDomains(): void + /** + * Tests for User::controller() + */ + public function testController(): void + { + $john = $this->getTestUser('john@kolab.org'); + + $this->assertSame($john->id, $john->controller()->id); + + $jack = $this->getTestUser('jack@kolab.org'); + + $this->assertSame($john->id, $jack->controller()->id); + } + + /** + * Tests for User::domains() + */ + public function testDomains(): void { $user = $this->getTestUser('john@kolab.org'); $domains = []; @@ -96,8 +129,20 @@ $domains[] = $domain->namespace; } - $this->assertContains('kolabnow.com', $domains); + $this->assertContains(\config('app.domain'), $domains); $this->assertContains('kolab.org', $domains); + + // Jack is not the wallet controller, so for him the list should not + // include John's domains, kolab.org specifically + $user = $this->getTestUser('jack@kolab.org'); + $domains = []; + + foreach ($user->domains() as $domain) { + $domains[] = $domain->namespace; + } + + $this->assertContains(\config('app.domain'), $domains); + $this->assertNotContains('kolab.org', $domains); } public function testUserQuota(): void @@ -158,7 +203,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(['UserAlias1@UserAccount.com']); + + $aliases = $user->aliases()->get(); + $this->assertCount(1, $aliases); + $this->assertSame('useralias1@useraccount.com', $aliases[0]['alias']); + + // Add another alias + $user->setAliases(['UserAlias1@UserAccount.com', 'UserAlias2@UserAccount.com']); + + $aliases = $user->aliases()->orderBy('alias')->get(); + $this->assertCount(2, $aliases); + $this->assertSame('useralias1@useraccount.com', $aliases[0]->alias); + $this->assertSame('useralias2@useraccount.com', $aliases[1]->alias); + + // Remove an alias + $user->setAliases(['UserAlias1@UserAccount.com']); + + $aliases = $user->aliases()->get(); + $this->assertCount(1, $aliases); + $this->assertSame('useralias1@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); + } } diff --git a/src/tests/Unit/Rules/ExternalEmailTest.php b/src/tests/Unit/Rules/ExternalEmailTest.php new file mode 100644 --- /dev/null +++ b/src/tests/Unit/Rules/ExternalEmailTest.php @@ -0,0 +1,52 @@ + $email], + ['email' => [new ExternalEmail()]] + ); + + $result = null; + if ($v->fails()) { + $result = $v->errors()->toArray()['email'][0]; + } + + $this->assertSame($expected_result, $result); + } +} diff --git a/src/tests/Unit/Rules/UserEmailDomainTest.php b/src/tests/Unit/Rules/UserEmailDomainTest.php new file mode 100644 --- /dev/null +++ b/src/tests/Unit/Rules/UserEmailDomainTest.php @@ -0,0 +1,18 @@ +markTestIncomplete(); + } +} diff --git a/src/tests/Unit/Rules/UserEmailLocalTest.php b/src/tests/Unit/Rules/UserEmailLocalTest.php new file mode 100644 --- /dev/null +++ b/src/tests/Unit/Rules/UserEmailLocalTest.php @@ -0,0 +1,18 @@ +markTestIncomplete(); + } +}