[Pkg-owncloud-commits] [owncloud] 112/123: a new approach to display the error message
David Prévot
taffit at moszumanska.debian.org
Tue May 19 23:55:25 UTC 2015
This is an automated email from the git hooks/post-receive script.
taffit pushed a commit to branch stable8
in repository owncloud.
commit 887be709f5f0dbbc6ad7b1cc1a9793d04421c5b9
Author: Bjoern Schiessle <schiessle at owncloud.com>
Date: Tue May 12 18:49:25 2015 +0200
a new approach to display the error message
---
apps/encryption/appinfo/application.php | 3 +-
apps/encryption/lib/crypto/encryption.php | 40 ++++++++++
apps/encryption/lib/util.php | 31 +++++++-
apps/encryption/settings/settings-personal.php | 3 +-
apps/encryption/tests/lib/UtilTest.php | 18 +++--
apps/files_sharing/lib/sharedstorage.php | 8 +-
.../exceptions/decryptionfailedexception.php | 11 +++
lib/private/files.php | 88 ++++++++++++----------
lib/private/files/storage/wrapper/encryption.php | 24 ++++++
.../exceptions/genericencryptionexception.php | 11 ++-
lib/public/encryption/iencryptionmodule.php | 12 +++
settings/changepassword/controller.php | 3 +-
tests/lib/files/storage/wrapper/encryption.php | 9 ++-
tests/lib/files/stream/encryption.php | 3 +-
14 files changed, 208 insertions(+), 56 deletions(-)
diff --git a/apps/encryption/appinfo/application.php b/apps/encryption/appinfo/application.php
index 0d6f57f..79b2ad3 100644
--- a/apps/encryption/appinfo/application.php
+++ b/apps/encryption/appinfo/application.php
@@ -209,7 +209,8 @@ class Application extends \OCP\AppFramework\App {
$c->query('Crypt'),
$server->getLogger(),
$server->getUserSession(),
- $server->getConfig());
+ $server->getConfig(),
+ $server->getUserManager());
});
}
diff --git a/apps/encryption/lib/crypto/encryption.php b/apps/encryption/lib/crypto/encryption.php
index a4abcd7..0fd85fa 100644
--- a/apps/encryption/lib/crypto/encryption.php
+++ b/apps/encryption/lib/crypto/encryption.php
@@ -270,6 +270,15 @@ class Encryption implements IEncryptionModule {
* @return mixed decrypted data
*/
public function decrypt($data) {
+ if (empty($this->fileKey)) {
+ $msg = $this->l->t('Can not decrypt this file, probably this is a shared file. Please ask the file owner to reshare the file with you.');
+ $this->logger->error('Can not decrypt this file,
+ probably this is a shared file.
+ Please ask the file owner to reshare the file with you.');
+
+ throw new DecryptionFailedException($msg);
+ }
+
$result = '';
if (!empty($data)) {
$result = $this->crypt->symmetricDecryptFileContent($data, $this->fileKey, $this->cipher);
@@ -346,6 +355,36 @@ class Encryption implements IEncryptionModule {
}
/**
+ * check if the encryption module is able to read the file,
+ * e.g. if all encryption keys exists
+ *
+ * @param string $path
+ * @param string $uid user for whom we want to check if he can read the file
+ * @return bool
+ * @throws DecryptionFailedException
+ */
+ public function isReadable($path, $uid) {
+ $fileKey = $this->keyManager->getFileKey($path, $uid);
+ if (empty($fileKey)) {
+ $owner = $this->util->getOwner($path);
+ if ($owner !== $uid) {
+ // if it is a shared file we throw a exception with a useful
+ // error message because in this case it means that the file was
+ // shared with the user at a point where the user didn't had a
+ // valid private/public key
+ $msg = 'Encryption module "' . $this->getDisplayName() .
+ '" is not able to read ' . $path;
+ $hint = $this->l->t('Can not read this file, probably this is a shared file. Please ask the file owner to reshare the file with you.');
+ $this->logger->warning($msg);
+ throw new DecryptionFailedException($msg, 0, null, $hint);
+ }
+ return false;
+ }
+
+ return true;
+ }
+
+ /**
* @param string $path
* @return string
*/
@@ -360,4 +399,5 @@ class Encryption implements IEncryptionModule {
return $realPath;
}
+
}
diff --git a/apps/encryption/lib/util.php b/apps/encryption/lib/util.php
index 51d5241..afed96a 100644
--- a/apps/encryption/lib/util.php
+++ b/apps/encryption/lib/util.php
@@ -29,6 +29,7 @@ use OCA\Encryption\Crypto\Crypt;
use OCP\IConfig;
use OCP\ILogger;
use OCP\IUser;
+use OCP\IUserManager;
use OCP\IUserSession;
use OCP\PreConditionNotMetException;
@@ -53,6 +54,10 @@ class Util {
* @var IConfig
*/
private $config;
+ /**
+ * @var IUserManager
+ */
+ private $userManager;
/**
* Util constructor.
@@ -62,18 +67,21 @@ class Util {
* @param ILogger $logger
* @param IUserSession $userSession
* @param IConfig $config
+ * @param IUserManager $userManager
*/
public function __construct(View $files,
Crypt $crypt,
ILogger $logger,
IUserSession $userSession,
- IConfig $config
+ IConfig $config,
+ IUserManager $userManager
) {
$this->files = $files;
$this->crypt = $crypt;
$this->logger = $logger;
$this->user = $userSession && $userSession->isLoggedIn() ? $userSession->getUser() : false;
$this->config = $config;
+ $this->userManager = $userManager;
}
/**
@@ -117,5 +125,26 @@ class Util {
return $this->files->file_exists($uid . '/files');
}
+ /**
+ * get owner from give path, path relative to data/ expected
+ *
+ * @param string $path relative to data/
+ * @return string
+ * @throws \BadMethodCallException
+ */
+ public function getOwner($path) {
+ $owner = '';
+ $parts = explode('/', $path, 3);
+ if (count($parts) > 1) {
+ $owner = $parts[1];
+ if ($this->userManager->userExists($owner) === false) {
+ throw new \BadMethodCallException('Unknown user: ' .
+ 'method expects path to a user folder relative to the data folder');
+ }
+
+ }
+
+ return $owner;
+ }
}
diff --git a/apps/encryption/settings/settings-personal.php b/apps/encryption/settings/settings-personal.php
index 003a27d..3815626 100644
--- a/apps/encryption/settings/settings-personal.php
+++ b/apps/encryption/settings/settings-personal.php
@@ -35,7 +35,8 @@ $util = new \OCA\Encryption\Util(
$crypt,
\OC::$server->getLogger(),
$userSession,
- \OC::$server->getConfig());
+ \OC::$server->getConfig(),
+ \OC::$server->getUserManager());
$keyManager = new \OCA\Encryption\KeyManager(
\OC::$server->getEncryptionKeyStorage(),
diff --git a/apps/encryption/tests/lib/UtilTest.php b/apps/encryption/tests/lib/UtilTest.php
index eab912b..18cf038 100644
--- a/apps/encryption/tests/lib/UtilTest.php
+++ b/apps/encryption/tests/lib/UtilTest.php
@@ -28,11 +28,17 @@ use Test\TestCase;
class UtilTest extends TestCase {
private static $tempStorage = [];
+
+ /** @var \PHPUnit_Framework_MockObject_MockObject */
private $configMock;
+
+ /** @var \PHPUnit_Framework_MockObject_MockObject */
private $filesMock;
- /**
- * @var Util
- */
+
+ /** @var \PHPUnit_Framework_MockObject_MockObject */
+ private $userManagerMock;
+
+ /** @var Util */
private $instance;
public function testSetRecoveryForUser() {
@@ -40,9 +46,6 @@ class UtilTest extends TestCase {
$this->assertArrayHasKey('recoveryEnabled', self::$tempStorage);
}
- /**
- *
- */
public function testIsRecoveryEnabledForUser() {
$this->assertTrue($this->instance->isRecoveryEnabledForUser('admin'));
@@ -62,6 +65,7 @@ class UtilTest extends TestCase {
protected function setUp() {
parent::setUp();
$this->filesMock = $this->getMock('OC\Files\View');
+ $this->userManagerMock = $this->getMock('\OCP\IUserManager');
$cryptMock = $this->getMockBuilder('OCA\Encryption\Crypto\Crypt')
->disableOriginalConstructor()
@@ -98,7 +102,7 @@ class UtilTest extends TestCase {
->method('setUserValue')
->will($this->returnCallback([$this, 'setValueTester']));
- $this->instance = new Util($this->filesMock, $cryptMock, $loggerMock, $userSessionMock, $configMock);
+ $this->instance = new Util($this->filesMock, $cryptMock, $loggerMock, $userSessionMock, $configMock, $this->userManagerMock);
}
/**
diff --git a/apps/files_sharing/lib/sharedstorage.php b/apps/files_sharing/lib/sharedstorage.php
index 33b7f88..10a37c7 100644
--- a/apps/files_sharing/lib/sharedstorage.php
+++ b/apps/files_sharing/lib/sharedstorage.php
@@ -217,7 +217,13 @@ class Shared extends \OC\Files\Storage\Common implements ISharedStorage {
}
public function isReadable($path) {
- return $this->file_exists($path);
+ $isReadable = false;
+ if ($source = $this->getSourcePath($path)) {
+ list($storage, $internalPath) = \OC\Files\Filesystem::resolvePath($source);
+ $isReadable = $storage->isReadable($internalPath);
+ }
+
+ return $isReadable && $this->file_exists($path);
}
public function isUpdatable($path) {
diff --git a/lib/private/encryption/exceptions/decryptionfailedexception.php b/lib/private/encryption/exceptions/decryptionfailedexception.php
index 406ae12..7e9fa21 100644
--- a/lib/private/encryption/exceptions/decryptionfailedexception.php
+++ b/lib/private/encryption/exceptions/decryptionfailedexception.php
@@ -27,4 +27,15 @@ use OCP\Encryption\Exceptions\GenericEncryptionException;
class DecryptionFailedException extends GenericEncryptionException {
+ /**
+ * @param string $message
+ * @param int $code
+ * @param \Exception $previous
+ * @param string $hint
+ */
+ public function __construct($message = '', $code = 0, \Exception $previous = null, $hint = '') {
+ parent::__construct($message, $code, $previous, $hint);
+
+}
+
}
diff --git a/lib/private/files.php b/lib/private/files.php
index 97f9d81..6a739fc 100644
--- a/lib/private/files.php
+++ b/lib/private/files.php
@@ -129,52 +129,60 @@ class OC_Files {
$zip = new ZipStreamer(false);
}
OC_Util::obEnd();
- if ($zip or \OC\Files\Filesystem::isReadable($filename)) {
- self::sendHeaders($filename, $name, $zip);
- } elseif (!\OC\Files\Filesystem::file_exists($filename)) {
- header("HTTP/1.0 404 Not Found");
- $tmpl = new OC_Template('', '404', 'guest');
- $tmpl->printPage();
- } else {
- header("HTTP/1.0 403 Forbidden");
- die('403 Forbidden');
- }
- if($only_header) {
- return ;
- }
- if ($zip) {
- $executionTime = intval(ini_get('max_execution_time'));
- set_time_limit(0);
- if ($get_type === self::ZIP_FILES) {
- foreach ($files as $file) {
- $file = $dir . '/' . $file;
- if (\OC\Files\Filesystem::is_file($file)) {
- $fh = \OC\Files\Filesystem::fopen($file, 'r');
- $zip->addFileFromStream($fh, basename($file));
- fclose($fh);
- } elseif (\OC\Files\Filesystem::is_dir($file)) {
- self::zipAddDir($file, $zip);
+
+ try {
+
+ if ($zip or \OC\Files\Filesystem::isReadable($filename)) {
+ self::sendHeaders($filename, $name, $zip);
+ } elseif (!\OC\Files\Filesystem::file_exists($filename)) {
+ header("HTTP/1.0 404 Not Found");
+ $tmpl = new OC_Template('', '404', 'guest');
+ $tmpl->printPage();
+ } else {
+ header("HTTP/1.0 403 Forbidden");
+ die('403 Forbidden');
+ }
+ if ($only_header) {
+ return;
+ }
+ if ($zip) {
+ $executionTime = intval(ini_get('max_execution_time'));
+ set_time_limit(0);
+ if ($get_type === self::ZIP_FILES) {
+ foreach ($files as $file) {
+ $file = $dir . '/' . $file;
+ if (\OC\Files\Filesystem::is_file($file)) {
+ $fh = \OC\Files\Filesystem::fopen($file, 'r');
+ $zip->addFileFromStream($fh, basename($file));
+ fclose($fh);
+ } elseif (\OC\Files\Filesystem::is_dir($file)) {
+ self::zipAddDir($file, $zip);
+ }
}
+ } elseif ($get_type === self::ZIP_DIR) {
+ $file = $dir . '/' . $files;
+ self::zipAddDir($file, $zip);
}
- } elseif ($get_type === self::ZIP_DIR) {
- $file = $dir . '/' . $files;
- self::zipAddDir($file, $zip);
- }
- $zip->finalize();
- set_time_limit($executionTime);
- } else {
- if ($xsendfile) {
- $view = \OC\Files\Filesystem::getView();
- /** @var $storage \OC\Files\Storage\Storage */
- list($storage) = $view->resolvePath($filename);
- if ($storage->isLocal()) {
- self::addSendfileHeader($filename);
+ $zip->finalize();
+ set_time_limit($executionTime);
+ } else {
+ if ($xsendfile) {
+ $view = \OC\Files\Filesystem::getView();
+ /** @var $storage \OC\Files\Storage\Storage */
+ list($storage) = $view->resolvePath($filename);
+ if ($storage->isLocal()) {
+ self::addSendfileHeader($filename);
+ } else {
+ \OC\Files\Filesystem::readfile($filename);
+ }
} else {
\OC\Files\Filesystem::readfile($filename);
}
- } else {
- \OC\Files\Filesystem::readfile($filename);
}
+ } catch (\Exception $ex) {
+ $l = \OC::$server->getL10N('core');
+ $hint = method_exists($ex, 'getHint') ? $ex->getHint() : '';
+ \OC_Template::printErrorPage($l->t('Can\'t read file'), $hint);
}
}
diff --git a/lib/private/files/storage/wrapper/encryption.php b/lib/private/files/storage/wrapper/encryption.php
index f7759d9..1683ff1 100644
--- a/lib/private/files/storage/wrapper/encryption.php
+++ b/lib/private/files/storage/wrapper/encryption.php
@@ -253,6 +253,30 @@ class Encryption extends Wrapper {
}
/**
+ * check if a file can be read
+ *
+ * @param string $path
+ * @return bool
+ */
+ public function isReadable($path) {
+
+ $isReadable = true;
+
+ $metaData = $this->getMetaData($path);
+ if (
+ !$this->is_dir($path) &&
+ isset($metaData['encrypted']) &&
+ $metaData['encrypted'] === true
+ ) {
+ $fullPath = $this->getFullPath($path);
+ $module = $this->getEncryptionModule($path);
+ $isReadable = $module->isReadable($fullPath, $this->uid);
+ }
+
+ return $this->storage->isReadable($path) && $isReadable;
+ }
+
+ /**
* see http://php.net/manual/en/function.copy.php
*
* @param string $path1
diff --git a/lib/public/encryption/exceptions/genericencryptionexception.php b/lib/public/encryption/exceptions/genericencryptionexception.php
index be96450..e97f00c 100644
--- a/lib/public/encryption/exceptions/genericencryptionexception.php
+++ b/lib/public/encryption/exceptions/genericencryptionexception.php
@@ -30,17 +30,26 @@ namespace OCP\Encryption\Exceptions;
*/
class GenericEncryptionException extends \Exception {
+ /** @var string */
+ protected $hint;
+
/**
* @param string $message
* @param int $code
* @param \Exception $previous
* @since 8.1.0
*/
- public function __construct($message = '', $code = 0, \Exception $previous = null) {
+ public function __construct($message = '', $code = 0, \Exception $previous = null, $hint = '') {
if (empty($message)) {
$message = 'Unspecified encryption exception';
}
parent::__construct($message, $code, $previous);
+
+ $this->hint = $hint;
+ }
+
+ public function getHint() {
+ return $this->hint;
}
}
diff --git a/lib/public/encryption/iencryptionmodule.php b/lib/public/encryption/iencryptionmodule.php
index 0dda042..975e577 100644
--- a/lib/public/encryption/iencryptionmodule.php
+++ b/lib/public/encryption/iencryptionmodule.php
@@ -119,4 +119,16 @@ interface IEncryptionModule {
* @since 8.1.0
*/
public function getUnencryptedBlockSize();
+
+ /**
+ * check if the encryption module is able to read the file,
+ * e.g. if all encryption keys exists
+ *
+ * @param string $path
+ * @param string $uid user for whom we want to check if he can read the file
+ * @return boolean
+ * @since 8.1.0
+ */
+ public function isReadable($path, $uid);
+
}
diff --git a/settings/changepassword/controller.php b/settings/changepassword/controller.php
index 94323fc..69b7ca7 100644
--- a/settings/changepassword/controller.php
+++ b/settings/changepassword/controller.php
@@ -89,7 +89,8 @@ class Controller {
$crypt,
\OC::$server->getLogger(),
\OC::$server->getUserSession(),
- \OC::$server->getConfig());
+ \OC::$server->getConfig(),
+ \OC::$server->getUserManager());
$keyManager = new \OCA\Encryption\KeyManager(
$keyStorage,
$crypt,
diff --git a/tests/lib/files/storage/wrapper/encryption.php b/tests/lib/files/storage/wrapper/encryption.php
index 97810c9..361592f 100644
--- a/tests/lib/files/storage/wrapper/encryption.php
+++ b/tests/lib/files/storage/wrapper/encryption.php
@@ -117,7 +117,7 @@ class Encryption extends \Test\Files\Storage\Storage {
$this->encryptionManager, $this->util, $logger, $file, null, $this->keyStore, $this->update
]
)
- ->setMethods(['getMetaData', 'getCache'])
+ ->setMethods(['getMetaData', 'getCache', 'getEncryptionModule'])
->getMock();
$this->instance->expects($this->any())
@@ -127,6 +127,10 @@ class Encryption extends \Test\Files\Storage\Storage {
$this->instance->expects($this->any())
->method('getCache')
->willReturn($this->cache);
+
+ $this->instance->expects($this->any())
+ ->method('getEncryptionModule')
+ ->willReturn($mockModule);
}
/**
@@ -135,7 +139,7 @@ class Encryption extends \Test\Files\Storage\Storage {
protected function buildMockModule() {
$this->encryptionModule = $this->getMockBuilder('\OCP\Encryption\IEncryptionModule')
->disableOriginalConstructor()
- ->setMethods(['getId', 'getDisplayName', 'begin', 'end', 'encrypt', 'decrypt', 'update', 'shouldEncrypt', 'getUnencryptedBlockSize'])
+ ->setMethods(['getId', 'getDisplayName', 'begin', 'end', 'encrypt', 'decrypt', 'update', 'shouldEncrypt', 'getUnencryptedBlockSize', 'isReadable'])
->getMock();
$this->encryptionModule->expects($this->any())->method('getId')->willReturn('UNIT_TEST_MODULE');
@@ -147,6 +151,7 @@ class Encryption extends \Test\Files\Storage\Storage {
$this->encryptionModule->expects($this->any())->method('update')->willReturn(true);
$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);
return $this->encryptionModule;
}
diff --git a/tests/lib/files/stream/encryption.php b/tests/lib/files/stream/encryption.php
index 892491c..113cd14 100644
--- a/tests/lib/files/stream/encryption.php
+++ b/tests/lib/files/stream/encryption.php
@@ -253,13 +253,14 @@ class Encryption extends \Test\TestCase {
protected function buildMockModule() {
$encryptionModule = $this->getMockBuilder('\OCP\Encryption\IEncryptionModule')
->disableOriginalConstructor()
- ->setMethods(['getId', 'getDisplayName', 'begin', 'end', 'encrypt', 'decrypt', 'update', 'shouldEncrypt', 'getUnencryptedBlockSize'])
+ ->setMethods(['getId', 'getDisplayName', 'begin', 'end', 'encrypt', 'decrypt', 'update', 'shouldEncrypt', 'getUnencryptedBlockSize', 'isReadable'])
->getMock();
$encryptionModule->expects($this->any())->method('getId')->willReturn('UNIT_TEST_MODULE');
$encryptionModule->expects($this->any())->method('getDisplayName')->willReturn('Unit test module');
$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('encrypt')->willReturnCallback(function($data) {
// simulate different block size by adding some padding to the data
if (isset($data[6125])) {
--
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-owncloud/owncloud.git
More information about the Pkg-owncloud-commits
mailing list