diff --git a/src/app/Http/Controllers/API/SignupController.php b/src/app/Http/Controllers/API/SignupController.php index 47366767..5eddb531 100644 --- a/src/app/Http/Controllers/API/SignupController.php +++ b/src/app/Http/Controllers/API/SignupController.php @@ -1,246 +1,257 @@ all(), [ 'email' => 'required', 'name' => 'required', ] ); if ($v->fails()) { return response()->json(['status' => 'error', 'errors' => $v->errors()], 422); } // 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 $code = SignupCode::create([ 'data' => [ 'email' => $request->email, 'name' => $request->name, ] ]); // Send email/sms message if ($is_phone) { SignupVerificationSMS::dispatch($code); } else { SignupVerificationEmail::dispatch($code); } return response()->json(['status' => 'success', 'code' => $code->code]); } /** * Validation of the verification code. * * @param Illuminate\Http\Request HTTP request * * @return \Illuminate\Http\JsonResponse JSON response */ public function verify(Request $request) { // Validate the request args $v = Validator::make( $request->all(), [ 'code' => 'required', 'short_code' => 'required', ] ); if ($v->fails()) { return response()->json(['status' => 'error', 'errors' => $v->errors()], 422); } // Validate the verification code $code = SignupCode::find($request->code); if (empty($code) || $code->isExpired() || Str::upper($request->short_code) !== Str::upper($code->short_code) ) { $errors = ['short_code' => "The code is invalid or expired."]; return response()->json(['status' => 'error', 'errors' => $errors], 422); } // For signup last-step mode remember the code object, so we can delete it // with single SQL query (->delete()) instead of two (::destroy()) $this->code = $code; // Return user name and email/phone from the codes database on success return response()->json([ 'status' => 'success', 'email' => $code->data['email'], 'name' => $code->data['name'], ]); } /** * Finishes the signup process by creating the user account. * * @param Illuminate\Http\Request HTTP request * * @return \Illuminate\Http\JsonResponse JSON response */ public function signup(Request $request) { // Validate input $v = Validator::make( $request->all(), [ 'domain' => 'required|min:3', 'login' => 'required|min:2', 'password' => 'required|min:3|confirmed', ] ); if ($v->fails()) { return response()->json(['status' => 'error', 'errors' => $v->errors()], 422); } $login = $request->login . '@' . $request->domain; // Validate login (email) if ($error = $this->validateEmail($login, true)) { return response()->json(['status' => 'error', 'errors' => ['login' => $error]], 422); } // Validate verification codes (again) $v = $this->verify($request); if ($v->status() !== 200) { return $v; } $code_data = $v->getData(); $user_name = $code_data->name; $user_email = $code_data->email; // We allow only ASCII, so we can safely lower-case the email address $login = Str::lower($login); $user = User::create( [ // TODO: Save the external email (or phone) 'name' => $user_name, 'email' => $login, 'password' => $request->password, ] ); // Remove the verification code $this->code->delete(); $token = auth()->login($user); return response()->json([ 'access_token' => $token, 'token_type' => 'bearer', 'expires_in' => Auth::guard()->factory()->getTTL() * 60, ]); } /** * Checks if the input string is a valid email address or a phone number * * @param string $email Email address or phone number * @param bool &$is_phone Will be set to True if the string is valid phone number * - * @return string Error message on validation error + * @return string Error message label on validation error */ protected function validatePhoneOrEmail($input, &$is_phone = false) { $is_phone = false; + return $this->validateEmail($input); + + // TODO: Phone number support +/* if (strpos($input, '@')) { return $this->validateEmail($input); } $input = str_replace(array('-', ' '), '', $input); if (!preg_match('/^\+?[0-9]{9,12}$/', $input)) { - return __('validation.noemailorphone'); + return 'validation.noemailorphone'; } $is_phone = true; +*/ } /** * Email address validation * * @param string $email Email address * @param bool $signup Enables additional checks for signup mode * - * @return string Error message on validation error + * @return string Error message label on validation error */ protected function validateEmail($email, $signup = false) { $v = Validator::make(['email' => $email], ['email' => 'required|email']); if ($v->fails()) { - return __('validation.emailinvalid'); + return 'validation.emailinvalid'; + } + + list($local, $domain) = explode('@', $email); + + // don't allow @localhost and other no-fqdn + if (strpos($domain, '.') === false) { + return 'validation.emailinvalid'; } // Extended checks for an address that is supposed to become a login to Kolab if ($signup) { - list($local, $domain) = explode('@', $email); - // Local part validation if (!preg_match('/^[A-Za-z0-9_.-]+$/', $local)) { - return __('validation.emailinvalid'); + return 'validation.emailinvalid'; } + // TODO: check if specified domain is allowed for signup + // Check if the local part is not one of exceptions $exceptions = '/^(admin|administrator|sales|root)$/i'; if (preg_match($exceptions, $local)) { - return __('validation.emailexists'); + return 'validation.emailexists'; } // Check if user with specified login already exists + // TODO: Aliases if (User::where('email', $email)->first()) { - return __('validation.emailexists'); + return 'validation.emailexists'; } - - // TODO: check if specified domain is ours } } } diff --git a/src/resources/vue/components/Signup.vue b/src/resources/vue/components/Signup.vue index e60d1716..5368dfc5 100644 --- a/src/resources/vue/components/Signup.vue +++ b/src/resources/vue/components/Signup.vue @@ -1,166 +1,166 @@ diff --git a/src/tests/Feature/Controller/SignupTest.php b/src/tests/Feature/Controller/SignupTest.php index 3d4ae022..2d805f99 100644 --- a/src/tests/Feature/Controller/SignupTest.php +++ b/src/tests/Feature/Controller/SignupTest.php @@ -1,197 +1,246 @@ 'SignupControllerTest1@SignupControllerTest.com' ] ); - - $user->delete(); } /** * {@inheritDoc} * * @return void */ public function tearDown(): void { $user = User::firstOrCreate( [ 'email' => 'SignupControllerTest1@SignupControllerTest.com' ] ); $user->delete(); parent::tearDown(); } public function testSignupInitInvalidInput() { // Empty input data $data = []; $response = $this->post('/api/auth/signup/init', $data); $json = $response->json(); $response->assertStatus(422); $this->assertSame('error', $json['status']); $this->assertCount(2, $json['errors']); $this->assertArrayHasKey('email', $json['errors']); $this->assertArrayHasKey('name', $json['errors']); // Data with missing name $data = [ 'email' => 'UsersApiControllerTest1@UsersApiControllerTest.com', 'password' => 'simple123', 'password_confirmation' => 'simple123' ]; $response = $this->post('/api/auth/signup/init', $data); $json = $response->json(); $response->assertStatus(422); $this->assertSame('error', $json['status']); $this->assertCount(1, $json['errors']); $this->assertArrayHasKey('name', $json['errors']); // Data with invalid email (but not phone number) $data = [ 'email' => '@example.org', 'name' => 'Signup User', 'password' => 'simple123', 'password_confirmation' => 'simple123' ]; $response = $this->post('/api/auth/signup/init', $data); $json = $response->json(); $response->assertStatus(422); $this->assertSame('error', $json['status']); $this->assertCount(1, $json['errors']); $this->assertArrayHasKey('email', $json['errors']); // TODO: Test phone validation } public function testSignupInitValidInput() { $data = [ 'email' => 'UsersApiControllerTest1@UsersApiControllerTest.com', 'name' => 'Signup User', 'password' => 'simple123', 'password_confirmation' => 'simple123' ]; $response = $this->post('/api/auth/signup/init', $data); $json = $response->json(); $response->assertStatus(200); $this->assertCount(2, $json); $this->assertSame('success', $json['status']); $this->assertNotEmpty($json['code']); // TODO: Test verification email/sms return [ 'code' => $json['code'], 'email' => $data['email'], 'name' => $data['name'], ]; } /** * @depends testSignupInitValidInput */ public function testSignupVerifyInvalidInput(array $result) { // Empty data $data = []; $response = $this->post('/api/auth/signup/verify', $data); $json = $response->json(); $response->assertStatus(422); $this->assertCount(2, $json); $this->assertSame('error', $json['status']); $this->assertArrayHasKey('code', $json['errors']); $this->assertArrayHasKey('short_code', $json['errors']); // Data with existing code but missing short_code $data = [ 'code' => $result['code'], ]; $response = $this->post('/api/auth/signup/verify', $data); $json = $response->json(); $response->assertStatus(422); $this->assertCount(2, $json); $this->assertSame('error', $json['status']); $this->assertArrayHasKey('short_code', $json['errors']); // Data with invalid short_code $data = [ 'code' => $result['code'], 'short_code' => 'XXXX', ]; $response = $this->post('/api/auth/signup/verify', $data); $json = $response->json(); $response->assertStatus(422); $this->assertCount(2, $json); $this->assertSame('error', $json['status']); $this->assertArrayHasKey('short_code', $json['errors']); // TODO: Test expired code } /** * @depends testSignupInitValidInput */ public function testSignupVerifyValidInput(array $result) { $code = SignupCode::find($result['code']); $data = [ 'code' => $code->code, 'short_code' => $code->short_code, ]; $response = $this->post('/api/auth/signup/verify', $data); $json = $response->json(); $response->assertStatus(200); $this->assertCount(3, $json); $this->assertSame('success', $json['status']); $this->assertSame($result['email'], $json['email']); $this->assertSame($result['name'], $json['name']); } public function testSignup() { // TODO } + + /** + * List of email address validation cases for testValidateEmail() + */ + public function dataValidateEmail() + { + $domain = 'example.org'; + + return [ + // general cases (invalid) + ['', false, 'validation.emailinvalid'], + ['example.org', false, 'validation.emailinvalid'], + ['@example.org', false, 'validation.emailinvalid'], + ['test@localhost', false, 'validation.emailinvalid'], + // general cases (valid) + ['test@domain.tld', false, null], + ['&@example.org', false, null], + // kolab identity cases + ['admin@' . $domain, true, 'validation.emailexists'], + ['administrator@' . $domain, true, 'validation.emailexists'], + ['sales@' . $domain, true, 'validation.emailexists'], + ['root@' . $domain, true, 'validation.emailexists'], + ['&@' . $domain, true, 'validation.emailinvalid'], + // existing account + ['SignupControllerTest1@SignupControllerTest.com', true, 'validation.emailexists'], + // valid for signup + ['test.test@' . $domain, true, null], + ['test_test@' . $domain, true, null], + ['test-test@' . $domain, true, null], + ]; + } + + /** + * Signup email validation. + * + * Note: Technicly these are mostly 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, $signup, $expected_result) + { + $method = new \ReflectionMethod('App\Http\Controllers\API\SignupController', 'validateEmail'); + $method->setAccessible(true); + + $is_phone = false; + $result = $method->invoke(new SignupController, $email, $signup); + + $this->assertSame($expected_result, $result); + } }