Skip to content

Commit 7d69981

Browse files
andrasbacsaiandrasbacsai
committed
fix: harden Vultr create, Gmail identity, and provider retries
Wrap Vultr server creation in DB transactions and delete the remote instance when local persistence fails (API and Livewire). Scope plus/dot email normalization to gmail.com/googlemail.com only. Use throw:false on DigitalOcean/Vultr HTTP retries, and normalize service log line counts via normalizeLogLines.
1 parent 2719d66 commit 7d69981

15 files changed

Lines changed: 429 additions & 63 deletions

‎app/Http/Controllers/Api/ServiceApplicationsController.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -490,7 +490,7 @@ public function logs_by_uuid(Request $request): JsonResponse
490490
], 400);
491491
}
492492

493-
$lines = (int) ($request->query('lines', 100) ?: 100);
493+
$lines = normalizeLogLines($request->query('lines'));
494494
$logs = getContainerLogs($server, $containerName, $lines);
495495

496496
return response()->json([

‎app/Http/Controllers/Api/ServiceDatabasesController.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,7 @@ public function logs(Request $request): JsonResponse
311311
return response()->json(['message' => 'Service database container is not running.'], 400);
312312
}
313313

314-
$lines = (int) ($request->query('lines', 100) ?: 100);
314+
$lines = normalizeLogLines($request->query('lines'));
315315

316316
return response()->json([
317317
'logs' => getContainerLogs($server, $containerName, $lines),

‎app/Http/Controllers/Api/VultrController.php‎

Lines changed: 62 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use App\Services\VultrService;
1616
use Illuminate\Http\JsonResponse;
1717
use Illuminate\Http\Request;
18+
use Illuminate\Support\Facades\DB;
1819
use OpenApi\Attributes as OA;
1920

2021
class VultrController extends Controller
@@ -286,6 +287,10 @@ public function createServer(Request $request): JsonResponse
286287
return response()->json(['message' => 'Private key not found.'], 404);
287288
}
288289

290+
$vultrService = null;
291+
$vultrInstanceId = null;
292+
$server = null;
293+
289294
try {
290295
$vultrService = new VultrService($token->token);
291296
$publicKey = $privateKey->getPublicKey();
@@ -317,33 +322,41 @@ public function createServer(Request $request): JsonResponse
317322
}
318323

319324
$vultrInstance = $vultrService->createInstance($params);
325+
$vultrInstanceId = (string) $vultrInstance['id'];
320326
$ipAddress = $vultrService->getPublicIp($vultrInstance, $request->disable_public_ipv4, $request->enable_ipv6) ?? Server::PLACEHOLDER_IP;
321327

322-
$server = Server::create([
323-
'name' => $normalizedServerName,
324-
'ip' => $ipAddress,
325-
'user' => 'root',
326-
'port' => 22,
327-
'team_id' => $teamId,
328-
'private_key_id' => $privateKey->id,
329-
'cloud_provider_token_id' => $token->id,
330-
'vultr_instance_id' => $vultrInstance['id'],
331-
'vultr_instance_status' => $vultrInstance['status'] ?? null,
332-
]);
333-
334-
$vultrInstance = $vultrService->waitForPublicIp($vultrInstance, $request->disable_public_ipv4, $request->enable_ipv6);
335-
$assignedIpAddress = $vultrService->getPublicIp($vultrInstance, $request->disable_public_ipv4, $request->enable_ipv6);
336-
if ($assignedIpAddress && $assignedIpAddress !== $server->ip) {
337-
$ipAddress = $assignedIpAddress;
338-
$server->update([
339-
'ip' => $assignedIpAddress,
340-
'vultr_instance_status' => $vultrInstance['status'] ?? $server->vultr_instance_status,
328+
$server = DB::transaction(function () use ($normalizedServerName, $ipAddress, $teamId, $privateKey, $token, $vultrInstanceId, $vultrInstance): Server {
329+
$server = Server::create([
330+
'name' => $normalizedServerName,
331+
'ip' => $ipAddress,
332+
'user' => 'root',
333+
'port' => 22,
334+
'team_id' => $teamId,
335+
'private_key_id' => $privateKey->id,
336+
'cloud_provider_token_id' => $token->id,
337+
'vultr_instance_id' => $vultrInstanceId,
338+
'vultr_instance_status' => $vultrInstance['status'] ?? null,
341339
]);
342-
}
343340

344-
$server->proxy->set('status', 'exited');
345-
$server->proxy->set('type', ProxyTypes::TRAEFIK->value);
346-
$server->save();
341+
$server->proxy->set('status', 'exited');
342+
$server->proxy->set('type', ProxyTypes::TRAEFIK->value);
343+
$server->save();
344+
345+
return $server;
346+
});
347+
348+
try {
349+
$vultrInstance = $vultrService->waitForPublicIp($vultrInstance, $request->disable_public_ipv4, $request->enable_ipv6);
350+
$assignedIpAddress = $vultrService->getPublicIp($vultrInstance, $request->disable_public_ipv4, $request->enable_ipv6);
351+
if ($assignedIpAddress && $assignedIpAddress !== $server->ip) {
352+
$server->update([
353+
'ip' => $assignedIpAddress,
354+
'vultr_instance_status' => $vultrInstance['status'] ?? $server->vultr_instance_status,
355+
]);
356+
}
357+
} catch (\Throwable $e) {
358+
report($e);
359+
}
347360

348361
if ($request->instant_validate) {
349362
ValidateServer::dispatch($server);
@@ -353,27 +366,48 @@ public function createServer(Request $request): JsonResponse
353366
'team_id' => $teamId,
354367
'server_uuid' => $server->uuid,
355368
'server_name' => $server->name,
356-
'vultr_instance_id' => $vultrInstance['id'],
357-
'ip' => $ipAddress,
369+
'vultr_instance_id' => $vultrInstanceId,
370+
'ip' => $server->ip,
358371
]);
359372

360373
return response()->json([
361374
'uuid' => $server->uuid,
362-
'vultr_instance_id' => $vultrInstance['id'],
363-
'ip' => $ipAddress,
375+
'vultr_instance_id' => $vultrInstanceId,
376+
'ip' => $server->ip,
364377
])->setStatusCode(201);
365378
} catch (RateLimitException $e) {
379+
$this->deleteUntrackedInstance($vultrService, $vultrInstanceId, $server);
380+
366381
$response = response()->json(['message' => $e->getMessage()], 429);
367382
if ($e->retryAfter !== null) {
368383
$response->header('Retry-After', $e->retryAfter);
369384
}
370385

371386
return $response;
372-
} catch (\Throwable) {
387+
} catch (\Throwable $e) {
388+
$this->deleteUntrackedInstance($vultrService, $vultrInstanceId, $server);
389+
390+
logger()->error('Failed to create Vultr server', [
391+
'error' => $e->getMessage(),
392+
]);
393+
373394
return response()->json(['message' => 'Failed to create Vultr server.'], 500);
374395
}
375396
}
376397

398+
private function deleteUntrackedInstance(?VultrService $vultrService, ?string $vultrInstanceId, ?Server $server): void
399+
{
400+
if (! $vultrService || ! $vultrInstanceId || $server) {
401+
return;
402+
}
403+
404+
try {
405+
$vultrService->deleteInstance($vultrInstanceId);
406+
} catch (\Throwable $e) {
407+
report($e);
408+
}
409+
}
410+
377411
private function findMatchingSshKey(array $sshKeys, string $publicKey): ?array
378412
{
379413
$normalizedPublicKey = $this->normalizePublicKey($publicKey);

‎app/Livewire/Server/New/ByVultr.php‎

Lines changed: 42 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
use Illuminate\Foundation\Auth\Access\AuthorizesRequests;
1515
use Illuminate\Http\Client\RequestException;
1616
use Illuminate\Support\Collection;
17+
use Illuminate\Support\Facades\DB;
1718
use Livewire\Attributes\Locked;
1819
use Livewire\Component;
1920

@@ -377,9 +378,8 @@ private function providerDataErrorMessage(string $providerName, \Throwable $e, s
377378
return "{$providerName} API error: {$details}";
378379
}
379380

380-
private function createVultrServer(string $token): array
381+
private function createVultrServer(VultrService $vultrService): array
381382
{
382-
$vultrService = new VultrService($token);
383383
$privateKey = PrivateKey::ownedByCurrentTeam()->findOrFail($this->private_key_id);
384384
$publicKey = $privateKey->getPublicKey();
385385
$existingKey = $this->findMatchingSshKey($vultrService->getSshKeys(), $publicKey);
@@ -419,6 +419,10 @@ public function submit(): mixed
419419
return null;
420420
}
421421

422+
$vultrService = null;
423+
$vultrInstanceId = null;
424+
$server = null;
425+
422426
try {
423427
$this->authorize('create', Server::class);
424428

@@ -437,20 +441,29 @@ public function submit(): mixed
437441
}
438442

439443
$vultrService = new VultrService($this->getVultrToken());
440-
$vultrInstance = $this->createVultrServer($this->getVultrToken());
444+
$vultrInstance = $this->createVultrServer($vultrService);
445+
$vultrInstanceId = (string) $vultrInstance['id'];
441446
$ipAddress = $vultrService->getPublicIp($vultrInstance, $this->disable_public_ipv4, $this->enable_ipv6) ?? Server::PLACEHOLDER_IP;
442447

443-
$server = Server::create([
444-
'name' => strtolower(trim($this->server_name)),
445-
'ip' => $ipAddress,
446-
'user' => 'root',
447-
'port' => 22,
448-
'team_id' => currentTeam()->id,
449-
'private_key_id' => $this->private_key_id,
450-
'cloud_provider_token_id' => $this->selected_token_id,
451-
'vultr_instance_id' => $vultrInstance['id'],
452-
'vultr_instance_status' => $vultrInstance['status'] ?? null,
453-
]);
448+
$server = DB::transaction(function () use ($ipAddress, $vultrInstanceId, $vultrInstance): Server {
449+
$server = Server::create([
450+
'name' => strtolower(trim($this->server_name)),
451+
'ip' => $ipAddress,
452+
'user' => 'root',
453+
'port' => 22,
454+
'team_id' => currentTeam()->id,
455+
'private_key_id' => $this->private_key_id,
456+
'cloud_provider_token_id' => $this->selected_token_id,
457+
'vultr_instance_id' => $vultrInstanceId,
458+
'vultr_instance_status' => $vultrInstance['status'] ?? null,
459+
]);
460+
461+
$server->proxy->set('status', 'exited');
462+
$server->proxy->set('type', ProxyTypes::TRAEFIK->value);
463+
$server->save();
464+
465+
return $server;
466+
});
454467

455468
try {
456469
$vultrInstance = $vultrService->waitForPublicIp($vultrInstance, $this->disable_public_ipv4, $this->enable_ipv6);
@@ -466,10 +479,6 @@ public function submit(): mixed
466479
report($e);
467480
}
468481

469-
$server->proxy->set('status', 'exited');
470-
$server->proxy->set('type', ProxyTypes::TRAEFIK->value);
471-
$server->save();
472-
473482
if ($this->from_onboarding) {
474483
currentTeam()->update([
475484
'show_boarding' => false,
@@ -479,10 +488,25 @@ public function submit(): mixed
479488

480489
return redirectRoute($this, 'server.show', [$server->uuid]);
481490
} catch (\Throwable $e) {
491+
$this->deleteUntrackedInstance($vultrService, $vultrInstanceId, $server);
492+
482493
return handleError($e, $this);
483494
}
484495
}
485496

497+
private function deleteUntrackedInstance(?VultrService $vultrService, ?string $vultrInstanceId, ?Server $server): void
498+
{
499+
if (! $vultrService || ! $vultrInstanceId || $server) {
500+
return;
501+
}
502+
503+
try {
504+
$vultrService->deleteInstance($vultrInstanceId);
505+
} catch (\Throwable $e) {
506+
report($e);
507+
}
508+
}
509+
486510
public function render()
487511
{
488512
return view('livewire.server.new.by-vultr');

‎app/Services/DigitalOceanService.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ private function request(string $method, string $endpoint, array $data = []): ar
2828
}
2929

3030
return $attempt * 100;
31-
})
31+
}, throw: false)
3232
->{$method}($this->baseUrl.$endpoint, $data);
3333

3434
if (! $response->successful()) {

‎app/Services/VultrService.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ private function request(string $method, string $endpoint, array $data = []): ar
1717
'Authorization' => 'Bearer '.$this->token,
1818
])
1919
->timeout(30)
20-
->retry(3, fn (int $attempt) => $attempt * 100)
20+
->retry(3, fn (int $attempt) => $attempt * 100, throw: false)
2121
->{$method}($this->baseUrl.$endpoint, $data);
2222

2323
if (! $response->successful()) {

‎bootstrap/helpers/email.php‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,12 @@ function normalize_email_identity(?string $email): ?string
88
return null;
99
}
1010

11-
[$localPart, $domain] = explode('@', Str::lower($email), 2);
12-
$localPart = Str::before($localPart, '+');
13-
$localPart = str_replace('.', '', $localPart);
11+
[$localPart, $domain] = explode('@', Str::lower(trim($email)), 2);
12+
13+
if (in_array($domain, ['gmail.com', 'googlemail.com'], true)) {
14+
$localPart = Str::before($localPart, '+');
15+
$localPart = str_replace('.', '', $localPart);
16+
}
1417

1518
if (blank($localPart) || blank($domain)) {
1619
return null;

‎tests/Feature/ForgotPasswordRateLimitTest.php‎

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,9 @@
2727

2828
it('rate limits dotted plus-address forgot password variants of the same email identity across ips', function () {
2929
$emails = [
30-
'ke.vinmcfadden+one@btinternet.com',
31-
'kevin.mcfadden+two@btinternet.com',
32-
'k.e.v.i.n.m.c.f.a.d.d.e.n+three@btinternet.com',
30+
'ke.vinmcfadden+one@gmail.com',
31+
'kevin.mcfadden+two@gmail.com',
32+
'k.e.v.i.n.m.c.f.a.d.d.e.n+three@gmail.com',
3333
];
3434

3535
foreach ($emails as $index => $email) {
@@ -42,7 +42,24 @@
4242

4343
$this->withServerVariables(['REMOTE_ADDR' => '203.0.113.99'])
4444
->post('/forgot-password', [
45-
'email' => 'k.evin.mcfadden+four@btinternet.com',
45+
'email' => 'k.evin.mcfadden+four@gmail.com',
4646
])
4747
->assertTooManyRequests();
4848
});
49+
50+
it('keeps distinct dotted and plus-addressed mailboxes in separate forgot password buckets on ordinary domains', function () {
51+
$emails = [
52+
'john.smith@example.com',
53+
'johnsmith@example.com',
54+
'johnsmith+one@example.com',
55+
'johnsmith+two@example.com',
56+
];
57+
58+
foreach ($emails as $index => $email) {
59+
$this->withServerVariables(['REMOTE_ADDR' => '203.0.113.'.($index + 120)])
60+
->post('/forgot-password', [
61+
'email' => $email,
62+
])
63+
->assertSessionHasNoErrors();
64+
}
65+
});

‎tests/Feature/RegistrationRateLimitTest.php‎

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
use App\Models\InstanceSettings;
44
use App\Models\User;
55
use Illuminate\Foundation\Testing\RefreshDatabase;
6+
use Illuminate\Support\Facades\RateLimiter;
67

78
uses(RefreshDatabase::class);
89

@@ -42,9 +43,9 @@
4243

4344
it('rate limits dotted plus-address variants of the same email identity across ips', function () {
4445
$emails = [
45-
'ke.vinmcfadden+one@btinternet.com',
46-
'kevin.mcfadden+two@btinternet.com',
47-
'k.e.v.i.n.m.c.f.a.d.d.e.n+three@btinternet.com',
46+
'ke.vinmcfadden+one@gmail.com',
47+
'kevin.mcfadden+two@gmail.com',
48+
'k.e.v.i.n.m.c.f.a.d.d.e.n+three@gmail.com',
4849
];
4950

5051
foreach ($emails as $index => $email) {
@@ -64,9 +65,33 @@
6465
$this->withServerVariables(['REMOTE_ADDR' => '203.0.113.99'])
6566
->post('/register', [
6667
'name' => 'Blocked User',
67-
'email' => 'k.evin.mcfadden+four@btinternet.com',
68+
'email' => 'k.evin.mcfadden+four@gmail.com',
6869
'password' => 'Password1!@',
6970
'password_confirmation' => 'Password1!@',
7071
])
7172
->assertTooManyRequests();
7273
});
74+
75+
it('keeps distinct dotted and plus-addressed mailboxes in separate rate limit buckets on ordinary domains', function () {
76+
$registrationIpKey = 'registration:ip:'.sha1('127.0.0.1');
77+
$emails = [
78+
'john.smith@example.com',
79+
'johnsmith@example.com',
80+
'johnsmith+one@example.com',
81+
'johnsmith+two@example.com',
82+
];
83+
84+
foreach ($emails as $index => $email) {
85+
$this->post('/register', [
86+
'name' => "Distinct User {$index}",
87+
'email' => $email,
88+
'password' => 'Password1!@',
89+
'password_confirmation' => 'Password1!@',
90+
])
91+
->assertRedirect();
92+
93+
auth()->logout();
94+
$this->flushSession();
95+
RateLimiter::clear($registrationIpKey);
96+
}
97+
});

0 commit comments

Comments
 (0)