From cb61d4d8d1d1f05894e6d59feac4fe10c6f527a4 Mon Sep 17 00:00:00 2001 From: Eric Doughty-Papassideris Date: Sun, 9 Mar 2025 23:36:58 +0100 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8=20back:=20delete=20user=20data=20admi?= =?UTF-8?q?nistrative=20endpoint=20(#799)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../config/custom-environment-variables.json | 3 +- .../controller/delete-user-controller.ts | 28 +- .../core/platform/services/admin/service.ts | 313 ++++++++++++------ .../src/core/platform/services/admin/utils.ts | 15 +- .../services/admin/web/delete-user-routes.ts | 23 +- .../storage/connectors/S3/s3-service.ts | 30 +- .../storage/default-storage-strategy.ts | 3 +- .../storage/oneof-storage-strategy.ts | 11 +- .../platform/services/storage/provider.ts | 11 +- .../services/storage/storage-service.ts | 13 +- .../src/services/console/client-interface.ts | 6 + .../src/services/console/clients/internal.ts | 5 + .../src/services/console/clients/remote.ts | 12 +- .../node/src/services/files/services/index.ts | 13 +- .../src/services/user/services/companies.ts | 21 +- .../services/user/services/users/service.ts | 40 ++- .../services/workspaces/services/workspace.ts | 13 +- .../test/e2e/users/users.deletion.spec.ts | 177 +++++----- .../storage/oneof-storage-strategy.test.ts | 4 +- .../docker-compose.dev.tests.opensearch.yml | 1 + tdrive/docker-compose.tests.yml | 1 + 21 files changed, 485 insertions(+), 258 deletions(-) diff --git a/tdrive/backend/node/config/custom-environment-variables.json b/tdrive/backend/node/config/custom-environment-variables.json index 535b317d..b14e85f3 100644 --- a/tdrive/backend/node/config/custom-environment-variables.json +++ b/tdrive/backend/node/config/custom-environment-variables.json @@ -102,7 +102,8 @@ "useSSL": "STORAGE_S3_USE_SSL", "accessKey": "STORAGE_S3_ACCESS_KEY", "secretKey": "STORAGE_S3_SECRET_KEY", - "disableRemove": "STORAGE_S3_DISABLE_REMOVE" + "disableRemove": "STORAGE_S3_DISABLE_REMOVE", + "overrideDisableRemoveForUserAccountDeletion": "STORAGE_S3_OVERRIDE_DISABLE_REMOVE_FOR_USER_ACCOUNT_DELETION" } }, "email-pusher": { diff --git a/tdrive/backend/node/src/core/platform/services/admin/controller/delete-user-controller.ts b/tdrive/backend/node/src/core/platform/services/admin/controller/delete-user-controller.ts index 80c6b935..0fb2be94 100644 --- a/tdrive/backend/node/src/core/platform/services/admin/controller/delete-user-controller.ts +++ b/tdrive/backend/node/src/core/platform/services/admin/controller/delete-user-controller.ts @@ -12,31 +12,35 @@ export class AdminDeleteUserController { return this._repos; } - /** Begin or forward the deletion process of a user */ - async deleteUser(userId: string): Promise<"failed" | "deleting" | "done"> { + /** Begin or forward the deletion process of a user, if `deleteData` is false, only anonymises the user entry */ + async deleteUser(userId: string, deleteData: boolean): Promise<"failed" | "deleting" | "done"> { try { - await gr.services.users.anonymizeAndDelete({ id: userId }, { - user: { server_request: true }, - company: { id: "// TODO: REPLACE WITH COMPANY ID" }, - } as unknown as ExecutionContext); + if ( + await gr.services.users.anonymizeAndDelete( + { id: userId }, + { + user: { server_request: true }, + } as unknown as ExecutionContext, + deleteData, + ) + ) + return "done"; const existingUser = await (await this.getRepos()).user.findOne({ id: userId }); if (existingUser?.deleted) { if (existingUser.delete_process_started_epoch > 0) return "deleting"; } } catch (err) { adminLogger.error({ err, userId }, "User deletion error"); - console.log(err); //TODO: NONONO return "failed"; } return "done"; } - /** Get an array of user IDs that are incompletely deleted */ + /** Get an array of 2 item arrays with `[ user IDs, delete_process_started_epoch ]` that are incompletely deleted */ async listUsersPendingDeletion() { - return ( + const users = ( await (await this.getRepos()).user.find({}, { $gt: [["delete_process_started_epoch", 0]] }) - ) - .getEntities() - .map(({ id }) => id); + ).getEntities(); + return users.map(({ id, delete_process_started_epoch }) => [id, delete_process_started_epoch]); } } diff --git a/tdrive/backend/node/src/core/platform/services/admin/service.ts b/tdrive/backend/node/src/core/platform/services/admin/service.ts index f6c7396f..cd3410eb 100644 --- a/tdrive/backend/node/src/core/platform/services/admin/service.ts +++ b/tdrive/backend/node/src/core/platform/services/admin/service.ts @@ -6,9 +6,16 @@ import { adminLogger, buildUserDeletionRepositories, descendDriveItemsDepthFirstRandomOrder, + findFileOfVersion, loadRawVersionsOfItemForDeletion, runInBatchesAreAllTrue, } from "./utils"; +import type Repository from "../database/services/orm/repository/repository"; +import type { Logger } from "pino"; +import type { DriveFile } from "../../../../services/documents/entities/drive-file"; +import { getFilePath, getUserPath } from "../../../../services/files/services"; +import type { File } from "../../../../services/files/entities/file"; +import type { FileVersion } from "../../../../services/documents/entities/file-version"; export default class AdminServiceImpl implements AdminServiceAPI { version: "1"; @@ -28,15 +35,19 @@ export default class AdminServiceImpl implements AdminServiceAPI { private async deleteS3Paths( logger: pino.Logger, paths: string[], - batchSize = 5, + batchSize = 10, ): Promise { return await runInBatchesAreAllTrue(batchSize, paths, async paths => Promise.all( - paths.map(path => { + paths.map(async path => { try { - return gr.platformServices.storage.remove("x" + path); + return await gr.platformServices.storage.remove( + path, + undefined, + undefined, + "admin:user_account_deletion", + ); } catch (err) { - console.log(err); logger.error({ err, path }, "Error deleting storage item"); return false; } @@ -45,109 +56,219 @@ export default class AdminServiceImpl implements AdminServiceAPI { ); } + /** Remove a DB entity, returns false if it threw an error, just logs if no entries were deleted and returns true */ + private async safeDBDelete( + deleteUserLogger: Logger, + repo: Repository, + item: T, + ): Promise { + try { + const result = await repo.remove(item); + if (!result) + // No error but nothing deleted, just move on + deleteUserLogger.warn({ item, result }, `Failed to delete ${repo.table}, 0 result count`); + return true; + } catch (err) { + deleteUserLogger.error({ err, item }, `Error deleting ${repo.table}`); + } + return false; + } + + /** Delete all the storage for a File, and if succesful, the File entry from the database */ + private async safeDeleteFile(deleteUserLogger: Logger, file: File): Promise { + const paths = await gr.platformServices.storage.enumeratePathsForFile(getFilePath(file)); + if (paths.length === 0) deleteUserLogger.warn({ file, paths }, "No paths found for File"); + else if (!(await this.deleteS3Paths(deleteUserLogger, paths))) { + deleteUserLogger.error({ file, paths }, "Failed to delete paths"); + return false; + } + return await this.safeDBDelete(deleteUserLogger, (await this.repos).file, file); + } + + /** Delette version and potential related File with {@link safeDeleteFile} */ + private async safeDeleteVersion( + deleteUserLogger: Logger, + version: FileVersion, + file?: false | File, + ): Promise { + if (file !== false) { + file = file || (await findFileOfVersion(await this.repos, version)); + if (file && !(await this.safeDeleteFile(deleteUserLogger, file))) return false; + } + return await this.safeDBDelete(deleteUserLogger, (await this.repos).fileVersion, version); + } + + /** + * Attempt to delete the DriveFile from the database, including: + * - related FileVersion and File entities, + * - all paths in storage + * - any search document + * + * @warn Does not check the DriveFile has no children + */ + private async safeDeleteSingleDriveFile( + deleteUserLogger: Logger, + item: DriveFile, + ): Promise { + let canDeleteItem = true; + const versionsAssets = await loadRawVersionsOfItemForDeletion(await this.repos, item, false); + for (const { version, file } of versionsAssets) { + if ( + !(await this.safeDeleteVersion(deleteUserLogger, version, item.is_directory ? false : file)) + ) + canDeleteItem = false; + } + try { + await (await this.repos).search.driveFile.service.remove([item as never]); + } catch (err) { + // canDeleteItem = false; // Search documents would be filtered out by the missing DriveFile, so continue + deleteUserLogger.error({ err, item }, "Error deleting drive item search entry"); + } + if (canDeleteItem) + if (!(await this.safeDBDelete(deleteUserLogger, (await this.repos).driveFile, item))) + return false; + return canDeleteItem; + } + + /** + * This call deletes all data related to a user. This means anything uploaded by the + * user, including shared items, (but not things shared by another with the user). + * + * The call is meant to be called repeatedly until it succeeds, and should resist + * concurrent calls eventually succeeding (though avoid if possible, it's not productive). + * It should also resist being interrupted by a timeout mid-way, and resuming on a later + * call. + * + * Multiple phases are involved, some must succeed to continue, some failures are ignored + * as the output should be filtered in other ways. + * + * - Depth-first recurse DriveFiles by parent_id + * - For files + * - For each FileVersion: + * - Delete all storage blobs + * - Delete related File + * - Delete related FileVersion + * - Attempt to remove from search index + * - Delete DriveFile + * - For directories: delete if all children were deleted + * + * - Locate `DriveFile`s, `FileVersion`s then `File`s that have the creator_id + * set to the user being deleted, and delete them individually in a similar + * way to above + * - Delete company users and external users + * - Set user's `delete_process_started_epoch` field to 0 if succesful + * + * @param user {@link User} entity to delete + * @returns `true` if the user was completely deleted + */ public async deleteUser(user: User): Promise { - /* - //TODO: NONONO + const deleteUserLogger = adminLogger.child({ adminOp: "DeleteUser", user: user.id }); -// todo: do test with wrong s3 delete path to ignore specifically only non existing errors - - Phases: - - Recurse drive item - - if file - - Each version - - delete all s3 - - then file - - then the version - - delete search document - - delete item - - if folder - delete if empty - - search items by creator - - search files by userid - - versions ? - - search leftover s3 - - // search for file by user id after deletion - // search s3 for prefix company/userid - */ - const deleteUserLogger = adminLogger.child({ adminOp: "DeleteUser", user }); const result = await descendDriveItemsDepthFirstRandomOrder( await this.repos, "user_" + user.id, async (item, children, _parents) => { - let canDeleteItem = true; - if (!item.is_directory) { - const versionsAssets = await loadRawVersionsOfItemForDeletion( - await this.repos, - item, - true, + if (children === undefined || children.every(x => !!x)) { + if (item.is_directory && children.length == 0) + deleteUserLogger.warn({ item }, "Deleting empty directory"); + if (!(await this.safeDeleteSingleDriveFile(deleteUserLogger, item))) return false; + } else { + deleteUserLogger.error( + { item }, + "Not deleting directory as at least one child failed to delete", ); - for (const { version, file, paths } of versionsAssets) { - if (paths.length > 0 && !(await this.deleteS3Paths(deleteUserLogger, paths))) { - deleteUserLogger.error({ paths }, "Failed to delete paths"); - canDeleteItem = false; - } else { - try { - if (file) { - const result = await (await this.repos).file.remove(file); - if (!result) - // No error but nothing deleted, just move on - deleteUserLogger.warn({ file, result }, "Failed to delete file"); - } - } catch (err) { - canDeleteItem = false; - deleteUserLogger.error({ err, file }, "Error deleting file"); - } - if (canDeleteItem && version) - try { - if (version) { - const result = await (await this.repos).fileVersion.remove(version); - if (!result) - // No error but nothing deleted, just move on - deleteUserLogger.warn({ version, result }, "Failed to delete version"); - } - } catch (err) { - canDeleteItem = false; - deleteUserLogger.error({ err, version }, "Error deleting version"); - } - } - } + return false; } - if (canDeleteItem) { - if (children === undefined || children.every(x => !!x)) { - if (item.is_directory && children.length == 0) - deleteUserLogger.warn({ item }, "Deleting empty directory"); - try { - await (await this.repos).search.driveFile.service.remove([item as never]); - } catch (err) { - canDeleteItem = false; - deleteUserLogger.error({ err, item }, "Error deleting drive item search entry"); - } - if (canDeleteItem) - try { - const result = await (await this.repos).driveFile.remove(item); - if (!result) - // No error but nothing deleted, just move on - deleteUserLogger.warn({ item, result }, "Failed to delete drive item"); - } catch (err) { - canDeleteItem = false; - deleteUserLogger.error({ err, item }, "Error deleting drive item"); - } - } else { - deleteUserLogger.error( - { item }, - "Not deleting directory as at least one child failed to delete", - ); - canDeleteItem = false; - } - } - return canDeleteItem; + return true; }, ); + + // Stop if deleting root DriveFiles didn't succeed if (!result.every(x => !!x)) return false; - //TODO: error checking and such + + // Stray items out of parent_id etc tree attached to the user in other ways + let canContinueDeleting = true; + + const driveItemsByCreator = ( + await (await this.repos).driveFile.find({ creator: user.id }) + ).getEntities(); + deleteUserLogger.info( + { canContinueDeleting, driveItemsByCreator: driveItemsByCreator.length }, + "Stray driveItemsByCreator", + ); + for (const creatorDriveFile of driveItemsByCreator) + if (!(await this.safeDeleteSingleDriveFile(deleteUserLogger, creatorDriveFile))) + canContinueDeleting = false; + if (!canContinueDeleting) return false; + + const versionsByCreator = ( + await (await this.repos).fileVersion.find({ creator_id: user.id }) + ).getEntities(); + deleteUserLogger.info( + { canContinueDeleting, versionsByCreator: versionsByCreator.length }, + "Stray versionsByCreator", + ); + for (const creatorVersion of versionsByCreator) + if (!(await this.safeDeleteVersion(deleteUserLogger, creatorVersion))) + canContinueDeleting = false; + if (!canContinueDeleting) return false; + deleteUserLogger.info( + { canContinueDeleting, versionsByCreator: versionsByCreator.length }, + "Stray versionsByCreator", + ); + + const filesByCreator = (await (await this.repos).file.find({ user_id: user.id })).getEntities(); + deleteUserLogger.info( + { canContinueDeleting, filesByCreator: filesByCreator.length }, + "Stray filesByCreator", + ); + for (const creatorFile of filesByCreator) + if (!(await this.safeDeleteFile(deleteUserLogger, creatorFile))) canContinueDeleting = false; + if (!canContinueDeleting) return false; + + // Remove stray S3 data by prefix folder, then de-associate from company if ok + const companyUsers = ( + await (await this.repos).companyUser.find({ user_id: user.id }) + ).getEntities(); + deleteUserLogger.info( + { canContinueDeleting, companyUsers: companyUsers.length }, + "companyUsers", + ); + for (const companyUser of companyUsers) { + const paths = await gr.platformServices.storage.enumeratePathsForFile( + getUserPath(user.id, companyUser.group_id), + ); + if (!(await this.deleteS3Paths(deleteUserLogger, paths))) canContinueDeleting = false; + else if ( + !(await this.safeDBDelete(deleteUserLogger, (await this.repos).companyUser, companyUser)) + ) { + deleteUserLogger.warn({ companyUser }, "Failed to delete database entry"); + canContinueDeleting = false; + } + } + + const externalUsers = ( + await (await this.repos).externalUser.find({ user_id: user.id }) + ).getEntities(); + deleteUserLogger.info( + { canContinueDeleting, externalUsers: externalUsers.length }, + "externalUsers", + ); + for (const externalUser of externalUsers) + if ( + !(await this.safeDBDelete(deleteUserLogger, (await this.repos).externalUser, externalUser)) + ) + canContinueDeleting = false; + await (await this.repos).search.user.service.remove([user as never]); - user.delete_process_started_epoch = 0; - await (await this.repos).user.save(user); - return true; + + if (canContinueDeleting) { + const latestUser = await (await this.repos).user.findOne({ id: user.id }); + deleteUserLogger.info("User deletion complete, zeroing delete_process_started_epoch"); + latestUser.delete_process_started_epoch = 0; + await (await this.repos).user.save(latestUser); + } + + return canContinueDeleting; } } diff --git a/tdrive/backend/node/src/core/platform/services/admin/utils.ts b/tdrive/backend/node/src/core/platform/services/admin/utils.ts index 497b432f..6b9d1521 100644 --- a/tdrive/backend/node/src/core/platform/services/admin/utils.ts +++ b/tdrive/backend/node/src/core/platform/services/admin/utils.ts @@ -70,7 +70,7 @@ export async function buildUserDeletionRepositories( db: DatabaseServiceAPI = gr.database, search: SearchServiceAPI = gr.platformServices.search, ) { - return { + const result = { driveFile: await db.getRepository(DriveFileTYPE, DriveFile), file: await db.getRepository(FileTYPE, File), fileVersion: await db.getRepository(FileVersionTYPE, FileVersion), @@ -81,13 +81,22 @@ export async function buildUserDeletionRepositories( company: await db.getRepository(CompanyType, Company), // This one is not typical because it's only valid for remote account type: session: gr.services.console.getSessionRepo() as Repository | null, - //TODO: checkout what to do with session and user_online (seen in prod) + //TODO: checkout what to do with user_online (seen in prod) search: { driveFile: await search.getRepository(DriveFileTYPE, DriveFile), user: await search.getRepository(UserTYPE, User), }, }; + await Promise.all(Object.values(result).map(item => "init" in item && item.init())); + return result; +} + +export async function findFileOfVersion( + repos: TUserDeletionRepos, + version: FileVersion, +): Promise { + return await repos.file.findOne({ id: version.file_metadata.external_id }); } /** Return versions, sorted by oldest first, and related DB and S3 entries for a given DriveFile */ @@ -106,7 +115,7 @@ export async function loadRawVersionsOfItemForDeletion( (await repos.fileVersion.find({ drive_item_id: item.id }, { sort: { date_added: "asc" } })) .getEntities() .map(async version => { - const file = await repos.file.findOne({ id: version.file_metadata.external_id }); + const file = await findFileOfVersion(repos, version); if (!loadStoragePaths) return { version, file }; const paths = file ? await gr.platformServices.storage.enumeratePathsForFile(getFilePath(file)) diff --git a/tdrive/backend/node/src/core/platform/services/admin/web/delete-user-routes.ts b/tdrive/backend/node/src/core/platform/services/admin/web/delete-user-routes.ts index 5dd7180a..13cc44b2 100644 --- a/tdrive/backend/node/src/core/platform/services/admin/web/delete-user-routes.ts +++ b/tdrive/backend/node/src/core/platform/services/admin/web/delete-user-routes.ts @@ -14,30 +14,35 @@ function authenticateAdminQuery(request: FastifyRequest, reply: FastifyReply) { return true; } -type TUserDeleteQueryBody = TQueryBody & { userId: string }; +type TUserDeleteQueryBody = TQueryBody & { userId: string; deleteData: boolean }; function getUserIfValidQuery(request: FastifyRequest, reply: FastifyReply) { - if (!authenticateAdminQuery(request, reply)) return false; + if (!authenticateAdminQuery(request, reply)) return { userId: "", deleteData: false }; const body = request.body as TUserDeleteQueryBody; if (!body.userId?.length) { reply.status(400).send(); - return false; + return { userId: "", deleteData: false }; } - return body.userId; + return body; } const routes: FastifyPluginCallback = async (fastify: FastifyInstance, _opts, next) => { + const urlRoot = "/api/user/delete"; const config = getConfig(); const controller = new AdminDeleteUserController(); if (config?.endpointSecret?.length) { - fastify.post("/user/delete", async (request, reply) => { - const userId = getUserIfValidQuery(request, reply); + fastify.post(urlRoot, async (request, reply) => { + const { userId, deleteData } = getUserIfValidQuery(request, reply); if (!userId) return false; - return reply.send({ status: await controller.deleteUser(userId) }); + if (userId == "e2e_simulate_timeout") + return new Promise(() => { + return; + }); + return { status: await controller.deleteUser(userId, deleteData) }; }); - fastify.post("/user/delete/pending", async (request, reply) => { + fastify.post(`${urlRoot}/pending`, async (request, reply) => { if (!authenticateAdminQuery(request, reply)) return false; - return reply.send(await controller.listUsersPendingDeletion()); + return await controller.listUsersPendingDeletion(); }); } next(); diff --git a/tdrive/backend/node/src/core/platform/services/storage/connectors/S3/s3-service.ts b/tdrive/backend/node/src/core/platform/services/storage/connectors/S3/s3-service.ts index dbe5cc0b..59960e61 100644 --- a/tdrive/backend/node/src/core/platform/services/storage/connectors/S3/s3-service.ts +++ b/tdrive/backend/node/src/core/platform/services/storage/connectors/S3/s3-service.ts @@ -15,7 +15,13 @@ export type S3Configuration = { useSSL: boolean; accessKey: string; secretKey: string; + /** If `true`, S3 file removal is disabled (see exception of {@link overrideDisableRemoveForUserAccountDeletion}) */ disableRemove: boolean; + /** + * If {@link disableRemove} is `true`, but the deletion is in the context of a user account deletion by an administrator, + * then override and delete from S3 if `overrideDisableRemoveForUserAccountDeletion` is also `true`. + */ + overrideDisableRemoveForUserAccountDeletion: boolean; }; export default class S3ConnectorService implements StorageConnectorAPI { @@ -31,6 +37,12 @@ export default class S3ConnectorService implements StorageConnectorAPI { if (confCopy.useSSL && typeof confCopy.useSSL === "string") { confCopy.useSSL = !(!confCopy.useSSL || confCopy.useSSL === "false"); } + if ( + confCopy.overrideDisableRemoveForUserAccountDeletion && + typeof confCopy.overrideDisableRemoveForUserAccountDeletion === "string" + ) + confCopy.overrideDisableRemoveForUserAccountDeletion = + confCopy.overrideDisableRemoveForUserAccountDeletion === "true"; this.client = new Minio.Client(confCopy); this.minioConfiguration = confCopy; this.id = this.minioConfiguration.id; @@ -114,16 +126,28 @@ export default class S3ConnectorService implements StorageConnectorAPI { return this.client.getObject(this.minioConfiguration.bucket, path); } - async remove(path: string): Promise { + async remove( + path: string, + _options?: undefined, + _context?: undefined, + deletionCause?: "admin:user_account_deletion", + ): Promise { try { - if (this.minioConfiguration.disableRemove) { + if ( + this.minioConfiguration.disableRemove && + (!this.minioConfiguration.overrideDisableRemoveForUserAccountDeletion || + deletionCause !== "admin:user_account_deletion") + ) { logger.info(`File ${path} wasn't removed, file removal is disabled in configuration`); return true; } else { + // This call never fails, whether path exists or not await this.client.removeObject(this.minioConfiguration.bucket, path); return true; } - } catch (err) {} + } catch (err) { + logger.error({ err, path }, "Error deleting S3 path"); + } return false; } diff --git a/tdrive/backend/node/src/core/platform/services/storage/default-storage-strategy.ts b/tdrive/backend/node/src/core/platform/services/storage/default-storage-strategy.ts index 99dcc5bf..1b1fef54 100644 --- a/tdrive/backend/node/src/core/platform/services/storage/default-storage-strategy.ts +++ b/tdrive/backend/node/src/core/platform/services/storage/default-storage-strategy.ts @@ -48,7 +48,8 @@ export class DefaultStorageStrategy implements StorageConnectorAPI { path: string, options?: DeleteOptions, context?: ExecutionContext, + deletionCause?: undefined | "admin:user_account_deletion", ): Promise => { - return this.connector.remove(path, options, context); + return this.connector.remove(path, options, context, deletionCause); }; } diff --git a/tdrive/backend/node/src/core/platform/services/storage/oneof-storage-strategy.ts b/tdrive/backend/node/src/core/platform/services/storage/oneof-storage-strategy.ts index 865c024e..03cbe9bd 100644 --- a/tdrive/backend/node/src/core/platform/services/storage/oneof-storage-strategy.ts +++ b/tdrive/backend/node/src/core/platform/services/storage/oneof-storage-strategy.ts @@ -172,8 +172,15 @@ export class OneOfStorageStrategy implements StorageConnectorAPI { * @param path - The path of the file to be removed. * @param options */ - remove = async (path: string, options?: DeleteOptions): Promise => { - return Promise.all(this.storages.map(storage => storage.remove(path, options))).then(array => { + remove = async ( + path: string, + options?: DeleteOptions, + context?: undefined, + deletionCause?: "admin:user_account_deletion", + ): Promise => { + return Promise.all( + this.storages.map(storage => storage.remove(path, options, context, deletionCause)), + ).then(array => { return array.reduce((a, b) => a && b); }); }; diff --git a/tdrive/backend/node/src/core/platform/services/storage/provider.ts b/tdrive/backend/node/src/core/platform/services/storage/provider.ts index f7131634..67d0f9c4 100644 --- a/tdrive/backend/node/src/core/platform/services/storage/provider.ts +++ b/tdrive/backend/node/src/core/platform/services/storage/provider.ts @@ -61,8 +61,17 @@ export interface StorageConnectorAPI extends IServiceDiagnosticProvider { * Remove a path * * @param path + * @param deletionCause In case of exceptional uses of deletion, eg. legal compliance, a reason + * for the deletion can be provided here and evaluated by the StorageConnector. + * + * In normal use, pass `undefined` */ - remove(path: string, options?: DeleteOptions, context?: ExecutionContext): Promise; + remove( + path: string, + options?: DeleteOptions, + context?: ExecutionContext, + deletionCause?: undefined | "admin:user_account_deletion", + ): Promise; /** * Enumerate all physical storage paths related to the provided file path diff --git a/tdrive/backend/node/src/core/platform/services/storage/storage-service.ts b/tdrive/backend/node/src/core/platform/services/storage/storage-service.ts index 816c46d6..34611965 100644 --- a/tdrive/backend/node/src/core/platform/services/storage/storage-service.ts +++ b/tdrive/backend/node/src/core/platform/services/storage/storage-service.ts @@ -172,12 +172,18 @@ export default class StorageService extends TdriveService implements return stream; } - async remove(path: string, options?: DeleteOptions) { + async remove( + path: string, + options?: DeleteOptions, + context?: undefined, + deletionCause?: undefined | "admin:user_account_deletion", + ) { try { for (let count = 1; count <= (options?.totalChunks || 1); count++) { const chunk = options?.totalChunks ? `${path}/chunk${count}` : path; - await this.getConnector().remove(chunk); + await this.getConnector().remove(chunk, options, context, deletionCause); } + if (!options) await this.getConnector().remove(path, options, context, deletionCause); return true; } catch (err) { logger.error("Unable to remove file %s", err); @@ -207,7 +213,8 @@ export default class StorageService extends TdriveService implements * "useSSL": false, * "accessKey": "ABCD", * "secretKey": "x1yz", - * "disableRemove": false + * "disableRemove": false, + * "overrideDisableRemoveForUserAccountDeletion": true, * }, * "local": { * "path": "/tdrive" diff --git a/tdrive/backend/node/src/services/console/client-interface.ts b/tdrive/backend/node/src/services/console/client-interface.ts index c18d1c0d..01878cc5 100644 --- a/tdrive/backend/node/src/services/console/client-interface.ts +++ b/tdrive/backend/node/src/services/console/client-interface.ts @@ -68,5 +68,11 @@ export interface ConsoleServiceClient { backChannelLogout(logoutToken: string): Promise; + /** + * Similar to backChannelLogout, but must be called by administrative authorized + * code in case of user immediate kick out + */ + userWasDeletedForceLogout(userId: string): Promise; + resendVerificationEmail(email: string): Promise; } diff --git a/tdrive/backend/node/src/services/console/clients/internal.ts b/tdrive/backend/node/src/services/console/clients/internal.ts index b4b830c0..a113e5b1 100644 --- a/tdrive/backend/node/src/services/console/clients/internal.ts +++ b/tdrive/backend/node/src/services/console/clients/internal.ts @@ -115,6 +115,11 @@ export class ConsoleInternalClient implements ConsoleServiceClient { throw new Error("Method should not be implemented."); } + async userWasDeletedForceLogout(_userId: string) { + logger.info("Internal: userWasDeletedForceLogout"); + throw new Error("Method should not be implemented."); + } + async verifyJwtSid(_sid: string): Promise { logger.info("Internal: verifyJwtSid"); } diff --git a/tdrive/backend/node/src/services/console/clients/remote.ts b/tdrive/backend/node/src/services/console/clients/remote.ts index 72af137f..1ce06ded 100644 --- a/tdrive/backend/node/src/services/console/clients/remote.ts +++ b/tdrive/backend/node/src/services/console/clients/remote.ts @@ -199,7 +199,7 @@ export class ConsoleRemoteClient implements ConsoleServiceClient { if (!user) { throw CrudException.notFound(`User ${consoleUserId} doesn't exists`); } - await gr.services.companies.removeUserFromCompany({ id: company.id }, { id: user.id }); + await gr.services.companies.removeUserFromCompany({ id: company.id }, user.id); } async removeUser(consoleUserId: string): Promise { @@ -328,6 +328,16 @@ export class ConsoleRemoteClient implements ConsoleServiceClient { } } + async userWasDeletedForceLogout(userId: string) { + const sessionRepository = gr.services.console.getSessionRepo(); + if (!sessionRepository) return; + const sessions = (await sessionRepository.find({ sub: userId })).getEntities(); + for (const session of sessions) { + session.revoked_at = new Date().getTime(); + await sessionRepository.save(session); + } + } + async verifyJwtSid(sid: string): Promise { const sessionRepository = gr.services.console.getSessionRepo(); if (sid) { diff --git a/tdrive/backend/node/src/services/files/services/index.ts b/tdrive/backend/node/src/services/files/services/index.ts index e6330ab7..b5ac6f64 100644 --- a/tdrive/backend/node/src/services/files/services/index.ts +++ b/tdrive/backend/node/src/services/files/services/index.ts @@ -478,10 +478,17 @@ export class FileServiceImpl { return this.algorithm; } } + +/** Get the storage path prefix specific to a user of a company */ +export const getUserPath = (user_id: string, company_id: string): string => { + return `${gr.platformServices.storage.getHomeDir()}/files/${company_id}/${ + user_id || "anonymous" + }/`; +}; + +/** Get the storage path prefix specific to a given File of a user of a company */ export const getFilePath = ( entity: File | { company_id: string; user_id?: string; id: string }, ): string => { - return `${gr.platformServices.storage.getHomeDir()}/files/${entity.company_id}/${ - entity.user_id || "anonymous" - }/${entity.id}`; + return `${getUserPath(entity.user_id, entity.company_id)}${entity.id}`; }; diff --git a/tdrive/backend/node/src/services/user/services/companies.ts b/tdrive/backend/node/src/services/user/services/companies.ts index 8b3255df..53046ed6 100644 --- a/tdrive/backend/node/src/services/user/services/companies.ts +++ b/tdrive/backend/node/src/services/user/services/companies.ts @@ -13,7 +13,7 @@ import { import Repository, { FindOptions, } from "../../../core/platform/services/database/services/orm/repository/repository"; -import { UserPrimaryKey } from "../entities/user"; +import User, { UserPrimaryKey } from "../entities/user"; import Company, { CompanyPrimaryKey, CompanySearchKey, @@ -178,13 +178,13 @@ export class CompanyServiceImpl { async removeUserFromCompany( companyPk: CompanyPrimaryKey, - userPk: UserPrimaryKey, + userId: string, context?: ExecutionContext, ): Promise> { const entity = await this.companyUserRepository.findOne( { group_id: companyPk.id, - user_id: userPk.id, + user_id: userId, }, {}, context, @@ -192,7 +192,7 @@ export class CompanyServiceImpl { if (entity) { await Promise.all([this.companyUserRepository.remove(entity, context)]); - const user = await gr.services.users.get(userPk); + const user = await gr.services.users.get({ id: userId }); if ((user.cache?.companies || []).includes(companyPk.id)) { // Update user cache with companies user.cache.companies = user.cache.companies.filter(id => id != companyPk.id); @@ -304,14 +304,17 @@ export class CompanyServiceImpl { } async ensureDeletedUserNotInCompanies(userPk: UserPrimaryKey): Promise { - const user = await gr.services.users.get(userPk); - if (user.deleted) { - const companies = await this.getAllForUser(user.id); + const user = await gr.services.users.get({ id: userPk.id }); + if (user?.deleted ?? (userPk as User).deleted) { + const companies = await this.getAllForUser(user?.id ?? userPk.id); for (const company of companies) { logger.warn(`User ${userPk.id} is deleted so removed from company ${company.id}`); - await this.removeUserFromCompany(company, user); + await this.removeUserFromCompany(company, user?.id ?? userPk.id); try { - await gr.services.workspaces.ensureUserNotInCompanyIsNotInWorkspace(userPk, company.id); + await gr.services.workspaces.ensureUserNotInCompanyIsNotInWorkspace( + user?.id ?? userPk.id, + company.id, + ); } catch (err) { logger.error({ err }, "Error removing user from company from workspace"); } diff --git a/tdrive/backend/node/src/services/user/services/users/service.ts b/tdrive/backend/node/src/services/user/services/users/service.ts index 3ab0326c..0755217f 100644 --- a/tdrive/backend/node/src/services/user/services/users/service.ts +++ b/tdrive/backend/node/src/services/user/services/users/service.ts @@ -14,7 +14,7 @@ import Repository, { FindFilter, FindOptions, } from "../../../../core/platform/services/database/services/orm/repository/repository"; -import User, { UserPrimaryKey } from "../../entities/user"; +import User, { getInstance as getUserInstance, UserPrimaryKey } from "../../entities/user"; import { ListUserOptions, SearchUserOptions } from "./types"; import CompanyUser from "../../entities/company_user"; import SearchRepository from "../../../../core/platform/services/search/repository"; @@ -154,32 +154,38 @@ export class UserServiceImpl { return new DeleteResult("user", instance, !!instance); } - async anonymizeAndDelete(pk: UserPrimaryKey, context?: ExecutionContext) { + /** If `deleteData` is false, then the user is only marked deleted and no data is actually deleted */ + async anonymizeAndDelete(pk: UserPrimaryKey, context?: ExecutionContext, deleteData?: boolean) { const user = await this.get(pk); if (context.user.server_request || context.user.id === user.id) { - const userCopy = { ...user } as User; - //We keep a part of the user id as new name - const partialId = user.id.toString().split("-")[0]; + const userCopy = getUserInstance(user); - user.username_canonical = `deleted-user-${partialId}`; - user.email_canonical = `${partialId}@tdrive.removed`; - user.first_name = ""; - user.last_name = ""; - user.phone = ""; - user.picture = ""; - user.thumbnail_id = null; - user.status_icon = null; - user.deleted = true; - user.delete_process_started_epoch = new Date().getTime(); + if (!user.deleted) { + //We keep a part of the user id as new name + const partialId = user.id.toString().split("-")[0]; - await this.save(user); + user.username_canonical = `deleted-user-${partialId}`; + user.email_canonical = `${partialId}@tdrive.removed`; + user.first_name = ""; + user.last_name = ""; + user.phone = ""; + user.picture = ""; + user.thumbnail_id = null; + user.status_icon = null; + user.deleted = true; + user.delete_process_started_epoch = new Date().getTime(); + + await gr.services.console.getClient().userWasDeletedForceLogout(user.id); + + await this.save(user); + } localEventBus.publish("user:deleted", { user: user, }); - await gr.platformServices.admin.deleteUser(userCopy); + return deleteData && (await gr.platformServices.admin.deleteUser(userCopy)); } } diff --git a/tdrive/backend/node/src/services/workspaces/services/workspace.ts b/tdrive/backend/node/src/services/workspaces/services/workspace.ts index b1d4af45..675100e0 100644 --- a/tdrive/backend/node/src/services/workspaces/services/workspace.ts +++ b/tdrive/backend/node/src/services/workspaces/services/workspace.ts @@ -110,7 +110,10 @@ export class WorkspaceServiceImpl implements TdriveServiceProvider, Initializabl //If user deleted from a company, remove it from all workspace localEventBus.subscribe("company:user:deleted", async data => { if (data?.user?.id && data?.company?.id) - gr.services.workspaces.ensureUserNotInCompanyIsNotInWorkspace(data.user, data.company.id); + gr.services.workspaces.ensureUserNotInCompanyIsNotInWorkspace( + data.user.id, + data.company.id, + ); }); return this; @@ -717,7 +720,7 @@ export class WorkspaceServiceImpl implements TdriveServiceProvider, Initializabl } async ensureUserNotInCompanyIsNotInWorkspace( - userPk: UserPrimaryKey, + userId: string, companyId: string, context?: ExecutionContext, ): Promise { @@ -725,15 +728,15 @@ export class WorkspaceServiceImpl implements TdriveServiceProvider, Initializabl for (const workspace of workspaces) { const companyUser = await gr.services.companies.getCompanyUser( { id: workspace.company_id }, - userPk, + { id: userId }, context, ); if (!companyUser) { logger.warn( - `User ${userPk.id} is not in company ${workspace.company_id} so removing from workspace ${workspace.id}`, + `User ${userId} is not in company ${workspace.company_id} so removing from workspace ${workspace.id}`, ); await this.removeUser( - { workspaceId: workspace.id, userId: userPk.id }, + { workspaceId: workspace.id, userId: userId }, companyId, context, ).then(() => null); diff --git a/tdrive/backend/node/test/e2e/users/users.deletion.spec.ts b/tdrive/backend/node/test/e2e/users/users.deletion.spec.ts index 952fd421..c8ebb853 100644 --- a/tdrive/backend/node/test/e2e/users/users.deletion.spec.ts +++ b/tdrive/backend/node/test/e2e/users/users.deletion.spec.ts @@ -1,25 +1,21 @@ -import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it } from "@jest/globals"; +import { afterAll, beforeAll, describe, expect, it } from "@jest/globals"; import { init, TestPlatform } from "../setup"; -import { CompanyLimitsEnum } from "../../../src/services/user/web/types"; import UserApi from "../common/user-api"; -import type { DriveExecutionContext } from "../../../src/services/documents/types"; import { DriveFile, TYPE as DriveFileType } from "../../../src/services/documents/entities/drive-file"; import { File } from "../../../src/services/files/entities/file"; import { FileVersion, TYPE as FileVersionType } from "../../../src/services/documents/entities/file-version"; -import User, { TYPE as UserType } from "../../../src/services/user/entities/user"; -import ExternalUser, { TYPE as ExternalUserType } from "../../../src/services/user/entities/external_user"; -import { getThumbnailRoute } from "../../../src/services/files/web/routes"; +import type User from "../../../src/services/user/entities/user"; import { getFilePath } from "../../../src/services/files/services"; -const FileType = "files"; -import { buildUserDeletionRepositories, descendDriveItemsDepthFirstRandomOrder } from "../../../src/core/platform/services/admin/utils"; +import { buildUserDeletionRepositories } from "../../../src/core/platform/services/admin/utils"; import { getConfig } from "../../../src/core/platform/framework/api/admin"; const loadRepositories = async (platform: TestPlatform) => buildUserDeletionRepositories(platform!.database, platform!.search); describe("The users deletion API", () => { - const url = "/internal/services/users/v1"; let platform: TestPlatform | undefined; + let adminUser: UserApi; let currentUser: UserApi; + let userToTestPending: UserApi; let myUserId: string; let myDriveId: string; let myCompanyId: string; @@ -31,7 +27,7 @@ describe("The users deletion API", () => { driveFileByCreator: async (userId: string) => repos.driveFile.find({ creator: userId }), driveFileByParent: async (userId: string) => repos.driveFile.find({ parent_id: "user_" + userId }), fileVersion: async (userId: string) => repos.fileVersion.find({ creator_id: userId }), - // file: async (userId: string) => repos.file.find({ user_id: userId }), // user_id is encrypted for reasons + file: async (userId: string) => repos.file.find({ user_id: userId }), missedDriveFile: async (userId: string) => repos.missedDriveFile.find({ creator: userId }), // user: async (userId: string) => repos.user.find({ id: userId }), // ignored because it stays with deleted=true companyUser: async (userId: string) => repos.companyUser.find({ user_id: userId }), @@ -126,6 +122,15 @@ describe("The users deletion API", () => { } } + async function getSearchEntriesFromFoundEntries(items: ReturnType) { + const searchDoc = async ({id}) => (await repos.search.driveFile.search({ id }, { pagination: {limitStr: "100"} })).getEntities()[0]; + return { + fileAtRoot: await searchDoc(items.fileAtRoot!.driveFile), + fileInRootFolder: await searchDoc(items.fileInRootFolder!.driveFile), + fileInSubRoot: await searchDoc(items.fileInSubRoot!.driveFile), + } + } + beforeAll(async () => { platform = await init({ services: [ @@ -146,7 +151,9 @@ describe("The users deletion API", () => { ], }); + adminUser = await UserApi.getInstance(platform, true, { companyRole: "admin" }); currentUser = await UserApi.getInstance(platform); + userToTestPending = await UserApi.getInstance(platform, true); myUserId = currentUser.user.id; myDriveId = "user_" + currentUser.user.id; myCompanyId = currentUser.workspace.company_id; @@ -154,15 +161,15 @@ describe("The users deletion API", () => { }); afterAll(async () => { - await platform!.tearDown(); + platform && await platform.tearDown(); platform = undefined; }); - async function sendDeleteUser(secret: string, userId: string, expected: number = 200) { + async function sendDeleteUser(secret: string, userId: string, expected: number = 200, deleteData: boolean = true) { const response = await platform!.app.inject({ method: "POST", - url: `/admin/user/delete`, - body: { secret, userId }, + url: `/admin/api/user/delete`, + body: { secret, userId, deleteData }, }); expect(response.statusCode).toBe(expected); if (expected === 200) return JSON.parse(response.body); @@ -172,7 +179,7 @@ describe("The users deletion API", () => { async function requestPendingUserDeletions(secret: string, expected: number = 200) { const response = await platform!.app.inject({ method: "POST", - url: `/admin/user/delete/pending`, + url: `/admin/api/user/delete/pending`, body: { secret }, }); expect(response.statusCode).toBe(expected); @@ -180,12 +187,11 @@ describe("The users deletion API", () => { return response; } - describe("The DELETE /users/:id route", () => { + describe("admin/api/user/delete routes", () => { it("should have a secret setup or none of this test will work and we should know", () => { expect(adminConfig.endpointSecret).toBeTruthy(); }); - //TODO: NONONONO it("should start with a non deleted user with some files", async () => { const user = await currentUser.getUser(); expect(user).toMatchObject({ @@ -201,49 +207,21 @@ describe("The users deletion API", () => { expect(initialFakeFiles.fileInRootFolder.id).toBeTruthy(); expect(initialFakeFiles.fileInSubRoot.id).toBeTruthy(); - // const docs = await currentUser.browseDocuments(myDriveId, {}); - // console.error(JSON.stringify(docs, null, 2)); // works, but projected values - // const docs = await platform!.documentService.browse(myDriveId, { }, { user: { id: myUserId }, company: { id: "string" } } as DriveExecutionContext); - // console.error(JSON.stringifya(docs, null, 2)); // doesn't work, empty children + // Wait for indexing, inspired by ../documents/documents-search.spec.ts + await new Promise(resolve => setTimeout(resolve, 5000)); - // await currentUser.uploadAllFilesOneByOne("user_" + currentUser.user.id); - - // Wait for indexing, inspired by tdrive/backend/node/test/e2e/documents/documents-search.spec.ts - // await new Promise(resolve => setTimeout(resolve, 10000)); // TODO: search doesn't work anyway - // const searchResult = await currentUser.searchDocument({search: "d"}); - // console.log({searchResult}); - // const a = (await repos.search.driveFile.search({}, { $text: { $search: "d" } })).getEntities()[0]; - // console.log({a}); const tree = await loadDriveFiles(myDriveId); - console.log((await userTreeToString(tree))); - - const lines: string[] = []; - const res = await descendDriveItemsDepthFirstRandomOrder(repos, myDriveId, async (item, children, parents) => { - //TODO: NONONO: - // recurse each drive item - // delete all s3 - // then the version - // then files - // search for file by user id after deletion - // search s3 for prefix company/userid - // - const indent = new Array(parents.length + 1).join("\t"); - lines.push(`${indent} ${item.is_directory ? "📂" : "📄"} ${item.name} (${item.id}) (${children?.length ?? 0} children)`); - return children; - }); - console.error(lines.join('\n')); - console.error(JSON.stringify(res, null, 2)); - - - console.log((await repos.search.driveFile.search({}, { $text: { $search: "" } }, undefined)).getEntities()); - // console.log((await repos.search.user.search({}, { $text: { $search: "" } }, undefined)).getEntities()); - const foundEntries = findEntriesInUserTree(tree); expect(foundEntries.rootFolder?.driveFile).toMatchObject({ id: initialFakeFiles.rootFolder.id, parent_id: myDriveId, name: "root_folder" }); expect(foundEntries.subRootFolder?.driveFile).toMatchObject({ id: initialFakeFiles.subRootFolder.id, parent_id: initialFakeFiles.rootFolder.id, name: "sub_root_folder" }); expect(foundEntries.fileAtRoot?.driveFile).toMatchObject({ id: initialFakeFiles.fileAtRoot.id }); expect(foundEntries.fileInRootFolder?.driveFile).toMatchObject({ id: initialFakeFiles.fileInRootFolder.id }); expect(foundEntries.fileInSubRoot?.driveFile).toMatchObject({ id: initialFakeFiles.fileInSubRoot.id }); + + const searchDocs = await getSearchEntriesFromFoundEntries(foundEntries); + expect(searchDocs.fileAtRoot).toMatchObject({ id: initialFakeFiles.fileAtRoot.id }); + expect(searchDocs.fileInRootFolder).toMatchObject({ id: initialFakeFiles.fileInRootFolder.id }); + expect(searchDocs.fileInSubRoot).toMatchObject({ id: initialFakeFiles.fileInSubRoot.id }); }); it("should reject queries without valid secret", async () => { @@ -260,54 +238,73 @@ describe("The users deletion API", () => { expect(responseBody).toStrictEqual([]); }); - it("should be immediately listed after deletion", async () => { - console.log({myCompanyId, myUserId}); - await sendDeleteUser(adminConfig.endpointSecret!, myUserId, 200); - // TODO: wait for message queue stuff ? - const responseBody = await requestPendingUserDeletions(adminConfig.endpointSecret!, 200); - expect(responseBody).toStrictEqual([myUserId]); + it("should timeout with temp test user ID", async () => { + let isResolved = false; + const waitable = sendDeleteUser(adminConfig.endpointSecret!, "e2e_simulate_timeout", 400); + waitable.then(() => isResolved = true, () => isResolved = true); + expect(new Promise(resolve => setTimeout(() => resolve(isResolved), 3000))).resolves.toBeFalsy(); }); - it("the user should be marked at least as pending deletion", async () => { - const user = await currentUser.getUser(); + it("should be immediately listed after deletion", async () => { + const deleteResult = await sendDeleteUser(adminConfig.endpointSecret!, userToTestPending.user.id, 200, false); + expect(deleteResult).toMatchObject({ status: "deleting" }); + const pendingDeletes = await requestPendingUserDeletions(adminConfig.endpointSecret!, 200); + expect(pendingDeletes).toHaveLength(1); + expect(pendingDeletes[0]).toHaveLength(2); + expect(pendingDeletes[0][0]).toBe(userToTestPending.user.id); + expect(pendingDeletes[0][1]).toBeGreaterThan(0); + const userBefore = await adminUser.getUser(userToTestPending.user.id); + expect(userBefore).toMatchObject({ + id: userToTestPending.user.id, + deleted: true, + }); + expect((userBefore as unknown as User).delete_process_started_epoch).toBeGreaterThan(0); + const actualDeleteResult = await sendDeleteUser(adminConfig.endpointSecret!, userToTestPending.user.id, 200, true); + expect(actualDeleteResult).toMatchObject({ status: "done" }); + expect(await requestPendingUserDeletions(adminConfig.endpointSecret!, 200)).toStrictEqual([ ]); + const userAfter = await adminUser.getUser(userToTestPending.user.id); + expect(userAfter).toMatchObject({ + id: userToTestPending.user.id, + deleted: true, + }); + expect((userAfter as unknown as User).delete_process_started_epoch).toBe(0); + }); + + it("the user deletion should complete inline in e2e", async () => { + const deleteResult = await sendDeleteUser(adminConfig.endpointSecret!, myUserId, 200, true); + expect(deleteResult).toMatchObject({ status: "done" }); + }); + + it("the user should no longer have any documents", async () => { + const tree = await loadDriveFiles(myDriveId); + const foundEntries = findEntriesInUserTree(tree); + expect(foundEntries.rootFolder?.driveFile).toBeUndefined(); + expect(foundEntries.subRootFolder?.driveFile).toBeUndefined(); + expect(foundEntries.fileAtRoot?.driveFile).toBeUndefined(); + expect(foundEntries.fileInRootFolder?.driveFile).toBeUndefined(); + expect(foundEntries.fileInSubRoot?.driveFile).toBeUndefined(); + }); + + it("the user should be marked deleted", async () => { + const user = await adminUser.getUser(currentUser.user.id); expect(user).toMatchObject({ id: myUserId, deleted: true, }); - expect((user as unknown as User).delete_process_started_epoch).toBeGreaterThan(0); + expect((user as unknown as User).delete_process_started_epoch).toBe(0); }); - // TODO: NONONO - it.skip("the user should not be able to login", async () => { - expect((await currentUser.login()).statusCode).toBe(401); + it("the user should not be able to continue with API", async () => { + expect(await currentUser.browseDocuments(myDriveId)).toMatchObject({ + "error": "Unauthorized", + }); }); - //TODO: NONONONO - it.skip("should have no objects left other than user (and untestable files)", async () => { - let totalCount = 0; - for (const [key, fn] of Object.entries(listByUserIn)) { - const result = (await fn(myUserId)).getEntities(); - totalCount += result.length; - if (result.length) - console.error(`unexpected ${key} still there`, result); - } - expect(totalCount).toBe(0); + it("should have no objects left other than user (and untestable files)", async () => { + expect(await (await Promise.all( + Object.entries(listByUserIn) + .flatMap(async ([key, fn]) => (await fn(myUserId)).getEntities().map(e => ({"": e.constructor.name, ...e}))) + )).flat()).toHaveLength(0); }); - - //TODO: NONONONO - it.skip("test todo wip messy thing", async () => { - const tree = await loadDriveFiles(myDriveId); - console.log(await userTreeToString(tree)); - const foundEntries = findEntriesInUserTree(tree); - const allPaths = foundEntries.fileAtRoot?.versions?.flatMap(({storagePaths}) => storagePaths); - console.error({allPaths}) - }); - - it("in the end the user should have nothing, and be deleted with no process epoch", async () => { - const tree = await loadDriveFiles(myDriveId); - return; //TODO: NONONO Don't check user deletion until it's done etc - - }); - }); }); diff --git a/tdrive/backend/node/test/unit/core/services/storage/oneof-storage-strategy.test.ts b/tdrive/backend/node/test/unit/core/services/storage/oneof-storage-strategy.test.ts index 57655486..4be970f6 100644 --- a/tdrive/backend/node/test/unit/core/services/storage/oneof-storage-strategy.test.ts +++ b/tdrive/backend/node/test/unit/core/services/storage/oneof-storage-strategy.test.ts @@ -168,8 +168,8 @@ describe('OneOfStorageStrategy', () => { it('should remove the file from all storages', async () => { const result = await oneOfStorageStrategy.remove('test/path'); - expect(storage1.remove).toHaveBeenCalledWith('test/path', undefined); - expect(storage2.remove).toHaveBeenCalledWith('test/path', undefined); + expect(storage1.remove).toHaveBeenCalledWith('test/path', undefined, undefined, undefined); + expect(storage2.remove).toHaveBeenCalledWith('test/path', undefined, undefined, undefined); expect(result).toBe(true); }); diff --git a/tdrive/docker-compose.dev.tests.opensearch.yml b/tdrive/docker-compose.dev.tests.opensearch.yml index 69cb853d..550ea2d7 100644 --- a/tdrive/docker-compose.dev.tests.opensearch.yml +++ b/tdrive/docker-compose.dev.tests.opensearch.yml @@ -56,6 +56,7 @@ services: environment: - LOG_LEVEL=error - NODE_ENV=test + - ADMIN_ENDPOINT_SECRET=the_admin_endpoint_secret - DB_DRIVER=postgres - DB_POSTGRES_DBNAME=tdrive - DB_POSTGRES_HOST=postgres diff --git a/tdrive/docker-compose.tests.yml b/tdrive/docker-compose.tests.yml index 2b887a95..dd41c13f 100644 --- a/tdrive/docker-compose.tests.yml +++ b/tdrive/docker-compose.tests.yml @@ -71,6 +71,7 @@ services: environment: - LOG_LEVEL=error - NODE_ENV=test + - ADMIN_ENDPOINT_SECRET=the_admin_endpoint_secret # - ACCOUNTS_TYPE=remote - SEARCH_DRIVER=mongodb - DB_DRIVER=mongodb