Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 31 additions & 2 deletions lib/private/Setup/AbstractDatabase.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,12 @@
use Psr\Log\LoggerInterface;

abstract class AbstractDatabase {
/**
* Installer options configuring an encrypted database connection.
* @var string[]
*/
protected const array CONNECTION_ENCRYPTION_OPTIONS = ['dbdriveroptions'];

protected string $dbUser;
protected string $dbPassword;
protected string $dbName;
Expand Down Expand Up @@ -47,6 +53,13 @@
if (substr_count($config['dbname'], '.') >= 1) {
$errors[] = $this->trans->t('You cannot use dots in the database name %s', [$this->dbprettyname]);
}
foreach (static::CONNECTION_ENCRYPTION_OPTIONS as $option) {
if (isset($config[$option]) && !is_array($config[$option])) {
// Fail instead of ignoring the option, otherwise the instance would be
// installed with an unencrypted connection without the admin noticing.
$errors[] = $this->trans->t('The database option "%1$s" for %2$s has to be a list of values', [$option, $this->dbprettyname]);

Check failure on line 60 in lib/private/Setup/AbstractDatabase.php

View workflow job for this annotation

GitHub Actions / static-code-analysis

UndefinedThisPropertyFetch

lib/private/Setup/AbstractDatabase.php:60:109: UndefinedThisPropertyFetch: Instance property OC\Setup\AbstractDatabase::$dbprettyname is not defined (see https://psalm.dev/041)
}
}
return $errors;
}

Expand All @@ -62,11 +75,27 @@
// accept `false` both as bool and string, since setting config values from env will result in a string
$this->tryCreateDbUser = $createUserConfig !== false && $createUserConfig !== 'false';

$this->config->setValues([
$configValues = [
'dbname' => $dbName,
'dbhost' => $dbHost,
'dbtableprefix' => $dbTablePrefix,
]);
];

// An encrypted connection can only be configured through the system config, so the
// options have to be persisted before the first connection is opened.
foreach (static::CONNECTION_ENCRYPTION_OPTIONS as $option) {
if (empty($config[$option])) {
continue;
}
if (!is_array($config[$option])) {
// Rejected by validate() already, but subclasses may not use that check
$this->logger->error('Ignoring database option "{option}" passed to the installer because it is not a list of values', ['option' => $option]);
continue;
}
$configValues[$option] = $config[$option];
}

$this->config->setValues($configValues);

$this->dbUser = $dbUser;
$this->dbPassword = $dbPass;
Expand Down
3 changes: 3 additions & 0 deletions lib/private/Setup/PostgreSQL.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@
class PostgreSQL extends AbstractDatabase {
public $dbprettyname = 'PostgreSQL';

// #[\Override] TODO: Uncomment this when we only support PHP 8.5+ support
protected const array CONNECTION_ENCRYPTION_OPTIONS = [...parent::CONNECTION_ENCRYPTION_OPTIONS, 'pgsql_ssl'];

/**
* @throws DatabaseSetupException
*/
Expand Down
110 changes: 108 additions & 2 deletions tests/lib/Setup/AbstractDatabaseTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,16 @@
use Test\TestCase;

class AbstractDatabaseTest extends TestCase {
/**
* Numeric literal instead of PDO::MYSQL_ATTR_SSL_CA: the constant is deprecated
* since PHP 8.5 and only defined when the MySQL driver is available.
*/
private const MYSQL_ATTR_SSL_CA = 1008;

private SystemConfig&MockObject $config;
private ConnectionFactory&MockObject $connectionFactory;
private Connection&MockObject $connection;
private LoggerInterface&MockObject $logger;
private TestDatabase $database;

#[\Override]
Expand All @@ -30,11 +37,16 @@ protected function setUp(): void {
$this->config = $this->createMock(SystemConfig::class);
$this->connectionFactory = $this->createMock(ConnectionFactory::class);
$this->connection = $this->createMock(Connection::class);
$this->logger = $this->createMock(LoggerInterface::class);

$l10n = $this->createMock(IL10N::class);
$l10n->method('t')
->willReturnCallback(fn (string $text, array $parameters = []) => vsprintf($text, $parameters));

$this->database = new TestDatabase(
$this->createMock(IL10N::class),
$l10n,
$this->config,
$this->createMock(LoggerInterface::class),
$this->logger,
$this->createMock(ISecureRandom::class),
);
$this->database->connectionFactory = $this->connectionFactory;
Expand Down Expand Up @@ -75,6 +87,100 @@ public function testInitializeFallsBackToLocalhost(): void {
]);
}

/**
* The connection encryption options are only read from the system config, so they have
* to be persisted by initialize() - before any connection is opened by setupDatabase().
*/
public function testInitializePersistsDriverOptions(): void {
$this->config->expects($this->once())
->method('setValues')
->with([
'dbname' => 'nextcloud',
'dbhost' => 'db.example.org',
'dbtableprefix' => 'oc_',
'dbdriveroptions' => [self::MYSQL_ATTR_SSL_CA => '/ca.pem'],
]);

$this->database->initialize($this->options([
'dbdriveroptions' => [self::MYSQL_ATTR_SSL_CA => '/ca.pem'],
]));
}

/**
* Only the options of the database being set up may be persisted, every database
* configures an encrypted connection differently.
*/
public function testInitializeSkipsOptionsOfOtherDatabases(): void {
$this->config->expects($this->once())
->method('setValues')
->with([
'dbname' => 'nextcloud',
'dbhost' => 'db.example.org',
'dbtableprefix' => 'oc_',
]);

$this->database->initialize($this->options([
'pgsql_ssl' => ['mode' => 'verify-full'],
]));
}

public static function emptyEncryptionOptions(): array {
return [
'not provided' => [[]],
'empty array' => [['dbdriveroptions' => []]],
'null' => [['dbdriveroptions' => null]],
];
}

#[\PHPUnit\Framework\Attributes\DataProvider('emptyEncryptionOptions')]
public function testInitializeSkipsEmptyEncryptionOptions(array $additional): void {
$this->config->expects($this->once())
->method('setValues')
->with([
'dbname' => 'nextcloud',
'dbhost' => 'db.example.org',
'dbtableprefix' => 'oc_',
]);

$this->database->initialize($this->options($additional));
}

/**
* A malformed option must never be persisted, as that would end up configuring an
* unencrypted connection while the admin expects an encrypted one.
*/
public function testInitializeRejectsMalformedEncryptionOptions(): void {
$this->config->expects($this->once())
->method('setValues')
->with([
'dbname' => 'nextcloud',
'dbhost' => 'db.example.org',
'dbtableprefix' => 'oc_',
]);
$this->logger->expects($this->once())
->method('error');

$this->database->initialize($this->options(['dbdriveroptions' => '/ca.pem']));
}

public function testValidateRejectsMalformedEncryptionOptions(): void {
$errors = $this->database->validate($this->options(['dbdriveroptions' => '/ca.pem']));

$this->assertEquals([
'The database option "dbdriveroptions" for Test has to be a list of values',
], $errors);
}

public function testValidateAcceptsEncryptionOptions(): void {
$errors = $this->database->validate($this->options([
'dbdriveroptions' => [self::MYSQL_ATTR_SSL_CA => '/ca.pem'],
// not an option of this database, so it is not validated either
'pgsql_ssl' => 'verify-full',
]));

$this->assertEquals([], $errors);
}

/**
* Host, database name and table prefix must not be passed as additional parameters:
* they are resolved from the system config by the connection factory, so that setup
Expand Down
129 changes: 129 additions & 0 deletions tests/lib/Setup/PostgreSQLTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
<?php

declare(strict_types=1);
/**
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

namespace Test\Setup;

use OC\Setup\PostgreSQL;
use OC\SystemConfig;
use OCP\IL10N;
use OCP\Security\ISecureRandom;
use PHPUnit\Framework\MockObject\MockObject;
use Psr\Log\LoggerInterface;
use Test\TestCase;

class PostgreSQLTest extends TestCase {
private const PGSQL_SSL = [
'mode' => 'verify-full',
'rootcert' => '/rootCA.crt',
'cert' => '/client.crt',
'key' => '/client.key',
];

private SystemConfig&MockObject $config;
private LoggerInterface&MockObject $logger;
private PostgreSQL $database;

#[\Override]
protected function setUp(): void {
parent::setUp();

$this->config = $this->createMock(SystemConfig::class);
$this->logger = $this->createMock(LoggerInterface::class);

$l10n = $this->createMock(IL10N::class);
$l10n->method('t')
->willReturnCallback(fn (string $text, array $parameters = []) => vsprintf($text, $parameters));

$this->database = new PostgreSQL(
$l10n,
$this->config,
$this->logger,
$this->createMock(ISecureRandom::class),
);
}

/**
* PostgreSQL is configured through its own set of connection parameters instead of PDO
* driver options. They are only read from the system config, so they have to be
* persisted by initialize() - before any connection is opened by setupDatabase().
*/
public function testInitializePersistsPgsqlSsl(): void {
$this->config->expects($this->once())
->method('setValues')
->with([
'dbname' => 'nextcloud',
'dbhost' => 'db.example.org',
'dbtableprefix' => 'oc_',
'pgsql_ssl' => self::PGSQL_SSL,
]);

$this->database->initialize($this->options(['pgsql_ssl' => self::PGSQL_SSL]));
}

public static function emptyPgsqlSsl(): array {
return [
'not provided' => [[]],
'empty array' => [['pgsql_ssl' => []]],
'null' => [['pgsql_ssl' => null]],
];
}

#[\PHPUnit\Framework\Attributes\DataProvider('emptyPgsqlSsl')]
public function testInitializeSkipsEmptyPgsqlSsl(array $additional): void {
$this->config->expects($this->once())
->method('setValues')
->with([
'dbname' => 'nextcloud',
'dbhost' => 'db.example.org',
'dbtableprefix' => 'oc_',
]);

$this->database->initialize($this->options($additional));
}

/**
* A malformed option must never be persisted, as that would end up configuring an
* unencrypted connection while the admin expects an encrypted one.
*/
public function testInitializeRejectsMalformedPgsqlSsl(): void {
$this->config->expects($this->once())
->method('setValues')
->with([
'dbname' => 'nextcloud',
'dbhost' => 'db.example.org',
'dbtableprefix' => 'oc_',
]);
$this->logger->expects($this->once())
->method('error');

$this->database->initialize($this->options(['pgsql_ssl' => 'verify-full']));
}

public function testValidateRejectsMalformedPgsqlSsl(): void {
$errors = $this->database->validate($this->options(['pgsql_ssl' => 'verify-full']));

$this->assertEquals([
'The database option "pgsql_ssl" for PostgreSQL has to be a list of values',
], $errors);
}

public function testValidateAcceptsPgsqlSsl(): void {
$errors = $this->database->validate($this->options(['pgsql_ssl' => self::PGSQL_SSL]));

$this->assertEquals([], $errors);
}

private function options(array $additional = []): array {
return array_merge([
'dbuser' => 'admin',
'dbpass' => 'admin-password',
'dbname' => 'nextcloud',
'dbhost' => 'db.example.org',
], $additional);
}
}
3 changes: 1 addition & 2 deletions tests/lib/Setup/TestDatabase.php
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
<?php

declare(strict_types=1);
/**
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

declare(strict_types=1);

namespace Test\Setup;

use OC\DB\Connection;
Expand Down
Loading