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 @@ -94,13 +94,11 @@ // Generate the verification code $code = SignupCode::create([ - 'data' => [ - 'email' => $request->email, - 'first_name' => $request->first_name, - 'last_name' => $request->last_name, - 'plan' => $request->plan, - 'voucher' => $request->voucher, - ] + 'email' => $request->email, + 'first_name' => $request->first_name, + 'last_name' => $request->last_name, + 'plan' => $request->plan, + 'voucher' => $request->voucher, ]); // Send email/sms message @@ -157,10 +155,10 @@ // domains list for selection and "plan type" flag return response()->json([ 'status' => 'success', - 'email' => $code->data['email'], - 'first_name' => $code->data['first_name'], - 'last_name' => $code->data['last_name'], - 'voucher' => $code->data['voucher'], + 'email' => $code->email, + 'first_name' => $code->first_name, + 'last_name' => $code->last_name, + 'voucher' => $code->voucher, 'is_domain' => $has_domain, 'domains' => $has_domain ? [] : Domain::getPublicDomains(), ]); @@ -277,8 +275,8 @@ { 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(); + if ($this->code && $this->code->plan) { + $plan = Plan::where('title', $this->code->plan)->first(); } // ...otherwise use the default plan diff --git a/src/app/Http/Controllers/Controller.php b/src/app/Http/Controllers/Controller.php --- a/src/app/Http/Controllers/Controller.php +++ b/src/app/Http/Controllers/Controller.php @@ -30,8 +30,9 @@ 401 => "Unauthorized", 403 => "Access denied", 404 => "Not found", - 422 => "Input validation error", 405 => "Method not allowed", + 422 => "Input validation error", + 429 => "Too many requests", 500 => "Internal server error", ]; diff --git a/src/app/Jobs/SignupVerificationEmail.php b/src/app/Jobs/SignupVerificationEmail.php --- a/src/app/Jobs/SignupVerificationEmail.php +++ b/src/app/Jobs/SignupVerificationEmail.php @@ -58,6 +58,6 @@ */ public function handle() { - Mail::to($this->code->data['email'])->send(new SignupVerification($this->code)); + Mail::to($this->code->email)->send(new SignupVerification($this->code)); } } diff --git a/src/app/Mail/SignupVerification.php b/src/app/Mail/SignupVerification.php --- a/src/app/Mail/SignupVerification.php +++ b/src/app/Mail/SignupVerification.php @@ -41,9 +41,9 @@ sprintf('/signup/%s-%s', $this->code->short_code, $this->code->code) ); - $username = $this->code->data['first_name'] ?? ''; - if (!empty($this->code->data['last_name'])) { - $username .= ' ' . $this->code->data['last_name']; + $username = $this->code->first_name ?? ''; + if (!empty($this->code->last_name)) { + $username .= ' ' . $this->code->last_name; } $username = trim($username); @@ -73,11 +73,9 @@ $code = new SignupCode([ 'code' => Str::random(SignupCode::CODE_LENGTH), 'short_code' => SignupCode::generateShortCode(), - 'data' => [ - 'email' => 'test@' . \config('app.domain'), - 'first_name' => 'Firstname', - 'last_name' => 'Lastname', - ], + 'email' => 'test@' . \config('app.domain'), + 'first_name' => 'Firstname', + 'last_name' => 'Lastname', ]); $mail = new self($code); diff --git a/src/app/Observers/SignupCodeObserver.php b/src/app/Observers/SignupCodeObserver.php --- a/src/app/Observers/SignupCodeObserver.php +++ b/src/app/Observers/SignupCodeObserver.php @@ -35,5 +35,33 @@ } $code->expires_at = Carbon::now()->addHours($exp_hours); + $code->ip_address = \App\Utils::requestIp(); + + if ($code->email) { + $parts = explode('@', $code->email); + + $code->local_part = $parts[0]; + $code->domain_part = $parts[1]; + } + } + + /** + * Handle the "updating" event. + * + * @param SignupCode $code The code being updated. + * + * @return void + */ + public function updating(SignupCode $code) + { + if ($code->email) { + $parts = explode('@', $code->email); + + $code->local_part = $parts[0]; + $code->domain_part = $parts[1]; + } else { + $code->local_part = null; + $code->domain_part = null; + } } } diff --git a/src/app/Observers/VerificationCodeObserver.php b/src/app/Observers/VerificationCodeObserver.php --- a/src/app/Observers/VerificationCodeObserver.php +++ b/src/app/Observers/VerificationCodeObserver.php @@ -3,7 +3,6 @@ namespace App\Observers; use App\VerificationCode; -use App\SignupCode; use Carbon\Carbon; use Illuminate\Support\Str; diff --git a/src/app/SignupCode.php b/src/app/SignupCode.php --- a/src/app/SignupCode.php +++ b/src/app/SignupCode.php @@ -4,17 +4,24 @@ use Carbon\Carbon; use Illuminate\Database\Eloquent\Model; +use Illuminate\Database\Eloquent\SoftDeletes; /** * The eloquent definition of a SignupCode. * * @property string $code - * @property array $data + * @property string $email * @property \Carbon\Carbon $expires_at + * @property string $first_name + * @property string $last_name + * @property string $plan * @property string $short_code + * @property string $voucher */ class SignupCode extends Model { + use SoftDeletes; + public const SHORTCODE_LENGTH = 5; public const CODE_LENGTH = 32; @@ -43,26 +50,21 @@ */ protected $keyType = 'string'; - /** - * Indicates if the model should be timestamped. - * - * @var bool - */ - public $timestamps = false; - /** * The attributes that are mass assignable. * * @var array */ - protected $fillable = ['code', 'short_code', 'data', 'expires_at']; - - /** - * The attributes that should be cast to native types. - * - * @var array - */ - protected $casts = ['data' => 'array']; + protected $fillable = [ + 'code', + 'email', + 'expires_at', + 'first_name', + 'last_name', + 'plan', + 'short_code', + 'voucher' + ]; /** * The attributes that should be mutated to dates. diff --git a/src/app/User.php b/src/app/User.php --- a/src/app/User.php +++ b/src/app/User.php @@ -8,7 +8,6 @@ use App\Traits\UserAliasesTrait; use App\Traits\SettingsTrait; use App\Wallet; -use Illuminate\Notifications\Notifiable; use Illuminate\Database\Eloquent\SoftDeletes; use Illuminate\Foundation\Auth\User as Authenticatable; use Iatstuti\Database\Support\NullableFields; @@ -24,7 +23,6 @@ */ class User extends Authenticatable implements JWTSubject { - use Notifiable; use NullableFields; use UserAliasesTrait; use SettingsTrait; diff --git a/src/app/Utils.php b/src/app/Utils.php --- a/src/app/Utils.php +++ b/src/app/Utils.php @@ -203,9 +203,7 @@ } // Convert the hexadecimal string to a binary string - # Using pack() here - # Newer PHP version can use hex2bin() - $lastaddrbin = pack('H*', $lastAddrHex); + $lastaddrbin = hex2bin($lastAddrHex); // And create an IPv6 address from the binary string $lastaddrstr = inet_ntop($lastaddrbin); @@ -284,6 +282,17 @@ return implode($join, $randStrs); } + /** + * Returns the IP address of the HTTP request + * + * @return string IP address + */ + public static function requestIp() + { + // TODO: Verify if it works with proxies + return request()->ip(); + } + /** * Returns a UUID in the form of an integer. * diff --git a/src/app/VerificationCode.php b/src/app/VerificationCode.php --- a/src/app/VerificationCode.php +++ b/src/app/VerificationCode.php @@ -2,45 +2,70 @@ namespace App; -use App\SignupCode; +use Carbon\Carbon; use Illuminate\Database\Eloquent\Model; /** * The eloquent definition of a VerificationCode * + * @property string $code * @property string $mode * @property \App\User $user + * @property int $user_id + * @property string $short_code */ -class VerificationCode extends SignupCode +class VerificationCode extends Model { // Code expires after so many hours - public const CODE_EXP_HOURS = 8; public const SHORTCODE_LENGTH = 8; + public const CODE_LENGTH = 32; + + // Code expires after so many hours + public const CODE_EXP_HOURS = 8; /** - * The attributes that are mass assignable. + * The primary key associated with the table. * - * @var array + * @var string */ - protected $fillable = ['user_id', 'code', 'short_code', 'mode', 'expires_at']; + protected $primaryKey = 'code'; + + /** + * Indicates if the IDs are auto-incrementing. + * + * @var bool + */ + public $incrementing = false; /** - * The attributes that should be cast to native types. + * The "type" of the auto-incrementing ID. + * + * @var string + */ + protected $keyType = 'string'; + + /** + * Indicates if the model should be timestamped. + * + * @var bool + */ + public $timestamps = false; + + /** + * The attributes that are mass assignable. * * @var array */ - protected $casts = []; + protected $fillable = ['user_id', 'code', 'short_code', 'mode', 'expires_at']; /** - * The user to which this setting belongs. + * The attributes that should be mutated to dates. * - * @return \Illuminate\Database\Eloquent\Relations\BelongsTo + * @var array */ - public function user() - { - return $this->belongsTo('\App\User', 'user_id', 'id'); - } + protected $dates = ['expires_at']; + /** * Generate a short code (for human). @@ -53,4 +78,25 @@ return \App\Utils::randStr($code_length); } + + /** + * Check if code is expired. + * + * @return bool True if code is expired, False otherwise + */ + public function isExpired() + { + // @phpstan-ignore-next-line + return $this->expires_at ? Carbon::now()->gte($this->expires_at) : false; + } + + /** + * The user to which this setting belongs. + * + * @return \Illuminate\Database\Eloquent\Relations\BelongsTo + */ + public function user() + { + return $this->belongsTo('\App\User', 'user_id', 'id'); + } } diff --git a/src/database/migrations/2021_03_25_100000_signup_code_refactor.php b/src/database/migrations/2021_03_25_100000_signup_code_refactor.php new file mode 100644 --- /dev/null +++ b/src/database/migrations/2021_03_25_100000_signup_code_refactor.php @@ -0,0 +1,98 @@ +string('email'); + $table->string('first_name')->nullable(); + $table->string('last_name')->nullable(); + $table->string('plan', 128)->nullable(); + $table->string('voucher', 32)->nullable(); + $table->string('local_part')->nullable(); + $table->string('domain_part')->nullable(); + $table->string('ip_address')->nullable(); + $table->timestamp('created_at')->useCurrent(); + $table->timestamp('updated_at')->useCurrent(); + $table->softDeletes(); + } + ); + + DB::table('signup_codes')->get()->each(function ($code) { + if (empty($code->data)) { + return; + } + + $data = json_decode($code->data); + + if (!empty($data->email)) { + $parts = explode('@', $data->email); + + $data->local_part = $parts[0] ?? null; + $data->domain_part = $parts[1] ?? null; + } + + DB::table('signup_codes') + ->where('code', $code->code) + ->update([ + 'email' => $data->email ?? null, + 'first_name' => $data->first_name ?? null, + 'last_name' => $data->last_name ?? null, + 'plan' => $data->plan ?? null, + 'voucher' => $data->voucher ?? null, + 'local_part' => $data->local_part ?? null, + 'domain_part' => $data->domain_part ?? null, + ]); + }); + + Schema::table( + 'signup_codes', + function (Blueprint $table) { + $table->dropColumn('data'); + } + ); + } + + /** + * Reverse the migrations. + * + * @return void + */ + public function down() + { + Schema::table( + 'signup_codes', + function (Blueprint $table) { + $table->text('data'); + + $table->dropColumn([ + 'created_at', + 'updated_at', + 'deleted_at', + 'ip_address', + 'email', + 'local_part', + 'domain_part', + 'first_name', + 'last_name', + 'plan', + 'voucher', + ]); + } + ); + } +} diff --git a/src/tests/Browser/SignupTest.php b/src/tests/Browser/SignupTest.php --- a/src/tests/Browser/SignupTest.php +++ b/src/tests/Browser/SignupTest.php @@ -72,13 +72,11 @@ // Test valid code $this->browse(function (Browser $browser) { $code = SignupCode::create([ - 'data' => [ - 'email' => 'User@example.org', - 'first_name' => 'User', - 'last_name' => 'Name', - 'plan' => 'individual', - 'voucher' => '', - ] + 'email' => 'User@example.org', + 'first_name' => 'User', + 'last_name' => 'Name', + 'plan' => 'individual', + 'voucher' => '', ]); $browser->visit('/signup/' . $code->short_code . '-' . $code->code) 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 @@ -200,10 +200,10 @@ $code = TestCase::getObjectProperty($job, 'code'); return $code->code === $json['code'] - && $code->data['plan'] === $data['plan'] - && $code->data['email'] === $data['email'] - && $code->data['first_name'] === $data['first_name'] - && $code->data['last_name'] === $data['last_name']; + && $code->plan === $data['plan'] + && $code->email === $data['email'] + && $code->first_name === $data['first_name'] + && $code->last_name === $data['last_name']; }); // Try the same with voucher @@ -222,11 +222,11 @@ $code = TestCase::getObjectProperty($job, 'code'); return $code->code === $json['code'] - && $code->data['plan'] === $data['plan'] - && $code->data['email'] === $data['email'] - && $code->data['voucher'] === $data['voucher'] - && $code->data['first_name'] === $data['first_name'] - && $code->data['last_name'] === $data['last_name']; + && $code->plan === $data['plan'] + && $code->email === $data['email'] + && $code->voucher === $data['voucher'] + && $code->first_name === $data['first_name'] + && $code->last_name === $data['last_name']; }); return [ @@ -551,10 +551,10 @@ $code = TestCase::getObjectProperty($job, 'code'); return $code->code === $json['code'] - && $code->data['plan'] === $data['plan'] - && $code->data['email'] === $data['email'] - && $code->data['first_name'] === $data['first_name'] - && $code->data['last_name'] === $data['last_name']; + && $code->plan === $data['plan'] + && $code->email === $data['email'] + && $code->first_name === $data['first_name'] + && $code->last_name === $data['last_name']; }); // Verify the code diff --git a/src/tests/Feature/Jobs/SignupVerificationEmailTest.php b/src/tests/Feature/Jobs/SignupVerificationEmailTest.php --- a/src/tests/Feature/Jobs/SignupVerificationEmailTest.php +++ b/src/tests/Feature/Jobs/SignupVerificationEmailTest.php @@ -22,11 +22,9 @@ parent::setUp(); $this->code = SignupCode::create([ - 'data' => [ - 'email' => 'SignupVerificationEmailTest1@' . \config('app.domain'), - 'first_name' => "Test", - 'last_name' => "Job" - ] + 'email' => 'SignupVerificationEmailTest1@' . \config('app.domain'), + 'first_name' => "Test", + 'last_name' => "Job" ]); } @@ -60,7 +58,7 @@ // Assert the mail was sent to the code's email Mail::assertSent(SignupVerification::class, function ($mail) { - return $mail->hasTo($this->code->data['email']); + return $mail->hasTo($this->code->email); }); } } diff --git a/src/tests/Feature/SignupCodeTest.php b/src/tests/Feature/SignupCodeTest.php --- a/src/tests/Feature/SignupCodeTest.php +++ b/src/tests/Feature/SignupCodeTest.php @@ -16,22 +16,22 @@ public function testSignupCodeCreate() { $data = [ - 'data' => [ - 'email' => 'User@email.org', - ] + 'email' => 'User@email.org', ]; $now = Carbon::now(); $code = SignupCode::create($data); - $code_length = env('VERIFICATION_CODE_LENGTH', SignupCode::SHORTCODE_LENGTH); $exp = Carbon::now()->addHours(env('SIGNUP_CODE_EXPIRY', SignupCode::CODE_EXP_HOURS)); $this->assertFalse($code->isExpired()); $this->assertTrue(strlen($code->code) === SignupCode::CODE_LENGTH); $this->assertTrue(strlen($code->short_code) === $code_length); - $this->assertSame($data['data'], $code->data); + $this->assertSame($data['email'], $code->email); + $this->assertSame('User', $code->local_part); + $this->assertSame('email.org', $code->domain_part); + $this->assertSame('127.0.0.1', $code->ip_address); $this->assertInstanceOf(Carbon::class, $code->expires_at); $this->assertSame($code->expires_at->toDateTimeString(), $exp->toDateTimeString()); @@ -39,5 +39,11 @@ $this->assertInstanceOf(SignupCode::class, $inst); $this->assertSame($inst->code, $code->code); + + $inst->email = 'other@email.com'; + $inst->save(); + + $this->assertSame('other', $inst->local_part); + $this->assertSame('email.com', $inst->domain_part); } } diff --git a/src/tests/Unit/Mail/SignupVerificationTest.php b/src/tests/Unit/Mail/SignupVerificationTest.php --- a/src/tests/Unit/Mail/SignupVerificationTest.php +++ b/src/tests/Unit/Mail/SignupVerificationTest.php @@ -20,11 +20,9 @@ $code = new SignupCode([ 'code' => 'code', 'short_code' => 'short-code', - 'data' => [ - 'email' => 'test@email', - 'first_name' => 'First', - 'last_name' => 'Last', - ], + 'email' => 'test@email', + 'first_name' => 'First', + 'last_name' => 'Last', ]); $mail = $this->fakeMail(new SignupVerification($code));