Skip to content

Commit

Permalink
BC-4573-rewrite-files-user-data-deletion (#4303)
Browse files Browse the repository at this point in the history
* move files repo to a dedicated files module

* add barrel file for the files deletion console job

* remove unused factory function

* add ownerId and refOwnerModel (required) fields to the file model in new (NestJS-based) server part

* add more File model fields from the legacy model

* add parent field in the new Field model

* reorder some fields in the new file model

* remove some unused imports, rename file properties interface to match current code style guide

* add share tokens to the file props

* reorder some fields

* add lockId field to the new File entity

* add embedded security check object

* modify some File entity fields to not trigger unnecessary entities loading on the ORM

* add model for the files permission

* add method for finding all the files with given permissionRefId

* add method to find all files by owner userId, refactor findByPermissionRefId to properly return entities mapped from the raw documents

* add File entity method that removes permissions with given refId from the entity object

* add file entity method that marks file for deletion

* add file entity method that verify if file is already marked for deletion

* add test cases for saving the file entity after removing some file permissions or marking the file as deleted

* rename File entity class to FileEntity to make it compliant with the current convention

* rename storage provider entity class to StorgeProviderEntity

* rename file security check entity class

* rename some classes

* add complete domain layer for the files module

* replace use of the files-related enums located in the shared entities module before with the new enums defined in the files module domain layer

* add getProps method for all the new domain objects of the files module

* add mappers for all the domain objects

* rename entity key

* remove unused user file factory reference

* remove unnecessary use of the user file factory as the FileEntity is not really needed there

* remove unnecessary use of the user file factory

* reorganize some code, move the user file factory to the files repo as its only used there now

* remove file factory export as it was moved inside of the files module

* rename file record security check to avoid naming collision, move the file entities to the files module

* add required injections for the FileEntity (as it was moved from the ALL_ENTITIES), split the files-related entities to a separate files

* add complete unit tests for the files-related entities

* modify one comment

* fix imports order

* modify a few imports

* move user file factory to a separate submodule

* add files service with the markFilesOwnedByUserForDeletion() method

* add more unit tests for the markFilesOwnedByUserForDeletion() method

* fix one import

* replace filter with forEach

* add method to remove user permissions to any files with just a single test case as for now

* rename filesRepo var to just repo

* add more unit tests for the removeUserPermissionsToAnyFiles() method

* add files use case with the user data deletion method

* modify method to copy file entity in unit tests to make it more generic

* replace explicit file entity constructor calls with the file factory calls (wherever possible)

* rollback not currently used code to not make the PR too big

* replace plain string class name with the callback type

* add unit test for creating directory with the complete props object

* rollback adding new UC for the files module as it is not yet used and it'll be added in the other ticket once all the authorization-level requirements will be defined

* fix some imports

* move all consts definition to the top of the test suite

* remove all the empty lines in the files module files imports

* rename userFileFactory to fileEntityFactory

* replace explicit file entity constructor calls with the factory calls

* fix imports

* change name of the file that contains the FileEntity factory

* replace all the FilesPermissionEntity constructor calls with the new factory calls

* fix DeleteFilesUC unit tests

* change the way of describing unit tests for the file-security-check.entity.spec.ts file

* add local factory for the legacy file mock

* move the legacy file entity mock factory to the @shared/testing module as it's also needed in the task copy service unit tests

* change DeleteFilesUC to DeleteFilesUc to keep it consistent with the majority of the modules in the project

* remove some unnecessary setup function calls

* move consts inside of the setup function, add some internal setup functions (rest of the "global" setup function calls will also be moved into the specific "describe" blocks)

* move setup functions inside of the respective describe blocks

* reformat some of the unit tests

* modify the way the other files in the files.repo.spec.ts are described and organized

* reformat file-permission.entity.spec.ts file unit tests

* reformat file.entity.spec.ts file unit tests

* remove unused import

* fix exports from the shared testing factory submodule index.ts file

* remove redeclared var

* remove unused var
  • Loading branch information
bn-pass authored Sep 19, 2023
1 parent b0e9f42 commit 560418e
Show file tree
Hide file tree
Showing 53 changed files with 1,702 additions and 320 deletions.
3 changes: 2 additions & 1 deletion apps/server/src/console/console.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { ConsoleWriterModule } from '@shared/infra/console/console-writer/consol
import { KeycloakModule } from '@shared/infra/identity-management/keycloak/keycloak.module';
import { DB_PASSWORD, DB_URL, DB_USERNAME, createConfigModuleOptions } from '@src/config';
import { FilesModule } from '@src/modules/files';
import { FileEntity } from '@src/modules/files/entity';
import { FileRecord } from '@src/modules/files-storage/entity';
import { ManagementModule } from '@src/modules/management/management.module';
import { serverConfig } from '@src/modules/server';
Expand All @@ -28,7 +29,7 @@ import { ServerConsole } from './server.console';
clientUrl: DB_URL,
password: DB_PASSWORD,
user: DB_USERNAME,
entities: [...ALL_ENTITIES, FileRecord],
entities: [...ALL_ENTITIES, FileEntity, FileRecord],
allowGlobalContext: true,
findOneOrFailHandler: (entityName: string, where: Dictionary | IPrimaryKey) =>
new NotFoundException(`The requested ${entityName}: ${JSON.stringify(where)} has not been found.`),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
import { createMock, DeepMocked } from '@golevelup/ts-jest';
import { Test, TestingModule } from '@nestjs/testing';
import { ComponentType, IComponentProperties } from '@shared/domain';
import { courseFactory, fileFactory, lessonFactory, schoolFactory, setupEntities } from '@shared/testing';
import {
courseFactory,
lessonFactory,
schoolFactory,
legacyFileEntityMockFactory,
setupEntities,
} from '@shared/testing';
import { CopyElementType, CopyHelperService } from '@src/modules/copy-helper';
import { CopyFilesService } from './copy-files.service';
import { FilesStorageClientAdapterService } from './files-storage-client.service';
Expand Down Expand Up @@ -52,16 +58,16 @@ describe('copy files service', () => {
describe('copy files of entity', () => {
const setup = () => {
const school = schoolFactory.build();
const file1 = fileFactory.buildWithId({ name: 'file.jpg' });
const file2 = fileFactory.buildWithId({ name: 'file.jpg' });
const file1 = legacyFileEntityMockFactory.build();
const file2 = legacyFileEntityMockFactory.build();
const imageHTML1 = getImageHTML(file1.id, file1.name);
const imageHTML2 = getImageHTML(file2.id, file2.name);
return { file1, file2, school, imageHTML1, imageHTML2 };
return { school, imageHTML1, imageHTML2 };
};

describe('copy files of lesson', () => {
const lessonSetup = () => {
const { file1, file2, school, imageHTML1, imageHTML2 } = setup();
const { school, imageHTML1, imageHTML2 } = setup();
const originalCourse = courseFactory.build({ school });
const textContent: IComponentProperties = {
title: '',
Expand All @@ -86,7 +92,7 @@ describe('copy files service', () => {
const mockedFileDto = { id: 'mockedFileId', sourceId: 'mockedSourceId', name: 'mockedName' };

filesStorageClientAdapterService.copyFilesOfParent.mockResolvedValue([mockedFileDto]);
return { originalLesson, copyLesson, file1, file2, schoolId: school.id, userId, mockedFileDto };
return { originalLesson, copyLesson, schoolId: school.id, userId, mockedFileDto };
};

it('should return fileUrlReplacements', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { PreviewInputMimeTypes } from '../interface';
import {
FileRecord,
FileRecordParentType,
FileSecurityCheck,
FileRecordSecurityCheck,
IFileRecordProperties,
PreviewStatus,
ScanStatus,
Expand Down Expand Up @@ -95,21 +95,21 @@ describe('FileRecord Entity', () => {
});
});

describe('FileSecurityCheck', () => {
describe('FileRecordSecurityCheck', () => {
it('should set the requestToken via the constructor', () => {
const securityCheck = new FileSecurityCheck({ requestToken: '08154711' });
const securityCheck = new FileRecordSecurityCheck({ requestToken: '08154711' });
expect(securityCheck.requestToken).toEqual('08154711');
expect(securityCheck.status).toEqual(securityCheck.status);
expect(securityCheck.reason).toEqual(securityCheck.reason);
});
it('should set the status via the constructor', () => {
const securityCheck = new FileSecurityCheck({ status: ScanStatus.PENDING });
const securityCheck = new FileRecordSecurityCheck({ status: ScanStatus.PENDING });
expect(securityCheck.status).toEqual(ScanStatus.PENDING);
expect(securityCheck.requestToken).toEqual(securityCheck.requestToken);
expect(securityCheck.reason).toEqual(securityCheck.reason);
});
it('should set the reason via the constructor', () => {
const securityCheck = new FileSecurityCheck({ reason: 'test-reason' });
const securityCheck = new FileRecordSecurityCheck({ reason: 'test-reason' });
expect(securityCheck.reason).toEqual('test-reason');
expect(securityCheck.status).toEqual(securityCheck.status);
expect(securityCheck.requestToken).toEqual(securityCheck.requestToken);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Embeddable, Embedded, Entity, Enum, Index, Property } from '@mikro-orm/core';
import { ObjectId } from '@mikro-orm/mongodb';
import { BadRequestException } from '@nestjs/common';
import { BaseEntityWithTimestamps, type EntityId } from '@shared/domain';
import { BaseEntityWithTimestamps, EntityId } from '@shared/domain';
import { v4 as uuid } from 'uuid';
import { ErrorType } from '../error';
import { PreviewInputMimeTypes } from '../interface/preview-input-mime-types.enum';
Expand Down Expand Up @@ -33,13 +33,13 @@ export enum PreviewStatus {
PREVIEW_NOT_POSSIBLE_WRONG_MIME_TYPE = 'preview_not_possible_wrong_mime_type',
}

export interface IFileSecurityCheckProperties {
export interface IFileRecordSecurityCheckProperties {
status?: ScanStatus;
reason?: string;
requestToken?: string;
}
@Embeddable()
export class FileSecurityCheck {
export class FileRecordSecurityCheck {
@Enum()
status: ScanStatus = ScanStatus.PENDING;

Expand All @@ -55,7 +55,7 @@ export class FileSecurityCheck {
@Property()
updatedAt = new Date();

constructor(props: IFileSecurityCheckProperties) {
constructor(props: IFileRecordSecurityCheckProperties) {
if (props.status !== undefined) {
this.status = props.status;
}
Expand Down Expand Up @@ -111,8 +111,8 @@ export class FileRecord extends BaseEntityWithTimestamps {
@Property()
mimeType: string; // TODO mime-type enum?

@Embedded(() => FileSecurityCheck, { object: true, nullable: false })
securityCheck: FileSecurityCheck;
@Embedded(() => FileRecordSecurityCheck, { object: true, nullable: false })
securityCheck: FileRecordSecurityCheck;

@Index()
@Enum()
Expand Down Expand Up @@ -161,7 +161,7 @@ export class FileRecord extends BaseEntityWithTimestamps {
if (props.isCopyFrom) {
this._isCopyFrom = new ObjectId(props.isCopyFrom);
}
this.securityCheck = new FileSecurityCheck({});
this.securityCheck = new FileRecordSecurityCheck({});
this.deletedSince = props.deletedSince;
}

Expand Down
4 changes: 2 additions & 2 deletions apps/server/src/modules/files-storage/files-storage.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { RabbitMQWrapperModule } from '@shared/infra/rabbitmq/rabbitmq.module';
import { S3ClientModule } from '@shared/infra/s3-client';
import { DB_PASSWORD, DB_URL, DB_USERNAME, createConfigModuleOptions } from '@src/config';
import { LoggerModule } from '@src/core/logger';
import { FileRecord, FileSecurityCheck } from './entity';
import { FileRecord, FileRecordSecurityCheck } from './entity';
import { config, s3Config } from './files-storage.config';
import { FileRecordRepo } from './repo';
import { FilesStorageService } from './service/files-storage.service';
Expand Down Expand Up @@ -45,7 +45,7 @@ const defaultMikroOrmOptions: MikroOrmModuleSyncOptions = {
clientUrl: DB_URL,
password: DB_PASSWORD,
user: DB_USERNAME,
entities: [...ALL_ENTITIES, FileRecord, FileSecurityCheck],
entities: [...ALL_ENTITIES, FileRecord, FileRecordSecurityCheck],

// debug: true, // use it for locally debugging of querys
}),
Expand Down
1 change: 1 addition & 0 deletions apps/server/src/modules/files/domain/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from './types';
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export const enum FileOwnerModel {
USER = 'user',
COURSE = 'course',
TEAMS = 'teams',
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
export const enum FilePermissionReferenceModel {
USER = 'user',
ROLE = 'role',
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
export const enum FileSecurityCheckStatus {
PENDING = 'pending',
VERIFIED = 'verified',
BLOCKED = 'blocked',
WONT_CHECK = 'wont-check',
}
3 changes: 3 additions & 0 deletions apps/server/src/modules/files/domain/types/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export * from './file-security-check-status.enum';
export * from './file-permission-reference-model.enum';
export * from './file-owner-model.enum';
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
import { ObjectId } from '@mikro-orm/mongodb';
import { FilePermissionReferenceModel } from '../domain';
import { FilePermissionEntity } from './file-permission.entity';

describe(FilePermissionEntity.name, () => {
describe('constructor', () => {
const setup = () => {
const refId = new ObjectId();
const refPermModel = FilePermissionReferenceModel.USER;

return { refId, refPermModel };
};

describe('when passed a minimal valid props object', () => {
it(`should return a valid ${FilePermissionEntity.name} object with proper default fields values and with the values taken from the passed props object`, () => {
const { refId, refPermModel } = setup();

const entity = new FilePermissionEntity({
refId: refId.toHexString(),
refPermModel,
});

expect(entity).toEqual(
expect.objectContaining({
refId,
refPermModel,
write: true,
read: true,
create: true,
delete: true,
})
);
});
});

describe('when passed a complete (fully filled) props object', () => {
it(`should return a valid ${FilePermissionEntity.name} object with proper fields values taken from the passed props object`, () => {
const { refId, refPermModel } = setup();

const entity = new FilePermissionEntity({
refId: refId.toHexString(),
refPermModel,
write: false,
read: false,
create: false,
delete: false,
});

expect(entity).toEqual(
expect.objectContaining({
refId,
refPermModel,
write: false,
read: false,
create: false,
delete: false,
})
);
});
});
});
});
55 changes: 55 additions & 0 deletions apps/server/src/modules/files/entity/file-permission.entity.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
import { Embeddable, Enum, Property } from '@mikro-orm/core';
import { ObjectId } from '@mikro-orm/mongodb';
import { EntityId } from '@shared/domain';
import { FilePermissionReferenceModel } from '../domain';

export interface FilePermissionEntityProps {
refId: EntityId;
refPermModel: FilePermissionReferenceModel;
write?: boolean;
read?: boolean;
create?: boolean;
delete?: boolean;
}

@Embeddable()
export class FilePermissionEntity {
@Property({ nullable: false })
refId: ObjectId;

@Enum({ nullable: false })
refPermModel: FilePermissionReferenceModel;

@Property()
write = true;

@Property()
read = true;

@Property()
create = true;

@Property()
delete = true;

constructor(props: FilePermissionEntityProps) {
this.refId = new ObjectId(props.refId);
this.refPermModel = props.refPermModel;

if (props.write !== undefined) {
this.write = props.write;
}

if (props.read !== undefined) {
this.read = props.read;
}

if (props.create !== undefined) {
this.create = props.create;
}

if (props.delete !== undefined) {
this.delete = props.delete;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
import { validate as validateUUID } from 'uuid';
import { FileSecurityCheckStatus } from '../domain';
import { FileSecurityCheckEntity } from './file-security-check.entity';

describe(FileSecurityCheckEntity.name, () => {
describe('constructor', () => {
const verifyTimestamps = (entity: FileSecurityCheckEntity) => {
const currentTime = new Date().getTime();

const createdAtTime = entity.createdAt.getTime();

expect(createdAtTime).toBeGreaterThan(0);
expect(createdAtTime).toBeLessThanOrEqual(currentTime);

const updatedAtTime = entity.updatedAt.getTime();

expect(updatedAtTime).toBeGreaterThan(0);
expect(updatedAtTime).toBeLessThanOrEqual(currentTime);
};

describe('when passed an empty props object', () => {
it(`should return a valid ${FileSecurityCheckEntity.name} object with proper default fields values`, () => {
const entity = new FileSecurityCheckEntity({});

verifyTimestamps(entity);
expect(entity).toEqual(
expect.objectContaining({
status: FileSecurityCheckStatus.PENDING,
reason: 'not yet scanned',
})
);
expect(entity.requestToken).toBeDefined();
expect(entity.requestToken?.length).toBeGreaterThan(0);
expect(validateUUID(entity.requestToken as string)).toEqual(true);
});
});

describe('when passed a complete (fully filled) props object', () => {
it(`should return a valid ${FileSecurityCheckEntity.name} object with fields values taken from the passed props object`, () => {
const status = FileSecurityCheckStatus.VERIFIED;
const reason = 'AV scanning done';
const requestToken = 'b9ebf8d9-6029-4d6c-bd93-4cace483df3c';

const entity = new FileSecurityCheckEntity({
status,
reason,
requestToken,
});

verifyTimestamps(entity);
expect(entity).toEqual(
expect.objectContaining({
status,
reason,
})
);
expect(entity.requestToken).toEqual(requestToken);
});
});
});
});
Loading

0 comments on commit 560418e

Please sign in to comment.