From de1d234ea5db6f7c8bc99865b1dfa6b63398a7b7 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Thu, 20 Aug 2026 09:34:05 +0200 Subject: [PATCH] fix(auth): require confirmation before accepting magic links (#11413) --- app/Http/Controllers/Controller.php | 150 ++++++++++++------ app/Models/TeamInvitation.php | 11 +- app/Providers/FortifyServiceProvider.php | 7 + resources/views/invitation/accept.blade.php | 5 +- routes/web.php | 5 +- tests/Feature/InvitationLinkHandlingTest.php | 122 +++++++++++++- .../LinkLoginEmailVerificationTest.php | 4 +- 7 files changed, 238 insertions(+), 66 deletions(-) diff --git a/app/Http/Controllers/Controller.php b/app/Http/Controllers/Controller.php index c723d811a..9b6b315ce 100644 --- a/app/Http/Controllers/Controller.php +++ b/app/Http/Controllers/Controller.php @@ -8,12 +8,15 @@ use App\Providers\RouteServiceProvider; use Illuminate\Auth\Events\Verified; use Illuminate\Contracts\Encryption\DecryptException; +use Illuminate\Contracts\View\View; use Illuminate\Foundation\Auth\Access\AuthorizesRequests; use Illuminate\Foundation\Validation\ValidatesRequests; +use Illuminate\Http\RedirectResponse; use Illuminate\Http\Request; use Illuminate\Routing\Controller as BaseController; use Illuminate\Support\Facades\Auth; use Illuminate\Support\Facades\Crypt; +use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Hash; use Illuminate\Support\Facades\Password; use Illuminate\Support\Str; @@ -95,61 +98,105 @@ public function forgot_password(Request $request) return response()->json(['message' => 'Transactional emails are not active'], 400); } - public function link() + public function link(): View|RedirectResponse { $token = request()->get('token'); - if (is_string($token) && $token !== '') { - try { - $decrypted = Crypt::decryptString($token); - } catch (DecryptException) { - return redirect()->route('login')->with('error', 'Invalid credentials.'); - } - - if (! str_contains($decrypted, '@@@')) { - return redirect()->route('login')->with('error', 'Invalid credentials.'); - } - - $payload = explode('@@@', $decrypted, 3); - if (count($payload) === 3) { - [$email, $invitationUuid, $password] = $payload; - } else { - [$email, $password] = $payload; - $invitationUuid = null; - } - - $email = Str::lower($email); - $user = User::whereEmail($email)->first(); - if (! $user) { - return redirect()->route('login'); - } - - $invitation = TeamInvitation::query() - ->where('email', $email) - ->when($invitationUuid, fn ($query) => $query->where('uuid', $invitationUuid)) - ->first(); - if (! $invitation || ! $this->invitationLinkMatchesToken($invitation, $token) || ! $invitation->isValid()) { - return redirect()->route('login')->with('error', 'Invitation has expired or been revoked.'); - } - - if (Hash::check($password, $user->password)) { - $team = $invitation->team; - if (! $user->teams()->where('team_id', $team->id)->exists()) { - $user->teams()->attach($team->id, ['role' => $invitation->role]); - } - $invitation->delete(); - - $user->forceFill([ - 'password' => Hash::make(Str::random(64)), - ])->save(); - - Auth::login($user); - session(['currentTeam' => $team]); - - return redirect()->route('dashboard'); - } + $credentials = is_string($token) ? $this->magicLinkCredentials($token) : null; + if (! $credentials) { + return redirect()->route('login')->with('error', 'Invitation has expired or been revoked.'); } - return redirect()->route('login')->with('error', 'Invalid credentials.'); + [$user, $invitation] = $credentials; + + return view('invitation.accept', [ + 'invitation' => $invitation, + 'team' => $invitation->team, + 'alreadyMember' => $user->teams()->where('team_id', $invitation->team_id)->exists(), + 'formAction' => route('auth.link.accept'), + 'token' => $token, + ]); + } + + public function acceptLink(Request $request): RedirectResponse + { + $token = $request->input('token'); + if (! is_string($token)) { + return redirect()->route('login')->with('error', 'Invitation has expired or been revoked.'); + } + + $acceptedInvitation = DB::transaction(function () use ($token) { + $credentials = $this->magicLinkCredentials($token, lockForUpdate: true); + if (! $credentials) { + return null; + } + + [$user, $invitation] = $credentials; + $team = $invitation->team; + if (! $user->teams()->where('team_id', $team->id)->exists()) { + $user->teams()->attach($team->id, ['role' => $invitation->role]); + } + + $user->forceFill([ + 'password' => Hash::make(Str::random(64)), + ])->save(); + $invitation->delete(); + + return [$user, $team]; + }); + + if (! $acceptedInvitation) { + return redirect()->route('login')->with('error', 'Invitation has expired or been revoked.'); + } + + [$user, $team] = $acceptedInvitation; + + Auth::login($user); + session(['currentTeam' => $team]); + + return redirect()->route('dashboard'); + } + + /** + * @return array{0: User, 1: TeamInvitation}|null + */ + private function magicLinkCredentials(string $token, bool $lockForUpdate = false): ?array + { + if ($token === '') { + return null; + } + + try { + $decrypted = Crypt::decryptString($token); + } catch (DecryptException) { + return null; + } + + $payload = explode('@@@', $decrypted, 3); + if (count($payload) === 3) { + [$email, $invitationUuid, $password] = $payload; + } elseif (count($payload) === 2) { + [$email, $password] = $payload; + $invitationUuid = null; + } else { + return null; + } + + $email = Str::lower($email); + $user = User::query()->where('email', $email)->first(); + $invitationQuery = TeamInvitation::query() + ->where('email', $email) + ->when($lockForUpdate, fn ($query) => $query->lockForUpdate()); + $invitation = $invitationUuid + ? $invitationQuery->where('uuid', $invitationUuid)->first() + : $invitationQuery->get()->first( + fn (TeamInvitation $invitation) => $this->invitationLinkMatchesToken($invitation, $token) + ); + + if (! $user || ! $invitation || $invitation->hasExpired() || ! $this->invitationLinkMatchesToken($invitation, $token)) { + return null; + } + + return Hash::check($password, $user->password) ? [$user, $invitation] : null; } private function invitationLinkMatchesToken(TeamInvitation $invitation, string $token): bool @@ -185,6 +232,7 @@ public function showInvitation() 'invitation' => $invitation, 'team' => $invitation->team, 'alreadyMember' => $alreadyMember, + 'formAction' => route('team.invitation.accept', $invitation->uuid), ]); } diff --git a/app/Models/TeamInvitation.php b/app/Models/TeamInvitation.php index c322982ed..4258a82b8 100644 --- a/app/Models/TeamInvitation.php +++ b/app/Models/TeamInvitation.php @@ -33,11 +33,9 @@ public static function ownedByCurrentTeam() return TeamInvitation::whereTeamId(currentTeam()->id); } - public function isValid() + public function isValid(): bool { - $createdAt = $this->created_at; - $diff = $createdAt->diffInDays(now()); - if ($diff <= config('constants.invitation.link.expiration_days')) { + if (! $this->hasExpired()) { return true; } else { $this->delete(); @@ -49,4 +47,9 @@ public function isValid() return false; } } + + public function hasExpired(): bool + { + return $this->created_at->diffInDays(now()) > config('constants.invitation.link.expiration_days'); + } } diff --git a/app/Providers/FortifyServiceProvider.php b/app/Providers/FortifyServiceProvider.php index 1b201fb3f..65d968774 100644 --- a/app/Providers/FortifyServiceProvider.php +++ b/app/Providers/FortifyServiceProvider.php @@ -152,6 +152,13 @@ public function boot(): void return Limit::perMinute(5)->by($email.'|'.$realIp); }); + RateLimiter::for('magic-link', function (Request $request) { + $realIp = $request->server('REMOTE_ADDR') ?? $request->ip(); + $token = (string) $request->input('token'); + + return Limit::perMinute(5)->by(hash('sha256', $token.'|'.$realIp)); + }); + RateLimiter::for('two-factor', function (Request $request) { return Limit::perMinute(5)->by($request->session()->get('login.id')); }); diff --git a/resources/views/invitation/accept.blade.php b/resources/views/invitation/accept.blade.php index 12c3ddbb6..7509a9969 100644 --- a/resources/views/invitation/accept.blade.php +++ b/resources/views/invitation/accept.blade.php @@ -21,8 +21,11 @@ You are already a member of this team. Dismiss the invitation to continue. @endif -
+ @csrf + @isset($token) + + @endisset {{ $alreadyMember ? 'Dismiss invitation' : 'Accept invitation' }} diff --git a/routes/web.php b/routes/web.php index ff3c0890c..d9b43a790 100644 --- a/routes/web.php +++ b/routes/web.php @@ -112,9 +112,8 @@ Route::get('/realtime', [Controller::class, 'realtime_test'])->middleware('auth'); Route::get('/verify', [Controller::class, 'verify'])->middleware('auth')->name('verify.email'); Route::get('/email/verify/{id}/{hash}', [Controller::class, 'email_verify'])->middleware(['auth'])->name('verify.verify'); -Route::middleware(['throttle:login'])->group(function () { - Route::get('/auth/link', [Controller::class, 'link'])->name('auth.link'); -}); +Route::get('/auth/link', [Controller::class, 'link'])->name('auth.link'); +Route::post('/auth/link', [Controller::class, 'acceptLink'])->middleware('throttle:magic-link')->name('auth.link.accept'); Route::get('/auth/{provider}/redirect', [OauthController::class, 'redirect'])->name('auth.redirect'); Route::get('/auth/{provider}/callback', [OauthController::class, 'callback'])->name('auth.callback'); diff --git a/tests/Feature/InvitationLinkHandlingTest.php b/tests/Feature/InvitationLinkHandlingTest.php index 3b473f60e..31081e726 100644 --- a/tests/Feature/InvitationLinkHandlingTest.php +++ b/tests/Feature/InvitationLinkHandlingTest.php @@ -1,5 +1,6 @@ withoutVite(); $this->withoutMiddleware([DecideWhatToDoWithUser::class, CheckForcePasswordReset::class]); Once::flush(); Config::set('app.maintenance.driver', 'file'); @@ -56,10 +58,86 @@ function createInvitationLinkFixture(array $invitationAttributes = []): array return [$team, $user, $password, $token, $invitation]; } -it('accepts a valid magic link invitation only once and rotates the temporary password', function () { +it('shows a valid magic link invitation without consuming it', function () { [$team, $user, $password, $token] = createInvitationLinkFixture(); $this->get(route('auth.link', ['token' => $token])) + ->assertSuccessful() + ->assertViewIs('invitation.accept') + ->assertSee($team->name) + ->assertSee('Accept invitation'); + + $this->assertGuest(); + $this->assertDatabaseHas('team_invitations', ['email' => $user->email]); + expect($user->teams()->where('team_id', $team->id)->exists())->toBeFalse(); + + $user->refresh(); + expect(Hash::check($password, $user->password))->toBeTrue(); +}); + +it('finds the matching invitation for a legacy token when the email has multiple invitations', function () { + $team = Team::factory()->create(); + $user = User::factory()->create([ + 'email' => 'legacy-invitee@example.com', + 'password' => Hash::make($password = 'temporary-password-123'), + ]); + $legacyToken = Crypt::encryptString("{$user->email}@@@{$password}"); + + TeamInvitation::create([ + 'team_id' => Team::factory()->create()->id, + 'uuid' => (string) new Cuid2(32), + 'email' => $user->email, + 'role' => 'member', + 'link' => route('auth.link', ['token' => Crypt::encryptString("{$user->email}@@@another-password")]), + 'via' => 'link', + ]); + + TeamInvitation::create([ + 'team_id' => $team->id, + 'uuid' => (string) new Cuid2(32), + 'email' => $user->email, + 'role' => 'member', + 'link' => route('auth.link', ['token' => $legacyToken]), + 'via' => 'link', + ]); + + $this->get(route('auth.link', ['token' => $legacyToken])) + ->assertSuccessful() + ->assertViewHas('team', $team); +}); + +it('does not count confirmation requests against the acceptance throttle', function () { + [, $user, , $token] = createInvitationLinkFixture(); + + foreach (range(1, 5) as $attempt) { + $this->get(route('auth.link', ['token' => $token])) + ->assertSuccessful(); + } + + $this->post(route('auth.link.accept'), ['token' => $token]) + ->assertRedirect(route('dashboard')); + + $this->assertAuthenticatedAs($user); +}); + +it('throttles acceptance independently for different magic link tokens from the same IP', function () { + [, $user, , $token] = createInvitationLinkFixture(); + + foreach (range(1, 5) as $attempt) { + $this->post(route('auth.link.accept'), ['token' => 'another-token']) + ->assertRedirect(route('login')); + } + + $this->post(route('auth.link.accept'), ['token' => $token]) + ->assertRedirect(route('dashboard')); + + $this->assertAuthenticatedAs($user); +}); + +it('accepts a valid magic link invitation on post only once and rotates the temporary password', function () { + [$team, $user, $password, $token] = createInvitationLinkFixture(); + + $this->post(route('auth.link.accept'), ['token' => $token]) ->assertRedirect(route('dashboard')); $this->assertAuthenticatedAs($user); @@ -72,16 +150,37 @@ function createInvitationLinkFixture(array $invitationAttributes = []): array auth()->logout(); session()->flush(); - $this->get(route('auth.link', ['token' => $token])) + $this->post(route('auth.link.accept'), ['token' => $token]) ->assertRedirect(route('login')); $this->assertGuest(); }); +it('rolls back invitation redemption when password rotation fails', function () { + [$team, $user, $password, $token, $invitation] = createInvitationLinkFixture(); + $this->withoutExceptionHandling(); + + User::updating(function (User $updatingUser) use ($user) { + if ($updatingUser->is($user)) { + throw new RuntimeException('Password rotation failed.'); + } + }); + + expect(fn () => $this->post(route('auth.link.accept'), ['token' => $token])) + ->toThrow(RuntimeException::class, 'Password rotation failed.'); + + $this->assertDatabaseHas('team_invitations', ['id' => $invitation->id]); + expect($user->teams()->where('team_id', $team->id)->exists())->toBeFalse(); + + $user->refresh(); + expect(Hash::check($password, $user->password))->toBeTrue(); + $this->assertGuest(); +}); + it('accepts a magic link when opened from a different public origin', function () { [$team, $user, $password, $token] = createInvitationLinkFixture(); - $this->get('https://coolify.example.com/auth/link?token='.urlencode($token)) + $this->post('https://coolify.example.com/auth/link', ['token' => $token]) ->assertRedirect(route('dashboard')); $this->assertAuthenticatedAs($user); @@ -98,7 +197,7 @@ function createInvitationLinkFixture(array $invitationAttributes = []): array [$team, $user, $password, $token] = createInvitationLinkFixture(); - $this->get(route('auth.link', ['token' => $token])) + $this->post(route('auth.link.accept'), ['token' => $token]) ->assertRedirect(route('dashboard')); expect(DB::table('sessions')->where('user_id', $user->id)->exists())->toBeTrue(); @@ -170,7 +269,7 @@ function createInvitationLinkFixture(array $invitationAttributes = []): array ->assertRedirect(route('login')); $this->assertGuest(); - $this->assertDatabaseMissing('team_invitations', ['id' => $invitation->id]); + $this->assertDatabaseHas('team_invitations', ['id' => $invitation->id]); }); it('rejects a malformed magic link token', function () { @@ -179,3 +278,16 @@ function createInvitationLinkFixture(array $invitationAttributes = []): array $this->assertGuest(); }); + +it('declares the magic link method contracts', function () { + $linkReturnType = (new ReflectionMethod(Controller::class, 'link'))->getReturnType(); + $acceptLinkReturnType = (new ReflectionMethod(Controller::class, 'acceptLink'))->getReturnType(); + $credentialsMethod = new ReflectionMethod(Controller::class, 'magicLinkCredentials'); + $isValidReturnType = (new ReflectionMethod(TeamInvitation::class, 'isValid'))->getReturnType(); + + expect((string) $linkReturnType)->toContain('Illuminate\\Contracts\\View\\View') + ->and((string) $linkReturnType)->toContain('Illuminate\\Http\\RedirectResponse') + ->and((string) $acceptLinkReturnType)->toBe('Illuminate\\Http\\RedirectResponse') + ->and($credentialsMethod->getDocComment())->toContain('@return array{0: User, 1: TeamInvitation}|null') + ->and((string) $isValidReturnType)->toBe('bool'); +}); diff --git a/tests/Feature/LinkLoginEmailVerificationTest.php b/tests/Feature/LinkLoginEmailVerificationTest.php index 39ade93eb..c962e86ec 100644 --- a/tests/Feature/LinkLoginEmailVerificationTest.php +++ b/tests/Feature/LinkLoginEmailVerificationTest.php @@ -50,7 +50,7 @@ 'via' => 'link', ]); - $this->get(route('auth.link', ['token' => $token])); + $this->post(route('auth.link.accept'), ['token' => $token]); $user->refresh(); expect($user->email_verified_at)->toBeNull(); @@ -77,7 +77,7 @@ 'via' => 'link', ]); - $this->get(route('auth.link', ['token' => $token])) + $this->post(route('auth.link.accept'), ['token' => $token]) ->assertRedirect(route('dashboard')); expect(auth()->id())->toBe($user->id);