diff --git a/api/app/Http/Controllers/Api/V1/LoginController.php b/api/app/Http/Controllers/Api/V1/LoginController.php index ff62b2fb..c790f57b 100644 --- a/api/app/Http/Controllers/Api/V1/LoginController.php +++ b/api/app/Http/Controllers/Api/V1/LoginController.php @@ -12,7 +12,7 @@ use App\Http\Resources\Api\V1\MeResource; use App\Models\User; use App\Services\MfaService; use Illuminate\Http\JsonResponse; -use Illuminate\Support\Facades\Auth; +use Illuminate\Support\Facades\Hash; use Illuminate\Support\Facades\Log; final class LoginController extends Controller @@ -25,7 +25,12 @@ final class LoginController extends Controller public function __invoke(LoginRequest $request): JsonResponse { - if (! Auth::attempt($request->only('email', 'password'))) { + // Validate credentials WITHOUT creating a session. + // Auth::attempt() must NOT be used here — it establishes a Laravel + // session, which could grant access before MFA verification. + $user = User::where('email', $request->validated('email'))->first(); + + if (! $user || ! Hash::check($request->validated('password'), $user->password)) { Log::warning('Failed login attempt', [ 'email' => $request->validated('email'), 'ip' => $request->ip(), @@ -35,8 +40,6 @@ final class LoginController extends Controller return $this->unauthorized('Invalid credentials'); } - $user = Auth::user(); - // MFA enabled and confirmed — check trusted device or require MFA if ($user->mfa_enabled && $user->mfa_confirmed_at) { $fingerprint = $request->header('X-Device-Fingerprint'); @@ -44,8 +47,11 @@ final class LoginController extends Controller return $this->issueToken($user, $request); } - // MFA required — return session token instead of auth token - Auth::guard('web')->logout(); + // Revoke ALL existing tokens so old sessions cannot bypass MFA. + // The only way to get a new token is through MfaVerifyController + // after a successful code verification. + $user->tokens()->delete(); + $mfaSession = $this->mfaService->createMfaSession($user, $request->ip()); // Auto-send email code if email is the preferred method @@ -57,11 +63,15 @@ final class LoginController extends Controller } } + // Return MFA challenge — NO auth token, NO auth cookie. + // Expire the auth cookie to invalidate any stale browser session. + $cookieName = $this->resolveCookieName($request); + return response()->json([ 'success' => true, 'mfa_required' => true, ...$mfaSession, - ]); + ])->withCookie($this->forgetAuthCookie($cookieName)); } // MFA required by policy but not yet set up — issue token with flag diff --git a/api/tests/Feature/Auth/MfaLoginFlowTest.php b/api/tests/Feature/Auth/MfaLoginFlowTest.php index 59b4945e..2371322a 100644 --- a/api/tests/Feature/Auth/MfaLoginFlowTest.php +++ b/api/tests/Feature/Auth/MfaLoginFlowTest.php @@ -257,6 +257,299 @@ class MfaLoginFlowTest extends TestCase ]); } + // ─── Security tests: no auth before MFA ─── + + public function test_login_with_mfa_does_not_return_auth_token(): void + { + $user = $this->createUserWithTotp(); + + $response = $this->postJson('/api/v1/auth/login', [ + 'email' => $user->email, + 'password' => 'password', + ]); + + $response->assertOk() + ->assertJson(['mfa_required' => true]) + ->assertJsonMissing(['data' => ['user' => []]]) + ->assertJsonStructure(['mfa_session_token', 'methods']); + + // No new Sanctum tokens should have been created + $this->assertDatabaseCount('personal_access_tokens', 0); + } + + public function test_login_with_mfa_expires_auth_cookie(): void + { + $user = $this->createUserWithTotp(); + + $response = $this->postJson('/api/v1/auth/login', [ + 'email' => $user->email, + 'password' => 'password', + ]); + + $response->assertOk() + ->assertJson(['mfa_required' => true]); + + // The response must include a Set-Cookie that expires the auth cookie + $cookies = $response->headers->getCookies(); + $authCookie = collect($cookies)->first(fn ($c) => str_starts_with($c->getName(), 'crewli_')); + + $this->assertNotNull($authCookie, 'Response must include an auth cookie header'); + $this->assertTrue( + $authCookie->getExpiresTime() < time(), + 'Auth cookie must be expired (forget cookie)', + ); + } + + public function test_login_with_mfa_revokes_existing_tokens(): void + { + $user = $this->createUserWithTotp(); + + // Create a pre-existing token (simulate previous login) + $oldToken = $user->createToken('old-session')->plainTextToken; + $this->assertDatabaseCount('personal_access_tokens', 1); + + // Login triggers MFA → should revoke old tokens + $this->postJson('/api/v1/auth/login', [ + 'email' => $user->email, + 'password' => 'password', + ])->assertJson(['mfa_required' => true]); + + $this->assertDatabaseCount('personal_access_tokens', 0); + } + + public function test_old_token_cannot_access_api_after_mfa_login(): void + { + $user = $this->createUserWithTotp(); + + // Create a pre-existing token + $oldToken = $user->createToken('old-session')->plainTextToken; + $this->assertDatabaseCount('personal_access_tokens', 1); + + // Login triggers MFA → revokes tokens + $this->postJson('/api/v1/auth/login', [ + 'email' => $user->email, + 'password' => 'password', + ])->assertJson(['mfa_required' => true]); + + // Token record deleted from database + $this->assertDatabaseCount('personal_access_tokens', 0); + + // Reset the auth guard so it re-queries the database for the token + app('auth')->forgetGuards(); + + // Old token must no longer work + $this->withHeader('Authorization', 'Bearer ' . $oldToken) + ->getJson('/api/v1/auth/me') + ->assertUnauthorized(); + } + + public function test_mfa_session_token_cannot_be_used_as_auth_token(): void + { + $user = $this->createUserWithTotp(); + + $loginResponse = $this->postJson('/api/v1/auth/login', [ + 'email' => $user->email, + 'password' => 'password', + ]); + + $mfaSessionToken = $loginResponse->json('mfa_session_token'); + + // Try to use the MFA session token as a Bearer token + $this->withHeader('Authorization', 'Bearer ' . $mfaSessionToken) + ->getJson('/api/v1/auth/me') + ->assertUnauthorized(); + } + + public function test_mfa_session_consumed_after_successful_verify(): void + { + $user = $this->createUserWithTotp(); + $secret = decrypt($user->mfa_secret); + + $loginResponse = $this->postJson('/api/v1/auth/login', [ + 'email' => $user->email, + 'password' => 'password', + ]); + + $sessionToken = $loginResponse->json('mfa_session_token'); + $validCode = $this->google2fa->getCurrentOtp($secret); + + // First verify succeeds + $this->postJson('/api/v1/auth/mfa/verify', [ + 'mfa_session_token' => $sessionToken, + 'code' => $validCode, + 'method' => 'totp', + ])->assertOk(); + + // Second verify with same session token fails + $this->postJson('/api/v1/auth/mfa/verify', [ + 'mfa_session_token' => $sessionToken, + 'code' => $validCode, + 'method' => 'totp', + ])->assertStatus(422) + ->assertJson(['message' => 'MFA-sessie verlopen. Log opnieuw in.']); + } + + public function test_mfa_session_not_consumed_by_failed_verify(): void + { + $user = $this->createUserWithTotp(); + $secret = decrypt($user->mfa_secret); + + $loginResponse = $this->postJson('/api/v1/auth/login', [ + 'email' => $user->email, + 'password' => 'password', + ]); + + $sessionToken = $loginResponse->json('mfa_session_token'); + + // Wrong code — session should survive + $this->postJson('/api/v1/auth/mfa/verify', [ + 'mfa_session_token' => $sessionToken, + 'code' => '000000', + 'method' => 'totp', + ])->assertStatus(422); + + // Correct code — session should still be valid + $validCode = $this->google2fa->getCurrentOtp($secret); + + $this->postJson('/api/v1/auth/mfa/verify', [ + 'mfa_session_token' => $sessionToken, + 'code' => $validCode, + 'method' => 'totp', + ])->assertOk(); + } + + public function test_mfa_verify_returns_auth_cookie_only_on_success(): void + { + $user = $this->createUserWithTotp(); + $secret = decrypt($user->mfa_secret); + + $loginResponse = $this->postJson('/api/v1/auth/login', [ + 'email' => $user->email, + 'password' => 'password', + ]); + + $sessionToken = $loginResponse->json('mfa_session_token'); + + // Failed verify — no auth cookie + $failResponse = $this->postJson('/api/v1/auth/mfa/verify', [ + 'mfa_session_token' => $sessionToken, + 'code' => '000000', + 'method' => 'totp', + ]); + + $failResponse->assertStatus(422); + $failCookies = collect($failResponse->headers->getCookies()); + $failAuthCookie = $failCookies->first(fn ($c) => str_starts_with($c->getName(), 'crewli_') && $c->getExpiresTime() > time()); + $this->assertNull($failAuthCookie, 'Failed verify must not set an auth cookie'); + + // Successful verify — auth cookie present + $validCode = $this->google2fa->getCurrentOtp($secret); + + $successResponse = $this->postJson('/api/v1/auth/mfa/verify', [ + 'mfa_session_token' => $sessionToken, + 'code' => $validCode, + 'method' => 'totp', + ]); + + $successResponse->assertOk(); + $successCookies = collect($successResponse->headers->getCookies()); + $successAuthCookie = $successCookies->first(fn ($c) => str_starts_with($c->getName(), 'crewli_') && $c->getExpiresTime() > time()); + $this->assertNotNull($successAuthCookie, 'Successful verify must set an auth cookie'); + } + + public function test_mfa_session_expires_after_ttl(): void + { + $user = $this->createUserWithTotp(); + $secret = decrypt($user->mfa_secret); + + $loginResponse = $this->postJson('/api/v1/auth/login', [ + 'email' => $user->email, + 'password' => 'password', + ]); + + $sessionToken = $loginResponse->json('mfa_session_token'); + + // Travel past the 10-minute TTL + $this->travel(11)->minutes(); + + $validCode = $this->google2fa->getCurrentOtp($secret); + + $this->postJson('/api/v1/auth/mfa/verify', [ + 'mfa_session_token' => $sessionToken, + 'code' => $validCode, + 'method' => 'totp', + ])->assertStatus(422) + ->assertJson(['message' => 'MFA-sessie verlopen. Log opnieuw in.']); + } + + public function test_email_code_consumed_after_use(): void + { + $user = User::factory()->create([ + 'mfa_enabled' => true, + 'mfa_method' => MfaMethod::EMAIL->value, + 'mfa_confirmed_at' => now(), + ]); + + $loginResponse = $this->postJson('/api/v1/auth/login', [ + 'email' => $user->email, + 'password' => 'password', + ]); + + $sessionToken = $loginResponse->json('mfa_session_token'); + + $emailCode = MfaEmailCode::where('user_id', $user->id) + ->where('used', false) + ->first(); + + // First use succeeds + $this->postJson('/api/v1/auth/mfa/verify', [ + 'mfa_session_token' => $sessionToken, + 'code' => $emailCode->code, + 'method' => 'email', + ])->assertOk(); + + // Get a new MFA session for the replay test + $loginResponse2 = $this->postJson('/api/v1/auth/login', [ + 'email' => $user->email, + 'password' => 'password', + ]); + + $sessionToken2 = $loginResponse2->json('mfa_session_token'); + + // Same code cannot be reused + $this->postJson('/api/v1/auth/mfa/verify', [ + 'mfa_session_token' => $sessionToken2, + 'code' => $emailCode->code, + 'method' => 'email', + ])->assertStatus(422); + } + + public function test_trusted_device_expired_after_30_days_requires_mfa(): void + { + $user = $this->createUserWithTotp(); + $fingerprint = 'test-device-fp'; + + app(MfaService::class)->trustDevice($user, $fingerprint, '127.0.0.1', 'Test'); + + // Within 30 days — no MFA + $this->postJson('/api/v1/auth/login', [ + 'email' => $user->email, + 'password' => 'password', + ], ['X-Device-Fingerprint' => $fingerprint]) + ->assertOk() + ->assertJsonMissing(['mfa_required' => true]); + + // After 31 days — MFA required + $this->travel(31)->days(); + + $this->postJson('/api/v1/auth/login', [ + 'email' => $user->email, + 'password' => 'password', + ], ['X-Device-Fingerprint' => $fingerprint]) + ->assertOk() + ->assertJson(['mfa_required' => true]); + } + private function createUserWithTotp(): User { $user = User::factory()->create();