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

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

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

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' );

apps/files_encryption/js/settings-admin.js

@@ -1,5 +1,6 @@
 /**
- * Copyright (c) 2011, Robin Appelman <icewind1991@gmail.com>
+ * Copyright (c) 2013, Sam Tuke <samtuke@owncloud.com>, Robin Appelman 
+ * <icewind1991@gmail.com>
  * 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 );
 	}
 })

apps/files_encryption/js/settings-personal.js

@@ -0,0 +1,23 @@
+/**
+ * Copyright (c) 2013, Sam Tuke <samtuke@owncloud.com>
+ * 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 );
+				}
+			);
+		}
+	);
+})

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

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;

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

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();

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();

apps/files_encryption/templates/settings-personal.php

@@ -3,6 +3,7 @@
 		<legend>
 			<?php p($l->t( 'Encryption' )); ?>
 		</legend>
+		
 		<p>
 			<?php p($l->t( 'File encryption is enabled.' )); ?>
 		</p>
@@ -10,6 +11,7 @@
 		<p>
 			<?php p($l->t( 'The following file types will not be encrypted:' )); ?>
 		</p>
+		
 		<ul>
 			<?php foreach( $_["blacklist"] as $type ): ?>
 			<li>
@@ -18,5 +20,25 @@
 			<?php endforeach; ?>
 		</ul>
 		<?php endif; ?>
+		
+		<p>
+			<?php p($l->t( "Enable password recovery by sharing all files with administrator:" )); ?>
+			<br />
+			<input 
+			type='radio'
+			name='userEnableRecovery'
+			value='1'
+			<?php echo ( $_["recoveryEnabled"] == 1 ? 'checked="checked"' : '' ); ?> />
+			<?php p($l->t( "Enabled" )); ?>
+			<br />
+			
+			<input 
+			type='radio'
+			name='userEnableRecovery'
+			value='0'
+			<?php echo ( $_["recoveryEnabled"] == 0 ? 'checked="checked"' : '' ); ?> />
+			<?php p($l->t( "Disabled" )); ?>
+		</p>
+		
 	</fieldset>
 </form>

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