Skip to content

Commit 676d25c

Browse files
wait for lock properly (#5638)
This PR is a minimal change that acquires a lock on the target model's row, rather than calling sharedLock() on the model instance.
1 parent a94b8bd commit 676d25c

2 files changed

Lines changed: 81 additions & 1 deletion

File tree

app/Services/Databases/DatabasePasswordService.php

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,10 @@ public function handle(Database|int $database): string
3232
$password = Utilities::randomStringWithSpecialCharacters(24);
3333

3434
$this->connection->transaction(function () use ($database, $password) {
35-
$database->sharedLock()->update([
35+
// Lock the row to serialize concurrent rotations of the same database.
36+
$database->newQuery()->whereKey($database->getKey())->lockForUpdate()->firstOrFail();
37+
38+
$database->update([
3639
'password' => $this->encrypter->encrypt($password),
3740
]);
3841

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
<?php
2+
3+
namespace Pterodactyl\Tests\Integration\Services\Databases;
4+
5+
use Mockery\MockInterface;
6+
use Pterodactyl\Models\Database;
7+
use Pterodactyl\Models\DatabaseHost;
8+
use Pterodactyl\Tests\Integration\IntegrationTestCase;
9+
use Pterodactyl\Repositories\Eloquent\DatabaseRepository;
10+
use Pterodactyl\Services\Databases\DatabasePasswordService;
11+
12+
class DatabasePasswordServiceTest extends IntegrationTestCase
13+
{
14+
private MockInterface $repository;
15+
16+
/**
17+
* Setup tests.
18+
*/
19+
public function setUp(): void
20+
{
21+
parent::setUp();
22+
23+
$this->repository = $this->mock(DatabaseRepository::class);
24+
}
25+
26+
/**
27+
* Test that a database password is rotated correctly.
28+
*/
29+
public function testDatabasePasswordCanBeRotated()
30+
{
31+
$server = $this->createServerModel();
32+
$host = DatabaseHost::factory()->create(['node_id' => $server->node_id]);
33+
34+
$database = Database::factory()->create([
35+
'server_id' => $server->id,
36+
'database_host_id' => $host->id,
37+
'password' => encrypt('original'),
38+
]);
39+
40+
$other = Database::factory()->create([
41+
'server_id' => $server->id,
42+
'database_host_id' => $host->id,
43+
'password' => encrypt('unchanged'),
44+
]);
45+
46+
$password = null;
47+
48+
$this->repository->expects('dropUser')->with($database->username, $database->remote);
49+
$this->repository->expects('createUser')->with(
50+
$database->username,
51+
$database->remote,
52+
\Mockery::on(function ($value) use (&$password) {
53+
$password = $value;
54+
55+
return true;
56+
}),
57+
$database->max_connections
58+
);
59+
$this->repository->expects('assignUserToDatabase')->with($database->database, $database->username, $database->remote);
60+
$this->repository->expects('flush')->withNoArgs();
61+
62+
$response = $this->getService()->handle($database);
63+
64+
// The new password is returned, set on the host, and stored.
65+
$this->assertSame(24, strlen($response));
66+
$this->assertSame($response, $password);
67+
$this->assertSame($response, decrypt($database->refresh()->password));
68+
69+
// Other databases are untouched.
70+
$this->assertSame('unchanged', decrypt($other->refresh()->password));
71+
}
72+
73+
private function getService(): DatabasePasswordService
74+
{
75+
return $this->app->make(DatabasePasswordService::class);
76+
}
77+
}

0 commit comments

Comments
 (0)