Skip to content

Fix upload conflict in encrypted folder #10560

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
tobiasKaminsky wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Fix upload conflict in encrypted folder #10560

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
70f4e3f
Fix upload conflict in encrypted folder
tobiasKaminsky Jul 18, 2022
0761687
wip
tobiasKaminsky Sep 9, 2022
3575ca4
wip
tobiasKaminsky Sep 12, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions app/src/androidTest/java/com/nextcloud/client/EndToEndRandomIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion app/src/main/java/com/owncloud/android/MainApp.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ public static AppComponent getAppComponent() {
@SuppressFBWarnings("ST")
@Override
public void onCreate() {
enableStrictMode();
// enableStrictMode();

viewThemeUtils = viewThemeUtilsProvider.get();

Expand Down
72 changes: 53 additions & 19 deletions app/src/main/java/com/owncloud/android/operations/UploadFileOperation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<Boolean, DecryptedFolderMetadata> metadataPair = EncryptionUtils.retrieveMetadata(parentFile,
client,
privateKey,
Expand All @@ -484,22 +478,31 @@ 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) {
result = collisionResult;
return collisionResult;
}

mFile.setDecryptedRemotePath(parentFile.getDecryptedRemotePath() + originalFile.getName());
/***** 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() + mFile.getFileName());
String expectedPath = FileStorageUtils.getDefaultSavePathFor(user.getAccountName(), mFile);
expectedFile = new File(expectedPath);

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
Expand Down Expand Up @@ -608,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
Expand Down Expand Up @@ -677,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,
Expand All @@ -687,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;
}

Expand Down Expand Up @@ -961,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");
Expand Down Expand Up @@ -1105,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;
}
Expand Down Expand Up @@ -1142,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();
Expand Down
8 changes: 7 additions & 1 deletion app/src/main/java/com/owncloud/android/ui/activity/ConflictsResolveActivity.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -141,6 +142,9 @@ protected void onCreate(Bundle savedInstanceState) {
uploadsStorageManager.removeUpload(upload);
break;
case KEEP_SERVER: // Download
if (newFile.isEncrypted()) {
return;

@surinder-tsys surinder-tsys Sep 6, 2023

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of return it should be break so that finish() function on line number 161 has to execute else the activity won't finish at all.

}
if (!shouldDeleteLocal()) {
// Overwrite local file
Intent intent = new Intent(getBaseContext(), FileDownloader.class);
Expand Down Expand Up @@ -231,7 +235,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());
Expand Down
4 changes: 2 additions & 2 deletions app/src/main/java/com/owncloud/android/ui/adapter/UploadListAdapter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion app/src/main/java/com/owncloud/android/ui/dialog/ConflictsResolveDialog.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down