From 3eb3e437c8a0520192ec7c1018d4d1c55e780dc0 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Mon, 19 Dec 2016 15:35:31 +0100 Subject: [PATCH] Add proper tests Signed-off-by: Lukas Reschke --- lib/private/IntegrityCheck/Checker.php | 30 ++++--- .../Helpers/FileAccessHelper.php | 8 +- tests/lib/Command/Integrity/SignAppTest.php | 62 ++++++++++++-- tests/lib/Command/Integrity/SignCoreTest.php | 60 +++++++++++-- tests/lib/IntegrityCheck/CheckerTest.php | 84 ++++++++++++++++--- .../Helpers/FileAccessHelperTest.php | 25 ++++++ 6 files changed, 220 insertions(+), 49 deletions(-) diff --git a/lib/private/IntegrityCheck/Checker.php b/lib/private/IntegrityCheck/Checker.php index a3d9d906d8..4588c006da 100644 --- a/lib/private/IntegrityCheck/Checker.php +++ b/lib/private/IntegrityCheck/Checker.php @@ -272,19 +272,18 @@ class Checker { X509 $certificate, RSA $privateKey) { $appInfoDir = $path . '/appinfo'; - $this->fileAccessHelper->assertDirectoryExists($path); - $this->fileAccessHelper->assertDirectoryExists($appInfoDir); - - $iterator = $this->getFolderIterator($path); - $hashes = $this->generateHashes($iterator, $path); - $signature = $this->createSignatureData($hashes, $certificate, $privateKey); try { - $this->fileAccessHelper->file_put_contents( - $appInfoDir . '/signature.json', + $this->fileAccessHelper->assertDirectoryExists($appInfoDir); + + $iterator = $this->getFolderIterator($path); + $hashes = $this->generateHashes($iterator, $path); + $signature = $this->createSignatureData($hashes, $certificate, $privateKey); + $this->fileAccessHelper->file_put_contents( + $appInfoDir . '/signature.json', json_encode($signature, JSON_PRETTY_PRINT) ); } catch (\Exception $e){ - if (!$this->fileAccessHelper->is_writeable($appInfoDir)){ + if (!$this->fileAccessHelper->is_writable($appInfoDir)) { throw new \Exception($appInfoDir . ' is not writable'); } throw $e; @@ -303,19 +302,18 @@ class Checker { RSA $rsa, $path) { $coreDir = $path . '/core'; - $this->fileAccessHelper->assertDirectoryExists($path); - $this->fileAccessHelper->assertDirectoryExists($coreDir); - - $iterator = $this->getFolderIterator($path, $path); - $hashes = $this->generateHashes($iterator, $path); - $signatureData = $this->createSignatureData($hashes, $certificate, $rsa); try { + + $this->fileAccessHelper->assertDirectoryExists($coreDir); + $iterator = $this->getFolderIterator($path, $path); + $hashes = $this->generateHashes($iterator, $path); + $signatureData = $this->createSignatureData($hashes, $certificate, $rsa); $this->fileAccessHelper->file_put_contents( $coreDir . '/signature.json', json_encode($signatureData, JSON_PRETTY_PRINT) ); } catch (\Exception $e){ - if (!$this->fileAccessHelper->is_writeable($coreDir)){ + if (!$this->fileAccessHelper->is_writable($coreDir)) { throw new \Exception($coreDir . ' is not writable'); } throw $e; diff --git a/lib/private/IntegrityCheck/Helpers/FileAccessHelper.php b/lib/private/IntegrityCheck/Helpers/FileAccessHelper.php index c319345789..a7e378c165 100644 --- a/lib/private/IntegrityCheck/Helpers/FileAccessHelper.php +++ b/lib/private/IntegrityCheck/Helpers/FileAccessHelper.php @@ -58,7 +58,7 @@ class FileAccessHelper { * @throws \Exception */ public function file_put_contents($filename, $data) { - $bytesWritten = file_put_contents($filename, $data); + $bytesWritten = @file_put_contents($filename, $data); if ($bytesWritten === false || $bytesWritten !== strlen($data)){ throw new \Exception('Failed to write into ' . $filename); } @@ -69,15 +69,15 @@ class FileAccessHelper { * @param string $path * @return bool */ - public function is_writeable($path){ - return is_writeable($path); + public function is_writable($path) { + return is_writable($path); } /** * @param string $path * @throws \Exception */ - public function assertDirectoryExists($path){ + public function assertDirectoryExists($path) { if (!is_dir($path)) { throw new \Exception('Directory ' . $path . ' does not exist.'); } diff --git a/tests/lib/Command/Integrity/SignAppTest.php b/tests/lib/Command/Integrity/SignAppTest.php index 71d9946ee8..013290b56e 100644 --- a/tests/lib/Command/Integrity/SignAppTest.php +++ b/tests/lib/Command/Integrity/SignAppTest.php @@ -29,13 +29,13 @@ use Symfony\Component\Console\Output\OutputInterface; use Test\TestCase; class SignAppTest extends TestCase { - /** @var Checker */ + /** @var Checker|\PHPUnit_Framework_MockObject_MockObject */ private $checker; /** @var SignApp */ private $signApp; - /** @var FileAccessHelper */ + /** @var FileAccessHelper|\PHPUnit_Framework_MockObject_MockObject */ private $fileAccessHelper; - /** @var IURLGenerator */ + /** @var IURLGenerator|\PHPUnit_Framework_MockObject_MockObject */ private $urlGenerator; public function setUp() { @@ -75,7 +75,7 @@ class SignAppTest extends TestCase { ->method('writeln') ->with('This command requires the --path, --privateKey and --certificate.'); - $this->invokePrivate($this->signApp, 'execute', [$inputInterface, $outputInterface]); + $this->assertNull(self::invokePrivate($this->signApp, 'execute', [$inputInterface, $outputInterface])); } public function testExecuteWithMissingPrivateKey() { @@ -103,7 +103,7 @@ class SignAppTest extends TestCase { ->method('writeln') ->with('This command requires the --path, --privateKey and --certificate.'); - $this->invokePrivate($this->signApp, 'execute', [$inputInterface, $outputInterface]); + $this->assertNull(self::invokePrivate($this->signApp, 'execute', [$inputInterface, $outputInterface])); } public function testExecuteWithMissingCertificate() { @@ -131,7 +131,7 @@ class SignAppTest extends TestCase { ->method('writeln') ->with('This command requires the --path, --privateKey and --certificate.'); - $this->invokePrivate($this->signApp, 'execute', [$inputInterface, $outputInterface]); + $this->assertNull(self::invokePrivate($this->signApp, 'execute', [$inputInterface, $outputInterface])); } public function testExecuteWithNotExistingPrivateKey() { @@ -165,7 +165,7 @@ class SignAppTest extends TestCase { ->method('writeln') ->with('Private key "privateKey" does not exists.'); - $this->invokePrivate($this->signApp, 'execute', [$inputInterface, $outputInterface]); + $this->assertNull(self::invokePrivate($this->signApp, 'execute', [$inputInterface, $outputInterface])); } public function testExecuteWithNotExistingCertificate() { @@ -204,7 +204,51 @@ class SignAppTest extends TestCase { ->method('writeln') ->with('Certificate "certificate" does not exists.'); - $this->invokePrivate($this->signApp, 'execute', [$inputInterface, $outputInterface]); + $this->assertNull(self::invokePrivate($this->signApp, 'execute', [$inputInterface, $outputInterface])); + } + + public function testExecuteWithException() { + $inputInterface = $this->createMock(InputInterface::class); + $outputInterface = $this->createMock(OutputInterface::class); + + $inputInterface + ->expects($this->at(0)) + ->method('getOption') + ->with('path') + ->will($this->returnValue('AppId')); + $inputInterface + ->expects($this->at(1)) + ->method('getOption') + ->with('privateKey') + ->will($this->returnValue('privateKey')); + $inputInterface + ->expects($this->at(2)) + ->method('getOption') + ->with('certificate') + ->will($this->returnValue('certificate')); + + $this->fileAccessHelper + ->expects($this->at(0)) + ->method('file_get_contents') + ->with('privateKey') + ->will($this->returnValue(\OC::$SERVERROOT . '/tests/data/integritycheck/core.key')); + $this->fileAccessHelper + ->expects($this->at(1)) + ->method('file_get_contents') + ->with('certificate') + ->will($this->returnValue(\OC::$SERVERROOT . '/tests/data/integritycheck/core.crt')); + + $this->checker + ->expects($this->once()) + ->method('writeAppSignature') + ->willThrowException(new \Exception('My error message')); + + $outputInterface + ->expects($this->at(0)) + ->method('writeln') + ->with('Error: My error message'); + + $this->assertSame(1, self::invokePrivate($this->signApp, 'execute', [$inputInterface, $outputInterface])); } public function testExecute() { @@ -247,6 +291,6 @@ class SignAppTest extends TestCase { ->method('writeln') ->with('Successfully signed "AppId"'); - $this->invokePrivate($this->signApp, 'execute', [$inputInterface, $outputInterface]); + $this->assertSame(0, self::invokePrivate($this->signApp, 'execute', [$inputInterface, $outputInterface])); } } diff --git a/tests/lib/Command/Integrity/SignCoreTest.php b/tests/lib/Command/Integrity/SignCoreTest.php index f3c242ae9f..35e52b951d 100644 --- a/tests/lib/Command/Integrity/SignCoreTest.php +++ b/tests/lib/Command/Integrity/SignCoreTest.php @@ -28,12 +28,12 @@ use Symfony\Component\Console\Output\OutputInterface; use Test\TestCase; class SignCoreTest extends TestCase { - /** @var Checker */ + /** @var Checker|\PHPUnit_Framework_MockObject_MockObject */ private $checker; + /** @var FileAccessHelper|\PHPUnit_Framework_MockObject_MockObject */ + private $fileAccessHelper; /** @var SignCore */ private $signCore; - /** @var FileAccessHelper */ - private $fileAccessHelper; public function setUp() { parent::setUp(); @@ -65,7 +65,7 @@ class SignCoreTest extends TestCase { ->method('writeln') ->with('--privateKey, --certificate and --path are required.'); - $this->invokePrivate($this->signCore, 'execute', [$inputInterface, $outputInterface]); + $this->assertNull(self::invokePrivate($this->signCore, 'execute', [$inputInterface, $outputInterface])); } public function testExecuteWithMissingCertificate() { @@ -88,7 +88,7 @@ class SignCoreTest extends TestCase { ->method('writeln') ->with('--privateKey, --certificate and --path are required.'); - $this->invokePrivate($this->signCore, 'execute', [$inputInterface, $outputInterface]); + $this->assertNull(self::invokePrivate($this->signCore, 'execute', [$inputInterface, $outputInterface])); } public function testExecuteWithNotExistingPrivateKey() { @@ -122,7 +122,7 @@ class SignCoreTest extends TestCase { ->method('writeln') ->with('Private key "privateKey" does not exists.'); - $this->invokePrivate($this->signCore, 'execute', [$inputInterface, $outputInterface]); + $this->assertNull(self::invokePrivate($this->signCore, 'execute', [$inputInterface, $outputInterface])); } public function testExecuteWithNotExistingCertificate() { @@ -161,7 +161,51 @@ class SignCoreTest extends TestCase { ->method('writeln') ->with('Certificate "certificate" does not exists.'); - $this->invokePrivate($this->signCore, 'execute', [$inputInterface, $outputInterface]); + $this->assertNull(self::invokePrivate($this->signCore, 'execute', [$inputInterface, $outputInterface])); + } + + public function testExecuteWithException() { + $inputInterface = $this->createMock(InputInterface::class); + $outputInterface = $this->createMock(OutputInterface::class); + + $inputInterface + ->expects($this->at(0)) + ->method('getOption') + ->with('privateKey') + ->will($this->returnValue('privateKey')); + $inputInterface + ->expects($this->at(1)) + ->method('getOption') + ->with('certificate') + ->will($this->returnValue('certificate')); + $inputInterface + ->expects($this->at(2)) + ->method('getOption') + ->with('path') + ->will($this->returnValue('certificate')); + + $this->fileAccessHelper + ->expects($this->at(0)) + ->method('file_get_contents') + ->with('privateKey') + ->will($this->returnValue(\OC::$SERVERROOT . '/tests/data/integritycheck/core.key')); + $this->fileAccessHelper + ->expects($this->at(1)) + ->method('file_get_contents') + ->with('certificate') + ->will($this->returnValue(\OC::$SERVERROOT . '/tests/data/integritycheck/core.crt')); + + $this->checker + ->expects($this->once()) + ->method('writeCoreSignature') + ->willThrowException(new \Exception('My exception message')); + + $outputInterface + ->expects($this->at(0)) + ->method('writeln') + ->with('Error: My exception message'); + + $this->assertEquals(1, self::invokePrivate($this->signCore, 'execute', [$inputInterface, $outputInterface])); } public function testExecute() { @@ -204,6 +248,6 @@ class SignCoreTest extends TestCase { ->method('writeln') ->with('Successfully signed "core"'); - $this->invokePrivate($this->signCore, 'execute', [$inputInterface, $outputInterface]); + $this->assertEquals(0, self::invokePrivate($this->signCore, 'execute', [$inputInterface, $outputInterface])); } } diff --git a/tests/lib/IntegrityCheck/CheckerTest.php b/tests/lib/IntegrityCheck/CheckerTest.php index 963a638bfd..049017cb5e 100644 --- a/tests/lib/IntegrityCheck/CheckerTest.php +++ b/tests/lib/IntegrityCheck/CheckerTest.php @@ -34,19 +34,19 @@ use OCP\ICacheFactory; use OCP\App\IAppManager; class CheckerTest extends TestCase { - /** @var EnvironmentHelper */ + /** @var EnvironmentHelper|\PHPUnit_Framework_MockObject_MockObject */ private $environmentHelper; - /** @var AppLocator */ + /** @var AppLocator|\PHPUnit_Framework_MockObject_MockObject */ private $appLocator; /** @var Checker */ private $checker; - /** @var FileAccessHelper */ + /** @var FileAccessHelper|\PHPUnit_Framework_MockObject_MockObject */ private $fileAccessHelper; - /** @var IConfig */ + /** @var IConfig|\PHPUnit_Framework_MockObject_MockObject */ private $config; - /** @var ICacheFactory */ + /** @var ICacheFactory|\PHPUnit_Framework_MockObject_MockObject */ private $cacheFactory; - /** @var IAppManager */ + /** @var IAppManager|\PHPUnit_Framework_MockObject_MockObject */ private $appManager; public function setUp() { @@ -77,8 +77,20 @@ class CheckerTest extends TestCase { /** * @expectedException \Exception + * @expectedExceptionMessage Exception message */ public function testWriteAppSignatureOfNotExistingApp() { + $this->fileAccessHelper + ->expects($this->at(0)) + ->method('assertDirectoryExists') + ->with('NotExistingApp/appinfo') + ->willThrowException(new \Exception('Exception message')); + $this->fileAccessHelper + ->expects($this->at(1)) + ->method('is_writable') + ->with('NotExistingApp/appinfo') + ->willReturn(true); + $keyBundle = file_get_contents(__DIR__ .'/../../data/integritycheck/SomeApp.crt'); $rsaPrivateKey = file_get_contents(__DIR__ .'/../../data/integritycheck/SomeApp.key'); $rsa = new RSA(); @@ -90,13 +102,14 @@ class CheckerTest extends TestCase { /** * @expectedException \Exception + * @expectedExceptionMessageRegExp /[a-zA-Z\/_-]+ is not writable/ */ - public function testWriteAppSignatureWrongPermissions(){ + public function testWriteAppSignatureWrongPermissions() { $this->fileAccessHelper ->expects($this->once()) ->method('file_put_contents') - ->will($this->throwException(new \Exception)) - ; + ->will($this->throwException(new \Exception('Exception message'))); + $keyBundle = file_get_contents(__DIR__ .'/../../data/integritycheck/SomeApp.crt'); $rsaPrivateKey = file_get_contents(__DIR__ .'/../../data/integritycheck/SomeApp.key'); $rsa = new RSA(); @@ -460,6 +473,54 @@ class CheckerTest extends TestCase { $this->assertSame([], $this->checker->verifyAppSignature('SomeApp')); } + /** + * @expectedException \Exception + * @expectedExceptionMessage Exception message + */ + public function testWriteCoreSignatureWithException() { + $this->fileAccessHelper + ->expects($this->at(0)) + ->method('assertDirectoryExists') + ->will($this->throwException(new \Exception('Exception message'))); + $this->fileAccessHelper + ->expects($this->at(1)) + ->method('is_writable') + ->with(__DIR__ . '/core') + ->willReturn(true); + + $keyBundle = file_get_contents(__DIR__ .'/../../data/integritycheck/SomeApp.crt'); + $rsaPrivateKey = file_get_contents(__DIR__ .'/../../data/integritycheck/SomeApp.key'); + $rsa = new RSA(); + $rsa->loadKey($rsaPrivateKey); + $x509 = new X509(); + $x509->loadX509($keyBundle); + $this->checker->writeCoreSignature($x509, $rsa, __DIR__); + } + + /** + * @expectedException \Exception + * @expectedExceptionMessageRegExp /[a-zA-Z\/_-]+ is not writable/ + */ + public function testWriteCoreSignatureWrongPermissions() { + $this->fileAccessHelper + ->expects($this->at(0)) + ->method('assertDirectoryExists') + ->will($this->throwException(new \Exception('Exception message'))); + $this->fileAccessHelper + ->expects($this->at(1)) + ->method('is_writable') + ->with(__DIR__ . '/core') + ->willReturn(false); + + $keyBundle = file_get_contents(__DIR__ .'/../../data/integritycheck/SomeApp.crt'); + $rsaPrivateKey = file_get_contents(__DIR__ .'/../../data/integritycheck/SomeApp.key'); + $rsa = new RSA(); + $rsa->loadKey($rsaPrivateKey); + $x509 = new X509(); + $x509->loadX509($keyBundle); + $this->checker->writeCoreSignature($x509, $rsa, __DIR__); + } + public function testWriteCoreSignature() { $expectedSignatureFileData = '{ "hashes": { @@ -965,7 +1026,7 @@ class CheckerTest extends TestCase { ->method('verifyCoreSignature'); $this->appLocator ->expects($this->at(0)) - ->Method('getAllApps') + ->method('getAllApps') ->will($this->returnValue([ 'files', 'calendar', @@ -1091,7 +1152,6 @@ class CheckerTest extends TestCase { ->with('integrity.check.disabled', false) ->will($this->returnValue(true)); - $result = $this->invokePrivate($this->checker, 'isCodeCheckEnforced'); - $this->assertSame(false, $result); + $this->assertFalse(self::invokePrivate($this->checker, 'isCodeCheckEnforced')); } } diff --git a/tests/lib/IntegrityCheck/Helpers/FileAccessHelperTest.php b/tests/lib/IntegrityCheck/Helpers/FileAccessHelperTest.php index 740b14e61c..de4aeec78c 100644 --- a/tests/lib/IntegrityCheck/Helpers/FileAccessHelperTest.php +++ b/tests/lib/IntegrityCheck/Helpers/FileAccessHelperTest.php @@ -40,4 +40,29 @@ class FileAccessHelperTest extends TestCase { $this->fileAccessHelper->file_put_contents($filePath, $data); $this->assertSame($data, $this->fileAccessHelper->file_get_contents($filePath)); } + + /** + * @expectedException \Exception + * @expectedExceptionMessage Failed to write into /anabsolutelynotexistingfolder/on/the/system.txt + */ + public function testFile_put_contentsWithException() { + $this->fileAccessHelper->file_put_contents('/anabsolutelynotexistingfolder/on/the/system.txt', 'MyFiles'); + } + + public function testIs_writable() { + $this->assertFalse($this->fileAccessHelper->is_writable('/anabsolutelynotexistingfolder/on/the/system.txt')); + $this->assertTrue($this->fileAccessHelper->is_writable(\OC::$server->getTempManager()->getTemporaryFile('MyFile'))); + } + + /** + * @expectedException \Exception + * @expectedExceptionMessage Directory /anabsolutelynotexistingfolder/on/the/system does not exist. + */ + public function testAssertDirectoryExistsWithException() { + $this->fileAccessHelper->assertDirectoryExists('/anabsolutelynotexistingfolder/on/the/system'); + } + + public function testAssertDirectoryExists() { + $this->fileAccessHelper->assertDirectoryExists(\OC::$server->getTempManager()->getTemporaryFolder('/testfolder/')); + } }