From 1b241aeddb1bbcb081edd0ccf03be2908c7ab948 Mon Sep 17 00:00:00 2001 From: ritual Date: Sun, 22 Feb 2026 20:02:09 +0000 Subject: [PATCH] First round of refinements on the login system... There's a lot more to do on the to-do list --- app/DTOs/MagicTokenResult.php | 14 + .../Controllers/Auth/MagicLinkController.php | 63 ++--- app/Models/MagicLoginToken.php | 31 +- app/Services/MagicLinkAuthService.php | 33 ++- ...000005_secure_magic_login_tokens_table.php | 33 +++ tests/Feature/Auth/MagicLinkAuthTest.php | 170 +++++++++-- tests/Unit/MagicLoginTokenTest.php | 265 ++++++++++-------- 7 files changed, 390 insertions(+), 219 deletions(-) create mode 100644 app/DTOs/MagicTokenResult.php create mode 100644 database/migrations/2026_02_22_000005_secure_magic_login_tokens_table.php diff --git a/app/DTOs/MagicTokenResult.php b/app/DTOs/MagicTokenResult.php new file mode 100644 index 0000000..3bb27c0 --- /dev/null +++ b/app/DTOs/MagicTokenResult.php @@ -0,0 +1,14 @@ + 'required|email', ]); - $email = $request->email; - $ip = $request->ip(); - $userAgent = $request->userAgent(); + $result = $this->authService->sendMagicLink( + $request->email, + $request->ip(), + $request->userAgent() + ); - try { - $token = $this->authService->sendMagicLink($email, $ip, $userAgent); + $loginUrl = URL::temporarySignedRoute( + 'magic-link.verify', + now()->addMinutes(15), + ['token' => $result->plainToken] + ); - // Generate signed URL valid for 15 minutes - $loginUrl = URL::temporarySignedRoute( - 'magic-link.verify', - now()->addMinutes(15), - ['token' => $token->plain_token] - ); + Mail::to($request->email)->queue(new MagicLoginLink($loginUrl, $result->plainCode, 15)); - // Queue the magic link email - Mail::to($email)->queue(new MagicLoginLink($loginUrl, $token->plain_code, 15)); - - return redirect()->route('verify-code') - ->with('status', 'Check your email for your login code!') - ->with('email', $email); - } catch (ValidationException $e) { - throw $e; - } + return redirect()->route('verify-code') + ->with('status', 'Check your email for your login code!') + ->with('email', $request->email); } /** * Show the code verification form. */ - public function showCodeForm(Request $request) + public function showCodeForm() { return view('auth.verify-code'); } @@ -72,14 +65,11 @@ class MagicLinkController extends Controller */ public function verifyLink(Request $request) { - // Validate the signed URL if (!$request->hasValidSignature()) { return redirect()->route('login')->with('error', 'Invalid or expired magic link.'); } - $token = $request->token; - - if ($this->authService->verifyMagicLink($token)) { + if ($this->authService->verifyMagicLink($request->query('token'))) { $request->session()->regenerate(); return redirect()->route('dashboard'); @@ -98,22 +88,15 @@ class MagicLinkController extends Controller 'code' => 'required|digits:6', ]); - $email = $request->email; - $code = $request->code; + if ($this->authService->verifyCode($request->email, $request->code)) { + $request->session()->regenerate(); - try { - if ($this->authService->verifyCode($email, $code)) { - $request->session()->regenerate(); - - return redirect()->route('dashboard'); - } - - return back()->withErrors([ - 'code' => 'Invalid or expired code.', - ]); - } catch (ValidationException $e) { - throw $e; + return redirect()->route('dashboard'); } + + return back()->withErrors([ + 'code' => 'Invalid or expired code.', + ]); } /** diff --git a/app/Models/MagicLoginToken.php b/app/Models/MagicLoginToken.php index 992a984..41fd613 100755 --- a/app/Models/MagicLoginToken.php +++ b/app/Models/MagicLoginToken.php @@ -2,6 +2,7 @@ namespace App\Models; +use App\DTOs\MagicTokenResult; use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Eloquent\Factories\HasFactory; use Illuminate\Database\Eloquent\Model; @@ -20,9 +21,7 @@ class MagicLoginToken extends Model protected $fillable = [ 'email_hash', 'token_hash', - 'plain_token', 'code_hash', - 'plain_code', 'expires_at', 'ip_address', 'user_agent', @@ -44,23 +43,23 @@ class MagicLoginToken extends Model /** * Generate a new magic login token for the given email. + * Plain token and code are returned in the DTO but never persisted. */ - public static function generate(string $email, ?string $ip = null, ?string $ua = null): self + public static function generate(string $email, ?string $ip = null, ?string $ua = null): MagicTokenResult { - $emailHash = User::hashEmail($email); - $token = Str::random(64); - $code = str_pad((string) random_int(0, 999999), 6, '0', STR_PAD_LEFT); + $plainToken = Str::random(64); + $plainCode = str_pad((string) random_int(0, 999999), 6, '0', STR_PAD_LEFT); - return self::create([ - 'email_hash' => $emailHash, - 'token_hash' => Hash::make($token), - 'plain_token' => $token, - 'code_hash' => Hash::make($code), - 'plain_code' => $code, + $model = self::create([ + 'email_hash' => User::hashEmail($email), + 'token_hash' => hash('sha256', $plainToken), + 'code_hash' => Hash::make($plainCode), 'expires_at' => now()->addMinutes(15), 'ip_address' => $ip, 'user_agent' => $ua, ]); + + return new MagicTokenResult($model, $plainToken, $plainCode); } /** @@ -79,14 +78,6 @@ class MagicLoginToken extends Model $this->update(['used_at' => now()]); } - /** - * Verify the provided token matches the plain token. - */ - public function verifyToken(string $token): bool - { - return $this->plain_token === $token; - } - /** * Verify the provided code matches the hashed code. */ diff --git a/app/Services/MagicLinkAuthService.php b/app/Services/MagicLinkAuthService.php index 123842a..fa28c1c 100755 --- a/app/Services/MagicLinkAuthService.php +++ b/app/Services/MagicLinkAuthService.php @@ -2,6 +2,7 @@ namespace App\Services; +use App\DTOs\MagicTokenResult; use App\Models\MagicLoginToken; use App\Models\User; use Illuminate\Support\Facades\Auth; @@ -15,9 +16,9 @@ class MagicLinkAuthService * * @throws ValidationException */ - public function sendMagicLink(string $email, ?string $ip, ?string $ua): MagicLoginToken + public function sendMagicLink(string $email, ?string $ip, ?string $ua): MagicTokenResult { - $key = 'magic-link:' . hash('sha256', strtolower(trim($email))); + $key = 'magic-link:' . User::hashEmail($email); if (RateLimiter::tooManyAttempts($key, 5)) { $seconds = RateLimiter::availableIn($key); @@ -31,10 +32,8 @@ class MagicLinkAuthService RateLimiter::hit($key, 3600); - $user = User::findByEmail($email); - - if (!$user) { - $user = User::create([ + if (!User::findByEmail($email)) { + User::create([ 'email_hash' => User::hashEmail($email), ]); } @@ -48,7 +47,7 @@ class MagicLinkAuthService public function verifyMagicLink(string $token): bool { $magicToken = MagicLoginToken::valid() - ->where('plain_token', $token) + ->where('token_hash', hash('sha256', $token)) ->first(); if (!$magicToken) { @@ -63,6 +62,11 @@ class MagicLinkAuthService $magicToken->markAsUsed(); + if (is_null($user->email_verified_at)) { + $user->email_verified_at = now(); + $user->save(); + } + Auth::login($user, true); return true; @@ -70,6 +74,8 @@ class MagicLinkAuthService /** * Verify a magic code and log the user in. + * + * @throws ValidationException */ public function verifyCode(string $email, string $code): bool { @@ -77,7 +83,13 @@ class MagicLinkAuthService $key = 'magic-code:' . $emailHash; if (RateLimiter::tooManyAttempts($key, 5)) { - return false; + $seconds = RateLimiter::availableIn($key); + + throw ValidationException::withMessages([ + 'code' => [ + "Too many attempts. Please try again in {$seconds} seconds.", + ], + ]); } $magicToken = MagicLoginToken::valid() @@ -100,6 +112,11 @@ class MagicLinkAuthService RateLimiter::clear($key); + if (is_null($user->email_verified_at)) { + $user->email_verified_at = now(); + $user->save(); + } + Auth::login($user, true); return true; diff --git a/database/migrations/2026_02_22_000005_secure_magic_login_tokens_table.php b/database/migrations/2026_02_22_000005_secure_magic_login_tokens_table.php new file mode 100644 index 0000000..8920691 --- /dev/null +++ b/database/migrations/2026_02_22_000005_secure_magic_login_tokens_table.php @@ -0,0 +1,33 @@ +dropColumn(['plain_token', 'plain_code']); + $table->string('token_hash', 64)->change(); + $table->unique('token_hash'); + }); + } + + /** + * Reverse the migrations. + */ + public function down(): void + { + Schema::table('magic_login_tokens', function (Blueprint $table) { + $table->dropUnique(['token_hash']); + $table->string('token_hash')->change(); + $table->string('plain_token')->after('token_hash'); + $table->string('plain_code', 6)->after('code_hash'); + }); + } +}; diff --git a/tests/Feature/Auth/MagicLinkAuthTest.php b/tests/Feature/Auth/MagicLinkAuthTest.php index 3c662bf..9e46520 100755 --- a/tests/Feature/Auth/MagicLinkAuthTest.php +++ b/tests/Feature/Auth/MagicLinkAuthTest.php @@ -19,7 +19,7 @@ class MagicLinkAuthTest extends TestCase protected function setUp(): void { parent::setUp(); - RateLimiter::clear('magic-link:' . hash('sha256', strtolower('test@example.com'))); + RateLimiter::clear('magic-link:' . User::hashEmail('test@example.com')); RateLimiter::clear('magic-code:' . User::hashEmail('test@example.com')); } @@ -77,12 +77,12 @@ class MagicLinkAuthTest extends TestCase 'email_hash' => User::hashEmail('test@example.com'), ]); - $token = MagicLoginToken::generate('test@example.com', '127.0.0.1', 'TestAgent'); + $result = MagicLoginToken::generate('test@example.com', '127.0.0.1', 'TestAgent'); $signedUrl = URL::temporarySignedRoute( 'magic-link.verify', now()->addMinutes(15), - ['token' => $token->plain_token] + ['token' => $result->plainToken] ); $this->assertGuest(); @@ -93,8 +93,8 @@ class MagicLinkAuthTest extends TestCase $this->assertAuthenticated(); $this->assertEquals($user->id, Auth::id()); - $token->refresh(); - $this->assertNotNull($token->used_at); + $result->token->refresh(); + $this->assertNotNull($result->token->used_at); } public function test_valid_6_digit_code_logs_user_in(): void @@ -103,22 +103,21 @@ class MagicLinkAuthTest extends TestCase 'email_hash' => User::hashEmail('test@example.com'), ]); - $token = MagicLoginToken::generate('test@example.com', '127.0.0.1', 'TestAgent'); - $code = $token->plain_code; + $result = MagicLoginToken::generate('test@example.com', '127.0.0.1', 'TestAgent'); $this->assertGuest(); $response = $this->post('/verify-code', [ 'email' => 'test@example.com', - 'code' => $code, + 'code' => $result->plainCode, ]); $response->assertRedirect(route('dashboard')); $this->assertAuthenticated(); $this->assertEquals($user->id, Auth::id()); - $token->refresh(); - $this->assertNotNull($token->used_at); + $result->token->refresh(); + $this->assertNotNull($result->token->used_at); } public function test_invalid_code_rejected_with_error(): void @@ -127,11 +126,14 @@ class MagicLinkAuthTest extends TestCase 'email_hash' => User::hashEmail('test@example.com'), ]); - MagicLoginToken::generate('test@example.com', '127.0.0.1', 'TestAgent'); + $result = MagicLoginToken::generate('test@example.com', '127.0.0.1', 'TestAgent'); + + // Derive a code guaranteed to differ from the real one + $wrongCode = str_pad((string) (((int) $result->plainCode + 1) % 1000000), 6, '0', STR_PAD_LEFT); $response = $this->post('/verify-code', [ 'email' => 'test@example.com', - 'code' => '999999', + 'code' => $wrongCode, ]); $response->assertRedirect(); @@ -141,18 +143,18 @@ class MagicLinkAuthTest extends TestCase public function test_expired_token_rejected(): void { - $user = User::create([ + User::create([ 'email_hash' => User::hashEmail('test@example.com'), ]); - $token = MagicLoginToken::generate('test@example.com', '127.0.0.1', 'TestAgent'); + $result = MagicLoginToken::generate('test@example.com', '127.0.0.1', 'TestAgent'); - $token->update(['expires_at' => now()->subMinutes(1)]); + $result->token->update(['expires_at' => now()->subMinutes(1)]); $signedUrl = URL::temporarySignedRoute( 'magic-link.verify', now()->addMinutes(15), - ['token' => $token->plain_token] + ['token' => $result->plainToken] ); $this->assertGuest(); @@ -170,12 +172,12 @@ class MagicLinkAuthTest extends TestCase 'email_hash' => User::hashEmail('test@example.com'), ]); - $token = MagicLoginToken::generate('test@example.com', '127.0.0.1', 'TestAgent'); + $result = MagicLoginToken::generate('test@example.com', '127.0.0.1', 'TestAgent'); $signedUrl = URL::temporarySignedRoute( 'magic-link.verify', now()->addMinutes(15), - ['token' => $token->plain_token] + ['token' => $result->plainToken] ); $this->get($signedUrl); @@ -221,33 +223,40 @@ class MagicLinkAuthTest extends TestCase $errors->first('email') ); - RateLimiter::clear('magic-link:' . hash('sha256', strtolower($email))); + RateLimiter::clear('magic-link:' . User::hashEmail($email)); } public function test_rate_limiting_on_code_attempts(): void { - $user = User::create([ + User::create([ 'email_hash' => User::hashEmail('test@example.com'), ]); - MagicLoginToken::generate('test@example.com', '127.0.0.1', 'TestAgent'); + $first = MagicLoginToken::generate('test@example.com', '127.0.0.1', 'TestAgent'); + + // Derive a code guaranteed to be wrong so the loop never accidentally succeeds + $wrongCode = str_pad((string) (((int) $first->plainCode + 1) % 1000000), 6, '0', STR_PAD_LEFT); for ($i = 0; $i < 5; $i++) { $this->post('/verify-code', [ 'email' => 'test@example.com', - 'code' => '000000', + 'code' => $wrongCode, ]); } - $token = MagicLoginToken::generate('test@example.com', '127.0.0.1', 'TestAgent'); - $validCode = $token->plain_code; + // Even a fresh valid token is blocked once the limit is reached + $result = MagicLoginToken::generate('test@example.com', '127.0.0.1', 'TestAgent'); $response = $this->post('/verify-code', [ 'email' => 'test@example.com', - 'code' => $validCode, + 'code' => $result->plainCode, ]); $this->assertGuest(); + $response->assertSessionHasErrors('code'); + + $errors = session('errors'); + $this->assertStringContainsString('Too many attempts', $errors->first('code')); RateLimiter::clear('magic-code:' . User::hashEmail('test@example.com')); } @@ -262,13 +271,13 @@ class MagicLinkAuthTest extends TestCase public function test_invalid_signature_rejected(): void { - $user = User::create([ + User::create([ 'email_hash' => User::hashEmail('test@example.com'), ]); - $token = MagicLoginToken::generate('test@example.com', '127.0.0.1', 'TestAgent'); + $result = MagicLoginToken::generate('test@example.com', '127.0.0.1', 'TestAgent'); - $invalidUrl = route('magic-link.verify', ['token' => $token->plain_token]); + $invalidUrl = route('magic-link.verify', ['token' => $result->plainToken]); $response = $this->get($invalidUrl); @@ -279,16 +288,16 @@ class MagicLinkAuthTest extends TestCase public function test_used_token_cannot_be_reused(): void { - $user = User::create([ + User::create([ 'email_hash' => User::hashEmail('test@example.com'), ]); - $token = MagicLoginToken::generate('test@example.com', '127.0.0.1', 'TestAgent'); + $result = MagicLoginToken::generate('test@example.com', '127.0.0.1', 'TestAgent'); $signedUrl = URL::temporarySignedRoute( 'magic-link.verify', now()->addMinutes(15), - ['token' => $token->plain_token] + ['token' => $result->plainToken] ); $this->get($signedUrl); @@ -347,4 +356,103 @@ class MagicLinkAuthTest extends TestCase $response->assertSessionHasErrors('code'); } + + public function test_plain_values_not_stored_after_magic_link_request(): void + { + Mail::fake(); + + $this->post('/magic-link', ['email' => 'test@example.com']); + + $token = MagicLoginToken::where('email_hash', User::hashEmail('test@example.com'))->firstOrFail(); + + $this->assertArrayNotHasKey('plain_token', $token->getAttributes()); + $this->assertArrayNotHasKey('plain_code', $token->getAttributes()); + + // Confirm the hash is the correct length for SHA256 + $this->assertEquals(64, strlen($token->token_hash)); + } + + public function test_email_verified_on_first_login_via_link(): void + { + $user = User::create([ + 'email_hash' => User::hashEmail('test@example.com'), + ]); + + $this->assertNull($user->email_verified_at); + + $result = MagicLoginToken::generate('test@example.com', '127.0.0.1', 'TestAgent'); + + $signedUrl = URL::temporarySignedRoute( + 'magic-link.verify', + now()->addMinutes(15), + ['token' => $result->plainToken] + ); + + $this->get($signedUrl); + + $user->refresh(); + $this->assertNotNull($user->email_verified_at); + } + + public function test_email_verified_on_first_login_via_code(): void + { + $user = User::create([ + 'email_hash' => User::hashEmail('test@example.com'), + ]); + + $this->assertNull($user->email_verified_at); + + $result = MagicLoginToken::generate('test@example.com', '127.0.0.1', 'TestAgent'); + + $this->post('/verify-code', [ + 'email' => 'test@example.com', + 'code' => $result->plainCode, + ]); + + $user->refresh(); + $this->assertNotNull($user->email_verified_at); + } + + public function test_already_verified_email_not_overwritten(): void + { + $verifiedAt = now()->subDays(30); + + $user = User::create([ + 'email_hash' => User::hashEmail('test@example.com'), + 'email_verified_at' => $verifiedAt, + ]); + + $result = MagicLoginToken::generate('test@example.com', '127.0.0.1', 'TestAgent'); + + $signedUrl = URL::temporarySignedRoute( + 'magic-link.verify', + now()->addMinutes(15), + ['token' => $result->plainToken] + ); + + $this->get($signedUrl); + + $user->refresh(); + $this->assertEquals( + $verifiedAt->timestamp, + $user->email_verified_at->timestamp, + 'Existing email_verified_at should not be overwritten on subsequent logins' + ); + } + + public function test_code_attempt_with_no_existing_token_is_rejected(): void + { + User::create([ + 'email_hash' => User::hashEmail('test@example.com'), + ]); + + // No token generated — no valid token exists for this email + $response = $this->post('/verify-code', [ + 'email' => 'test@example.com', + 'code' => '123456', + ]); + + $response->assertSessionHasErrors('code'); + $this->assertGuest(); + } } diff --git a/tests/Unit/MagicLoginTokenTest.php b/tests/Unit/MagicLoginTokenTest.php index 7b591ab..b113713 100755 --- a/tests/Unit/MagicLoginTokenTest.php +++ b/tests/Unit/MagicLoginTokenTest.php @@ -2,11 +2,12 @@ namespace Tests\Unit; +use App\DTOs\MagicTokenResult; use App\Models\MagicLoginToken; use App\Models\User; +use App\Services\MagicLinkAuthService; use Illuminate\Foundation\Testing\RefreshDatabase; use Illuminate\Support\Facades\Hash; -use Illuminate\Support\Str; use Tests\TestCase; class MagicLoginTokenTest extends TestCase @@ -15,221 +16,218 @@ class MagicLoginTokenTest extends TestCase public function test_token_expiry_validation_works(): void { - $token = MagicLoginToken::generate('test@example.com', '127.0.0.1', 'TestAgent'); + $result = MagicLoginToken::generate('test@example.com', '127.0.0.1', 'TestAgent'); - $this->assertTrue($token->isValid(), 'Newly created token should be valid'); + $this->assertTrue($result->token->isValid(), 'Newly created token should be valid'); - $token->update(['expires_at' => now()->subMinutes(1)]); - $token->refresh(); + $result->token->update(['expires_at' => now()->subMinutes(1)]); + $result->token->refresh(); - $this->assertFalse($token->isValid(), 'Expired token should not be valid'); + $this->assertFalse($result->token->isValid(), 'Expired token should not be valid'); } public function test_marking_token_as_used_works(): void { - $token = MagicLoginToken::generate('test@example.com', '127.0.0.1', 'TestAgent'); + $result = MagicLoginToken::generate('test@example.com', '127.0.0.1', 'TestAgent'); - $this->assertNull($token->used_at, 'New token should not be marked as used'); - $this->assertTrue($token->isValid(), 'New token should be valid'); + $this->assertNull($result->token->used_at, 'New token should not be marked as used'); + $this->assertTrue($result->token->isValid(), 'New token should be valid'); - $token->markAsUsed(); - $token->refresh(); + $result->token->markAsUsed(); + $result->token->refresh(); - $this->assertNotNull($token->used_at, 'Token should be marked as used'); - $this->assertFalse($token->isValid(), 'Used token should not be valid'); + $this->assertNotNull($result->token->used_at, 'Token should be marked as used'); + $this->assertFalse($result->token->isValid(), 'Used token should not be valid'); } public function test_code_verification_works(): void { - $token = MagicLoginToken::generate('test@example.com', '127.0.0.1', 'TestAgent'); - $code = $token->plain_code; + $result = MagicLoginToken::generate('test@example.com', '127.0.0.1', 'TestAgent'); $this->assertTrue( - $token->verifyCode($code), + $result->token->verifyCode($result->plainCode), 'Correct code should verify successfully' ); - $this->assertFalse( - $token->verifyCode('000000'), - 'Incorrect code should not verify' - ); + // Derive a code that is guaranteed to differ from the generated one + $wrongCode = str_pad((string) (((int) $result->plainCode + 1) % 1000000), 6, '0', STR_PAD_LEFT); + $this->assertFalse($result->token->verifyCode($wrongCode), 'Different code should not verify'); - $this->assertFalse( - $token->verifyCode('999999'), - 'Wrong code should not verify' - ); + // Non-numeric strings can never match a bcrypt hash of a 6-digit code + $this->assertFalse($result->token->verifyCode('XXXXXX'), 'Non-numeric string should not verify'); } public function test_code_verification_uses_bcrypt(): void { - $token = MagicLoginToken::generate('test@example.com', '127.0.0.1', 'TestAgent'); + $result = MagicLoginToken::generate('test@example.com', '127.0.0.1', 'TestAgent'); $this->assertNotEquals( - $token->plain_code, - $token->code_hash, + $result->plainCode, + $result->token->code_hash, 'Code hash should not be plain text' ); $this->assertTrue( - Hash::check($token->plain_code, $token->code_hash), + Hash::check($result->plainCode, $result->token->code_hash), 'Code hash should use bcrypt' ); } - public function test_token_verification_works(): void + public function test_token_hash_uses_sha256(): void { - $token = MagicLoginToken::generate('test@example.com', '127.0.0.1', 'TestAgent'); - $plainToken = $token->plain_token; - - $this->assertTrue( - $token->verifyToken($plainToken), - 'Correct token should verify successfully' - ); - - $this->assertFalse( - $token->verifyToken(Str::random(64)), - 'Incorrect token should not verify' - ); - } - - public function test_token_verification_uses_plain_text_comparison(): void - { - $token = MagicLoginToken::generate('test@example.com', '127.0.0.1', 'TestAgent'); + $result = MagicLoginToken::generate('test@example.com', '127.0.0.1', 'TestAgent'); $this->assertEquals( - $token->plain_token, - $token->plain_token, - 'Plain token should be stored as-is for comparison' + hash('sha256', $result->plainToken), + $result->token->token_hash, + 'Token hash should be SHA256 of the plain token' ); - $this->assertTrue( - $token->verifyToken($token->plain_token), - 'Token verification should use plain text comparison' - ); + $this->assertEquals(64, strlen($result->token->token_hash), 'SHA256 hex is 64 chars'); } public function test_scope_valid_returns_only_valid_tokens(): void { - $validToken = MagicLoginToken::generate('valid@example.com', '127.0.0.1', 'TestAgent'); + $validResult = MagicLoginToken::generate('valid@example.com', '127.0.0.1', 'TestAgent'); - $expiredToken = MagicLoginToken::generate('expired@example.com', '127.0.0.1', 'TestAgent'); - $expiredToken->update(['expires_at' => now()->subMinutes(1)]); + $expiredResult = MagicLoginToken::generate('expired@example.com', '127.0.0.1', 'TestAgent'); + $expiredResult->token->update(['expires_at' => now()->subMinutes(1)]); - $usedToken = MagicLoginToken::generate('used@example.com', '127.0.0.1', 'TestAgent'); - $usedToken->markAsUsed(); + $usedResult = MagicLoginToken::generate('used@example.com', '127.0.0.1', 'TestAgent'); + $usedResult->token->markAsUsed(); $validTokens = MagicLoginToken::valid()->get(); $this->assertCount(1, $validTokens, 'Only one token should be valid'); - $this->assertEquals($validToken->id, $validTokens->first()->id); + $this->assertEquals($validResult->token->id, $validTokens->first()->id); } public function test_scope_expired_or_used_returns_correct_tokens(): void { - $validToken = MagicLoginToken::generate('valid@example.com', '127.0.0.1', 'TestAgent'); + $validResult = MagicLoginToken::generate('valid@example.com', '127.0.0.1', 'TestAgent'); - $expiredToken = MagicLoginToken::generate('expired@example.com', '127.0.0.1', 'TestAgent'); - $expiredToken->update(['expires_at' => now()->subMinutes(1)]); + $expiredResult = MagicLoginToken::generate('expired@example.com', '127.0.0.1', 'TestAgent'); + $expiredResult->token->update(['expires_at' => now()->subMinutes(1)]); - $usedToken = MagicLoginToken::generate('used@example.com', '127.0.0.1', 'TestAgent'); - $usedToken->markAsUsed(); + $usedResult = MagicLoginToken::generate('used@example.com', '127.0.0.1', 'TestAgent'); + $usedResult->token->markAsUsed(); $expiredOrUsedTokens = MagicLoginToken::expiredOrUsed()->get(); $this->assertCount(2, $expiredOrUsedTokens, 'Two tokens should be expired or used'); $ids = $expiredOrUsedTokens->pluck('id')->toArray(); - $this->assertContains($expiredToken->id, $ids); - $this->assertContains($usedToken->id, $ids); - $this->assertNotContains($validToken->id, $ids); + $this->assertContains($expiredResult->token->id, $ids); + $this->assertContains($usedResult->token->id, $ids); + $this->assertNotContains($validResult->token->id, $ids); + } + + public function test_generate_returns_magic_token_result(): void + { + $result = MagicLoginToken::generate('test@example.com', '127.0.0.1', 'TestAgent'); + + $this->assertInstanceOf(MagicTokenResult::class, $result); + $this->assertInstanceOf(MagicLoginToken::class, $result->token); + $this->assertNotEmpty($result->plainToken); + $this->assertNotEmpty($result->plainCode); } public function test_generated_code_is_6_digits(): void { - $token = MagicLoginToken::generate('test@example.com', '127.0.0.1', 'TestAgent'); + $result = MagicLoginToken::generate('test@example.com', '127.0.0.1', 'TestAgent'); $this->assertMatchesRegularExpression( '/^\d{6}$/', - $token->plain_code, + $result->plainCode, 'Code should be exactly 6 digits' ); } - public function test_generated_code_includes_leading_zeros(): void + public function test_generated_code_is_always_6_chars(): void { + // Generate enough tokens that low-value codes (which have leading zeros) are statistically likely, + // and confirm they are always returned as a 6-character zero-padded string. for ($i = 0; $i < 20; $i++) { - $token = MagicLoginToken::generate('test' . $i . '@example.com', '127.0.0.1', 'TestAgent'); + $result = MagicLoginToken::generate('test' . $i . '@example.com', '127.0.0.1', 'TestAgent'); - $this->assertEquals( - 6, - strlen($token->plain_code), - 'Code should always be 6 characters long, including leading zeros' - ); + $this->assertEquals(6, strlen($result->plainCode)); } } + public function test_leading_zero_padding_is_preserved(): void + { + // Directly verify the padding logic: a code of 42 must be stored as '000042'. + // We confirm this by checking str_pad behaviour matches what the model produces. + $padded = str_pad('42', 6, '0', STR_PAD_LEFT); + $this->assertEquals('000042', $padded); + $this->assertMatchesRegularExpression('/^\d{6}$/', $padded); + } + public function test_token_stores_ip_address_and_user_agent(): void { $ip = '192.168.1.1'; $userAgent = 'Mozilla/5.0 Test Browser'; - $token = MagicLoginToken::generate('test@example.com', $ip, $userAgent); + $result = MagicLoginToken::generate('test@example.com', $ip, $userAgent); - $this->assertEquals($ip, $token->ip_address); - $this->assertEquals($userAgent, $token->user_agent); + $this->assertEquals($ip, $result->token->ip_address); + $this->assertEquals($userAgent, $result->token->user_agent); } public function test_token_expiry_is_15_minutes(): void { $beforeCreation = now()->startOfSecond()->addMinutes(15); - $token = MagicLoginToken::generate('test@example.com', '127.0.0.1', 'TestAgent'); + $result = MagicLoginToken::generate('test@example.com', '127.0.0.1', 'TestAgent'); $afterCreation = now()->addMinutes(15); $this->assertTrue( - $token->expires_at->between($beforeCreation, $afterCreation), + $result->token->expires_at->between($beforeCreation, $afterCreation), 'Token should expire in 15 minutes' ); } - public function test_token_hash_uses_bcrypt(): void - { - $token = MagicLoginToken::generate('test@example.com', '127.0.0.1', 'TestAgent'); - - $this->assertNotEquals( - $token->plain_token, - $token->token_hash, - 'Token hash should not be plain text' - ); - - $this->assertTrue( - Hash::check($token->plain_token, $token->token_hash), - 'Token hash should use bcrypt' - ); - } - public function test_email_hash_is_stored_correctly(): void { $email = 'test@example.com'; $expectedHash = User::hashEmail($email); - $token = MagicLoginToken::generate($email, '127.0.0.1', 'TestAgent'); + $result = MagicLoginToken::generate($email, '127.0.0.1', 'TestAgent'); - $this->assertEquals($expectedHash, $token->email_hash); + $this->assertEquals($expectedHash, $result->token->email_hash); + } + + public function test_plain_values_are_not_persisted(): void + { + $result = MagicLoginToken::generate('test@example.com', '127.0.0.1', 'TestAgent'); + + $fresh = MagicLoginToken::find($result->token->id); + + $this->assertArrayNotHasKey('plain_token', $fresh->getAttributes()); + $this->assertArrayNotHasKey('plain_code', $fresh->getAttributes()); + } + + public function test_token_can_be_looked_up_by_hash(): void + { + $result = MagicLoginToken::generate('test@example.com', '127.0.0.1', 'TestAgent'); + + $found = MagicLoginToken::where('token_hash', hash('sha256', $result->plainToken))->first(); + + $this->assertNotNull($found); + $this->assertEquals($result->token->id, $found->id); } public function test_multiple_tokens_can_exist_for_same_email(): void { $email = 'test@example.com'; - $token1 = MagicLoginToken::generate($email, '127.0.0.1', 'TestAgent'); - $token2 = MagicLoginToken::generate($email, '127.0.0.1', 'TestAgent'); + $result1 = MagicLoginToken::generate($email, '127.0.0.1', 'TestAgent'); + $result2 = MagicLoginToken::generate($email, '127.0.0.1', 'TestAgent'); - $this->assertNotEquals($token1->id, $token2->id); - $this->assertNotEquals($token1->plain_token, $token2->plain_token); - $this->assertNotEquals($token1->plain_code, $token2->plain_code); - $this->assertEquals($token1->email_hash, $token2->email_hash); + $this->assertNotEquals($result1->token->id, $result2->token->id); + $this->assertNotEquals($result1->plainToken, $result2->plainToken); + $this->assertEquals($result1->token->email_hash, $result2->token->email_hash); } public function test_tokens_are_unique(): void @@ -237,30 +235,57 @@ class MagicLoginTokenTest extends TestCase $generatedTokens = []; for ($i = 0; $i < 10; $i++) { - $token = MagicLoginToken::generate('test' . $i . '@example.com', '127.0.0.1', 'TestAgent'); - $generatedTokens[] = $token->plain_token; + $result = MagicLoginToken::generate('test' . $i . '@example.com', '127.0.0.1', 'TestAgent'); + $generatedTokens[] = $result->plainToken; } - $uniqueTokens = array_unique($generatedTokens); - - $this->assertCount( - 10, - $uniqueTokens, - 'All generated tokens should be unique' - ); + $this->assertCount(10, array_unique($generatedTokens), 'All generated tokens should be unique'); } public function test_token_casts_dates_correctly(): void { - $token = MagicLoginToken::generate('test@example.com', '127.0.0.1', 'TestAgent'); + $result = MagicLoginToken::generate('test@example.com', '127.0.0.1', 'TestAgent'); - $this->assertInstanceOf(\Illuminate\Support\Carbon::class, $token->expires_at); - $this->assertNull($token->used_at); + $this->assertInstanceOf(\Illuminate\Support\Carbon::class, $result->token->expires_at); + $this->assertNull($result->token->used_at); - $token->markAsUsed(); - $token->refresh(); + $result->token->markAsUsed(); + $result->token->refresh(); - $this->assertInstanceOf(\Illuminate\Support\Carbon::class, $token->used_at); + $this->assertInstanceOf(\Illuminate\Support\Carbon::class, $result->token->used_at); } + public function test_cleanup_deletes_old_expired_and_used_tokens(): void + { + $service = new MagicLinkAuthService(); + + // Recent expired — should NOT be deleted (within 7-day retention window) + $recentExpired = MagicLoginToken::generate('recent@example.com', null, null); + $recentExpired->token->update(['expires_at' => now()->subMinutes(1)]); + + // Old expired — should be deleted + $oldExpired = MagicLoginToken::generate('oldexpired@example.com', null, null); + $oldExpired->token->update([ + 'expires_at' => now()->subDays(8), + 'created_at' => now()->subDays(8), + ]); + + // Old used — should be deleted + $oldUsed = MagicLoginToken::generate('oldused@example.com', null, null); + $oldUsed->token->update([ + 'used_at' => now()->subDays(8), + 'created_at' => now()->subDays(8), + ]); + + // Valid, recent — must never be touched + $valid = MagicLoginToken::generate('valid@example.com', null, null); + + $deleted = $service->cleanupExpiredTokens(); + + $this->assertEquals(2, $deleted); + $this->assertDatabaseHas('magic_login_tokens', ['id' => $recentExpired->token->id]); + $this->assertDatabaseHas('magic_login_tokens', ['id' => $valid->token->id]); + $this->assertDatabaseMissing('magic_login_tokens', ['id' => $oldExpired->token->id]); + $this->assertDatabaseMissing('magic_login_tokens', ['id' => $oldUsed->token->id]); + } }