Skip to content

fix(encryption): Refactor EncryptionWrapper, fix zero-byte size, add S3 tests #57279

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
cuppett wants to merge 5 commits into
base: master
Choose a base branch
from
Open

fix(encryption): Refactor EncryptionWrapper, fix zero-byte size, add S3 tests #57279

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7eae917
fix(encryption): Refactor EncryptionWrapper with HomeMountPoint support
cuppett Apr 29, 2026
14f16f1
fix(encryption): Correctly report size for zero-byte encrypted files
cuppett Apr 29, 2026
eef0a39
test(encryption): Add S3 encryption test suite for primary object sto…
cuppett Apr 29, 2026
e34918a
fix(tests): Fix encryption test isolation between test runs
cuppett Apr 29, 2026
d99a8c6
refactor(encryption): Migrate appconfig keys to typed bool IAppConfig…
cuppett Apr 29, 2026
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
1 change: 1 addition & 0 deletions apps/encryption/lib/Settings/Admin.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ public function getForm() {
$crypt,
$this->userSession,
$this->config,
$this->appConfig,
$this->userManager);

// Check if an adminRecovery account is enabled for recovering files after lost pwd
Expand Down
17 changes: 4 additions & 13 deletions apps/encryption/lib/Util.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use OC\Files\View;
use OCA\Encryption\Crypto\Crypt;
use OCP\Files\Storage\IStorage;
use OCP\IAppConfig;
use OCP\IConfig;
use OCP\IUser;
use OCP\IUserManager;
Expand All @@ -25,6 +26,7 @@ public function __construct(
private Crypt $crypt,
IUserSession $userSession,
private IConfig $config,
private IAppConfig $appConfig,
private IUserManager $userManager,
) {
$this->user = $userSession->isLoggedIn() ? $userSession->getUser() : false;
Expand All @@ -51,13 +53,7 @@ public function isRecoveryEnabledForUser($uid) {
* @return bool
*/
public function shouldEncryptHomeStorage() {
$encryptHomeStorage = $this->config->getAppValue(
'encryption',
'encryptHomeStorage',
'1'
);

return ($encryptHomeStorage === '1');
return $this->appConfig->getValueBool('encryption', 'encryptHomeStorage', true);
}

/**
Expand All @@ -66,12 +62,7 @@ public function shouldEncryptHomeStorage() {
* @param bool $encryptHomeStorage
*/
public function setEncryptHomeStorage($encryptHomeStorage) {
$value = $encryptHomeStorage ? '1' : '0';
$this->config->setAppValue(
'encryption',
'encryptHomeStorage',
$value
);
$this->appConfig->setValueBool('encryption', 'encryptHomeStorage', (bool)$encryptHomeStorage);
}

/**
Expand Down
6 changes: 2 additions & 4 deletions apps/encryption/tests/Settings/AdminTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,17 +62,15 @@ public function testGetForm(): void {
$this->appConfig
->method('getValueBool')
->willReturnMap([
['encryption', 'recoveryAdminEnabled', true]
['encryption', 'recoveryAdminEnabled', true],
Comment thread
cuppett marked this conversation as resolved.
['encryption', 'encryptHomeStorage', true, true],
]);
$this->config
->method('getAppValue')
->willReturnCallback(function ($app, $key, $default) {
if ($app === 'encryption' && $key === 'recoveryAdminEnabled' && $default === '0') {
return '1';
}
if ($app === 'encryption' && $key === 'encryptHomeStorage' && $default === '1') {
return '1';
}
return $default;
});

Expand Down
29 changes: 16 additions & 13 deletions apps/encryption/tests/UtilTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
use OCA\Encryption\Util;
use OCP\Files\Mount\IMountPoint;
use OCP\Files\Storage\IStorage;
use OCP\IAppConfig;
use OCP\IConfig;
use OCP\IUser;
use OCP\IUserManager;
Expand All @@ -26,6 +27,7 @@ class UtilTest extends TestCase {
protected Util $instance;
protected static $tempStorage = [];

protected IAppConfig&MockObject $appConfigMock;
protected IConfig&MockObject $configMock;
protected View&MockObject $filesMock;
protected IUserManager&MockObject $userManagerMock;
Expand Down Expand Up @@ -78,6 +80,7 @@ protected function setUp(): void {
->willReturn(true);

$this->configMock = $this->createMock(IConfig::class);
$this->appConfigMock = $this->createMock(IAppConfig::class);

$this->configMock->expects($this->any())
->method('getUserValue')
Expand All @@ -87,7 +90,7 @@ protected function setUp(): void {
->method('setUserValue')
->willReturnCallback([$this, 'setValueTester']);

$this->instance = new Util($this->filesMock, $cryptMock, $userSessionMock, $this->configMock, $this->userManagerMock);
$this->instance = new Util($this->filesMock, $cryptMock, $userSessionMock, $this->configMock, $this->appConfigMock, $this->userManagerMock);
}

/**
Expand Down Expand Up @@ -136,13 +139,13 @@ public static function dataTestIsMasterKeyEnabled(): array {
}

/**
* @param string $returnValue return value from getAppValue()
* @param bool $returnValue return value from getValueBool()
* @param bool $expected
*/
#[\PHPUnit\Framework\Attributes\DataProvider(methodName: 'dataTestShouldEncryptHomeStorage')]
public function testShouldEncryptHomeStorage($returnValue, $expected): void {
$this->configMock->expects($this->once())->method('getAppValue')
->with('encryption', 'encryptHomeStorage', '1')
public function testShouldEncryptHomeStorage(bool $returnValue, bool $expected): void {
$this->appConfigMock->expects($this->once())->method('getValueBool')
->with('encryption', 'encryptHomeStorage', true)
->willReturn($returnValue);

$this->assertSame($expected,
Expand All @@ -151,26 +154,26 @@ public function testShouldEncryptHomeStorage($returnValue, $expected): void {

public static function dataTestShouldEncryptHomeStorage(): array {
return [
['1', true],
['0', false]
[true, true],
[false, false]
];
}

/**
* @param $value
* @param $expected
* @param bool $value
* @param bool $expected
*/
#[\PHPUnit\Framework\Attributes\DataProvider(methodName: 'dataTestSetEncryptHomeStorage')]
public function testSetEncryptHomeStorage($value, $expected): void {
$this->configMock->expects($this->once())->method('setAppValue')
public function testSetEncryptHomeStorage(bool $value, bool $expected): void {
$this->appConfigMock->expects($this->once())->method('setValueBool')
->with('encryption', 'encryptHomeStorage', $expected);
$this->instance->setEncryptHomeStorage($value);
}

public static function dataTestSetEncryptHomeStorage(): array {
return [
[true, '1'],
[false, '0']
[true, true],
[false, false]
];
}

Expand Down
3 changes: 2 additions & 1 deletion apps/provisioning_api/lib/Controller/AppConfigController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,8 @@ protected function verifyConfigKey(string $app, string $key, string $value) {
throw new \InvalidArgumentException('The given key can not be set');
}

if ($app === 'core' && $key === 'encryption_enabled' && $value !== 'yes') {
if ($app === 'core' && $key === 'encryption_enabled'
&& !in_array(strtolower(trim($value)), ['yes', '1', 'true', 'on'], true)) {
throw new \InvalidArgumentException('The given key can not be set');
}

Expand Down
6 changes: 6 additions & 0 deletions apps/provisioning_api/tests/Controller/AppConfigControllerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,10 @@ public static function dataVerifyConfigKey(): array {
['dav', 'public_route', ''],
['files', 'remote_route', ''],
['core', 'encryption_enabled', 'yes'],
['core', 'encryption_enabled', '1'],
['core', 'encryption_enabled', 'true'],
['core', 'encryption_enabled', 'YES'],
['core', 'encryption_enabled', 'on'],
];
}

Expand All @@ -384,6 +388,8 @@ public static function dataVerifyConfigKeyThrows(): array {
['contacts', 'types', ''],
['core', 'encryption_enabled', 'no'],
['core', 'encryption_enabled', ''],
['core', 'encryption_enabled', '0'],
['core', 'encryption_enabled', 'false'],
['core', 'public_files', ''],
['core', 'public_dav', ''],
['core', 'remote_files', ''],
Expand Down
15 changes: 0 additions & 15 deletions build/psalm-baseline.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1300,10 +1300,8 @@
</file>
<file src="apps/encryption/lib/Util.php">
<DeprecatedMethod>
<code><![CDATA[getAppValue]]></code>
<code><![CDATA[getAppValue]]></code>
<code><![CDATA[getUserValue]]></code>
<code><![CDATA[setAppValue]]></code>
<code><![CDATA[setUserValue]]></code>
</DeprecatedMethod>
<InternalMethod>
Expand Down Expand Up @@ -3023,19 +3021,6 @@
<code><![CDATA[\OC_Util::tearDownFS()]]></code>
</DeprecatedMethod>
</file>
<file src="core/Command/Encryption/Disable.php">
<DeprecatedMethod>
<code><![CDATA[getAppValue]]></code>
<code><![CDATA[setAppValue]]></code>
</DeprecatedMethod>
</file>
<file src="core/Command/Encryption/Enable.php">
<DeprecatedMethod>
<code><![CDATA[getAppValue]]></code>
<code><![CDATA[getAppValue]]></code>
<code><![CDATA[setAppValue]]></code>
</DeprecatedMethod>
</file>
<file src="core/Command/Encryption/MigrateKeyStorage.php">
<DeprecatedClass>
<code><![CDATA[\OC_Util::setupFS($uid)]]></code>
Expand Down
26 changes: 20 additions & 6 deletions core/Command/Encryption/DecryptAll.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
namespace OC\Core\Command\Encryption;

use OCP\App\IAppManager;
use OCP\Exceptions\AppConfigTypeConflictException;
use OCP\IAppConfig;
use OCP\IConfig;
use Symfony\Component\Console\Command\Command;
Expand Down Expand Up @@ -91,11 +92,16 @@ protected function execute(InputInterface $input, OutputInterface $output): int
return 1;
}

$originallyEnabled = $this->appConfig->getValueBool('core', 'encryption_enabled');
try {
$originallyEnabled = $this->appConfig->getValueBool('core', 'encryption_enabled', false);
} catch (AppConfigTypeConflictException) {
$raw = $this->appConfig->getValueString('core', 'encryption_enabled', 'no');
$originallyEnabled = in_array(strtolower(trim($raw)), ['1', 'true', 'yes', 'on'], true);
}
try {
if ($originallyEnabled) {
$output->write('Disable server side encryption... ');
$this->appConfig->setValueBool('core', 'encryption_enabled', false);
$this->writeEncryptionEnabled(false);
$output->writeln('done.');
} else {
$output->writeln('Server side encryption not enabled. Nothing to do.');
Expand Down Expand Up @@ -123,29 +129,37 @@ protected function execute(InputInterface $input, OutputInterface $output): int
$output->writeln(' aborted.');
if ($originallyEnabled) {
$output->writeln('Server side encryption remains enabled');
$this->appConfig->setValueBool('core', 'encryption_enabled', true);
$this->writeEncryptionEnabled(true);
}
} elseif (($uid !== '') && $originallyEnabled) {
$output->writeln('Server side encryption remains enabled');
$this->appConfig->setValueBool('core', 'encryption_enabled', true);
$this->writeEncryptionEnabled(true);
}
$this->resetMaintenanceAndTrashbin();
return 0;
}
if ($originallyEnabled) {
$output->write('Enable server side encryption... ');
$this->appConfig->setValueBool('core', 'encryption_enabled', true);
$this->writeEncryptionEnabled(true);
$output->writeln('done.');
}
$output->writeln('aborted');
return 1;
} catch (\Exception $e) {
// enable server side encryption again if something went wrong
if ($originallyEnabled) {
$this->appConfig->setValueBool('core', 'encryption_enabled', true);
$this->writeEncryptionEnabled(true);
}
$this->resetMaintenanceAndTrashbin();
throw $e;
}
}

private function writeEncryptionEnabled(bool $enabled): void {
try {
$this->appConfig->setValueBool('core', 'encryption_enabled', $enabled);
} catch (AppConfigTypeConflictException) {
$this->appConfig->setValueString('core', 'encryption_enabled', $enabled ? 'yes' : 'no');
}
}
}
20 changes: 16 additions & 4 deletions core/Command/Encryption/Disable.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,15 @@
*/
namespace OC\Core\Command\Encryption;

use OCP\IConfig;
use OCP\Exceptions\AppConfigTypeConflictException;
use OCP\IAppConfig;
use Symfony\Component\Console\Command\Command;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Output\OutputInterface;

class Disable extends Command {
public function __construct(
protected IConfig $config,
protected IAppConfig $appConfig,
) {
Comment thread
cuppett marked this conversation as resolved.
parent::__construct();
}
Expand All @@ -31,10 +32,21 @@ protected function configure() {

#[\Override]
protected function execute(InputInterface $input, OutputInterface $output): int {
if ($this->config->getAppValue('core', 'encryption_enabled', 'no') !== 'yes') {
try {
$isEnabled = $this->appConfig->getValueBool('core', 'encryption_enabled', false);
} catch (AppConfigTypeConflictException) {
$raw = $this->appConfig->getValueString('core', 'encryption_enabled', 'no');
$isEnabled = in_array(strtolower(trim($raw)), ['1', 'true', 'yes', 'on'], true);
}

if (!$isEnabled) {
$output->writeln('Encryption is already disabled');
} else {
$this->config->setAppValue('core', 'encryption_enabled', 'no');
try {
$this->appConfig->setValueBool('core', 'encryption_enabled', false);
} catch (AppConfigTypeConflictException) {
$this->appConfig->setValueString('core', 'encryption_enabled', 'no');
}
$output->writeln('<info>Encryption disabled</info>');
}
return 0;
Expand Down
22 changes: 17 additions & 5 deletions core/Command/Encryption/Enable.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,15 @@
namespace OC\Core\Command\Encryption;

use OCP\Encryption\IManager;
use OCP\IConfig;
use OCP\Exceptions\AppConfigTypeConflictException;
use OCP\IAppConfig;
use Symfony\Component\Console\Command\Command;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Output\OutputInterface;

class Enable extends Command {
public function __construct(
protected IConfig $config,
protected IAppConfig $appConfig,
protected IManager $encryptionManager,
) {
Comment thread
cuppett marked this conversation as resolved.
parent::__construct();
Expand All @@ -33,10 +34,21 @@ protected function configure() {

#[\Override]
protected function execute(InputInterface $input, OutputInterface $output): int {
if ($this->config->getAppValue('core', 'encryption_enabled', 'no') === 'yes') {
try {
$isEnabled = $this->appConfig->getValueBool('core', 'encryption_enabled', false);
} catch (AppConfigTypeConflictException) {
$raw = $this->appConfig->getValueString('core', 'encryption_enabled', 'no');
$isEnabled = in_array(strtolower(trim($raw)), ['1', 'true', 'yes', 'on'], true);
}

if ($isEnabled) {
$output->writeln('Encryption is already enabled');
} else {
$this->config->setAppValue('core', 'encryption_enabled', 'yes');
try {
$this->appConfig->setValueBool('core', 'encryption_enabled', true);
} catch (AppConfigTypeConflictException) {
$this->appConfig->setValueString('core', 'encryption_enabled', 'yes');
}
$output->writeln('<info>Encryption enabled</info>');
}
$output->writeln('');
Expand All @@ -46,7 +58,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
$output->writeln('<error>No encryption module is loaded</error>');
return 1;
}
$defaultModule = $this->config->getAppValue('core', 'default_encryption_module');
$defaultModule = $this->appConfig->getValueString('core', 'default_encryption_module', '');
if ($defaultModule === '') {
$output->writeln('<error>No default module is set</error>');
return 1;
Expand Down
1 change: 1 addition & 0 deletions lib/composer/composer/autoload_classmap.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2092,6 +2092,7 @@
'OC\\Repair\\RepairInvalidShares' => $baseDir . '/lib/private/Repair/RepairInvalidShares.php',
'OC\\Repair\\RepairLogoDimension' => $baseDir . '/lib/private/Repair/RepairLogoDimension.php',
'OC\\Repair\\RepairMimeTypes' => $baseDir . '/lib/private/Repair/RepairMimeTypes.php',
'OC\\Repair\\RetypeEncryptionConfigKeys' => $baseDir . '/lib/private/Repair/RetypeEncryptionConfigKeys.php',
'OC\\RichObjectStrings\\RichTextFormatter' => $baseDir . '/lib/private/RichObjectStrings/RichTextFormatter.php',
'OC\\RichObjectStrings\\Validator' => $baseDir . '/lib/private/RichObjectStrings/Validator.php',
'OC\\Route\\CachingRouter' => $baseDir . '/lib/private/Route/CachingRouter.php',
Expand Down
1 change: 1 addition & 0 deletions lib/composer/composer/autoload_static.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2133,6 +2133,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
'OC\\Repair\\RepairInvalidShares' => __DIR__ . '/../../..' . '/lib/private/Repair/RepairInvalidShares.php',
'OC\\Repair\\RepairLogoDimension' => __DIR__ . '/../../..' . '/lib/private/Repair/RepairLogoDimension.php',
'OC\\Repair\\RepairMimeTypes' => __DIR__ . '/../../..' . '/lib/private/Repair/RepairMimeTypes.php',
'OC\\Repair\\RetypeEncryptionConfigKeys' => __DIR__ . '/../../..' . '/lib/private/Repair/RetypeEncryptionConfigKeys.php',
'OC\\RichObjectStrings\\RichTextFormatter' => __DIR__ . '/../../..' . '/lib/private/RichObjectStrings/RichTextFormatter.php',
'OC\\RichObjectStrings\\Validator' => __DIR__ . '/../../..' . '/lib/private/RichObjectStrings/Validator.php',
'OC\\Route\\CachingRouter' => __DIR__ . '/../../..' . '/lib/private/Route/CachingRouter.php',
Expand Down
Loading
Loading