From 70f4e3f96abb30de3281b4deb5413d1441409ea8 Mon Sep 17 00:00:00 2001 From: tobiasKaminsky Date: Mon, 18 Jul 2022 10:55:24 +0200 Subject: [PATCH 1/3] Fix upload conflict in encrypted folder Signed-off-by: tobiasKaminsky --- .../nextcloud/client/EndToEndRandomIT.java | 17 +++++++++++++++ .../operations/UploadFileOperation.java | 21 +++++++++++-------- .../ui/activity/ConflictsResolveActivity.java | 1 + .../android/ui/adapter/UploadListAdapter.java | 4 ++-- .../ui/dialog/ConflictsResolveDialog.java | 2 +- 5 files changed, 33 insertions(+), 12 deletions(-) diff --git a/app/src/androidTest/java/com/nextcloud/client/EndToEndRandomIT.java b/app/src/androidTest/java/com/nextcloud/client/EndToEndRandomIT.java index d78b3d2b3ffe..89dd1be44660 100644 --- a/app/src/androidTest/java/com/nextcloud/client/EndToEndRandomIT.java +++ b/app/src/androidTest/java/com/nextcloud/client/EndToEndRandomIT.java @@ -562,6 +562,23 @@ public void reInit() throws Exception { useExistingKeys(); } + @Test + public void lockUnlock() throws Exception { + after(); + + init(); + + currentFolder = createFolder(0); + + // lock folder + String token = EncryptionUtils.lockFolder(currentFolder, client); + assertNotNull(token); + + // unlock folder + RemoteOperationResult unlockResult = EncryptionUtils.unlockFolder(currentFolder, client, token); + assertTrue(unlockResult.isSuccess()); + } + private void useExistingKeys() throws Exception { // download them from server GetPublicKeyOperation publicKeyOperation = new GetPublicKeyOperation(); diff --git a/app/src/main/java/com/owncloud/android/operations/UploadFileOperation.java b/app/src/main/java/com/owncloud/android/operations/UploadFileOperation.java index 947716ddc2d1..7b43c243dcad 100644 --- a/app/src/main/java/com/owncloud/android/operations/UploadFileOperation.java +++ b/app/src/main/java/com/owncloud/android/operations/UploadFileOperation.java @@ -468,14 +468,8 @@ private RemoteOperationResult encryptedUpload(OwnCloudClient client, OCFile pare if (result != null) { return result; } - /***** E2E *****/ - - token = EncryptionUtils.lockFolder(parentFile, client); - // immediately store it - mUpload.setFolderUnlockToken(token); - uploadsStorageManager.updateUpload(mUpload); - // Update metadata + // retrieve metadata Pair metadataPair = EncryptionUtils.retrieveMetadata(parentFile, client, privateKey, @@ -484,8 +478,6 @@ private RemoteOperationResult encryptedUpload(OwnCloudClient client, OCFile pare metadataExists = metadataPair.first; DecryptedFolderMetadata metadata = metadataPair.second; - /**** E2E *****/ - // check name collision RemoteOperationResult collisionResult = checkNameCollision(client, metadata, parentFile.isEncrypted()); if (collisionResult != null) { @@ -493,6 +485,17 @@ private RemoteOperationResult encryptedUpload(OwnCloudClient client, OCFile pare return collisionResult; } + /***** E2E *****/ + try { + token = EncryptionUtils.lockFolder(parentFile, client); + } catch (UploadException e) { + return new RemoteOperationResult(ResultCode.LOCK_FAILED); + } + // immediately store it + mUpload.setFolderUnlockToken(token); + uploadsStorageManager.updateUpload(mUpload); + + // Update metadata mFile.setDecryptedRemotePath(parentFile.getDecryptedRemotePath() + originalFile.getName()); String expectedPath = FileStorageUtils.getDefaultSavePathFor(user.getAccountName(), mFile); expectedFile = new File(expectedPath); diff --git a/app/src/main/java/com/owncloud/android/ui/activity/ConflictsResolveActivity.java b/app/src/main/java/com/owncloud/android/ui/activity/ConflictsResolveActivity.java index 8f41b3aea679..598348266f4b 100644 --- a/app/src/main/java/com/owncloud/android/ui/activity/ConflictsResolveActivity.java +++ b/app/src/main/java/com/owncloud/android/ui/activity/ConflictsResolveActivity.java @@ -81,6 +81,7 @@ public static Intent createIntent(OCFile file, intent.setFlags(intent.getFlags() | flag); } intent.putExtra(EXTRA_FILE, file); + intent.putExtra(EXTRA_EXISTING_FILE, file); intent.putExtra(EXTRA_USER, user); intent.putExtra(EXTRA_CONFLICT_UPLOAD_ID, conflictUploadId); diff --git a/app/src/main/java/com/owncloud/android/ui/adapter/UploadListAdapter.java b/app/src/main/java/com/owncloud/android/ui/adapter/UploadListAdapter.java index c52a53d369fa..47dea84f3ba7 100755 --- a/app/src/main/java/com/owncloud/android/ui/adapter/UploadListAdapter.java +++ b/app/src/main/java/com/owncloud/android/ui/adapter/UploadListAdapter.java @@ -495,10 +495,10 @@ private boolean checkAndOpenConflictResolutionDialog(User user, OCUpload item, String status) { String remotePath = item.getRemotePath(); - OCFile ocFile = storageManager.getFileByPath(remotePath); + OCFile ocFile = storageManager.getFileByDecryptedRemotePath(remotePath); if (ocFile == null) { // Remote file doesn't exist, try to refresh folder - OCFile folder = storageManager.getFileByPath(new File(remotePath).getParent() + "/"); + OCFile folder = storageManager.getFileByDecryptedRemotePath(new File(remotePath).getParent() + "/"); if (folder != null && folder.isFolder()) { this.refreshFolder(itemViewHolder, user, folder, (caller, result) -> { itemViewHolder.binding.uploadStatus.setText(status); diff --git a/app/src/main/java/com/owncloud/android/ui/dialog/ConflictsResolveDialog.java b/app/src/main/java/com/owncloud/android/ui/dialog/ConflictsResolveDialog.java index 47ca18a5be22..5a3f56539f28 100644 --- a/app/src/main/java/com/owncloud/android/ui/dialog/ConflictsResolveDialog.java +++ b/app/src/main/java/com/owncloud/android/ui/dialog/ConflictsResolveDialog.java @@ -178,7 +178,7 @@ public Dialog onCreateDialog(Bundle savedInstanceState) { listener.conflictDecisionMade(Decision.CANCEL); } }) - .setTitle(String.format(getString(R.string.conflict_file_headline), existingFile.getFileName())); + .setTitle(String.format(getString(R.string.conflict_file_headline), existingFile.getDecryptedFileName())); File parentFile = new File(existingFile.getRemotePath()).getParentFile(); if (parentFile != null) { From 0761687766860c32076043864e6daf7515016c7d Mon Sep 17 00:00:00 2001 From: tobiasKaminsky Date: Fri, 9 Sep 2022 14:18:43 +0200 Subject: [PATCH 2/3] wip Signed-off-by: tobiasKaminsky --- .../java/com/owncloud/android/MainApp.java | 2 +- .../operations/UploadFileOperation.java | 22 +++++++++++++------ .../ui/activity/ConflictsResolveActivity.java | 4 +++- 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/app/src/main/java/com/owncloud/android/MainApp.java b/app/src/main/java/com/owncloud/android/MainApp.java index c35036ad181e..9e5c0e36ba8a 100644 --- a/app/src/main/java/com/owncloud/android/MainApp.java +++ b/app/src/main/java/com/owncloud/android/MainApp.java @@ -280,7 +280,7 @@ public static AppComponent getAppComponent() { @SuppressFBWarnings("ST") @Override public void onCreate() { - enableStrictMode(); + // enableStrictMode(); viewThemeUtils = viewThemeUtilsProvider.get(); diff --git a/app/src/main/java/com/owncloud/android/operations/UploadFileOperation.java b/app/src/main/java/com/owncloud/android/operations/UploadFileOperation.java index 7b43c243dcad..f7df5ae5510b 100644 --- a/app/src/main/java/com/owncloud/android/operations/UploadFileOperation.java +++ b/app/src/main/java/com/owncloud/android/operations/UploadFileOperation.java @@ -502,7 +502,7 @@ private RemoteOperationResult encryptedUpload(OwnCloudClient client, OCFile pare result = copyFile(originalFile, expectedPath); if (!result.isSuccess()) { - return result; + return returnGracefully(temporalFile, token, parentFile, client, result); } // Get the last modification date of the file from the file system @@ -680,6 +680,20 @@ private RemoteOperationResult encryptedUpload(OwnCloudClient client, OCFile pare getStorageManager().saveConflict(mFile, mFile.getEtagInConflict()); } + return returnGracefully(temporalFile, token, parentFile, client, result); + } + + private RemoteOperationResult returnGracefully( + File temporalFile, + String token, + OCFile parentFile, + OwnCloudClient client, + RemoteOperationResult result) { + // delete temporal file + if (temporalFile != null && temporalFile.exists() && !temporalFile.delete()) { + Log_OC.e(TAG, "Could not delete temporal file " + temporalFile.getAbsolutePath()); + } + // unlock must be done always if (token != null) { RemoteOperationResult unlockFolderResult = EncryptionUtils.unlockFolder(parentFile, @@ -690,12 +704,6 @@ private RemoteOperationResult encryptedUpload(OwnCloudClient client, OCFile pare return unlockFolderResult; } } - - // delete temporal file - if (temporalFile != null && temporalFile.exists() && !temporalFile.delete()) { - Log_OC.e(TAG, "Could not delete temporal file " + temporalFile.getAbsolutePath()); - } - return result; } diff --git a/app/src/main/java/com/owncloud/android/ui/activity/ConflictsResolveActivity.java b/app/src/main/java/com/owncloud/android/ui/activity/ConflictsResolveActivity.java index 598348266f4b..1bb34c58ba38 100644 --- a/app/src/main/java/com/owncloud/android/ui/activity/ConflictsResolveActivity.java +++ b/app/src/main/java/com/owncloud/android/ui/activity/ConflictsResolveActivity.java @@ -232,7 +232,9 @@ private void startDialog() { fragmentTransaction.remove(prev); } - if (existingFile != null && getStorageManager().fileExists(newFile.getRemotePath())) { + // TODO renaming does not work? + // TODO check all three conflict options + if (existingFile != null && getStorageManager().getFileByDecryptedRemotePath(newFile.getRemotePath()) != null) { ConflictsResolveDialog dialog = ConflictsResolveDialog.newInstance(existingFile, newFile, userOptional.get()); From 3575ca440c097b2178969de14b8168e37134e8af Mon Sep 17 00:00:00 2001 From: tobiasKaminsky Date: Mon, 12 Sep 2022 08:52:41 +0200 Subject: [PATCH 3/3] wip Signed-off-by: tobiasKaminsky --- .../operations/UploadFileOperation.java | 29 +++++++++++++++++-- .../ui/activity/ConflictsResolveActivity.java | 3 ++ 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/com/owncloud/android/operations/UploadFileOperation.java b/app/src/main/java/com/owncloud/android/operations/UploadFileOperation.java index f7df5ae5510b..5ed1662c7fdb 100644 --- a/app/src/main/java/com/owncloud/android/operations/UploadFileOperation.java +++ b/app/src/main/java/com/owncloud/android/operations/UploadFileOperation.java @@ -496,7 +496,7 @@ private RemoteOperationResult encryptedUpload(OwnCloudClient client, OCFile pare uploadsStorageManager.updateUpload(mUpload); // Update metadata - mFile.setDecryptedRemotePath(parentFile.getDecryptedRemotePath() + originalFile.getName()); + mFile.setDecryptedRemotePath(parentFile.getDecryptedRemotePath() + mFile.getFileName()); String expectedPath = FileStorageUtils.getDefaultSavePathFor(user.getAccountName(), mFile); expectedFile = new File(expectedPath); @@ -611,7 +611,7 @@ private RemoteOperationResult encryptedUpload(OwnCloudClient client, OCFile pare } if (result.isSuccess()) { - mFile.setDecryptedRemotePath(parentFile.getDecryptedRemotePath() + originalFile.getName()); + mFile.setDecryptedRemotePath(parentFile.getDecryptedRemotePath() + mFile.getFileName()); mFile.setRemotePath(parentFile.getRemotePath() + encryptedFileName); // update metadata @@ -972,6 +972,26 @@ private RemoteOperationResult checkNameCollision(OwnCloudClient client, break; case OVERWRITE: Log_OC.d(TAG, "Overwriting file"); + if (encrypted) { + OCFile file = getStorageManager().getFileByDecryptedRemotePath(mRemotePath); + + if (file != null) { + RemoteOperationResult removeResult = new RemoveFileOperation(file, + false, + user, + true, + mContext, + getStorageManager()) + .execute(client); + + if (!removeResult.isSuccess()) { + throw new IllegalArgumentException("Failed to remove old encrypted file!"); + } + + } else { + throw new IllegalArgumentException("File to remove is null!"); + } + } break; case ASK_USER: Log_OC.d(TAG, "Name collision; asking the user what to do"); @@ -1116,6 +1136,7 @@ private void createNewOCFile(String newRemotePath) { newFile.setLastSyncDateForData(mFile.getLastSyncDateForData()); newFile.setStoragePath(mFile.getStoragePath()); newFile.setParentId(mFile.getParentId()); + newFile.setEncrypted(mFile.isEncrypted()); mOldFile = mFile; mFile = newFile; } @@ -1153,7 +1174,9 @@ private String getNewAvailableRemotePath(OwnCloudClient client, String remotePat return newPath; } - private boolean existsFile(OwnCloudClient client, String remotePath, DecryptedFolderMetadata metadata, + private boolean existsFile(OwnCloudClient client, + String remotePath, + DecryptedFolderMetadata metadata, boolean encrypted) { if (encrypted) { String fileName = new File(remotePath).getName(); diff --git a/app/src/main/java/com/owncloud/android/ui/activity/ConflictsResolveActivity.java b/app/src/main/java/com/owncloud/android/ui/activity/ConflictsResolveActivity.java index 1bb34c58ba38..6f05224a5394 100644 --- a/app/src/main/java/com/owncloud/android/ui/activity/ConflictsResolveActivity.java +++ b/app/src/main/java/com/owncloud/android/ui/activity/ConflictsResolveActivity.java @@ -142,6 +142,9 @@ protected void onCreate(Bundle savedInstanceState) { uploadsStorageManager.removeUpload(upload); break; case KEEP_SERVER: // Download + if (newFile.isEncrypted()) { + return; + } if (!shouldDeleteLocal()) { // Overwrite local file Intent intent = new Intent(getBaseContext(), FileDownloader.class);