Merge pull request #12828 from nextcloud/bugfix/handle-temp-encrypted-file

Bugfix Handle Temp Encrypted File Correctly

app/src/androidTest/java/com/owncloud/android/util/EncryptionTestIT.java

@@ -836,7 +836,7 @@ public class EncryptionTestIT extends AbstractIT {
 
         // Encryption
         Cipher encryptorCipher = EncryptionUtils.getCipher(Cipher.ENCRYPT_MODE, key, iv);
-        EncryptionUtils.encryptFile(file, encryptorCipher);
+        EncryptionUtils.encryptFile(user.getAccountName(), file, encryptorCipher);
         String encryptorCipherAuthTag = EncryptionUtils.getAuthenticationTag(encryptorCipher);
 
         // Decryption

app/src/main/java/com/nextcloud/client/jobs/upload/FileUploadHelper.kt

@@ -75,8 +75,10 @@ class FileUploadHelper {
     ) {
         val failedUploads = uploadsStorageManager.failedUploads
         if (failedUploads == null || failedUploads.isEmpty()) {
+            Log_OC.d(TAG, "Failed uploads are empty or null")
             return
         }
+
         retryUploads(
             uploadsStorageManager,
             connectivityService,
@@ -120,6 +122,7 @@ class FileUploadHelper {
         val charging = batteryStatus.isCharging || batteryStatus.isFull
         val isPowerSaving = powerManagementService.isPowerSavingEnabled
         var uploadUser = Optional.empty<User>()
+
         for (failedUpload in failedUploads) {
             // 1. extract failed upload owner account and cache it between loops (expensive query)
             if (!uploadUser.isPresent || !uploadUser.get().nameEquals(failedUpload.accountName)) {

app/src/main/java/com/nextcloud/client/jobs/upload/FileUploadWorker.kt

@@ -165,16 +165,16 @@ class FileUploadWorker(
             }
 
             if (user.isPresent) {
-                val operation = createUploadFileOperation(upload, user.get())
+                val uploadFileOperation = createUploadFileOperation(upload, user.get())
 
-                currentUploadFileOperation = operation
-                val result = upload(operation, user.get())
+                currentUploadFileOperation = uploadFileOperation
+                val result = upload(uploadFileOperation, user.get())
                 currentUploadFileOperation = null
 
                 fileUploaderDelegate.sendBroadcastUploadFinished(
-                    operation,
+                    uploadFileOperation,
                     result,
-                    operation.oldFile?.storagePath,
+                    uploadFileOperation.oldFile?.storagePath,
                     context,
                     localBroadcastManager
                 )
@@ -205,39 +205,39 @@ class FileUploadWorker(
     }
 
     @Suppress("TooGenericExceptionCaught", "DEPRECATION")
-    private fun upload(operation: UploadFileOperation, user: User): RemoteOperationResult<Any?> {
+    private fun upload(uploadFileOperation: UploadFileOperation, user: User): RemoteOperationResult<Any?> {
         lateinit var result: RemoteOperationResult<Any?>
 
         notificationManager.prepareForStart(
-            operation,
-            intents.startIntent(operation),
-            intents.notificationStartIntent(operation)
+            uploadFileOperation,
+            cancelPendingIntent = intents.startIntent(uploadFileOperation),
+            intents.notificationStartIntent(uploadFileOperation)
         )
 
         try {
-            val storageManager = operation.storageManager
+            val storageManager = uploadFileOperation.storageManager
             val ocAccount = OwnCloudAccount(user.toPlatformAccount(), context)
             val uploadClient = OwnCloudClientManagerFactory.getDefaultSingleton().getClientFor(ocAccount, context)
-            result = operation.execute(uploadClient)
+            result = uploadFileOperation.execute(uploadClient)
 
             val task = ThumbnailsCacheManager.ThumbnailGenerationTask(storageManager, user)
-            val file = File(operation.originalStoragePath)
-            val remoteId: String? = operation.file.remoteId
+            val file = File(uploadFileOperation.originalStoragePath)
+            val remoteId: String? = uploadFileOperation.file.remoteId
             task.execute(ThumbnailsCacheManager.ThumbnailGenerationTaskObject(file, remoteId))
         } catch (e: Exception) {
             Log_OC.e(TAG, "Error uploading", e)
             result = RemoteOperationResult<Any?>(e)
         } finally {
-            cleanupUploadProcess(result, operation)
+            cleanupUploadProcess(result, uploadFileOperation)
         }
 
         return result
     }
 
-    private fun cleanupUploadProcess(result: RemoteOperationResult<Any?>, operation: UploadFileOperation) {
+    private fun cleanupUploadProcess(result: RemoteOperationResult<Any?>, uploadFileOperation: UploadFileOperation) {
         if (!isStopped || !result.isCancelled) {
-            uploadsStorageManager.updateDatabaseUploadResult(result, operation)
-            notifyUploadResult(operation, result)
+            uploadsStorageManager.updateDatabaseUploadResult(result, uploadFileOperation)
+            notifyUploadResult(uploadFileOperation, result)
             notificationManager.dismissWorkerNotifications()
         }
     }
@@ -315,10 +315,11 @@ class FileUploadWorker(
 
         if (percent != lastPercent) {
             notificationManager.run {
-                updateUploadProgress(fileAbsoluteName, percent, currentUploadFileOperation)
-
                 val accountName = currentUploadFileOperation?.user?.accountName
                 val remotePath = currentUploadFileOperation?.remotePath
+                val filename = currentUploadFileOperation?.fileName ?: ""
+
+                updateUploadProgress(filename, percent, currentUploadFileOperation)
 
                 if (accountName != null && remotePath != null) {
                     val key: String =
@@ -329,7 +330,7 @@ class FileUploadWorker(
                         progressRate,
                         totalTransferredSoFar,
                         totalToTransfer,
-                        fileAbsoluteName
+                        filename
                     )
                 }
 

app/src/main/java/com/nextcloud/client/jobs/upload/UploadNotificationManager.kt

@@ -16,7 +16,6 @@ import android.os.Build
 import androidx.core.app.NotificationCompat
 import com.owncloud.android.R
 import com.owncloud.android.lib.common.operations.RemoteOperationResult
-import com.owncloud.android.lib.resources.files.FileUtils
 import com.owncloud.android.operations.UploadFileOperation
 import com.owncloud.android.ui.notifications.NotificationUtils
 import com.owncloud.android.utils.theme.ViewThemeUtils
@@ -44,14 +43,18 @@ class UploadNotificationManager(private val context: Context, viewThemeUtils: Vi
     }
 
     @Suppress("MagicNumber")
-    fun prepareForStart(upload: UploadFileOperation, pendingIntent: PendingIntent, startIntent: PendingIntent) {
+    fun prepareForStart(
+        uploadFileOperation: UploadFileOperation,
+        cancelPendingIntent: PendingIntent,
+        startIntent: PendingIntent
+    ) {
         notificationBuilder.run {
             setContentTitle(context.getString(R.string.uploader_upload_in_progress_ticker))
             setContentText(
                 String.format(
                     context.getString(R.string.uploader_upload_in_progress),
                     0,
-                    upload.fileName
+                    uploadFileOperation.fileName
                 )
             )
             setTicker(context.getString(R.string.foreground_service_upload))
@@ -62,13 +65,13 @@ class UploadNotificationManager(private val context: Context, viewThemeUtils: Vi
             addAction(
                 R.drawable.ic_action_cancel_grey,
                 context.getString(R.string.common_cancel),
-                pendingIntent
+                cancelPendingIntent
             )
 
             setContentIntent(startIntent)
         }
 
-        if (!upload.isInstantPicture && !upload.isInstantVideo) {
+        if (!uploadFileOperation.isInstantPicture && !uploadFileOperation.isInstantVideo) {
             showNotification()
         }
     }
@@ -138,11 +141,10 @@ class UploadNotificationManager(private val context: Context, viewThemeUtils: Vi
     }
 
     @Suppress("MagicNumber")
-    fun updateUploadProgress(filePath: String, percent: Int, currentOperation: UploadFileOperation?) {
+    fun updateUploadProgress(filename: String, percent: Int, currentOperation: UploadFileOperation?) {
         notificationBuilder.run {
             setProgress(100, percent, false)
-            val fileName = filePath.substring(filePath.lastIndexOf(FileUtils.PATH_SEPARATOR) + 1)
-            val text = String.format(context.getString(R.string.uploader_upload_in_progress), percent, fileName)
+            val text = String.format(context.getString(R.string.uploader_upload_in_progress), percent, filename)
             setContentText(text)
 
             showNotification()

app/src/main/java/com/owncloud/android/datamodel/FileDataStorageManager.java

@@ -56,7 +56,10 @@ import com.owncloud.android.utils.FileStorageUtils;
 import com.owncloud.android.utils.MimeType;
 import com.owncloud.android.utils.MimeTypeUtil;
 
+import org.apache.commons.io.FileUtils;
+
 import java.io.File;
+import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
@@ -338,6 +341,36 @@ public class FileDataStorageManager {
         return ocFile;
     }
 
+    public static void clearTempEncryptedFolder(String accountName) {
+        File tempEncryptedFolder =  new File(FileStorageUtils.getTemporalEncryptedFolderPath(accountName));
+
+        if (!tempEncryptedFolder.exists()) {
+            Log_OC.d(TAG,"tempEncryptedFolder not exists");
+            return;
+        }
+
+        try {
+            FileUtils.cleanDirectory(tempEncryptedFolder);
+
+            Log_OC.d(TAG,"tempEncryptedFolder cleared");
+        } catch (IOException exception) {
+            Log_OC.d(TAG,"Error caught at clearTempEncryptedFolder: " + exception);
+        }
+    }
+
+    public static File createTempEncryptedFolder(String accountName) {
+        File tempEncryptedFolder = new File(FileStorageUtils.getTemporalEncryptedFolderPath(accountName));
+
+        if (!tempEncryptedFolder.exists()) {
+            boolean isTempEncryptedFolderCreated = tempEncryptedFolder.mkdirs();
+            Log_OC.d(TAG, "tempEncryptedFolder created" + isTempEncryptedFolderCreated);
+        } else {
+            Log_OC.d(TAG, "tempEncryptedFolder already exists");
+        }
+
+        return tempEncryptedFolder;
+    }
+
     public void saveNewFile(OCFile newFile) {
         String remoteParentPath = new File(newFile.getRemotePath()).getParent();
         remoteParentPath = remoteParentPath.endsWith(OCFile.PATH_SEPARATOR) ?

app/src/main/java/com/owncloud/android/operations/UploadFileOperation.java

@@ -442,6 +442,7 @@ public class UploadFileOperation extends SyncOperation {
         File temporalFile = null;
         File originalFile = new File(mOriginalStoragePath);
         File expectedFile = null;
+        File encryptedTempFile = null;
         FileLock fileLock = null;
         long size;
 
@@ -552,7 +553,7 @@ public class UploadFileOperation extends SyncOperation {
             byte[] iv = EncryptionUtils.randomBytes(EncryptionUtils.ivLength);
             Cipher cipher = EncryptionUtils.getCipher(Cipher.ENCRYPT_MODE, key, iv);
             File file = new File(mFile.getStoragePath());
-            EncryptedFile encryptedFile = EncryptionUtils.encryptFile(file, cipher);
+            EncryptedFile encryptedFile = EncryptionUtils.encryptFile(user.getAccountName(), file, cipher);
 
             // new random file name, check if it exists in metadata
             String encryptedFileName = EncryptionUtils.generateUid();
@@ -567,9 +568,7 @@ public class UploadFileOperation extends SyncOperation {
                 }
             }
 
-            File encryptedTempFile = encryptedFile.getEncryptedFile();
-
-            /***** E2E *****/
+            encryptedTempFile = encryptedFile.getEncryptedFile();
 
             FileChannel channel = null;
             try {
@@ -712,8 +711,6 @@ public class UploadFileOperation extends SyncOperation {
                                                                  user,
                                                                  getStorageManager());
                 }
-
-                encryptedTempFile.delete();
             }
         } catch (FileNotFoundException e) {
             Log_OC.d(TAG, mFile.getStoragePath() + " not exists anymore");
@@ -755,6 +752,13 @@ public class UploadFileOperation extends SyncOperation {
             if (unlockFolderResult != null && !unlockFolderResult.isSuccess()) {
                 result = unlockFolderResult;
             }
+
+            if (encryptedTempFile != null) {
+                boolean isTempEncryptedFileDeleted = encryptedTempFile.delete();
+                Log_OC.e(TAG, "isTempEncryptedFileDeleted: " + isTempEncryptedFileDeleted);
+            } else {
+                Log_OC.e(TAG, "Encrypted temp file cannot be found");
+            }
         }
 
         if (result.isSuccess()) {

app/src/main/java/com/owncloud/android/ui/adapter/UploadListAdapter.java

@@ -141,9 +141,11 @@ public class UploadListAdapter extends SectionedRecyclerViewAdapter<SectionedVie
 
             if (itemId == R.id.action_upload_list_failed_clear) {
                 uploadsStorageManager.clearFailedButNotDelayedUploads();
+                clearTempEncryptedFolder();
                 loadUploadItemsFromDb();
-            } else {
+            } else if (itemId == R.id.action_upload_list_failed_retry) {
 
+                // FIXME For e2e resume is not working
                 new Thread(() -> {
                     FileUploadHelper.Companion.instance().retryFailedUploads(
                         uploadsStorageManager,
@@ -152,10 +154,11 @@ public class UploadListAdapter extends SectionedRecyclerViewAdapter<SectionedVie
                         powerManagementService);
                     parentActivity.runOnUiThread(this::loadUploadItemsFromDb);
                 }).start();
-
             }
+
             return true;
         });
+
         failedPopup.show();
     }
 
@@ -169,6 +172,7 @@ public class UploadListAdapter extends SectionedRecyclerViewAdapter<SectionedVie
             if (itemId == R.id.action_upload_list_cancelled_clear) {
                 uploadsStorageManager.clearCancelledUploadsForCurrentAccount();
                 loadUploadItemsFromDb();
+                clearTempEncryptedFolder();
             } else if (itemId == R.id.action_upload_list_cancelled_resume) {
                 retryCancelledUploads();
             }
@@ -179,6 +183,12 @@ public class UploadListAdapter extends SectionedRecyclerViewAdapter<SectionedVie
         popup.show();
     }
 
+    private void clearTempEncryptedFolder() {
+        Optional<User> user = parentActivity.getUser();
+        user.ifPresent(value -> FileDataStorageManager.clearTempEncryptedFolder(value.getAccountName()));
+    }
+
+    // FIXME For e2e resume is not working
     private void retryCancelledUploads() {
         new Thread(() -> {
             boolean showNotExistMessage = FileUploadHelper.Companion.instance().retryCancelledUploads(

app/src/main/java/com/owncloud/android/utils/EncryptionUtils.java

@@ -547,12 +547,12 @@ public final class EncryptionUtils {
         return Base64.decode(string, Base64.NO_WRAP);
     }
 
-    public static EncryptedFile encryptFile(File file, Cipher cipher) throws InvalidParameterSpecException {
-        // FIXME this won't work on low or write-protected storage
-        File encryptedFile = new File(file.getAbsolutePath() + ".enc.jpg");
-        encryptFileWithGivenCipher(file, encryptedFile, cipher);
+    public static EncryptedFile encryptFile(String accountName, File file, Cipher cipher) throws InvalidParameterSpecException, IOException {
+        File tempEncryptedFolder = FileDataStorageManager.createTempEncryptedFolder(accountName);
+        File tempEncryptedFile = File.createTempFile(file.getName(), null, tempEncryptedFolder);
+        encryptFileWithGivenCipher(file, tempEncryptedFile, cipher);
         String authenticationTagString = getAuthenticationTag(cipher);
-        return new EncryptedFile(encryptedFile, authenticationTagString);
+        return new EncryptedFile(tempEncryptedFile, authenticationTagString);
     }
 
     public static String getAuthenticationTag(Cipher cipher) throws InvalidParameterSpecException {
@@ -569,7 +569,7 @@ public final class EncryptionUtils {
     }
 
     public static void encryptFileWithGivenCipher(File inputFile, File encryptedFile, Cipher cipher) {
-        try( FileInputStream inputStream = new FileInputStream(inputFile);
+        try (FileInputStream inputStream = new FileInputStream(inputFile);
              FileOutputStream fileOutputStream = new FileOutputStream(encryptedFile);
              CipherOutputStream outputStream = new CipherOutputStream(fileOutputStream, cipher)) {
             byte[] buffer = new byte[4096];

app/src/main/java/com/owncloud/android/utils/FileStorageUtils.java

@@ -106,6 +106,17 @@ public final class FileStorageUtils {
         // that can be in the accountName since 0.1.190B
     }
 
+    public static String getTemporalEncryptedFolderPath(String accountName) {
+        return MainApp
+            .getAppContext()
+            .getFilesDir()
+            .getAbsolutePath()
+            + File.separator
+            + accountName
+            + File.separator
+            + "temp_encrypted_folder";
+    }
+
     /**
      * Get absolute path to tmp folder inside app folder for given accountName.
      */