Merge pull request #5174 from nextcloud/new-encryption-default

Use the master key by default

4 files changed, 59 insertions, 19 deletions

diff --git a/tests/Settings/Controller/UsersControllerTest.php b/tests/Settings/Controller/UsersControllerTest.php
index 0780f5219c0..cd08c834147 100644
--- a/tests/Settings/Controller/UsersControllerTest.php
+++ b/tests/Settings/Controller/UsersControllerTest.php
@@ -20,6 +20,8 @@ use OCP\AppFramework\Http\DataResponse;
 use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\BackgroundJob\IJobList;
 use OCP\Files\Config\IUserMountCache;
+use OCP\Encryption\IEncryptionModule;
+use OCP\Encryption\IManager;
 use OCP\IAvatar;
 use OCP\IAvatarManager;
 use OCP\IConfig;
@@ -82,6 +84,10 @@ class UsersControllerTest extends \Test\TestCase {
 	private $securityManager;
 	/** @var IUserMountCache |\PHPUnit_Framework_MockObject_MockObject */
 	private $userMountCache;
+	/** @var  IManager | \PHPUnit_Framework_MockObject_MockObject */
+	private $encryptionManager;
+	/** @var  IEncryptionModule  | \PHPUnit_Framework_MockObject_MockObject */
+	private $encryptionModule;
 
 	protected function setUp() {
 		parent::setUp();
@@ -104,6 +110,7 @@ class UsersControllerTest extends \Test\TestCase {
 		$this->crypto = $this->createMock(ICrypto::class);
 		$this->securityManager = $this->getMockBuilder(\OC\Security\IdentityProof\Manager::class)->disableOriginalConstructor()->getMock();
 		$this->jobList = $this->createMock(IJobList::class);
+		$this->encryptionManager = $this->createMock(IManager::class);
 		$this->l = $this->createMock(IL10N::class);
 		$this->l->method('t')
 			->will($this->returnCallback(function ($text, $parameters = []) {
@@ -111,6 +118,10 @@ class UsersControllerTest extends \Test\TestCase {
 			}));
 		$this->userMountCache = $this->createMock(IUserMountCache::class);
 
+		$this->encryptionModule = $this->createMock(IEncryptionModule::class);
+		$this->encryptionManager->expects($this->any())->method('getEncryptionModules')
+			->willReturn(['encryptionModule' => ['callback' => function() { return $this->encryptionModule;}]]);
+
 		/*
 		 * Set default avatar behaviour for whole test suite
 		 */
@@ -154,8 +165,8 @@ class UsersControllerTest extends \Test\TestCase {
 				$this->crypto,
 				$this->securityManager,
 				$this->jobList,
-				$this->userMountCache
-
+				$this->userMountCache,
+				$this->encryptionManager
 			);
 		} else {
 			return $this->getMockBuilder(UsersController::class)
@@ -182,6 +193,7 @@ class UsersControllerTest extends \Test\TestCase {
 						$this->securityManager,
 						$this->jobList,
 						$this->userMountCache,
+						$this->encryptionManager
 					]
 				)->setMethods($mockedMethods)->getMock();
 		}
@@ -1689,9 +1701,17 @@ class UsersControllerTest extends \Test\TestCase {
 		$this->assertEquals($expectedResult, $result);
 	}
 
-	public function testRestoreNotPossibleWithoutAdminRestore() {
+	/**
+	 * @dataProvider dataTestRestoreNotPossibleWithoutAdminRestore
+	 *
+	 * @param bool $masterKeyEnabled
+	 */
+	public function testRestoreNotPossibleWithoutAdminRestore($masterKeyEnabled) {
 		list($user, $expectedResult) = $this->mockUser();
 
+		// without the master key enabled we use per-user keys
+		$this->encryptionModule->expects($this->once())->method('needDetailedAccessList')->willReturn(!$masterKeyEnabled);
+
 		$this->appManager
 			->method('isEnabledForUser')
 			->with(
@@ -1699,7 +1719,8 @@ class UsersControllerTest extends \Test\TestCase {
 			)
 			->will($this->returnValue(true));
 
-		$expectedResult['isRestoreDisabled'] = true;
+		// without the master key enabled we use per-user keys -> restore is disabled
+		$expectedResult['isRestoreDisabled'] = !$masterKeyEnabled;
 
 		$subadmin = $this->getMockBuilder('\OC\SubAdmin')
 			->disableOriginalConstructor()
@@ -1718,6 +1739,13 @@ class UsersControllerTest extends \Test\TestCase {
 		$this->assertEquals($expectedResult, $result);
 	}
 
+	public function dataTestRestoreNotPossibleWithoutAdminRestore() {
+		return [
+			[true],
+			[false]
+		];
+	}
+
 	public function testRestoreNotPossibleWithoutUserRestore() {
 		list($user, $expectedResult) = $this->mockUser();
 
diff --git a/tests/lib/Files/Storage/Wrapper/EncryptionTest.php b/tests/lib/Files/Storage/Wrapper/EncryptionTest.php
index d310f110b94..a66ff14a778 100644
--- a/tests/lib/Files/Storage/Wrapper/EncryptionTest.php
+++ b/tests/lib/Files/Storage/Wrapper/EncryptionTest.php
@@ -212,7 +212,7 @@ class EncryptionTest extends Storage {
 	protected function buildMockModule() {
 		$this->encryptionModule = $this->getMockBuilder('\OCP\Encryption\IEncryptionModule')
 			->disableOriginalConstructor()
-			->setMethods(['getId', 'getDisplayName', 'begin', 'end', 'encrypt', 'decrypt', 'update', 'shouldEncrypt', 'getUnencryptedBlockSize', 'isReadable', 'encryptAll', 'prepareDecryptAll', 'isReadyForUser'])
+			->setMethods(['getId', 'getDisplayName', 'begin', 'end', 'encrypt', 'decrypt', 'update', 'shouldEncrypt', 'getUnencryptedBlockSize', 'isReadable', 'encryptAll', 'prepareDecryptAll', 'isReadyForUser', 'needDetailedAccessList'])
 			->getMock();
 
 		$this->encryptionModule->expects($this->any())->method('getId')->willReturn('UNIT_TEST_MODULE');
@@ -225,6 +225,7 @@ class EncryptionTest extends Storage {
 		$this->encryptionModule->expects($this->any())->method('shouldEncrypt')->willReturn(true);
 		$this->encryptionModule->expects($this->any())->method('getUnencryptedBlockSize')->willReturn(8192);
 		$this->encryptionModule->expects($this->any())->method('isReadable')->willReturn(true);
+		$this->encryptionModule->expects($this->any())->method('needDetailedAccessList')->willReturn(false);
 		return $this->encryptionModule;
 	}
 
diff --git a/tests/lib/Files/Stream/EncryptionTest.php b/tests/lib/Files/Stream/EncryptionTest.php
index e072dd6718d..1dc9dca0aad 100644
--- a/tests/lib/Files/Stream/EncryptionTest.php
+++ b/tests/lib/Files/Stream/EncryptionTest.php
@@ -58,7 +58,8 @@ class EncryptionTest extends \Test\TestCase {
 	/**
 	 * @dataProvider dataProviderStreamOpen()
 	 */
-	public function testStreamOpen($mode,
+	public function testStreamOpen($isMasterKeyUsed,
+								   $mode,
 								   $fullPath,
 								   $fileExists,
 								   $expectedSharePath,
@@ -69,6 +70,7 @@ class EncryptionTest extends \Test\TestCase {
 		// build mocks
 		$encryptionModuleMock = $this->getMockBuilder('\OCP\Encryption\IEncryptionModule')
 		->disableOriginalConstructor()->getMock();
+		$encryptionModuleMock->expects($this->any())->method('needDetailedAccessList')->willReturn(!$isMasterKeyUsed);
 		$encryptionModuleMock->expects($this->once())
 			->method('getUnencryptedBlockSize')->willReturn(99);
 		$encryptionModuleMock->expects($this->once())
@@ -80,12 +82,15 @@ class EncryptionTest extends \Test\TestCase {
 
 		$fileMock = $this->getMockBuilder('\OC\Encryption\File')
 			->disableOriginalConstructor()->getMock();
-		$fileMock->expects($this->once())->method('getAccessList')
-			->will($this->returnCallback(function($sharePath) use ($expectedSharePath) {
-				$this->assertSame($expectedSharePath, $sharePath);
-				return array();
-			}));
-
+		if ($isMasterKeyUsed) {
+			$fileMock->expects($this->never())->method('getAccessList');
+		} else {
+			$fileMock->expects($this->once())->method('getAccessList')
+				->will($this->returnCallback(function ($sharePath) use ($expectedSharePath) {
+					$this->assertSame($expectedSharePath, $sharePath);
+					return array();
+				}));
+		}
 		$utilMock = $this->getMockBuilder('\OC\Encryption\Util')
 			->disableOriginalConstructor()->getMock();
 		$utilMock->expects($this->any())
@@ -152,11 +157,14 @@ class EncryptionTest extends \Test\TestCase {
 	}
 
 	public function dataProviderStreamOpen() {
-		return array(
-			array('r', '/foo/bar/test.txt', true, '/foo/bar/test.txt', null, null, true),
-			array('r', '/foo/bar/test.txt', false, '/foo/bar', null, null, true),
-			array('w', '/foo/bar/test.txt', true, '/foo/bar/test.txt', 8192, 0, false),
-		);
+		return [
+			[false, 'r', '/foo/bar/test.txt', true, '/foo/bar/test.txt', null, null, true],
+			[false, 'r', '/foo/bar/test.txt', false, '/foo/bar', null, null, true],
+			[false, 'w', '/foo/bar/test.txt', true, '/foo/bar/test.txt', 8192, 0, false],
+			[true, 'r', '/foo/bar/test.txt', true, '/foo/bar/test.txt', null, null, true],
+			[true, 'r', '/foo/bar/test.txt', false, '/foo/bar', null, null, true],
+			[true, 'w', '/foo/bar/test.txt', true, '/foo/bar/test.txt', 8192, 0, false],
+		];
 	}
 
 	public function testWriteRead() {
@@ -193,7 +201,7 @@ class EncryptionTest extends \Test\TestCase {
 		$stream = $this->getStream($fileName, 'r', 6);
 		$this->assertEquals('barbar', fread($stream, 100));
 		fclose($stream);
-	
+
 		unlink($fileName);
 }
 
@@ -311,7 +319,7 @@ class EncryptionTest extends \Test\TestCase {
 	protected function buildMockModule() {
 		$encryptionModule = $this->getMockBuilder('\OCP\Encryption\IEncryptionModule')
 			->disableOriginalConstructor()
-			->setMethods(['getId', 'getDisplayName', 'begin', 'end', 'encrypt', 'decrypt', 'update', 'shouldEncrypt', 'getUnencryptedBlockSize', 'isReadable', 'encryptAll', 'prepareDecryptAll', 'isReadyForUser'])
+			->setMethods(['getId', 'getDisplayName', 'begin', 'end', 'encrypt', 'decrypt', 'update', 'shouldEncrypt', 'getUnencryptedBlockSize', 'isReadable', 'encryptAll', 'prepareDecryptAll', 'isReadyForUser', 'needDetailedAccessList'])
 			->getMock();
 
 		$encryptionModule->expects($this->any())->method('getId')->willReturn('UNIT_TEST_MODULE');
@@ -319,6 +327,7 @@ class EncryptionTest extends \Test\TestCase {
 		$encryptionModule->expects($this->any())->method('begin')->willReturn([]);
 		$encryptionModule->expects($this->any())->method('end')->willReturn('');
 		$encryptionModule->expects($this->any())->method('isReadable')->willReturn(true);
+		$encryptionModule->expects($this->any())->method('needDetailedAccessList')->willReturn(false);
 		$encryptionModule->expects($this->any())->method('encrypt')->willReturnCallback(function($data) {
 			// simulate different block size by adding some padding to the data
 			if (isset($data[6125])) {
diff --git a/tests/lib/Traits/EncryptionTrait.php b/tests/lib/Traits/EncryptionTrait.php
index 5e2ca4e561f..8a06d37fa7f 100644
--- a/tests/lib/Traits/EncryptionTrait.php
+++ b/tests/lib/Traits/EncryptionTrait.php
@@ -64,6 +64,7 @@ trait EncryptionTrait {
 		/** @var Setup $userSetup */
 		$userSetup = $container->query('UserSetup');
 		$userSetup->setupUser($name, $password);
+		$this->encryptionApp->setUp();
 		$keyManager->init($name, $password);
 	}
 
@@ -99,6 +100,7 @@ trait EncryptionTrait {
 		if ($this->config) {
 			$this->config->setAppValue('core', 'encryption_enabled', $this->encryptionWasEnabled);
 			$this->config->setAppValue('core', 'default_encryption_module', $this->originalEncryptionModule);
+			$this->config->deleteAppValue('encryption', 'useMasterKey');
 		}
 	}
 }