Merge branch 'improve-vault-deletion-method'

7 files changed, 172 insertions, 102 deletions

diff --git a/appinfo/routes.php b/appinfo/routes.php
index 37fb1fbe..a0476751 100644
--- a/appinfo/routes.php
+++ b/appinfo/routes.php
@@ -47,6 +47,7 @@ return [
 		['name' => 'file#uploadFile', 'url' => '/api/v2/file', 'verb' => 'POST'],
 		['name' => 'file#getFile', 'url' => '/api/v2/file/{file_id}', 'verb' => 'GET'],
 		['name' => 'file#deleteFile', 'url' => '/api/v2/file/{file_id}', 'verb' => 'DELETE'],
+		['name' => 'file#deleteFiles', 'url' => '/api/v2/files/delete', 'verb' => 'POST'],
 		['name' => 'file#updateFile', 'url' => '/api/v2/file/{file_id}', 'verb' => 'PATCH'],
 
 		//Sharing stuff
@@ -99,4 +100,4 @@ return [
 		['name' => 'admin#listRequests', 'url' => '/admin/delete-requests', 'verb' => 'GET'],
 		['name' => 'admin#acceptRequestDeletion', 'url' => '/admin/accept-delete-request', 'verb' => 'POST'],
 	]
-];
\ No newline at end of file
+];
diff --git a/controller/credentialcontroller.php b/controller/credentialcontroller.php
index 4984df6c..87c029c6 100644
--- a/controller/credentialcontroller.php
+++ b/controller/credentialcontroller.php
@@ -37,12 +37,12 @@ class CredentialController extends ApiController {
 
 	public function __construct($AppName,
 	                            IRequest $request,
-	                            $userId,
-	                            CredentialService $credentialService,
-	                            ActivityService $activityService,
-	                            CredentialRevisionService $credentialRevisionService,
-	                            ShareService $sharingService,
-	                            SettingsService $settings
+								$userId,
+		                        CredentialService $credentialService,
+		                        ActivityService $activityService,
+		                        CredentialRevisionService $credentialRevisionService,
+		                        ShareService $sharingService,
+		                        SettingsService $settings
 
 	) {
 		parent::__construct(
@@ -275,7 +275,7 @@ class CredentialController extends ApiController {
 		if ($credential instanceof Credential) {
 			$result = $this->credentialService->deleteCredential($credential);
 			//print_r($credential);
-			$this->deleteCredentialParts($credential);
+			$this->credentialService->deleteCredentialParts($credential, $this->userId);
 		} else {
 			$result = false;
 		}
@@ -283,25 +283,6 @@ class CredentialController extends ApiController {
 	}
 
 	/**
-	 * Delete leftovers from a credential
-	 * @param Credential $credential
-	 * @throws \Exception
-	 */
-	private function deleteCredentialParts(Credential $credential) {
-		$this->activityService->add(
-			'item_destroyed_self', array($credential->getLabel()),
-			'', array(),
-			'', $this->userId, Activity::TYPE_ITEM_ACTION);
-		$this->sharingService->unshareCredential($credential->getGuid());
-		foreach ($this->credentialRevisionService->getRevisions($credential->getId()) as $revision) {
-			$id = $revision['revision_id'];
-			if (isset($id)) {
-				$this->credentialRevisionService->deleteRevision($id, $this->userId);
-			}
-		}
-	}
-
-	/**
 	 * @NoAdminRequired
 	 * @NoCSRFRequired
 	 * @throws \Exception
diff --git a/controller/filecontroller.php b/controller/filecontroller.php
index c2d151a8..bd5cef40 100644
--- a/controller/filecontroller.php
+++ b/controller/filecontroller.php
@@ -11,19 +11,23 @@
 
 namespace OCA\Passman\Controller;
 
+use OCA\Passman\Service\FileService;
+use OCP\AppFramework\ApiController;
 use OCP\AppFramework\Db\DoesNotExistException;
-use OCP\IRequest;
 use OCP\AppFramework\Http\JSONResponse;
-use OCP\AppFramework\ApiController;
-use OCA\Passman\Service\FileService;
+use OCP\IRequest;
+use Psr\Log\LoggerInterface;
 
 class FileController extends ApiController {
 	private $userId;
 	private $fileService;
+	private $logger;
+
 	public function __construct($AppName,
-								IRequest $request,
+	                            IRequest $request,
 								$UserId,
-								FileService $fileService){
+		                        FileService $fileService,
+		                        LoggerInterface $logger) {
 		parent::__construct(
 			$AppName,
 			$request,
@@ -32,6 +36,7 @@ class FileController extends ApiController {
 			86400);
 		$this->userId = $UserId;
 		$this->fileService = $fileService;
+		$this->logger = $logger;
 	}
 
 
@@ -57,6 +62,7 @@ class FileController extends ApiController {
 	public function getFile($file_id) {
 		return new JSONResponse($this->fileService->getFile($file_id, $this->userId));
 	}
+
 	/**
 	 * @NoAdminRequired
 	 * @NoCSRFRequired
@@ -65,22 +71,44 @@ class FileController extends ApiController {
 		return new JSONResponse($this->fileService->deleteFile($file_id, $this->userId));
 	}
 
-	public function updateFile($file_id, $file_data, $filename){
-		try{
+	/**
+	 * @NoAdminRequired
+	 * @NoCSRFRequired
+	 */
+	public function deleteFiles($file_ids) {
+		$failed_file_ids = [];
+		if ($file_ids != null && !empty($file_ids)) {
+			$decoded_file_ids = json_decode($file_ids);
+			foreach ($decoded_file_ids as $file_id) {
+				try {
+					$this->fileService->deleteFile($file_id, $this->userId);
+				} catch (\Exception $e) {
+					$this->logger->error('Error deleting file (' . $file_id . ') in filecontroller:deleteFiles()',
+						['exception' => $e->getTrace(), 'message' => $e->getMessage()]);
+					$failed_file_ids[] = $file_id;
+					continue;
+				}
+			}
+		}
+		return new JSONResponse(array('ok' => empty($failed_file_ids), 'failed' => $failed_file_ids));
+	}
+
+	public function updateFile($file_id, $file_data, $filename) {
+		try {
 			$file = $this->fileService->getFile($file_id, $this->userId);
-		} catch (\Exception $doesNotExistException){
+		} catch (\Exception $doesNotExistException) {
 
 		}
-		if($file){
-			if($file_data) {
+		if ($file) {
+			if ($file_data) {
 				$file->setFileData($file_data);
 			}
-			if($filename) {
+			if ($filename) {
 				$file->setFilename($filename);
 			}
-			if($filename || $file_data){
+			if ($filename || $file_data) {
 				new JSONResponse($this->fileService->updateFile($file));
 			}
 		}
 	}
-}
\ No newline at end of file
+}
diff --git a/controller/vaultcontroller.php b/controller/vaultcontroller.php
index 357608eb..1455efa0 100644
--- a/controller/vaultcontroller.php
+++ b/controller/vaultcontroller.php
@@ -11,16 +11,18 @@
 
 namespace OCA\Passman\Controller;
 
+use OCA\Passman\Db\Credential;
+use OCA\Passman\Service\CredentialService;
 use OCA\Passman\Service\DeleteVaultRequestService;
-use OCA\Passman\Service\EncryptService;
+use OCA\Passman\Service\FileService;
 use OCA\Passman\Service\SettingsService;
+use OCA\Passman\Service\VaultService;
 use OCA\Passman\Utility\NotFoundJSONResponse;
+use OCP\AppFramework\ApiController;
 use OCP\AppFramework\Db\DoesNotExistException;
-use OCP\IRequest;
 use OCP\AppFramework\Http\JSONResponse;
-use OCP\AppFramework\ApiController;
-use OCA\Passman\Service\VaultService;
-use OCA\Passman\Service\CredentialService;
+use OCP\IRequest;
+use Psr\Log\LoggerInterface;
 
 
 class VaultController extends ApiController {
@@ -28,15 +30,19 @@ class VaultController extends ApiController {
 	private $vaultService;
 	private $credentialService;
 	private $settings;
+	private $fileService;
+	private $logger;
 	private $deleteVaultRequestService;
 
 	public function __construct($AppName,
-								IRequest $request,
+	                            IRequest $request,
 								$UserId,
-								VaultService $vaultService,
-								CredentialService $credentialService,
-								DeleteVaultRequestService $deleteVaultRequestService,
-								SettingsService $settings) {
+		                        VaultService $vaultService,
+		                        CredentialService $credentialService,
+		                        DeleteVaultRequestService $deleteVaultRequestService,
+		                        SettingsService $settings,
+		                        FileService $fileService,
+		                        LoggerInterface $logger) {
 		parent::__construct(
 			$AppName,
 			$request,
@@ -48,6 +54,8 @@ class VaultController extends ApiController {
 		$this->credentialService = $credentialService;
 		$this->deleteVaultRequestService = $deleteVaultRequestService;
 		$this->settings = $settings;
+		$this->fileService = $fileService;
+		$this->logger = $logger;
 	}
 
 	/**
@@ -63,7 +71,7 @@ class VaultController extends ApiController {
 			foreach ($vaults as $vault) {
 				$credential = $this->credentialService->getRandomCredentialByVaultId($vault->getId(), $this->userId);
 				$secret_field = $protected_credential_fields[array_rand($protected_credential_fields)];
-				if(isset($credential)) {
+				if (isset($credential)) {
 					array_push($result, array(
 						'vault_id' => $vault->getId(),
 						'guid' => $vault->getGuid(),
@@ -165,7 +173,30 @@ class VaultController extends ApiController {
 	 * @NoCSRFRequired
 	 */
 	public function delete($vault_guid) {
+		$failed_credential_guids = [];
+		try {
+			$vault = $this->vaultService->getByGuid($vault_guid, $this->userId);
+			$credentials = $this->credentialService->getCredentialsByVaultId($vault->getId(), $this->userId);
+
+			foreach ($credentials as $credential) {
+				if ($credential instanceof Credential) {
+					try {
+						// $credential = $this->credentialService->getCredentialByGUID($credential_guid, $this->userId);
+						$this->credentialService->deleteCredentiaL($credential);
+						$this->credentialService->deleteCredentialParts($credential, $this->userId);
+					} catch (\Exception $e) {
+						$this->logger->error('Error deleting credential (' . $credential->getId() . ') in vaultcontroller:delete()',
+							['exception' => $e->getTrace(), 'message' => $e->getMessage()]);
+						$failed_credential_guids[] = $credential->getGuid();
+						continue;
+					}
+				}
+			}
+		} catch (\Exception $e) {
+			return new NotFoundJSONResponse();
+		}
+
 		$this->vaultService->deleteVault($vault_guid, $this->userId);
-		return new JSONResponse(array('ok' => true));
+		return new JSONResponse(array('ok' => empty($failed_credential_guids), 'failed' => $failed_credential_guids));
 	}
-}
\ No newline at end of file
+}
diff --git a/js/app/controllers/settings.js b/js/app/controllers/settings.js
index 9fa1f155..21caea4a 100644
--- a/js/app/controllers/settings.js
+++ b/js/app/controllers/settings.js
@@ -87,7 +87,9 @@
 				});
 
 				var btn_txt = $translate.instant('bookmarklet.text');
-				var http = location.protocol, slashes = http.concat("//"), host = slashes.concat(window.location.hostname + ":" + window.location.port), complete = host + location.pathname;
+				var http = location.protocol, slashes = http.concat("//"),
+					host = slashes.concat(window.location.hostname + ":" + window.location.port),
+					complete = host + location.pathname;
 				$scope.bookmarklet = $sce.trustAsHtml("<a class=\"button\" href=\"javascript:(function(){var a=window,b=document,c=encodeURIComponent,e=c(document.title),d=a.open('" + complete + "/bookmarklet?url='+c(b.location)+'&title='+e,'bkmk_popup','left='+((a.screenX||a.screenLeft)+10)+',top='+((a.screenY||a.screenTop)+10)+',height=750px,width=475px,resizable=0,alwaysRaised=1');a.setTimeout(function(){d.focus()},300);})();\">" + btn_txt + "</a>");
 
 
@@ -271,43 +273,34 @@
 
 				$scope.confirm_vault_delete = false;
 				$scope.delete_vault_password = '';
-        $scope.delete_vault = function() {
-          if ($scope.confirm_vault_delete && $scope.delete_vault_password === VaultService.getActiveVault().vaultKey) {
-            getCurrentVaultCredentials(function(vault) {
-              var credentials = vault.credentials;
-              $scope.remove_pw = {
-                percent: 0,
-                done: 0,
-                total: vault.credentials.length,
-              };
-              var deleteCredential = function(index) {
-                $scope.translationData = {
-                  password: credentials[index].label,
-                };
-                CredentialService.destroyCredential(credentials[index].guid).then(function() {
-                  var percent = index / vault.credentials.length * 100;
-                  $scope.remove_pw = {
-                    percent: percent,
-                    done: index,
-                    total: vault.credentials.length,
-                  };
-                  if (index === credentials.length - 1) {
-                    VaultService.deleteVault(vault).then(function() {
-                      SettingsService.setSetting('defaultVaultPass', false);
-                      SettingsService.setSetting('defaultVault', null);
-                      $rootScope.$broadcast('logout');
-                      $location.path('/');
-                    });
-                    return;
-                  }
-                  deleteCredential(index + 1);
-                });
-              };
-              deleteCredential(0);
-            });
-          }
-
-        };
+				$scope.delete_vault = function () {
+					if ($scope.confirm_vault_delete && $scope.delete_vault_password === VaultService.getActiveVault().vaultKey) {
+						getCurrentVaultCredentials(function (vault) {
+							var credentials = vault.credentials;
+							$scope.remove_pw = {
+								percent: 0,
+								done: 0,
+								total: vault.credentials.length,
+							};
+
+							var file_ids = [];
+							for (const credential of credentials) {
+								var decryptedFiles = JSON.parse(EncryptService.decryptString(angular.copy(credential.files), VaultService.getActiveVault().vaultKey));
+								for (const file of decryptedFiles) {
+									file_ids.push(file.file_id);
+								}
+							}
+
+							VaultService.deleteVault(vault, file_ids).then(function () {
+								SettingsService.setSetting('defaultVaultPass', false);
+								SettingsService.setSetting('defaultVault', null);
+								$rootScope.$broadcast('logout');
+								$location.path('/');
+							});
+						});
+					}
+
+				};
 
 				$rootScope.$on('logout', function () {
 					$scope.active_vault = null;
diff --git a/js/app/services/vaultservice.js b/js/app/services/vaultservice.js
index 767466b5..188f7c06 100644
--- a/js/app/services/vaultservice.js
+++ b/js/app/services/vaultservice.js
@@ -66,7 +66,7 @@
 						return false;
 					} else {
 						_activeVault.vault_settings[key] = value;
-                        this.updateVault(_activeVault);
+						this.updateVault(_activeVault);
 					}
 
 				},
@@ -122,21 +122,27 @@
 						}
 					});
 				},
-				deleteVault: function (vault) {
+				deleteVault: function (vault, file_ids) {
 					var queryUrl = OC.generateUrl('apps/passman/api/v2/vaults/' + vault.guid);
-					return $http.delete(queryUrl).then(function (response) {
-						if (response.data) {
-							return response.data;
-						} else {
-							return response;
-						}
+					var deleteFilesUrl = OC.generateUrl('apps/passman/api/v2/files/delete');
+					var filesData = {
+						"file_ids": JSON.stringify(file_ids)
+					};
+					return $http.post(deleteFilesUrl, filesData).then(function () {
+						return $http.delete(queryUrl).then(function (response) {
+							if (response.data) {
+								return response.data;
+							} else {
+								return response;
+							}
+						});
 					});
 				},
 				clearVaultService: function () {
-					_activeVault=null;
+					_activeVault = null;
 				}
 			};
 
 			return service;
 		}]);
-}());
\ No newline at end of file
+}());
diff --git a/lib/Service/CredentialService.php b/lib/Service/CredentialService.php
index 69fcc973..1679dfda 100644
--- a/lib/Service/CredentialService.php
+++ b/lib/Service/CredentialService.php
@@ -23,6 +23,7 @@
 
 namespace OCA\Passman\Service;
 
+use OCA\Passman\Activity;
 use OCA\Passman\Db\Credential;
 use OCA\Passman\Db\CredentialMapper;
 use OCA\Passman\Db\SharingACL;
@@ -37,15 +38,25 @@ class CredentialService {
 
 	private CredentialMapper $credentialMapper;
 	private SharingACLMapper $sharingACL;
+	private ActivityService $activityService;
 	private ShareService $shareService;
 	private EncryptService $encryptService;
+	private CredentialRevisionService $credentialRevisionService;
 	private $server_key;
 
-	public function __construct(CredentialMapper $credentialMapper, SharingACLMapper $sharingACL, ShareService $shareService, EncryptService $encryptService, IConfig $config) {
+	public function __construct(CredentialMapper          $credentialMapper,
+	                            SharingACLMapper          $sharingACL,
+	                            ActivityService           $activityService,
+	                            ShareService              $shareService,
+	                            EncryptService            $encryptService,
+	                            CredentialRevisionService $credentialRevisionService,
+	                            IConfig                   $config) {
 		$this->credentialMapper = $credentialMapper;
 		$this->sharingACL = $sharingACL;
+		$this->activityService = $activityService;
 		$this->shareService = $shareService;
 		$this->encryptService = $encryptService;
+		$this->credentialRevisionService = $credentialRevisionService;
 		$this->server_key = $config->getSystemValue('passwordsalt', '');
 	}
 
@@ -100,6 +111,25 @@ class CredentialService {
 	}
 
 	/**
+	 * Delete leftovers from a credential
+	 * @param Credential $credential
+	 * @throws \Exception
+	 */
+	public function deleteCredentialParts(Credential $credential, $userId) {
+		$this->activityService->add(
+			'item_destroyed_self', array($credential->getLabel()),
+			'', array(),
+			'', $userId, Activity::TYPE_ITEM_ACTION);
+		$this->shareService->unshareCredential($credential->getGuid());
+		foreach ($this->credentialRevisionService->getRevisions($credential->getId()) as $revision) {
+			$id = $revision['revision_id'];
+			if (isset($id)) {
+				$this->credentialRevisionService->deleteRevision($id, $userId);
+			}
+		}
+	}
+
+	/**
 	 * Get credentials by vault id
 	 *
 	 * @param int $vault_id