diff --git a/apps/encryption/lib/Crypto/EncryptAll.php b/apps/encryption/lib/Crypto/EncryptAll.php index 5600526eac09c..087870425ee3c 100644 --- a/apps/encryption/lib/Crypto/EncryptAll.php +++ b/apps/encryption/lib/Crypto/EncryptAll.php @@ -10,7 +10,7 @@ namespace OCA\Encryption\Crypto; -use OC\Encryption\Exceptions\DecryptionFailedException; +use OC\Encryption\Exceptions\EncryptionFailedException; use OC\Files\SetupManager; use OC\Files\View; use OCA\Encryption\KeyManager; @@ -33,6 +33,9 @@ use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Console\Question\ConfirmationQuestion; +/** + * Handles bulk encryption of files for users. + */ class EncryptAll { /** @var array $userCache store one time passwords for the users */ @@ -168,7 +171,10 @@ protected function encryptAllUserFilesWithMasterKey(ProgressBar $progress): void } /** - * encrypt files from the given user + * Encrypt all files from the given user. + * + * Recursively traverses the user's files directory, skipping files and folders not owned by the user, + * and attempts to encrypt each file. */ protected function encryptUsersFiles(IUser $user, ProgressBar $progress, string $userCount): void { $this->setupUserFileSystem($user); @@ -178,64 +184,63 @@ protected function encryptUsersFiles(IUser $user, ProgressBar $progress, string while ($root = array_pop($directories)) { $content = $this->rootView->getDirectoryContent($root); + /** @var FileInfo $file */ foreach ($content as $file) { $path = $root . '/' . $file->getName(); - if ($file->isShared()) { - $progress->setMessage("Skip shared file/folder $path"); + + if ($file->getOwner() !== $uid) { + $progress->setMessage("Skipping shared/unowned file/folder $path"); $progress->advance(); continue; - } elseif ($file->getType() === FileInfo::TYPE_FOLDER) { + } + + if ($file->getType() === FileInfo::TYPE_FOLDER) { $directories[] = $path; continue; - } else { - $progress->setMessage("encrypt files for user $userCount: $path"); - $progress->advance(); - try { - if ($this->encryptFile($file, $path) === false) { - $progress->setMessage("encrypt files for user $userCount: $path (already encrypted)"); - $progress->advance(); - } - } catch (\Exception $e) { - $progress->setMessage("Failed to encrypt path $path: " . $e->getMessage()); + } + + $progress->setMessage("Encrypting file for user $userCount: $path"); + $progress->advance(); + + try { + if ($this->encryptFile($file, $path) === false) { + $progress->setMessage("Skipping already encrypted file $path for user $userCount"); $progress->advance(); - $this->logger->error( - 'Failed to encrypt path {path}', - [ - 'user' => $uid, - 'path' => $path, - 'exception' => $e, - ] - ); } + } catch (\Exception $e) { + $progress->setMessage("Failed to encrypt path $path: " . $e->getMessage()); + $progress->advance(); + $this->logger->error('Failed to encrypt path {path}', [ 'user' => $uid, 'path' => $path, 'exception' => $e, ]); } } } } protected function encryptFile(FileInfo $fileInfo, string $path): bool { - // skip already encrypted files if ($fileInfo->isEncrypted()) { - return true; + return false; } $source = $path; $target = $path . '.encrypted.' . time(); try { - $copySuccess = $this->rootView->copy($source, $target); - if ($copySuccess === false) { - /* Copy failed, abort */ - if ($this->rootView->file_exists($target)) { - $this->rootView->unlink($target); - } - throw new \Exception('Copy failed for ' . $source); + if ($this->rootView->copy($source, $target) === false) { + throw new EncryptionFailedException("Failed to copy $source -> $target"); } - $this->rootView->rename($target, $source); - } catch (DecryptionFailedException $e) { + + if ($this->rootView->rename($target, $source) === false) { + throw new EncryptionFailedException("Failed to rename $target -> $source"); + } + } catch (\Exception $e) { if ($this->rootView->file_exists($target)) { + $this->logger->debug( + "Cleaning up failed temp file $target after encryption exception", + [ 'user' => $fileInfo->getOwner(), 'path' => $path, ] + ); $this->rootView->unlink($target); } - return false; + throw $e; } return true; diff --git a/lib/private/Encryption/DecryptAll.php b/lib/private/Encryption/DecryptAll.php index 59d0eb03d28e1..e6394df685526 100644 --- a/lib/private/Encryption/DecryptAll.php +++ b/lib/private/Encryption/DecryptAll.php @@ -11,23 +11,33 @@ namespace OC\Encryption; use OC\Encryption\Exceptions\DecryptionFailedException; +use OC\Files\FileInfo; use OC\Files\View; use OCP\Encryption\IEncryptionModule; use OCP\Encryption\IManager; use OCP\IUserManager; +use Psr\Log\LoggerInterface; use Symfony\Component\Console\Helper\ProgressBar; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; +/** + * DecryptAll handles bulk decryption of files for users. + */ class DecryptAll { /** @var array> files which couldn't be decrypted */ protected array $failed = []; + protected readonly LoggerInterface $logger; + public function __construct( - protected IManager $encryptionManager, - protected IUserManager $userManager, - protected View $rootView, + protected readonly IManager $encryptionManager, + protected readonly IUserManager $userManager, + protected readonly View $rootView, ) { + // TODO: Inject LoggerInterface + $this->logger = \OC::$server->get(LoggerInterface::class); + // TODO: Inject SetupManager } /** @@ -51,6 +61,7 @@ public function decryptAll(InputInterface $input, OutputInterface $output, strin $this->failed = []; $this->decryptAllUsersFiles($output, $user); + // TODO: Drop below output; maybe still use $this->failed to return false (if we can't switch to void) /** @psalm-suppress RedundantCondition $this->failed is modified by decryptAllUsersFiles, not clear why psalm fails to see it */ if (empty($this->failed)) { $output->writeln('all files could be decrypted successfully!'); @@ -91,7 +102,7 @@ protected function prepareEncryptionModules(InputInterface $input, OutputInterfa } /** - * iterate over all user and encrypt their files + * iterate over all user and decrypt their files * * @param string $user which users files should be decrypted, default = all users */ @@ -129,7 +140,7 @@ protected function decryptAllUsersFiles(OutputInterface $output, string $user = $progress = new ProgressBar($output); $progress->setFormat(" %message% \n [%bar%]"); $progress->start(); - $progress->setMessage('starting to decrypt files...'); + $progress->setMessage('Decrypting files...'); $progress->advance(); $numberOfUsers = count($userList); @@ -140,50 +151,57 @@ protected function decryptAllUsersFiles(OutputInterface $output, string $user = $userNo++; } - $progress->setMessage('starting to decrypt files... finished'); + $progress->setMessage('Decrypting files... finished'); $progress->finish(); $output->writeln("\n\n"); } - /** - * encrypt files from the given user - */ - protected function decryptUsersFiles(string $uid, ProgressBar $progress, string $userCount): void { +/** + * Decrypt all files as the given user. + * + * Recursively traverses the user's files directory, skipping files and folders not owned by the user, + * and attempts to decrypt each file. + */ + protected function decryptUsersFiles(string $uid, ProgressBar $progress, string $userCount): void { $this->setupUserFS($uid); $directories = []; $directories[] = '/' . $uid . '/files'; while ($root = array_pop($directories)) { $content = $this->rootView->getDirectoryContent($root); + /** @var FileInfo $file */ foreach ($content as $file) { - // only decrypt files owned by the user - if ($file->getStorage()->instanceOfStorage('OCA\Files_Sharing\SharedStorage')) { + $path = $root . '/' . $file->getName(); + + if ($file->getOwner() !== $uid) { + $progress->setMessage("Skipping shared/unowned file/folder $path"); + $progress->advance(); continue; } - $path = $root . '/' . $file['name']; - if ($this->rootView->is_dir($path)) { + + if ($file->getType() === FileInfo::TYPE_FOLDER) { $directories[] = $path; continue; - } else { - try { - $progress->setMessage("decrypt files for user $userCount: $path"); + } + + $progress->setMessage("Decrypting file for user $userCount: $path"); + $progress->advance(); + + try { + if ($this->decryptFile($path) === false) { + $progress->setMessage("Skipping already decrypted file $path for user $userCount"); $progress->advance(); - if ($file->isEncrypted() === false) { - $progress->setMessage("decrypt files for user $userCount: $path (already decrypted)"); - $progress->advance(); - } else { - if ($this->decryptFile($path) === false) { - $progress->setMessage("decrypt files for user $userCount: $path (already decrypted)"); - $progress->advance(); - } - } - } catch (\Exception $e) { - if (isset($this->failed[$uid])) { - $this->failed[$uid][] = $path; - } else { - $this->failed[$uid] = [$path]; - } + } + } catch (\Exception $e) { + $progress->setMessage("Failed to decrypt path $path: " . $e->getMessage()); + $progress->advance(); + $this->logger->error('Failed to decrypt path {path}', [ 'user' => $uid, 'path' => $path, 'exception' => $e, ]); + // TODO: we can probably drop this since we're now outputting above like we do in EncryptAll + if (isset($this->failed[$uid])) { + $this->failed[$uid][] = $path; + } else { + $this->failed[$uid] = [$path]; } } } @@ -191,43 +209,56 @@ protected function decryptUsersFiles(string $uid, ProgressBar $progress, string } /** - * encrypt file + * Attempt to decrypt a single file. + * @param string $path The full filesystem path to the file. + * + * @throws DecryptionFailedException If file copy or rename fails during decryption. + * @throws \RuntimeException If file info cannot be retrieved or touch fails. + * + * @return bool True if decryption succeeded, false if file is already decrypted. */ protected function decryptFile(string $path): bool { - // skip already decrypted files $fileInfo = $this->rootView->getFileInfo($path); - if ($fileInfo !== false && !$fileInfo->isEncrypted()) { - return true; + + if ($fileInfo === false) { + throw new \RuntimeException("Could not retrieve file info for $path"); + } + + if (!$fileInfo->isEncrypted()) { + return false; } $source = $path; - $target = $path . '.decrypted.' . $this->getTimestamp(); + $target = $path . '.decrypted.' . time(); try { - $this->rootView->copy($source, $target); - $this->rootView->touch($target, $fileInfo->getMTime()); - $this->rootView->rename($target, $source); - } catch (DecryptionFailedException $e) { + if ($this->rootView->copy($source, $target) === false) { + throw new DecryptionFailedException("Failed to copy $source -> $target"); + } + + if ($this->rootView->touch($target, $fileInfo->getMTime()) === false) { + throw new \RuntimeException("Failed to update mtime for $target"); + } + + if ($this->rootView->rename($target, $source) === false) { + throw new DecryptionFailedException("Failed to rename $target -> $source"); + } + } catch (\Exception $e) { if ($this->rootView->file_exists($target)) { + $this->logger->debug("Cleaning up failed temp file $target after decryption exception", [ 'path' => $path, ]); $this->rootView->unlink($target); } - return false; + throw $e; } return true; } - /** - * get current timestamp - */ - protected function getTimestamp(): int { - return time(); - } - /** * setup user file system */ protected function setupUserFS(string $uid): void { + // TODO: Refactor to use injected SetupManager (like EncryptAll does) + the IUser objeect \OC_Util::tearDownFS(); \OC_Util::setupFS($uid); } diff --git a/tests/lib/Encryption/DecryptAllTest.php b/tests/lib/Encryption/DecryptAllTest.php index f7a6497e011be..6e6a3a6ae0bbd 100644 --- a/tests/lib/Encryption/DecryptAllTest.php +++ b/tests/lib/Encryption/DecryptAllTest.php @@ -11,13 +11,13 @@ namespace Test\Encryption; use OC\Encryption\DecryptAll; -use OC\Encryption\Exceptions\DecryptionFailedException; use OC\Encryption\Manager; use OC\Files\FileInfo; use OC\Files\View; use OCP\Files\Storage\IStorage; use OCP\IUserManager; use OCP\UserInterface; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\MockObject\MockObject; use Symfony\Component\Console\Formatter\OutputFormatterInterface; use Symfony\Component\Console\Helper\ProgressBar; @@ -26,54 +26,182 @@ use Test\TestCase; /** - * Class DecryptAllTest + * Unit tests for DecryptAll orchestration logic in lib/private/Encryption. * - * - * @package Test\Encryption + * This class covers high-level user and file decryption workflows as exposed by the main encryption service, + * differentiating it from tests in app-layer or crypto-layer classes that cover low-level session, key, or CLI logic. */ -#[\PHPUnit\Framework\Attributes\Group('DB')] class DecryptAllTest extends TestCase { - private IUserManager&MockObject $userManager; - private Manager&MockObject $encryptionManager; - private View&MockObject $view; - private InputInterface&MockObject $inputInterface; - private OutputInterface&MockObject $outputInterface; - private UserInterface&MockObject $userInterface; + private const TEST_USER = 'user1'; + private const TEST_FILE = 'test.txt'; + + private IUserManager&MockObject $mockUserManager; + private Manager&MockObject $mockEncryptionManager; + private View&MockObject $mockView; + private InputInterface&MockObject $mockInputInterface; + private OutputInterface&MockObject $mockOutputInterface; + private UserInterface&MockObject $mockUserInterface; private DecryptAll $instance; protected function setUp(): void { parent::setUp(); + $this->initializeMocks(); + $this->instance = new DecryptAll( + $this->mockEncryptionManager, + $this->mockUserManager, + $this->mockView + ); + } - $this->userManager = $this->getMockBuilder(IUserManager::class) - ->disableOriginalConstructor()->getMock(); - $this->encryptionManager = $this->getMockBuilder('OC\Encryption\Manager') - ->disableOriginalConstructor()->getMock(); - $this->view = $this->getMockBuilder(View::class) - ->disableOriginalConstructor()->getMock(); - $this->inputInterface = $this->getMockBuilder(InputInterface::class) - ->disableOriginalConstructor()->getMock(); - $this->outputInterface = $this->getMockBuilder(OutputInterface::class) - ->disableOriginalConstructor()->getMock(); - $this->outputInterface->expects($this->any())->method('isDecorated') - ->willReturn(false); - $this->userInterface = $this->getMockBuilder(UserInterface::class) - ->disableOriginalConstructor()->getMock(); + private function initializeMocks(): void { + $this->mockUserManager = $this->createMock(IUserManager::class); + $this->mockEncryptionManager = $this->createMock(Manager::class); + $this->mockView = $this->createMock(View::class); + $this->mockInputInterface = $this->createMock(InputInterface::class); + $this->mockOutputInterface = $this->createMock(OutputInterface::class); + $this->mockOutputInterface->method('isDecorated')->willReturn(false); + $this->mockUserInterface = $this->createMock(UserInterface::class); + } - /* We need format method to return a string */ - $outputFormatter = $this->createMock(OutputFormatterInterface::class); - $outputFormatter->method('format')->willReturn('foo'); - $outputFormatter->method('isDecorated')->willReturn(false); + /** + * Covers: scenarios where user does/does not exist when running decryptAll. + * Ensures expected messaging and correct function return. + */ + #[DataProvider('provideUserExistence')] + public function testDecryptAllWithNonexistentUser(bool $userExists): void { + $this->mockUserManager->method('userExists')->willReturn($userExists); - $this->outputInterface->expects($this->any())->method('getFormatter') - ->willReturn($outputFormatter); + if (!$userExists) { + $this->mockOutputInterface->expects($this->once())->method('writeln') + ->with(sprintf('User "%s" does not exist. Please check the username and try again', self::TEST_USER)); + } + + $result = $this->instance->decryptAll($this->mockInputInterface, $this->mockOutputInterface, self::TEST_USER); - $this->instance = new DecryptAll($this->encryptionManager, $this->userManager, $this->view); + $this->assertSame($userExists, $result); + } - $this->invokePrivate($this->instance, 'input', [$this->inputInterface]); - $this->invokePrivate($this->instance, 'output', [$this->outputInterface]); + public static function provideUserExistence(): array { + return [ + [true], // userExists + [false], // !userExists + ]; } + + /** + * Covers: encrypted vs. non-encrypted file decryption path and side effects. + * Ensures expected filesystem operations happen only when appropriate. + */ + #[DataProvider('provideDecryptionStatus')] + public function testDecryptFileBehavior(bool $isEncrypted): void { + // Configure fileInfo for encrypted/non-encrypted + $fileInfo = $this->createMock(FileInfo::class); + $fileInfo->method('isEncrypted')->willReturn($isEncrypted); + $this->mockView->method('getFileInfo')->willReturn($fileInfo); + + // Set up mock for the timestamp function + $instance = $this->getMockBuilder(DecryptAll::class) + ->setConstructorArgs([$this->mockEncryptionManager, $this->mockUserManager, $this->mockView]) + ->onlyMethods(['getTimestamp']) + ->getMock(); + if ($isEncrypted) { + // Only when encrypted, "copy" and "rename" should be called + $instance->method('getTimestamp')->willReturn(42); + + $this->mockView->expects($this->once()) + ->method('copy') + ->with(self::TEST_FILE, self::TEST_FILE . '.decrypted.42'); + $this->mockView->expects($this->once()) + ->method('rename') + ->with(self::TEST_FILE . '.decrypted.42', self::TEST_FILE); + } else { + // If not encrypted, these operations should not happen + $instance->expects($this->never())->method('getTimestamp'); + $this->mockView->expects($this->never())->method('copy'); + $this->mockView->expects($this->never())->method('rename'); + } + + $result = $instance->decryptFile(self::TEST_FILE); + + $this->assertTrue($result); + } + + public static function provideDecryptionStatus(): array { + return [ + [true], // isEncrypted + [false] // !isEncrypted + ]; + } + + /** + * Covers: workflow for successful module preparation and user file decryption + * Ensures the main orchestration calls collaborators as expected. + */ + public function testDecryptAllTriggersModuleAndUserWorkflows(): void { + $this->mockUserManager->method('userExists')->willReturn(true); + + // Partial mock to verify internal orchestration logic + $instance = $this->getMockBuilder(DecryptAll::class) + ->setConstructorArgs([ + $this->mockEncryptionManager, + $this->mockUserManager, + $this->mockView + ]) + ->onlyMethods(['prepareEncryptionModules', 'decryptAllUsersFiles']) + ->getMock(); + + // If module preparation succeeds, proceed with user file decryption + $instance->expects($this->once()) + ->method('prepareEncryptionModules') + ->with($this->mockInputInterface, $this->mockOutputInterface, self::TEST_USER) + ->willReturn(true); + + $instance->expects($this->once()) + ->method('decryptAllUsersFiles') + ->with($this->mockOutputInterface, self::TEST_USER); + + $result = $instance->decryptAll($this->mockInputInterface, $this->mockOutputInterface, self::TEST_USER); + $this->assertTrue($result); + } + + /** + * Covers: early abort if module preparation fails. + * Ensures "decryptAllUsersFiles" is not called and function returns false. + */ + public function testDecryptAllAbortsIfPreparationFails(): void { + $this->mockUserManager->method('userExists')->willReturn(true); + + $instance = $this->getMockBuilder(DecryptAll::class) + ->setConstructorArgs([ + $this->mockEncryptionManager, + $this->mockUserManager, + $this->mockView + ]) + ->onlyMethods(['prepareEncryptionModules', 'decryptAllUsersFiles']) + ->getMock(); + + $instance->expects($this->once()) + ->method('prepareEncryptionModules') + ->with($this->mockInputInterface, $this->mockOutputInterface, self::TEST_USER) + ->willReturn(false); + + $instance->expects($this->never()) + ->method('decryptAllUsersFiles'); + + $result = $instance->decryptAll($this->mockInputInterface, $this->mockOutputInterface, self::TEST_USER); + $this->assertFalse($result); + } + + /** + * TODO: + * - Add file operation failure (fileInfo/copy/rename) + * - Add teamfolder scenario + */ + + /*********** LEGACY TESTS ************/ + public static function dataDecryptAll(): array { return [ [true, 'user1', true],