Skip to content

Commit 2972d3f

Browse files
susnuxsusnux
authored
Merge pull request #62958 from nextcloud/feat/allow-ssl-config
feat(setup): allow to pass SSL/TLS config for database connection
2 parents b85f383 + 6ee7e43 commit 2972d3f

7 files changed

Lines changed: 276 additions & 14 deletions

File tree

‎build/psalm-baseline.xml‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4101,12 +4101,6 @@
41014101
<TypeDoesNotContainType>
41024102
<code><![CDATA[ctype_digit($this->dbPort)]]></code>
41034103
</TypeDoesNotContainType>
4104-
<UndefinedThisPropertyFetch>
4105-
<code><![CDATA[$this->dbprettyname]]></code>
4106-
<code><![CDATA[$this->dbprettyname]]></code>
4107-
<code><![CDATA[$this->dbprettyname]]></code>
4108-
<code><![CDATA[$this->dbprettyname]]></code>
4109-
</UndefinedThisPropertyFetch>
41104104
</file>
41114105
<file src="lib/private/Share20/DefaultShareProvider.php">
41124106
<FalsableReturnStatement>

‎lib/private/Setup/AbstractDatabase.php‎

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,14 @@
1919
use Psr\Log\LoggerInterface;
2020

2121
abstract class AbstractDatabase {
22+
/**
23+
* Installer options configuring an encrypted database connection.
24+
* @var string[]
25+
*/
26+
protected const array CONNECTION_ENCRYPTION_OPTIONS = ['dbdriveroptions'];
27+
28+
protected string $dbprettyname = 'abstract';
29+
2230
protected string $dbUser;
2331
protected string $dbPassword;
2432
protected string $dbName;
@@ -47,6 +55,13 @@ public function validate(array $config): array {
4755
if (substr_count($config['dbname'], '.') >= 1) {
4856
$errors[] = $this->trans->t('You cannot use dots in the database name %s', [$this->dbprettyname]);
4957
}
58+
foreach (static::CONNECTION_ENCRYPTION_OPTIONS as $option) {
59+
if (isset($config[$option]) && !is_array($config[$option])) {
60+
// Fail instead of ignoring the option, otherwise the instance would be
61+
// installed with an unencrypted connection without the admin noticing.
62+
$errors[] = $this->trans->t('The database option "%1$s" for %2$s has to be a list of values', [$option, $this->dbprettyname]);
63+
}
64+
}
5065
return $errors;
5166
}
5267

@@ -62,11 +77,27 @@ public function initialize(array $config): void {
6277
// accept `false` both as bool and string, since setting config values from env will result in a string
6378
$this->tryCreateDbUser = $createUserConfig !== false && $createUserConfig !== 'false';
6479

65-
$this->config->setValues([
80+
$configValues = [
6681
'dbname' => $dbName,
6782
'dbhost' => $dbHost,
6883
'dbtableprefix' => $dbTablePrefix,
69-
]);
84+
];
85+
86+
// An encrypted connection can only be configured through the system config, so the
87+
// options have to be persisted before the first connection is opened.
88+
foreach (static::CONNECTION_ENCRYPTION_OPTIONS as $option) {
89+
if (empty($config[$option])) {
90+
continue;
91+
}
92+
if (!is_array($config[$option])) {
93+
// Rejected by validate() already, but subclasses may not use that check
94+
$this->logger->error('Ignoring database option "{option}" passed to the installer because it is not a list of values', ['option' => $option]);
95+
continue;
96+
}
97+
$configValues[$option] = $config[$option];
98+
}
99+
100+
$this->config->setValues($configValues);
70101

71102
$this->dbUser = $dbUser;
72103
$this->dbPassword = $dbPass;

‎lib/private/Setup/OCI.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
use OC\DatabaseSetupException;
1212

1313
class OCI extends AbstractDatabase {
14-
public $dbprettyname = 'Oracle';
14+
public string $dbprettyname = 'Oracle';
1515

1616
protected $dbtablespace;
1717

‎lib/private/Setup/PostgreSQL.php‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,10 @@
1414
use OC\DB\QueryBuilder\Literal;
1515

1616
class PostgreSQL extends AbstractDatabase {
17-
public $dbprettyname = 'PostgreSQL';
17+
public string $dbprettyname = 'PostgreSQL';
18+
19+
// #[\Override] TODO: Uncomment this when we only support PHP 8.5+ support
20+
protected const array CONNECTION_ENCRYPTION_OPTIONS = [...parent::CONNECTION_ENCRYPTION_OPTIONS, 'pgsql_ssl'];
1821

1922
/**
2023
* @throws DatabaseSetupException

‎tests/lib/Setup/AbstractDatabaseTest.php‎

Lines changed: 108 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,16 @@
1818
use Test\TestCase;
1919

2020
class AbstractDatabaseTest extends TestCase {
21+
/**
22+
* Numeric literal instead of PDO::MYSQL_ATTR_SSL_CA: the constant is deprecated
23+
* since PHP 8.5 and only defined when the MySQL driver is available.
24+
*/
25+
private const MYSQL_ATTR_SSL_CA = 1008;
26+
2127
private SystemConfig&MockObject $config;
2228
private ConnectionFactory&MockObject $connectionFactory;
2329
private Connection&MockObject $connection;
30+
private LoggerInterface&MockObject $logger;
2431
private TestDatabase $database;
2532

2633
#[\Override]
@@ -30,11 +37,16 @@ protected function setUp(): void {
3037
$this->config = $this->createMock(SystemConfig::class);
3138
$this->connectionFactory = $this->createMock(ConnectionFactory::class);
3239
$this->connection = $this->createMock(Connection::class);
40+
$this->logger = $this->createMock(LoggerInterface::class);
41+
42+
$l10n = $this->createMock(IL10N::class);
43+
$l10n->method('t')
44+
->willReturnCallback(fn (string $text, array $parameters = []) => vsprintf($text, $parameters));
3345

3446
$this->database = new TestDatabase(
35-
$this->createMock(IL10N::class),
47+
$l10n,
3648
$this->config,
37-
$this->createMock(LoggerInterface::class),
49+
$this->logger,
3850
$this->createMock(ISecureRandom::class),
3951
);
4052
$this->database->connectionFactory = $this->connectionFactory;
@@ -75,6 +87,100 @@ public function testInitializeFallsBackToLocalhost(): void {
7587
]);
7688
}
7789

90+
/**
91+
* The connection encryption options are only read from the system config, so they have
92+
* to be persisted by initialize() - before any connection is opened by setupDatabase().
93+
*/
94+
public function testInitializePersistsDriverOptions(): void {
95+
$this->config->expects($this->once())
96+
->method('setValues')
97+
->with([
98+
'dbname' => 'nextcloud',
99+
'dbhost' => 'db.example.org',
100+
'dbtableprefix' => 'oc_',
101+
'dbdriveroptions' => [self::MYSQL_ATTR_SSL_CA => '/ca.pem'],
102+
]);
103+
104+
$this->database->initialize($this->options([
105+
'dbdriveroptions' => [self::MYSQL_ATTR_SSL_CA => '/ca.pem'],
106+
]));
107+
}
108+
109+
/**
110+
* Only the options of the database being set up may be persisted, every database
111+
* configures an encrypted connection differently.
112+
*/
113+
public function testInitializeSkipsOptionsOfOtherDatabases(): void {
114+
$this->config->expects($this->once())
115+
->method('setValues')
116+
->with([
117+
'dbname' => 'nextcloud',
118+
'dbhost' => 'db.example.org',
119+
'dbtableprefix' => 'oc_',
120+
]);
121+
122+
$this->database->initialize($this->options([
123+
'pgsql_ssl' => ['mode' => 'verify-full'],
124+
]));
125+
}
126+
127+
public static function emptyEncryptionOptions(): array {
128+
return [
129+
'not provided' => [[]],
130+
'empty array' => [['dbdriveroptions' => []]],
131+
'null' => [['dbdriveroptions' => null]],
132+
];
133+
}
134+
135+
#[\PHPUnit\Framework\Attributes\DataProvider('emptyEncryptionOptions')]
136+
public function testInitializeSkipsEmptyEncryptionOptions(array $additional): void {
137+
$this->config->expects($this->once())
138+
->method('setValues')
139+
->with([
140+
'dbname' => 'nextcloud',
141+
'dbhost' => 'db.example.org',
142+
'dbtableprefix' => 'oc_',
143+
]);
144+
145+
$this->database->initialize($this->options($additional));
146+
}
147+
148+
/**
149+
* A malformed option must never be persisted, as that would end up configuring an
150+
* unencrypted connection while the admin expects an encrypted one.
151+
*/
152+
public function testInitializeRejectsMalformedEncryptionOptions(): void {
153+
$this->config->expects($this->once())
154+
->method('setValues')
155+
->with([
156+
'dbname' => 'nextcloud',
157+
'dbhost' => 'db.example.org',
158+
'dbtableprefix' => 'oc_',
159+
]);
160+
$this->logger->expects($this->once())
161+
->method('error');
162+
163+
$this->database->initialize($this->options(['dbdriveroptions' => '/ca.pem']));
164+
}
165+
166+
public function testValidateRejectsMalformedEncryptionOptions(): void {
167+
$errors = $this->database->validate($this->options(['dbdriveroptions' => '/ca.pem']));
168+
169+
$this->assertEquals([
170+
'The database option "dbdriveroptions" for Test has to be a list of values',
171+
], $errors);
172+
}
173+
174+
public function testValidateAcceptsEncryptionOptions(): void {
175+
$errors = $this->database->validate($this->options([
176+
'dbdriveroptions' => [self::MYSQL_ATTR_SSL_CA => '/ca.pem'],
177+
// not an option of this database, so it is not validated either
178+
'pgsql_ssl' => 'verify-full',
179+
]));
180+
181+
$this->assertEquals([], $errors);
182+
}
183+
78184
/**
79185
* Host, database name and table prefix must not be passed as additional parameters:
80186
* they are resolved from the system config by the connection factory, so that setup

‎tests/lib/Setup/PostgreSQLTest.php‎

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
/**
5+
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
6+
* SPDX-License-Identifier: AGPL-3.0-or-later
7+
*/
8+
9+
namespace Test\Setup;
10+
11+
use OC\Setup\PostgreSQL;
12+
use OC\SystemConfig;
13+
use OCP\IL10N;
14+
use OCP\Security\ISecureRandom;
15+
use PHPUnit\Framework\MockObject\MockObject;
16+
use Psr\Log\LoggerInterface;
17+
use Test\TestCase;
18+
19+
class PostgreSQLTest extends TestCase {
20+
private const PGSQL_SSL = [
21+
'mode' => 'verify-full',
22+
'rootcert' => '/rootCA.crt',
23+
'cert' => '/client.crt',
24+
'key' => '/client.key',
25+
];
26+
27+
private SystemConfig&MockObject $config;
28+
private LoggerInterface&MockObject $logger;
29+
private PostgreSQL $database;
30+
31+
#[\Override]
32+
protected function setUp(): void {
33+
parent::setUp();
34+
35+
$this->config = $this->createMock(SystemConfig::class);
36+
$this->logger = $this->createMock(LoggerInterface::class);
37+
38+
$l10n = $this->createMock(IL10N::class);
39+
$l10n->method('t')
40+
->willReturnCallback(fn (string $text, array $parameters = []) => vsprintf($text, $parameters));
41+
42+
$this->database = new PostgreSQL(
43+
$l10n,
44+
$this->config,
45+
$this->logger,
46+
$this->createMock(ISecureRandom::class),
47+
);
48+
}
49+
50+
/**
51+
* PostgreSQL is configured through its own set of connection parameters instead of PDO
52+
* driver options. They are only read from the system config, so they have to be
53+
* persisted by initialize() - before any connection is opened by setupDatabase().
54+
*/
55+
public function testInitializePersistsPgsqlSsl(): void {
56+
$this->config->expects($this->once())
57+
->method('setValues')
58+
->with([
59+
'dbname' => 'nextcloud',
60+
'dbhost' => 'db.example.org',
61+
'dbtableprefix' => 'oc_',
62+
'pgsql_ssl' => self::PGSQL_SSL,
63+
]);
64+
65+
$this->database->initialize($this->options(['pgsql_ssl' => self::PGSQL_SSL]));
66+
}
67+
68+
public static function emptyPgsqlSsl(): array {
69+
return [
70+
'not provided' => [[]],
71+
'empty array' => [['pgsql_ssl' => []]],
72+
'null' => [['pgsql_ssl' => null]],
73+
];
74+
}
75+
76+
#[\PHPUnit\Framework\Attributes\DataProvider('emptyPgsqlSsl')]
77+
public function testInitializeSkipsEmptyPgsqlSsl(array $additional): void {
78+
$this->config->expects($this->once())
79+
->method('setValues')
80+
->with([
81+
'dbname' => 'nextcloud',
82+
'dbhost' => 'db.example.org',
83+
'dbtableprefix' => 'oc_',
84+
]);
85+
86+
$this->database->initialize($this->options($additional));
87+
}
88+
89+
/**
90+
* A malformed option must never be persisted, as that would end up configuring an
91+
* unencrypted connection while the admin expects an encrypted one.
92+
*/
93+
public function testInitializeRejectsMalformedPgsqlSsl(): void {
94+
$this->config->expects($this->once())
95+
->method('setValues')
96+
->with([
97+
'dbname' => 'nextcloud',
98+
'dbhost' => 'db.example.org',
99+
'dbtableprefix' => 'oc_',
100+
]);
101+
$this->logger->expects($this->once())
102+
->method('error');
103+
104+
$this->database->initialize($this->options(['pgsql_ssl' => 'verify-full']));
105+
}
106+
107+
public function testValidateRejectsMalformedPgsqlSsl(): void {
108+
$errors = $this->database->validate($this->options(['pgsql_ssl' => 'verify-full']));
109+
110+
$this->assertEquals([
111+
'The database option "pgsql_ssl" for PostgreSQL has to be a list of values',
112+
], $errors);
113+
}
114+
115+
public function testValidateAcceptsPgsqlSsl(): void {
116+
$errors = $this->database->validate($this->options(['pgsql_ssl' => self::PGSQL_SSL]));
117+
118+
$this->assertEquals([], $errors);
119+
}
120+
121+
private function options(array $additional = []): array {
122+
return array_merge([
123+
'dbuser' => 'admin',
124+
'dbpass' => 'admin-password',
125+
'dbname' => 'nextcloud',
126+
'dbhost' => 'db.example.org',
127+
], $additional);
128+
}
129+
}

‎tests/lib/Setup/TestDatabase.php‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,11 @@
11
<?php
22

3+
declare(strict_types=1);
34
/**
45
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
56
* SPDX-License-Identifier: AGPL-3.0-or-later
67
*/
78

8-
declare(strict_types=1);
9-
109
namespace Test\Setup;
1110

1211
use OC\DB\Connection;

0 commit comments

Comments
 (0)