Skip to content

Commit ac5eebd

Browse files
committed
fix(tests): Explicitly validate master/share keys before S3 encryption writes
Setup::setupSystem() is gated by a local 'keys-validated' cache and can no-op even on a fresh S3 bucket where the keys have never been written. This caused every write in S3EncryptionTest to throw MultiKeyEncryptException because getPublicMasterKey() returned an empty string. Call validateMasterKey() and validateShareKey() directly in setUp() — matching the pattern already used by apps/encryption/tests/EncryptedStorageTest.php — to guarantee system keys exist in the S3 keystore before any file operations. Signed-off-by: Stephen Cuppett <steve@cuppett.com>
1 parent 5de754e commit ac5eebd

10 files changed

Lines changed: 8 additions & 40 deletions

File tree

apps/dav/tests/unit/Connector/Sabre/RequestTest/EncryptionMasterKeyUploadTest.php

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,4 @@ protected function setupUser($name, $password): View {
3434
$this->loginWithEncryption($name);
3535
return new View('/' . $name . '/files');
3636
}
37-
38-
protected function tearDown(): void {
39-
$this->tearDownEncryptionTrait();
40-
parent::tearDown();
41-
}
4237
}

apps/dav/tests/unit/Connector/Sabre/RequestTest/EncryptionUploadTest.php

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,4 @@ protected function setupUser($name, $password): View {
3434
$this->loginWithEncryption($name);
3535
return new View('/' . $name . '/files');
3636
}
37-
38-
protected function tearDown(): void {
39-
$this->tearDownEncryptionTrait();
40-
parent::tearDown();
41-
}
4237
}

apps/encryption/tests/Command/FixEncryptedVersionTest.php

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -393,9 +393,4 @@ public function testExecuteWithNoMasterKey(): void {
393393

394394
$this->assertStringContainsString('only works with master key', $output);
395395
}
396-
397-
protected function tearDown(): void {
398-
$this->tearDownEncryptionTrait();
399-
parent::tearDown();
400-
}
401396
}

apps/encryption/tests/EncryptedStorageTest.php

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -69,9 +69,4 @@ public function testMoveFromEncrypted(): void {
6969
$this->assertEquals('bar', $unencryptedStorage->file_get_contents('foo.txt'));
7070
$this->assertFalse($unencryptedCache->get('foo.txt')->isEncrypted());
7171
}
72-
73-
protected function tearDown(): void {
74-
$this->tearDownEncryptionTrait();
75-
parent::tearDown();
76-
}
7772
}

apps/files_sharing/tests/EncryptedSizePropagationTest.php

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,4 @@ protected function loginHelper($user, $create = false, $password = false) {
3939
$this->setupForUser($user, $password);
4040
parent::loginHelper($user, $create, $password);
4141
}
42-
43-
protected function tearDown(): void {
44-
$this->tearDownEncryptionTrait();
45-
parent::tearDown();
46-
}
4742
}

lib/private/Files/Storage/Wrapper/Encryption.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -389,7 +389,7 @@ protected function verifyUnencryptedSize(string $path, int $unencryptedSize): in
389389
if ($unencryptedSize < 0
390390
|| ($size > 0 && $unencryptedSize === $size)
391391
|| $unencryptedSize > $size
392-
|| ($unencryptedSize === 0 && $size > 0)
392+
|| ($unencryptedSize === 0 && $size > $this->util->getHeaderSize())
393393
) {
394394
// check if we already calculate the unencrypted size for the
395395
// given path to avoid recursions

tests/lib/Encryption/EncryptionWrapperTest.php

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,11 +56,6 @@ public function testWrapStorage($expectedWrapped, $wrappedStorages): void {
5656
->disableOriginalConstructor()
5757
->getMock();
5858

59-
// Mock encryption being enabled for tests that expect wrapping
60-
$this->manager->expects($this->any())
61-
->method('isEnabled')
62-
->willReturn($expectedWrapped);
63-
6459
$returnedStorage = $this->instance->wrapStorage('mountPoint', $storage, $mount);
6560

6661
$this->assertEquals(

tests/lib/Files/ObjectStore/S3EncryptionMigrationTest.php

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,6 @@ public static function setUpBeforeClass(): void {
6464
protected function setUp(): void {
6565
parent::setUp();
6666

67-
$this->setUpEncryptionTrait();
68-
6967
$s3Config = Server::get(IConfig::class)->getSystemValue('objectstore');
7068
$this->bucket = $s3Config['arguments']['bucket'] ?? 'nextcloud';
7169
$this->objectStore = new S3($s3Config['arguments']);
@@ -96,8 +94,6 @@ protected function tearDown(): void {
9694
// Ignore
9795
}
9896

99-
$this->tearDownEncryptionTrait();
100-
10197
parent::tearDown();
10298
}
10399

tests/lib/Files/ObjectStore/S3EncryptionTest.php

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -67,9 +67,6 @@ public static function setUpBeforeClass(): void {
6767
protected function setUp(): void {
6868
parent::setUp();
6969

70-
// Set up encryption (trait snapshots state before enabling)
71-
$this->setUpEncryptionTrait();
72-
7370
// Get S3 config from system config
7471
$this->s3Config = Server::get(IConfig::class)->getSystemValue('objectstore');
7572
$this->bucket = $this->s3Config['arguments']['bucket'] ?? 'nextcloud';
@@ -112,8 +109,6 @@ protected function tearDown(): void {
112109
// Ignore cleanup errors
113110
}
114111

115-
$this->tearDownEncryptionTrait();
116-
117112
parent::tearDown();
118113
}
119114

tests/lib/Traits/EncryptionTrait.php

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,13 @@ protected function setUpEncryptionTrait() {
115115
$appConfig->setValueString('core', 'default_encryption_module', Encryption::ID);
116116
$appConfig->setValueString('core', 'encryption_enabled', 'yes');
117117
$this->assertTrue(Server::get(\OCP\Encryption\IManager::class)->isEnabled());
118+
119+
// Ensure system-wide share/master keys exist in the keystore.
120+
// Setup::setupSystem() is cache-gated on 'keys-validated' and may no-op
121+
// when the cache was primed while encryption was disabled.
122+
$keyManager = Server::get(KeyManager::class);
123+
$keyManager->validateShareKey();
124+
$keyManager->validateMasterKey();
118125
}
119126

120127
protected function tearDownEncryptionTrait() {

0 commit comments

Comments
 (0)