From 8aceb680a3442d0ac8287dfbf949b96835121d42 Mon Sep 17 00:00:00 2001 From: provokateurin Date: Tue, 1 Oct 2024 15:55:36 +0200 Subject: [PATCH] refactor(Wrapper\Encryption): Migrate to strong types Signed-off-by: provokateurin --- .../Files/Storage/Wrapper/Encryption.php | 74 ++---- .../Files/Storage/Wrapper/EncryptionTest.php | 230 +++++++++--------- 2 files changed, 135 insertions(+), 169 deletions(-) diff --git a/lib/private/Files/Storage/Wrapper/Encryption.php b/lib/private/Files/Storage/Wrapper/Encryption.php index f2f3cdb94f6fa..f40d7d66c185f 100644 --- a/lib/private/Files/Storage/Wrapper/Encryption.php +++ b/lib/private/Files/Storage/Wrapper/Encryption.php @@ -29,76 +29,32 @@ class Encryption extends Wrapper { use LocalTempFileTrait; - /** @var string */ - private $mountPoint; - - /** @var \OC\Encryption\Util */ - private $util; - - /** @var \OCP\Encryption\IManager */ - private $encryptionManager; - - private LoggerInterface $logger; - - /** @var string */ - private $uid; - - /** @var array */ - protected $unencryptedSize; - - /** @var \OCP\Encryption\IFile */ - private $fileHelper; - - /** @var IMountPoint */ - private $mount; - - /** @var IStorage */ - private $keyStorage; - - /** @var Update */ - private $update; - - /** @var Manager */ - private $mountManager; - - /** @var array remember for which path we execute the repair step to avoid recursions */ - private $fixUnencryptedSizeOf = []; - - /** @var ArrayCache */ - private $arrayCache; - + private string $mountPoint; + protected array $unencryptedSize = []; + private IMountPoint $mount; + /** for which path we execute the repair step to avoid recursions */ + private array $fixUnencryptedSizeOf = []; /** @var CappedMemoryCache */ private CappedMemoryCache $encryptedPaths; - - private $enabled = true; + private bool $enabled = true; /** * @param array $parameters */ public function __construct( $parameters, - ?IManager $encryptionManager = null, - ?Util $util = null, - ?LoggerInterface $logger = null, - ?IFile $fileHelper = null, - $uid = null, - ?IStorage $keyStorage = null, - ?Update $update = null, - ?Manager $mountManager = null, - ?ArrayCache $arrayCache = null, + private IManager $encryptionManager, + private Util $util, + private LoggerInterface $logger, + private IFile $fileHelper, + private string $uid, + private IStorage $keyStorage, + private Update $update, + private Manager $mountManager, + private ArrayCache $arrayCache, ) { $this->mountPoint = $parameters['mountPoint']; $this->mount = $parameters['mount']; - $this->encryptionManager = $encryptionManager; - $this->util = $util; - $this->logger = $logger; - $this->uid = $uid; - $this->fileHelper = $fileHelper; - $this->keyStorage = $keyStorage; - $this->unencryptedSize = []; - $this->update = $update; - $this->mountManager = $mountManager; - $this->arrayCache = $arrayCache; $this->encryptedPaths = new CappedMemoryCache(); parent::__construct($parameters); } diff --git a/tests/lib/Files/Storage/Wrapper/EncryptionTest.php b/tests/lib/Files/Storage/Wrapper/EncryptionTest.php index a2949938410bb..d1a9d782fd896 100644 --- a/tests/lib/Files/Storage/Wrapper/EncryptionTest.php +++ b/tests/lib/Files/Storage/Wrapper/EncryptionTest.php @@ -4,12 +4,18 @@ * SPDX-FileCopyrightText: 2016 ownCloud, Inc. * SPDX-License-Identifier: AGPL-3.0-only */ + namespace Test\Files\Storage\Wrapper; +use Exception; +use OC; use OC\Encryption\Exceptions\ModuleDoesNotExistsException; +use OC\Encryption\File; use OC\Encryption\Update; use OC\Encryption\Util; +use OC\Files\Cache\Cache; use OC\Files\Cache\CacheEntry; +use OC\Files\Mount\MountPoint; use OC\Files\Storage\Temporary; use OC\Files\Storage\Wrapper\Encryption; use OC\Files\View; @@ -23,6 +29,7 @@ use OCP\Files\Mount\IMountPoint; use OCP\ICacheFactory; use OCP\IConfig; +use PHPUnit\Framework\MockObject\MockObject; use Psr\Log\LoggerInterface; use Test\Files\Storage\Storage; @@ -30,87 +37,26 @@ class EncryptionTest extends Storage { /** * block size will always be 8192 for a PHP stream * @see https://bugs.php.net/bug.php?id=21641 - * @var integer - */ - protected $headerSize = 8192; - - /** - * @var Temporary - */ - private $sourceStorage; - - /** - * @var \OC\Files\Storage\Wrapper\Encryption | \PHPUnit\Framework\MockObject\MockObject */ + protected int $headerSize = 8192; + private Temporary $sourceStorage; + /** @var Encryption&MockObject */ protected $instance; - - /** - * @var \OC\Encryption\Keys\Storage | \PHPUnit\Framework\MockObject\MockObject - */ - private $keyStore; - - /** - * @var \OC\Encryption\Util | \PHPUnit\Framework\MockObject\MockObject - */ - private $util; - - /** - * @var \OC\Encryption\Manager | \PHPUnit\Framework\MockObject\MockObject - */ - private $encryptionManager; - - /** - * @var \OCP\Encryption\IEncryptionModule | \PHPUnit\Framework\MockObject\MockObject - */ - private $encryptionModule; - - /** - * @var \OC\Encryption\Update | \PHPUnit\Framework\MockObject\MockObject - */ - private $update; - - /** - * @var \OC\Files\Cache\Cache | \PHPUnit\Framework\MockObject\MockObject - */ - private $cache; - - /** - * @var \OC\Log | \PHPUnit\Framework\MockObject\MockObject - */ - private $logger; - - /** - * @var \OC\Encryption\File | \PHPUnit\Framework\MockObject\MockObject - */ - private $file; - - - /** - * @var \OC\Files\Mount\MountPoint | \PHPUnit\Framework\MockObject\MockObject - */ - private $mount; - - /** - * @var \OC\Files\Mount\Manager | \PHPUnit\Framework\MockObject\MockObject - */ - private $mountManager; - - /** - * @var \OC\Group\Manager | \PHPUnit\Framework\MockObject\MockObject - */ - private $groupManager; - - /** - * @var \OCP\IConfig | \PHPUnit\Framework\MockObject\MockObject - */ - private $config; - - /** @var \OC\Memcache\ArrayCache | \PHPUnit\Framework\MockObject\MockObject */ - private $arrayCache; - - - /** @var integer dummy unencrypted size */ - private $dummySize = -1; + private \OC\Encryption\Keys\Storage&MockObject $keyStore; + private Util&MockObject $util; + private \OC\Encryption\Manager&MockObject $encryptionManager; + private IEncryptionModule&MockObject $encryptionModule; + private Update&MockObject $update; + private Cache&MockObject $cache; + private LoggerInterface&MockObject $logger; + private File&MockObject $file; + private MountPoint&MockObject $mount; + private \OC\Files\Mount\Manager&MockObject $mountManager; + private \OC\Group\Manager&MockObject $groupManager; + private IConfig&MockObject $config; + private ArrayCache&MockObject $arrayCache; + /** dummy unencrypted size */ + private int $dummySize = -1; protected function setUp(): void { parent::setUp(); @@ -133,7 +79,7 @@ protected function setUp(): void { ->getMock(); $this->util = $this->getMockBuilder('\OC\Encryption\Util') - ->setMethods(['getUidAndFilename', 'isFile', 'isExcluded']) + ->setMethods(['getUidAndFilename', 'isFile', 'isExcluded', 'stripPartialFileExtension']) ->setConstructorArgs([new View(), new Manager( $this->config, $this->createMock(ICacheFactory::class), @@ -146,6 +92,11 @@ protected function setUp(): void { ->willReturnCallback(function ($path) { return ['user1', $path]; }); + $this->util->expects($this->any()) + ->method('stripPartialFileExtension') + ->willReturnCallback(function ($path) { + return $path; + }); $this->file = $this->getMockBuilder('\OC\Encryption\File') ->disableOriginalConstructor() @@ -189,7 +140,7 @@ protected function setUp(): void { $this->mountManager->method('findByStorageId') ->willReturn([]); - $this->instance = $this->getMockBuilder('\OC\Files\Storage\Wrapper\Encryption') + $this->instance = $this->getMockBuilder(Encryption::class) ->setConstructorArgs( [ [ @@ -198,7 +149,15 @@ protected function setUp(): void { 'mountPoint' => '/', 'mount' => $this->mount ], - $this->encryptionManager, $this->util, $this->logger, $this->file, null, $this->keyStore, $this->update, $this->mountManager, $this->arrayCache + $this->encryptionManager, + $this->util, + $this->logger, + $this->file, + '', + $this->keyStore, + $this->update, + $this->mountManager, + $this->arrayCache ] ) ->setMethods(['getMetaData', 'getCache', 'getEncryptionModule']) @@ -219,10 +178,7 @@ protected function setUp(): void { ->willReturn($mockModule); } - /** - * @return \PHPUnit\Framework\MockObject\MockObject - */ - protected function buildMockModule() { + protected function buildMockModule(): IEncryptionModule&MockObject { $this->encryptionModule = $this->getMockBuilder('\OCP\Encryption\IEncryptionModule') ->disableOriginalConstructor() ->setMethods(['getId', 'getDisplayName', 'begin', 'end', 'encrypt', 'decrypt', 'update', 'shouldEncrypt', 'getUnencryptedBlockSize', 'isReadable', 'encryptAll', 'prepareDecryptAll', 'isReadyForUser', 'needDetailedAccessList']) @@ -266,7 +222,7 @@ function ($path) use ($encrypted) { } ); - $this->instance = $this->getMockBuilder('\OC\Files\Storage\Wrapper\Encryption') + $this->instance = $this->getMockBuilder(Encryption::class) ->setConstructorArgs( [ [ @@ -275,7 +231,15 @@ function ($path) use ($encrypted) { 'mountPoint' => '/', 'mount' => $this->mount ], - $this->encryptionManager, $this->util, $this->logger, $this->file, null, $this->keyStore, $this->update, $this->mountManager, $this->arrayCache + $this->encryptionManager, + $this->util, + $this->logger, + $this->file, + '', + $this->keyStore, + $this->update, + $this->mountManager, + $this->arrayCache, ] ) ->setMethods(['getCache', 'verifyUnencryptedSize']) @@ -337,7 +301,7 @@ public function testFilesize(): void { ->method('get') ->willReturn(new CacheEntry(['encrypted' => true, 'path' => '/test.txt', 'size' => 0, 'fileid' => 1])); - $this->instance = $this->getMockBuilder('\OC\Files\Storage\Wrapper\Encryption') + $this->instance = $this->getMockBuilder(Encryption::class) ->setConstructorArgs( [ [ @@ -346,7 +310,15 @@ public function testFilesize(): void { 'mountPoint' => '/', 'mount' => $this->mount ], - $this->encryptionManager, $this->util, $this->logger, $this->file, null, $this->keyStore, $this->update, $this->mountManager, $this->arrayCache + $this->encryptionManager, + $this->util, + $this->logger, + $this->file, + '', + $this->keyStore, + $this->update, + $this->mountManager, + $this->arrayCache, ] ) ->setMethods(['getCache', 'verifyUnencryptedSize']) @@ -374,7 +346,7 @@ public function testVerifyUnencryptedSize($encryptedSize, $unencryptedSize, $fai $sourceStorage = $this->getMockBuilder('\OC\Files\Storage\Storage') ->disableOriginalConstructor()->getMock(); - $this->instance = $this->getMockBuilder('\OC\Files\Storage\Wrapper\Encryption') + $this->instance = $this->getMockBuilder(Encryption::class) ->setConstructorArgs( [ [ @@ -383,7 +355,15 @@ public function testVerifyUnencryptedSize($encryptedSize, $unencryptedSize, $fai 'mountPoint' => '/', 'mount' => $this->mount ], - $this->encryptionManager, $this->util, $this->logger, $this->file, null, $this->keyStore, $this->update, $this->mountManager, $this->arrayCache + $this->encryptionManager, + $this->util, + $this->logger, + $this->file, + '', + $this->keyStore, + $this->update, + $this->mountManager, + $this->arrayCache, ] ) ->setMethods(['fixUnencryptedSize']) @@ -396,7 +376,7 @@ public function testVerifyUnencryptedSize($encryptedSize, $unencryptedSize, $fai ->willReturnCallback( function () use ($failure, $expected) { if ($failure) { - throw new \Exception(); + throw new Exception(); } else { return $expected; } @@ -495,17 +475,25 @@ public function testRmdir($path, $rmdirResult, $isExcluded, $encryptionEnabled): $util = $this->getMockBuilder('\OC\Encryption\Util')->disableOriginalConstructor()->getMock(); $sourceStorage->expects($this->once())->method('rmdir')->willReturn($rmdirResult); - $util->expects($this->any())->method('isExcluded')-> willReturn($isExcluded); + $util->expects($this->any())->method('isExcluded')->willReturn($isExcluded); $this->encryptionManager->expects($this->any())->method('isEnabled')->willReturn($encryptionEnabled); - $encryptionStorage = new \OC\Files\Storage\Wrapper\Encryption( + $encryptionStorage = new Encryption( [ 'storage' => $sourceStorage, 'root' => 'foo', 'mountPoint' => '/mountPoint', 'mount' => $this->mount ], - $this->encryptionManager, $util, $this->logger, $this->file, null, $this->keyStore, $this->update + $this->encryptionManager, + $util, + $this->logger, + $this->file, + '', + $this->keyStore, + $this->update, + $this->mountManager, + $this->arrayCache, ); @@ -595,7 +583,7 @@ public function testGetHeader($path, $strippedPathExists, $strippedPath): void { return ['encrypted' => true, 'path' => $path]; }); - $instance = $this->getMockBuilder('\OC\Files\Storage\Wrapper\Encryption') + $instance = $this->getMockBuilder(Encryption::class) ->setConstructorArgs( [ [ @@ -604,7 +592,15 @@ public function testGetHeader($path, $strippedPathExists, $strippedPath): void { 'mountPoint' => '/', 'mount' => $this->mount ], - $this->encryptionManager, $util, $this->logger, $this->file, null, $this->keyStore, $this->update, $this->mountManager, $this->arrayCache + $this->encryptionManager, + $util, + $this->logger, + $this->file, + '', + $this->keyStore, + $this->update, + $this->mountManager, + $this->arrayCache, ] ) ->setMethods(['getCache', 'readFirstBlock']) @@ -649,15 +645,17 @@ public function dataTestGetHeader() { * * @dataProvider dataTestGetHeaderAddLegacyModule */ - public function testGetHeaderAddLegacyModule($header, $isEncrypted, $exists, $expected): void { + public function testGetHeaderAddLegacyModule($header, $isEncrypted, $strippedPathExists, $expected): void { $sourceStorage = $this->getMockBuilder('\OC\Files\Storage\Storage') ->disableOriginalConstructor()->getMock(); $sourceStorage->expects($this->once()) ->method('is_file') - ->willReturn($exists); + ->with('test.txt') + ->willReturn($strippedPathExists); $util = $this->getMockBuilder('\OC\Encryption\Util') + ->onlyMethods(['stripPartialFileExtension', 'parseRawHeader']) ->setConstructorArgs([new View(), new Manager( $this->config, $this->createMock(ICacheFactory::class), @@ -665,6 +663,11 @@ public function testGetHeaderAddLegacyModule($header, $isEncrypted, $exists, $ex $this->createMock(LoggerInterface::class), ), $this->groupManager, $this->config, $this->arrayCache]) ->getMock(); + $util->expects($this->any()) + ->method('stripPartialFileExtension') + ->willReturnCallback(function ($path) { + return $path; + }); $cache = $this->getMockBuilder('\OC\Files\Cache\Cache') ->disableOriginalConstructor()->getMock(); @@ -674,7 +677,7 @@ public function testGetHeaderAddLegacyModule($header, $isEncrypted, $exists, $ex return ['encrypted' => $isEncrypted, 'path' => $path]; }); - $instance = $this->getMockBuilder('\OC\Files\Storage\Wrapper\Encryption') + $instance = $this->getMockBuilder(Encryption::class) ->setConstructorArgs( [ [ @@ -683,7 +686,15 @@ public function testGetHeaderAddLegacyModule($header, $isEncrypted, $exists, $ex 'mountPoint' => '/', 'mount' => $this->mount ], - $this->encryptionManager, $util, $this->logger, $this->file, null, $this->keyStore, $this->update, $this->mountManager, $this->arrayCache + $this->encryptionManager, + $util, + $this->logger, + $this->file, + '', + $this->keyStore, + $this->update, + $this->mountManager, + $this->arrayCache, ] ) ->setMethods(['readFirstBlock', 'getCache']) @@ -729,7 +740,7 @@ public function testCopyBetweenStorageMinimumEncryptedVersion(): void { $storage2->expects($this->any()) ->method('fopen') ->willReturnCallback(function ($path, $mode) { - $temp = \OC::$server->getTempManager(); + $temp = OC::$server->getTempManager(); return fopen($temp->getTemporaryFile(), $mode); }); $storage2->method('getId') @@ -778,7 +789,7 @@ public function testCopyBetweenStorage($encryptionEnabled, $mountPointEncryption $storage2->expects($this->any()) ->method('fopen') ->willReturnCallback(function ($path, $mode) { - $temp = \OC::$server->getTempManager(); + $temp = OC::$server->getTempManager(); return fopen($temp->getTemporaryFile(), $mode); }); $storage2->method('getId') @@ -840,8 +851,8 @@ public function testCopyBetweenStorageVersions($sourceInternalPath, $targetInter $mountPoint = '/mountPoint'; - /** @var \OC\Files\Storage\Wrapper\Encryption |\PHPUnit\Framework\MockObject\MockObject $instance */ - $instance = $this->getMockBuilder('\OC\Files\Storage\Wrapper\Encryption') + /** @var Encryption |MockObject $instance */ + $instance = $this->getMockBuilder(Encryption::class) ->setConstructorArgs( [ [ @@ -854,7 +865,7 @@ public function testCopyBetweenStorageVersions($sourceInternalPath, $targetInter $this->util, $this->logger, $this->file, - null, + '', $this->keyStore, $this->update, $this->mountManager, @@ -956,7 +967,6 @@ public function testShouldEncrypt( $encryptionManager = $this->createMock(\OC\Encryption\Manager::class); $util = $this->createMock(Util::class); $fileHelper = $this->createMock(IFile::class); - $uid = null; $keyStorage = $this->createMock(IStorage::class); $update = $this->createMock(Update::class); $mountManager = $this->createMock(\OC\Files\Mount\Manager::class); @@ -974,7 +984,7 @@ public function testShouldEncrypt( $util, $this->logger, $fileHelper, - $uid, + '', $keyStorage, $update, $mountManager, @@ -985,7 +995,7 @@ public function testShouldEncrypt( ->getMock(); if ($encryptionModule === true) { - /** @var IEncryptionModule|\PHPUnit\Framework\MockObject\MockObject $encryptionModule */ + /** @var IEncryptionModule|MockObject $encryptionModule */ $encryptionModule = $this->createMock(IEncryptionModule::class); }