From ba29147e34a22142c4bfd2afa3f5b9f1e6efcd63 Mon Sep 17 00:00:00 2001 From: Sam Tuke Date: Wed, 1 May 2013 19:18:31 +0200 Subject: [PATCH] Fixed recoveryadmin settings in user and admin settings pages (js, templates, ajax) Renamed recovery methods in Util{} for clarity Added note about bug causing slow page load and redundant keypair generation recoveryAdmin functionality not yet complete --- apps/files_encryption/ajax/adminrecovery.php | 27 +++++-- apps/files_encryption/ajax/userrecovery.php | 9 +-- apps/files_encryption/appinfo/app.php | 4 +- .../js/{settings.js => settings-admin.js} | 13 +-- apps/files_encryption/js/settings-personal.js | 23 ++++++ apps/files_encryption/lib/keymanager.php | 2 +- apps/files_encryption/lib/session.php | 18 +++-- apps/files_encryption/lib/util.php | 79 ++++++++++++------- .../{settings.php => settings-admin.php} | 11 +-- apps/files_encryption/settings-personal.php | 20 +++++ .../{settings.php => settings-admin.php} | 0 .../templates/settings-personal.php | 22 ++++++ apps/files_encryption/tests/util.php | 14 ++-- 13 files changed, 176 insertions(+), 66 deletions(-) rename apps/files_encryption/js/{settings.js => settings-admin.js} (57%) create mode 100644 apps/files_encryption/js/settings-personal.js rename apps/files_encryption/{settings.php => settings-admin.php} (64%) rename apps/files_encryption/templates/{settings.php => settings-admin.php} (100%) diff --git a/apps/files_encryption/ajax/adminrecovery.php b/apps/files_encryption/ajax/adminrecovery.php index cec0cd4ddda..157fc8f313c 100644 --- a/apps/files_encryption/ajax/adminrecovery.php +++ b/apps/files_encryption/ajax/adminrecovery.php @@ -17,11 +17,12 @@ use OCA\Encryption; $return = $doSetup = false; +// Enable recoveryAdmin if ( isset( $_POST['adminEnableRecovery'] ) - && $_POST['adminEnableRecovery'] == 1 - && isset( $_POST['recoveryPassword'] ) - && ! empty ( $_POST['recoveryPassword'] ) + && 1 == $_POST['adminEnableRecovery'] +// && isset( $_POST['recoveryPassword'] ) +// && ! empty ( $_POST['recoveryPassword'] ) ) { // TODO: Let the admin set this themselves @@ -29,7 +30,7 @@ if ( // If desired recoveryAdmin UID is already in use if ( ! \OC_User::userExists( $recoveryAdminUid ) ) { - + // Create new recoveryAdmin user \OC_User::createUser( $recoveryAdminUid, $_POST['recoveryPassword'] ); @@ -55,11 +56,11 @@ if ( } - // If recoveryAdmin has passed other checks + // Setup recoveryAdmin user for encryption if ( $doSetup ) { $view = new \OC_FilesystemView( '/' ); - $util = new Util( $view, $recoveryAdminUid ); + $util = new \OCA\Encryption\Util( $view, $recoveryAdminUid ); // Ensure recoveryAdmin is ready for encryption (has usable keypair etc.) $util->setupServerSide( $_POST['recoveryPassword'] ); @@ -71,6 +72,20 @@ if ( } + // Set recoveryAdmin as enabled + OC_Appconfig::setValue( 'files_encryption', 'recoveryAdminEnabled', 1 ); + +// Disable recoveryAdmin +} elseif ( + isset( $_POST['adminEnableRecovery'] ) + && 0 == $_POST['adminEnableRecovery'] +) { + + // Set recoveryAdmin as enabled + OC_Appconfig::setValue( 'files_encryption', 'recoveryAdminEnabled', 0 ); + + $return = true; + } ($return) ? OC_JSON::success() : OC_JSON::error(); \ No newline at end of file diff --git a/apps/files_encryption/ajax/userrecovery.php b/apps/files_encryption/ajax/userrecovery.php index 56c18f7ad5b..f72be3181ef 100644 --- a/apps/files_encryption/ajax/userrecovery.php +++ b/apps/files_encryption/ajax/userrecovery.php @@ -13,21 +13,18 @@ use OCA\Encryption; \OCP\JSON::checkLoggedIn(); \OCP\JSON::checkAppEnabled( 'files_encryption' ); -\OCP\JSON::callCheck(); if ( isset( $_POST['userEnableRecovery'] ) + && ( 0 == $_POST['userEnableRecovery'] || 1 == $_POST['userEnableRecovery'] ) ) { - // Ensure preference is an integer - $recoveryEnabled = intval( $_POST['userEnableRecovery'] ); - $userId = \OCP\USER::getUser(); $view = new \OC_FilesystemView( '/' ); - $util = new Util( $view, $userId ); + $util = new \OCA\Encryption\Util( $view, $userId ); // Save recovery preference to DB - $result = $util->setRecovery( $recoveryEnabled ); + $result = $util->setRecoveryForUser( $_POST['userEnableRecovery'] ); if ( $result ) { diff --git a/apps/files_encryption/appinfo/app.php b/apps/files_encryption/appinfo/app.php index 9ae6c8331f8..a7253c43332 100644 --- a/apps/files_encryption/appinfo/app.php +++ b/apps/files_encryption/appinfo/app.php @@ -28,7 +28,7 @@ OCP\Util::connectHook('OC_Filesystem', 'post_rename', 'OCA\Encryption\Hooks', 'p stream_wrapper_register( 'crypt', 'OCA\Encryption\Stream' ); -$view = new OC_FilesystemView( '/' ); +$view = new \OC\Files\View( '/' ); $session = new OCA\Encryption\Session( $view ); @@ -50,5 +50,5 @@ if ( } // Register settings scripts -OCP\App::registerAdmin( 'files_encryption', 'settings' ); +OCP\App::registerAdmin( 'files_encryption', 'settings-admin' ); OCP\App::registerPersonal( 'files_encryption', 'settings-personal' ); diff --git a/apps/files_encryption/js/settings.js b/apps/files_encryption/js/settings-admin.js similarity index 57% rename from apps/files_encryption/js/settings.js rename to apps/files_encryption/js/settings-admin.js index 9a0bebf2478..48b5598d524 100644 --- a/apps/files_encryption/js/settings.js +++ b/apps/files_encryption/js/settings-admin.js @@ -1,5 +1,6 @@ /** - * Copyright (c) 2011, Robin Appelman + * Copyright (c) 2013, Sam Tuke , Robin Appelman + * * This file is licensed under the Affero General Public License version 3 or later. * See the COPYING-README file. */ @@ -17,11 +18,11 @@ $(document).ready(function(){ $( 'input:radio[name="adminEnableRecovery"]' ).change( function() { - var foo = $( this ).val(); + var recoveryStatus = $( this ).val(); $.post( - OC.filePath('files_encryption', 'ajax', 'adminrecovery.php') - , { adminEnableRecovery: foo, recoveryPassword: 'password' } + OC.filePath( 'files_encryption', 'ajax', 'adminrecovery.php' ) + , { adminEnableRecovery: recoveryStatus, recoveryPassword: 'password' } , function( data ) { alert( data ); } @@ -30,7 +31,7 @@ $(document).ready(function(){ ); function blackListChange(){ - var blackList=$('#encryption_blacklist').val().join(','); - OC.AppConfig.setValue('files_encryption','type_blacklist',blackList); + var blackList=$( '#encryption_blacklist' ).val().join( ',' ); + OC.AppConfig.setValue( 'files_encryption', 'type_blacklist', blackList ); } }) \ No newline at end of file diff --git a/apps/files_encryption/js/settings-personal.js b/apps/files_encryption/js/settings-personal.js new file mode 100644 index 00000000000..e4a1b7448f4 --- /dev/null +++ b/apps/files_encryption/js/settings-personal.js @@ -0,0 +1,23 @@ +/** + * Copyright (c) 2013, Sam Tuke + * This file is licensed under the Affero General Public License version 3 or later. + * See the COPYING-README file. + */ + +$(document).ready(function(){ + // Trigger ajax on recoveryAdmin status change + $( 'input:radio[name="userEnableRecovery"]' ).change( + function() { + + var recoveryStatus = $( this ).val(); + + $.post( + OC.filePath( 'files_encryption', 'ajax', 'userrecovery.php' ) + , { userEnableRecovery: recoveryStatus } + , function( data ) { + alert( data ); + } + ); + } + ); +}) \ No newline at end of file diff --git a/apps/files_encryption/lib/keymanager.php b/apps/files_encryption/lib/keymanager.php index 9f3cb8120ca..51d4f8ffc04 100755 --- a/apps/files_encryption/lib/keymanager.php +++ b/apps/files_encryption/lib/keymanager.php @@ -32,7 +32,7 @@ class Keymanager { /** * @brief retrieve the ENCRYPTED private key from a user * - * @return string private key or false + * @return string private key or false (hopefully) * @note the key returned by this method must be decrypted before use */ public static function getPrivateKey( \OC_FilesystemView $view, $user ) { diff --git a/apps/files_encryption/lib/session.php b/apps/files_encryption/lib/session.php index 0c6a7131fd9..0e6bb96605f 100644 --- a/apps/files_encryption/lib/session.php +++ b/apps/files_encryption/lib/session.php @@ -35,22 +35,28 @@ class Session { * * The ownCloud key pair is used to allow public link sharing even if encryption is enabled */ - public function __construct( \OC_FilesystemView $view ) { + public function __construct( $view ) { $this->view = $view; if ( ! $this->view->is_dir( 'owncloud_private_key' ) ) { - $this->view->mkdir('owncloud_private_key'); + $this->view->mkdir( 'owncloud_private_key' ); + } - if ( - ! $this->view->file_exists("/public-keys/owncloud.public.key") - || ! $this->view->file_exists("/owncloud_private_key/owncloud.private.key" ) + ! $this->view->file_exists( "/public-keys/owncloud.public.key" ) + || ! $this->view->file_exists( "/owncloud_private_key/owncloud.private.key" ) ) { - + + //FIXME: Bug: for some reason file_exists is returning + // false in above if statement, and causing new keys + // to be generated on each page load. At last check + // our app.php is being executed 18 times per page load + // , causing 18 new keypairs and huge performance hit. + $keypair = Crypt::createKeypair(); \OC_FileProxy::$enabled = false; diff --git a/apps/files_encryption/lib/util.php b/apps/files_encryption/lib/util.php index eabb34f7ab0..015125370bc 100644 --- a/apps/files_encryption/lib/util.php +++ b/apps/files_encryption/lib/util.php @@ -24,11 +24,8 @@ # Bugs # ---- # Sharing a file to a user without encryption set up will not provide them with access but won't notify the sharer -# Timeouts on first login due to encryption of very large files (fix in progress, as a result streaming is currently broken) # Sharing all files to admin for recovery purposes still in progress # Possibly public links are broken (not tested since last merge of master) -# encryptAll during login mangles paths: /files/files/ -# encryptAll is accessing files via encryption proxy - perhaps proxies should be disabled? # Missing features @@ -204,12 +201,18 @@ class Util { $this->view->file_put_contents( $this->privateKeyPath, $encryptedPrivateKey ); \OC_FileProxy::$enabled = true; - - // create database configuration - $sql = 'INSERT INTO `*PREFIX*encryption` (`uid`,`mode`,`recovery`) VALUES (?,?,?)'; - $args = array( $this->userId, 'server-side', 0); - $query = \OCP\DB::prepare( $sql ); - $query->execute( $args ); + + } + + // 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`) VALUES (?,?,?)'; + $args = array( $this->userId, 'server-side', 0); + $query = \OCP\DB::prepare( $sql ); + $query->execute( $args ); + } return true; @@ -218,11 +221,11 @@ class Util { /** * @brief Check whether pwd recovery is enabled for a given user - * @return bool + * @return 1 = yes, 0 = no, false = no record * @note If records are not being returned, check for a hidden space * at the start of the uid in db */ - public function recoveryEnabled() { + public function recoveryEnabledForUser() { $sql = 'SELECT recovery @@ -237,16 +240,25 @@ class Util { $result = $query->execute( $args ); - // Set default in case no records found - $recoveryEnabled = 0; + $recoveryEnabled = array(); while( $row = $result->fetchRow() ) { - $recoveryEnabled = $row['recovery']; + $recoveryEnabled[] = $row['recovery']; } - return $recoveryEnabled; + // If no record is found + if ( empty( $recoveryEnabled ) ) { + + return false; + + // If a record is found + } else { + + return $recoveryEnabled[0]; + + } } @@ -255,20 +267,33 @@ class Util { * @param bool $enabled Whether to enable or disable recovery * @return bool */ - public function setRecovery( $enabled ) { + public function setRecoveryForUser( $enabled ) { - $sql = 'UPDATE - *PREFIX*encryption - SET - recovery = ? - WHERE - uid = ?'; + $recoveryStatus = $this->recoveryEnabledForUser(); + + // If a record for this user already exists, update it + if ( false === $recoveryStatus ) { - // Ensure value is an integer - $enabled = intval( $enabled ); + $sql = 'INSERT INTO `*PREFIX*encryption` + (`uid`,`mode`,`recovery`) + VALUES (?,?,?)'; + + $args = array( $this->userId, 'server-side', $enabled ); - $args = array( $enabled, $this->userId ); - + // Create a new record instead + } else { + + $sql = 'UPDATE + *PREFIX*encryption + SET + recovery = ? + WHERE + uid = ?'; + + $args = array( $enabled, $this->userId ); + + } + $query = \OCP\DB::prepare( $sql ); if ( $query->execute( $args ) ) { @@ -888,7 +913,7 @@ class Util { public function getSharingUsersArray( $sharingEnabled, $filePath, $currentUserId = false ) { // Check if key recovery is enabled - $recoveryEnabled = $this->recoveryEnabled(); + $recoveryEnabled = $this->recoveryEnabledForUser(); // Make sure that a share key is generated for the owner too list($owner, $ownerPath) = $this->getUidAndFilename($filePath); diff --git a/apps/files_encryption/settings.php b/apps/files_encryption/settings-admin.php similarity index 64% rename from apps/files_encryption/settings.php rename to apps/files_encryption/settings-admin.php index 71d47f061af..b09515f0c33 100644 --- a/apps/files_encryption/settings.php +++ b/apps/files_encryption/settings-admin.php @@ -8,20 +8,21 @@ \OC_Util::checkAdminUser(); -$tmpl = new OCP\Template( 'files_encryption', 'settings' ); +$tmpl = new OCP\Template( 'files_encryption', 'settings-admin' ); $blackList = explode( ',', \OCP\Config::getAppValue( 'files_encryption', 'type_blacklist', '' ) ); // Check if an adminRecovery account is enabled for recovering files after lost pwd $view = new OC_FilesystemView( '' ); -$util = new \OCA\Encryption\Util( $view, \OCP\USER::getUser() ); -$recoveryEnabled = $util->recoveryEnabled(); + +$recoveryAdminEnabled = OC_Appconfig::getValue( 'files_encryption', 'recoveryAdminEnabled' ); +$recoveryAdminUid = OC_Appconfig::getValue( 'files_encryption', 'recoveryAdminUid' ); $tmpl->assign( 'blacklist', $blackList ); $tmpl->assign( 'encryption_mode', \OC_Appconfig::getValue( 'files_encryption', 'mode', 'none' ) ); -$tmpl->assign( 'recoveryEnabled', $recoveryEnabled ); +$tmpl->assign( 'recoveryEnabled', $recoveryAdminEnabled ); -\OCP\Util::addscript( 'files_encryption', 'settings' ); +\OCP\Util::addscript( 'files_encryption', 'settings-admin' ); \OCP\Util::addscript( 'core', 'multiselect' ); return $tmpl->fetchPage(); diff --git a/apps/files_encryption/settings-personal.php b/apps/files_encryption/settings-personal.php index c001bb0d725..f7ebc425120 100644 --- a/apps/files_encryption/settings-personal.php +++ b/apps/files_encryption/settings-personal.php @@ -10,6 +10,26 @@ $tmpl = new OCP\Template( 'files_encryption', 'settings-personal'); $blackList = explode( ',', \OCP\Config::getAppValue( 'files_encryption', 'type_blacklist', '' ) ); +// Add human readable message in case nothing is blacklisted +if ( + 1 == count( $blackList ) + && $blackList[0] == '' +) { + + // FIXME: Make this string translatable + $blackList[0] = "(None - all filetypes will be encrypted)"; + +} + +$user = \OCP\USER::getUser(); +$view = new \OC_FilesystemView( '/' ); +$util = new \OCA\Encryption\Util( $view, $user ); + +$recoveryEnabledForUser = $util->recoveryEnabledForUser(); + +\OCP\Util::addscript( 'files_encryption', 'settings-personal' ); + +$tmpl->assign( 'recoveryEnabled', $recoveryEnabledForUser ); $tmpl->assign( 'blacklist', $blackList ); return $tmpl->fetchPage(); diff --git a/apps/files_encryption/templates/settings.php b/apps/files_encryption/templates/settings-admin.php similarity index 100% rename from apps/files_encryption/templates/settings.php rename to apps/files_encryption/templates/settings-admin.php diff --git a/apps/files_encryption/templates/settings-personal.php b/apps/files_encryption/templates/settings-personal.php index 5f0accaed5f..0cefde36b61 100644 --- a/apps/files_encryption/templates/settings-personal.php +++ b/apps/files_encryption/templates/settings-personal.php @@ -3,6 +3,7 @@ t( 'Encryption' )); ?> +

t( 'File encryption is enabled.' )); ?>

@@ -10,6 +11,7 @@

t( 'The following file types will not be encrypted:' )); ?>

+
  • @@ -18,5 +20,25 @@
+ +

+ t( "Enable password recovery by sharing all files with administrator:" )); ?> +
+ /> + t( "Enabled" )); ?> +
+ + /> + t( "Disabled" )); ?> +

+ diff --git a/apps/files_encryption/tests/util.php b/apps/files_encryption/tests/util.php index e3ec0860fa5..d0a988f96b9 100755 --- a/apps/files_encryption/tests/util.php +++ b/apps/files_encryption/tests/util.php @@ -179,23 +179,23 @@ class Test_Enc_Util extends \PHPUnit_Framework_TestCase { } - function testRecoveryEnabled() { + function testRecoveryEnabledForUser() { $util = new Encryption\Util( $this->view, $this->userId ); // Record the value so we can return it to it's original state later - $enabled = $util->recoveryEnabled(); + $enabled = $util->recoveryEnabledForUser(); - $this->assertTrue( $util->setRecovery( 1 ) ); + $this->assertTrue( $util->setRecoveryForUser( 1 ) ); - $this->assertEquals( 1, $util->recoveryEnabled() ); + $this->assertEquals( 1, $util->recoveryEnabledForUser() ); - $this->assertTrue( $util->setRecovery( 0 ) ); + $this->assertTrue( $util->setRecoveryForUser( 0 ) ); - $this->assertEquals( 0, $util->recoveryEnabled() ); + $this->assertEquals( 0, $util->recoveryEnabledForUser() ); // Return the setting to it's previous state - $this->assertTrue( $util->setRecovery( $enabled ) ); + $this->assertTrue( $util->setRecoveryForUser( $enabled ) ); } -- 2.39.5