a new approach to display the error message

This commit is contained in:
Bjoern Schiessle 2015-05-12 18:49:25 +02:00
parent 73a3086945
commit 887be709f5
14 changed files with 209 additions and 57 deletions

View file

@ -209,7 +209,8 @@ class Application extends \OCP\AppFramework\App {
$c->query('Crypt'),
$server->getLogger(),
$server->getUserSession(),
$server->getConfig());
$server->getConfig(),
$server->getUserManager());
});
}

View file

@ -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);
@ -345,6 +354,36 @@ class Encryption implements IEncryptionModule {
return 6126;
}
/**
* 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;
}
}

View file

@ -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;
}
}

View file

@ -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(),

View file

@ -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);
}
/**

View file

@ -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) {

View file

@ -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);
}
}

View file

@ -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);
}
}
} elseif ($get_type === self::ZIP_DIR) {
$file = $dir . '/' . $files;
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');
}
$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);
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);
}
$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);
}
}

View file

@ -252,6 +252,30 @@ class Encryption extends Wrapper {
return $result;
}
/**
* 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
*

View file

@ -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;
}
}

View file

@ -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);
}

View file

@ -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,

View file

@ -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;
}

View file

@ -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])) {