diff --git a/apps/files_encryption/appinfo/update.php b/apps/files_encryption/appinfo/update.php new file mode 100644 index 0000000000..a29667ec6b --- /dev/null +++ b/apps/files_encryption/appinfo/update.php @@ -0,0 +1,10 @@ +dropTableEncryption(); +} diff --git a/apps/files_encryption/appinfo/version b/apps/files_encryption/appinfo/version index 2eb3c4fe4e..5a2a5806df 100644 --- a/apps/files_encryption/appinfo/version +++ b/apps/files_encryption/appinfo/version @@ -1 +1 @@ -0.5 +0.6 diff --git a/apps/files_encryption/hooks/hooks.php b/apps/files_encryption/hooks/hooks.php index d1ee4a97d1..66e2bccd59 100644 --- a/apps/files_encryption/hooks/hooks.php +++ b/apps/files_encryption/hooks/hooks.php @@ -530,8 +530,7 @@ class Hooks { public static function preDisable($params) { if ($params['app'] === 'files_encryption') { - $setMigrationStatus = \OC_DB::prepare('UPDATE `*PREFIX*encryption` SET `migration_status`=0'); - $setMigrationStatus->execute(); + \OC_Preferences::deleteAppFromAllUsers('files_encryption'); $session = new \OCA\Encryption\Session(new \OC\Files\View('/')); $session->setInitialized(\OCA\Encryption\Session::NOT_INITIALIZED); diff --git a/apps/files_encryption/lib/crypt.php b/apps/files_encryption/lib/crypt.php index 694791810c..ee2c4024e0 100755 --- a/apps/files_encryption/lib/crypt.php +++ b/apps/files_encryption/lib/crypt.php @@ -43,6 +43,7 @@ class Crypt { * return encryption mode client or server side encryption * @param string $user name (use system wide setting if name=null) * @return string 'client' or 'server' + * @note at the moment we only support server side encryption */ public static function mode($user = null) { diff --git a/apps/files_encryption/lib/migration.php b/apps/files_encryption/lib/migration.php new file mode 100644 index 0000000000..59389911b5 --- /dev/null +++ b/apps/files_encryption/lib/migration.php @@ -0,0 +1,50 @@ +. + * + */ + +namespace OCA\Files_Encryption; + + +class Migration { + + public function __construct($tableName = 'encryption') { + $this->tableName = $tableName; + } + + // migrate settings from oc_encryption to oc_preferences + public function dropTableEncryption() { + $tableName = $this->tableName; + if (!\OC_DB::tableExists($tableName)) { + return; + } + $sql = "select `uid`, max(`recovery_enabled`) as `recovery_enabled`, min(`migration_status`) as `migration_status` from `*PREFIX*$tableName` group by `uid`"; + $query = \OCP\DB::prepare($sql); + $result = $query->execute(array())->fetchAll(); + + foreach ($result as $row) { + \OC_Preferences::setValue($row['uid'], 'files_encryption', 'recovery_enabled', $row['recovery_enabled']); + \OC_Preferences::setValue($row['uid'], 'files_encryption', 'migration_status', $row['migration_status']); + } + + \OC_DB::dropTable($tableName); + } + +} diff --git a/apps/files_encryption/lib/util.php b/apps/files_encryption/lib/util.php index 434d23f4a5..ea2b1b10d6 100644 --- a/apps/files_encryption/lib/util.php +++ b/apps/files_encryption/lib/util.php @@ -194,22 +194,6 @@ class Util { } } - // If there's no record for this user's encryption preferences - if (false === $this->recoveryEnabledForUser()) { - - // create database configuration - $sql = 'INSERT INTO `*PREFIX*encryption` (`uid`,`mode`,`recovery_enabled`,`migration_status`) VALUES (?,?,?,?)'; - $args = array( - $this->userId, - 'server-side', - 0, - self::MIGRATION_OPEN - ); - $query = \OCP\DB::prepare($sql); - $query->execute($args); - - } - return true; } @@ -230,36 +214,9 @@ class Util { */ public function recoveryEnabledForUser() { - $sql = 'SELECT `recovery_enabled` FROM `*PREFIX*encryption` WHERE `uid` = ?'; + $recoveryMode = \OC_Preferences::getValue($this->userId, 'files_encryption', 'recovery_enabled', '0'); - $args = array($this->userId); - - $query = \OCP\DB::prepare($sql); - - $result = $query->execute($args); - - $recoveryEnabled = array(); - - if (\OCP\DB::isError($result)) { - \OCP\Util::writeLog('Encryption library', \OC_DB::getErrorMessage($result), \OCP\Util::ERROR); - } else { - $row = $result->fetchRow(); - if ($row && isset($row['recovery_enabled'])) { - $recoveryEnabled[] = $row['recovery_enabled']; - } - } - - // If no record is found - if (empty($recoveryEnabled)) { - - return false; - - // If a record is found - } else { - - return $recoveryEnabled[0]; - - } + return ($recoveryMode === '1') ? true : false; } @@ -270,32 +227,8 @@ class Util { */ public function setRecoveryForUser($enabled) { - $recoveryStatus = $this->recoveryEnabledForUser(); - - // If a record for this user already exists, update it - if (false === $recoveryStatus) { - - $sql = 'INSERT INTO `*PREFIX*encryption` (`uid`,`mode`,`recovery_enabled`) VALUES (?,?,?)'; - - $args = array( - $this->userId, - 'server-side', - $enabled - ); - - // Create a new record instead - } else { - - $sql = 'UPDATE `*PREFIX*encryption` SET `recovery_enabled` = ? WHERE `uid` = ?'; - - $args = array( - $enabled ? '1' : '0', - $this->userId - ); - - } - - return is_numeric(\OC_DB::executeAudited($sql, $args)); + $value = $enabled ? '1' : '0'; + return \OC_Preferences::setValue($this->userId, 'files_encryption', 'recovery_enabled', $value); } @@ -1133,24 +1066,16 @@ class Util { /** * set migration status * @param int $status + * @param int $preCondition only update migration status if the previous value equals $preCondition * @return boolean */ - private function setMigrationStatus($status) { + private function setMigrationStatus($status, $preCondition = null) { - $sql = 'UPDATE `*PREFIX*encryption` SET `migration_status` = ? WHERE `uid` = ?'; - $args = array($status, $this->userId); - $query = \OCP\DB::prepare($sql); - $manipulatedRows = $query->execute($args); + // convert to string if preCondition is set + $preCondition = ($preCondition === null) ? null : (string)$preCondition; - if ($manipulatedRows === 1) { - $result = true; - \OCP\Util::writeLog('Encryption library', "Migration status set to " . self::MIGRATION_OPEN, \OCP\Util::INFO); - } else { - $result = false; - \OCP\Util::writeLog('Encryption library', "Could not set migration status to " . self::MIGRATION_OPEN, \OCP\Util::WARN); - } + return \OC_Preferences::setValue($this->userId, 'files_encryption', 'migration_status', (string)$status, $preCondition); - return $result; } /** @@ -1159,7 +1084,7 @@ class Util { */ public function beginMigration() { - $result = $this->setMigrationStatus(self::MIGRATION_IN_PROGRESS); + $result = $this->setMigrationStatus(self::MIGRATION_IN_PROGRESS, self::MIGRATION_OPEN); if ($result) { \OCP\Util::writeLog('Encryption library', "Start migration to encryption mode for " . $this->userId, \OCP\Util::INFO); @@ -1199,46 +1124,16 @@ class Util { */ public function getMigrationStatus() { - $sql = 'SELECT `migration_status` FROM `*PREFIX*encryption` WHERE `uid` = ?'; - - $args = array($this->userId); - $query = \OCP\DB::prepare($sql); - - $result = $query->execute($args); - - $migrationStatus = array(); - - if (\OCP\DB::isError($result)) { - \OCP\Util::writeLog('Encryption library', \OC_DB::getErrorMessage($result), \OCP\Util::ERROR); - } else { - $row = $result->fetchRow(); - if ($row && isset($row['migration_status'])) { - $migrationStatus[] = $row['migration_status']; + $migrationStatus = false; + if (\OCP\User::userExists($this->userId)) { + $migrationStatus = \OC_Preferences::getValue($this->userId, 'files_encryption', 'migration_status'); + if ($migrationStatus === null) { + \OC_Preferences::setValue($this->userId, 'files_encryption', 'migration_status', (string)self::MIGRATION_OPEN); + $migrationStatus = self::MIGRATION_OPEN; } } - // If no record is found - if (empty($migrationStatus)) { - \OCP\Util::writeLog('Encryption library', "Could not get migration status for " . $this->userId . ", no record found", \OCP\Util::ERROR); - // insert missing entry in DB with status open if the user exists - if (\OCP\User::userExists($this->userId)) { - $sql = 'INSERT INTO `*PREFIX*encryption` (`uid`,`mode`,`recovery_enabled`,`migration_status`) VALUES (?,?,?,?)'; - $args = array( - $this->userId, - 'server-side', - 0, - self::MIGRATION_OPEN - ); - $query = \OCP\DB::prepare($sql); - $query->execute($args); - - return self::MIGRATION_OPEN; - } else { - return false; - } - } else { // If a record is found - return (int)$migrationStatus[0]; - } + return (int)$migrationStatus; } diff --git a/apps/files_encryption/templates/settings-personal.php b/apps/files_encryption/templates/settings-personal.php index 8139ece195..e9988df327 100644 --- a/apps/files_encryption/templates/settings-personal.php +++ b/apps/files_encryption/templates/settings-personal.php @@ -46,7 +46,7 @@ type='radio' name='userEnableRecovery' value='1' - /> + /> t( "Enabled" ) ); ?>
@@ -54,7 +54,7 @@ type='radio' name='userEnableRecovery' value='0' - /> + /> t( "Disabled" ) ); ?>
t( 'File recovery settings updated' ) ); ?>
t( 'Could not update file recovery' ) ); ?>
diff --git a/apps/files_encryption/appinfo/database.xml b/apps/files_encryption/tests/encryption_table.xml similarity index 83% rename from apps/files_encryption/appinfo/database.xml rename to apps/files_encryption/tests/encryption_table.xml index 4587930da0..c0f63dc0ef 100644 --- a/apps/files_encryption/appinfo/database.xml +++ b/apps/files_encryption/tests/encryption_table.xml @@ -1,11 +1,11 @@ - *dbname* - true - false - utf8 - - *dbprefix*encryption + *dbname* + true + false + utf8 +
+ *dbprefix*encryption_test uid @@ -36,4 +36,4 @@
- \ No newline at end of file + diff --git a/apps/files_encryption/tests/hooks.php b/apps/files_encryption/tests/hooks.php index 4370347261..95f5996bb8 100644 --- a/apps/files_encryption/tests/hooks.php +++ b/apps/files_encryption/tests/hooks.php @@ -100,6 +100,29 @@ class Test_Encryption_Hooks extends \PHPUnit_Framework_TestCase { \OC_User::deleteUser(\Test_Encryption_Hooks::TEST_ENCRYPTION_HOOKS_USER2); } + function testDisableHook() { + // encryption is enabled and running so we should have some user specific + // settings in oc_preferences + $query = \OC_DB::prepare('SELECT * FROM `*PREFIX*preferences` WHERE `appid` = ?'); + $result = $query->execute(array('files_encryption')); + $row = $result->fetchRow(); + $this->assertTrue(is_array($row)); + + // disabling the app should delete all user specific settings + \OCA\Encryption\Hooks::preDisable(array('app' => 'files_encryption')); + + // check if user specific settings for the encryption app are really gone + $query = \OC_DB::prepare('SELECT * FROM `*PREFIX*preferences` WHERE `appid` = ?'); + $result = $query->execute(array('files_encryption')); + $row = $result->fetchRow(); + $this->assertFalse($row); + + // relogin user to initialize the encryption again + $user = \OCP\User::getUser(); + \Test_Encryption_Util::loginHelper($user); + + } + function testDeleteHooks() { // remember files_trashbin state diff --git a/apps/files_encryption/tests/migration.php b/apps/files_encryption/tests/migration.php new file mode 100644 index 0000000000..3ef528c24b --- /dev/null +++ b/apps/files_encryption/tests/migration.php @@ -0,0 +1,130 @@ +. + * + */ + +use OCA\Encryption; +use OCA\Files_Encryption\Migration; + +class Test_Migration extends PHPUnit_Framework_TestCase { + + public function tearDown() { + if (OC_DB::tableExists('encryption_test')) { + OC_DB::dropTable('encryption_test'); + } + $this->assertTableNotExist('encryption_test'); + } + + public function setUp() { + if (OC_DB::tableExists('encryption_test')) { + OC_DB::dropTable('encryption_test'); + } + $this->assertTableNotExist('encryption_test'); + } + + public function testEncryptionTableDoesNotExist() { + + $this->assertTableNotExist('encryption_test'); + + $migration = new Migration('encryption_test'); + $migration->dropTableEncryption(); + + $this->assertTableNotExist('encryption_test'); + + } + + public function testDataMigration() { + + $this->assertTableNotExist('encryption_test'); + + // create test table + OC_DB::createDbFromStructure(__DIR__ . '/encryption_table.xml'); + $this->assertTableExist('encryption_test'); + + OC_DB::executeAudited('INSERT INTO `*PREFIX*encryption_test` values(?, ?, ?, ?)', + array('user1', 'server-side', 1, 1)); + + // preform migration + $migration = new Migration('encryption_test'); + $migration->dropTableEncryption(); + + // assert + $this->assertTableNotExist('encryption_test'); + + $rec = \OC_Preferences::getValue('user1', 'files_encryption', 'recovery_enabled'); + $mig = \OC_Preferences::getValue('user1', 'files_encryption', 'migration_status'); + + $this->assertEquals(1, $rec); + $this->assertEquals(1, $mig); + } + + public function testDuplicateDataMigration() { + + // create test table + OC_DB::createDbFromStructure(__DIR__ . '/encryption_table.xml'); + + // in case of duplicate entries we want to preserve 0 on migration status and 1 on recovery + $data = array( + array('user1', 'server-side', 1, 1), + array('user1', 'server-side', 1, 0), + array('user1', 'server-side', 0, 1), + array('user1', 'server-side', 0, 0), + ); + foreach ($data as $d) { + OC_DB::executeAudited( + 'INSERT INTO `*PREFIX*encryption_test` values(?, ?, ?, ?)', + $d); + } + + // preform migration + $migration = new Migration('encryption_test'); + $migration->dropTableEncryption(); + + // assert + $this->assertTableNotExist('encryption_test'); + + $rec = \OC_Preferences::getValue('user1', 'files_encryption', 'recovery_enabled'); + $mig = \OC_Preferences::getValue('user1', 'files_encryption', 'migration_status'); + + $this->assertEquals(1, $rec); + $this->assertEquals(0, $mig); + } + + /** + * @param string $table + */ + public function assertTableExist($table) { + $this->assertTrue(OC_DB::tableExists($table), 'Table ' . $table . ' does not exist'); + } + + /** + * @param string $table + */ + public function assertTableNotExist($table) { + $type=OC_Config::getValue( "dbtype", "sqlite" ); + if( $type == 'sqlite' || $type == 'sqlite3' ) { + // sqlite removes the tables after closing the DB + $this->assertTrue(true); + } else { + $this->assertFalse(OC_DB::tableExists($table), 'Table ' . $table . ' exists.'); + } + } + +} diff --git a/apps/files_encryption/tests/util.php b/apps/files_encryption/tests/util.php index 2b873bb308..cf2aa5f516 100755 --- a/apps/files_encryption/tests/util.php +++ b/apps/files_encryption/tests/util.php @@ -236,17 +236,15 @@ class Test_Encryption_Util extends \PHPUnit_Framework_TestCase { // Record the value so we can return it to it's original state later $enabled = $util->recoveryEnabledForUser(); - $this->assertTrue($util->setRecoveryForUser(1)); + $this->assertTrue($util->setRecoveryForUser(!$enabled)); - $this->assertEquals(1, $util->recoveryEnabledForUser()); + $this->assertEquals(!$enabled, $util->recoveryEnabledForUser()); - $this->assertTrue($util->setRecoveryForUser(0)); - - $this->assertEquals(0, $util->recoveryEnabledForUser()); - - // Return the setting to it's previous state $this->assertTrue($util->setRecoveryForUser($enabled)); + $this->assertEquals($enabled, $util->recoveryEnabledForUser()); + + } /** @@ -587,18 +585,7 @@ class Test_Encryption_Util extends \PHPUnit_Framework_TestCase { * @return boolean */ private function setMigrationStatus($status, $user) { - $sql = 'UPDATE `*PREFIX*encryption` SET `migration_status` = ? WHERE `uid` = ?'; - $args = array( - $status, - $user - ); - - $query = \OCP\DB::prepare($sql); - if ($query->execute($args)) { - return true; - } else { - return false; - } + return \OC_Preferences::setValue($user, 'files_encryption', 'migration_status', (string)$status); } } diff --git a/lib/private/legacy/preferences.php b/lib/private/legacy/preferences.php index 71d0b749f4..060274085f 100644 --- a/lib/private/legacy/preferences.php +++ b/lib/private/legacy/preferences.php @@ -84,14 +84,14 @@ class OC_Preferences{ * @param string $app app * @param string $key key * @param string $value value - * @return bool + * @param string $preCondition only set value if the key had a specific value before + * @return bool true if value was set, otherwise false * * Adds a value to the preferences. If the key did not exist before, it * will be added automagically. */ - public static function setValue( $user, $app, $key, $value ) { - self::$object->setValue( $user, $app, $key, $value ); - return true; + public static function setValue( $user, $app, $key, $value, $preCondition = null ) { + return self::$object->setValue( $user, $app, $key, $value, $preCondition ); } /** diff --git a/lib/private/preferences.php b/lib/private/preferences.php index 0dc5b26810..d1db25bbf0 100644 --- a/lib/private/preferences.php +++ b/lib/private/preferences.php @@ -165,44 +165,56 @@ class Preferences { * @param string $app app * @param string $key key * @param string $value value + * @param string $preCondition only set value if the key had a specific value before + * @return bool true if value was set, otherwise false * * Adds a value to the preferences. If the key did not exist before, it * will be added automagically. */ - public function setValue($user, $app, $key, $value) { + public function setValue($user, $app, $key, $value, $preCondition = null) { // Check if the key does exist $query = 'SELECT COUNT(*) FROM `*PREFIX*preferences`' . ' WHERE `userid` = ? AND `appid` = ? AND `configkey` = ?'; $count = $this->conn->fetchColumn($query, array($user, $app, $key)); $exists = $count > 0; - if (!$exists) { + $affectedRows = 0; + + if (!$exists && $preCondition === null) { $data = array( 'userid' => $user, 'appid' => $app, 'configkey' => $key, 'configvalue' => $value, ); - $this->conn->insert('*PREFIX*preferences', $data); - } else { - $data = array( - 'configvalue' => $value, - ); - $where = array( - 'userid' => $user, - 'appid' => $app, - 'configkey' => $key, - ); - $this->conn->update('*PREFIX*preferences', $data, $where); + $affectedRows = $this->conn->insert('*PREFIX*preferences', $data); + } elseif ($exists) { + $data = array($value, $user, $app, $key); + $sql = "UPDATE `*PREFIX*preferences` SET `configvalue` = ?" + . " WHERE `userid` = ? AND `appid` = ? AND `configkey` = ?"; + + if ($preCondition !== null) { + if (\OC_Config::getValue( 'dbtype', 'sqlite' ) === 'oci') { + //oracle hack: need to explicitly cast CLOB to CHAR for comparison + $sql .= " AND to_char(`configvalue`) = ?"; + } else { + $sql .= " AND `configvalue` = ?"; + } + $data[] = $preCondition; + } + $affectedRows = $this->conn->executeUpdate($sql, $data); } // only add to the cache if we already loaded data for the user - if (isset($this->cache[$user])) { + if ($affectedRows > 0 && isset($this->cache[$user])) { if (!isset($this->cache[$user][$app])) { $this->cache[$user][$app] = array(); } $this->cache[$user][$app][$key] = $value; } + + return ($affectedRows > 0) ? true : false; + } /** diff --git a/tests/lib/preferences.php b/tests/lib/preferences.php index 93c9704f6c..d31b0257ba 100644 --- a/tests/lib/preferences.php +++ b/tests/lib/preferences.php @@ -97,6 +97,42 @@ class Test_Preferences extends PHPUnit_Framework_TestCase { $this->assertEquals('othervalue', $value); } + public function testSetValueWithPreCondition() { + // remove existing key + $this->assertTrue(\OC_Preferences::deleteKey('Someuser', 'setvalueapp', 'newkey')); + + // add new preference with pre-condition should fails + $this->assertFalse(\OC_Preferences::setValue('Someuser', 'setvalueapp', 'newkey', 'newvalue', 'preCondition')); + $query = \OC_DB::prepare('SELECT `configvalue` FROM `*PREFIX*preferences` WHERE `userid` = ? AND `appid` = ? AND `configkey` = ?'); + $result = $query->execute(array('Someuser', 'setvalueapp', 'newkey')); + $row = $result->fetchRow(); + $this->assertFalse($row); + + // add new preference without pre-condition should insert the new value + $this->assertTrue(\OC_Preferences::setValue('Someuser', 'setvalueapp', 'newkey', 'newvalue')); + $query = \OC_DB::prepare('SELECT `configvalue` FROM `*PREFIX*preferences` WHERE `userid` = ? AND `appid` = ? AND `configkey` = ?'); + $result = $query->execute(array('Someuser', 'setvalueapp', 'newkey')); + $row = $result->fetchRow(); + $value = $row['configvalue']; + $this->assertEquals('newvalue', $value); + + // wrong pre-condition, value should stay the same + $this->assertFalse(\OC_Preferences::setValue('Someuser', 'setvalueapp', 'newkey', 'othervalue', 'preCondition')); + $query = \OC_DB::prepare('SELECT `configvalue` FROM `*PREFIX*preferences` WHERE `userid` = ? AND `appid` = ? AND `configkey` = ?'); + $result = $query->execute(array('Someuser', 'setvalueapp', 'newkey')); + $row = $result->fetchRow(); + $value = $row['configvalue']; + $this->assertEquals('newvalue', $value); + + // correct pre-condition, value should change + $this->assertTrue(\OC_Preferences::setValue('Someuser', 'setvalueapp', 'newkey', 'othervalue', 'newvalue')); + $query = \OC_DB::prepare('SELECT `configvalue` FROM `*PREFIX*preferences` WHERE `userid` = ? AND `appid` = ? AND `configkey` = ?'); + $result = $query->execute(array('Someuser', 'setvalueapp', 'newkey')); + $row = $result->fetchRow(); + $value = $row['configvalue']; + $this->assertEquals('othervalue', $value); + } + public function testDeleteKey() { $this->assertTrue(\OC_Preferences::deleteKey('Deleteuser', 'deleteapp', 'deletekey')); $query = \OC_DB::prepare('SELECT `configvalue` FROM `*PREFIX*preferences` WHERE `userid` = ? AND `appid` = ? AND `configkey` = ?'); @@ -165,19 +201,11 @@ class Test_Preferences_Object extends PHPUnit_Framework_TestCase { ) )); $connectionMock->expects($this->once()) - ->method('update') - ->with($this->equalTo('*PREFIX*preferences'), - $this->equalTo( - array( - 'configvalue' => 'v2', - )), - $this->equalTo( - array( - 'userid' => 'grg', - 'appid' => 'bar', - 'configkey' => 'foo', - ) - )); + ->method('executeUpdate') + ->with($this->equalTo("UPDATE `*PREFIX*preferences` SET `configvalue` = ?" + . " WHERE `userid` = ? AND `appid` = ? AND `configkey` = ?"), + $this->equalTo(array('v2', 'grg', 'bar', 'foo')) + ); $preferences = new OC\Preferences($connectionMock); $preferences->setValue('grg', 'bar', 'foo', 'v1');