From e591a65945388a82f73c55a0ef31a3e871e18545 Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Fri, 5 Mar 2021 00:25:06 +0100 Subject: [PATCH 1/8] UserEntity: Make userName unique Each username should only be given once. Signed-off-by: Philip Molares --- src/users/user.entity.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/users/user.entity.ts b/src/users/user.entity.ts index ca753c7bd..33e26114d 100644 --- a/src/users/user.entity.ts +++ b/src/users/user.entity.ts @@ -23,7 +23,9 @@ export class User { @PrimaryGeneratedColumn('uuid') id: string; - @Column() + @Column({ + unique: true, + }) userName: string; @Column() From c65ef80dd576c40a5df3737b0dcad1ebbc1819e6 Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Fri, 5 Mar 2021 00:26:57 +0100 Subject: [PATCH 2/8] UsersService: Add JSDoc to all methods Signed-off-by: Philip Molares --- src/users/users.service.ts | 35 +++++++++++++++++++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/src/users/users.service.ts b/src/users/users.service.ts index 7714d0dd5..1ec9de04c 100644 --- a/src/users/users.service.ts +++ b/src/users/users.service.ts @@ -7,7 +7,7 @@ import { Injectable } from '@nestjs/common'; import { InjectRepository } from '@nestjs/typeorm'; import { Repository } from 'typeorm'; -import { NotInDBError } from '../errors/errors'; +import { AlreadyInDBError, NotInDBError } from '../errors/errors'; import { ConsoleLoggerService } from '../logger/console-logger.service'; import { UserInfoDto } from './user-info.dto'; import { User } from './user.entity'; @@ -21,11 +21,25 @@ export class UsersService { this.logger.setContext(UsersService.name); } - createUser(userName: string, displayName: string): Promise { + /** + * @async + * Create a new user with a given userName and displayName + * @param userName - the userName the new user shall have + * @param displayName - the display the new user shall have + * @return {User} the user + * @throws {AlreadyInDBError} the userName is already taken. + */ + async createUser(userName: string, displayName: string): Promise { const user = User.create(userName, displayName); return this.userRepository.save(user); } + /** + * @async + * Delete the user with the specified userName + * @param userName - the username of the user to be delete + * @throws {NotInDBError} the userName has no user associated with it. + */ async deleteUser(userName: string): Promise { // TODO: Handle owned notes and edits const user = await this.userRepository.findOne({ @@ -34,6 +48,13 @@ export class UsersService { await this.userRepository.delete(user); } + /** + * @async + * Get the user specified by the username + * @param {string} userName the username by which the user is specified + * @param {boolean} [withTokens=false] if the returned user object should contain authTokens + * @return {User} the specified user + */ async getUserByUsername(userName: string, withTokens = false): Promise { const user = await this.userRepository.findOne({ where: { userName: userName }, @@ -45,6 +66,11 @@ export class UsersService { return user; } + /** + * Extract the photoUrl of the user or in case no photo url is present generate a deterministic user photo + * @param {User} user - the specified User + * @return the url of the photo + */ getPhotoUrl(user: User): string { if (user.photo) { return user.photo; @@ -54,6 +80,11 @@ export class UsersService { } } + /** + * Build UserInfoDto from a user. + * @param {User=} user - the user to use + * @return {(UserInfoDto|null)} the built UserInfoDto + */ toUserDto(user: User | null | undefined): UserInfoDto | null { if (!user) { this.logger.warn(`Recieved ${String(user)} argument!`, 'toUserDto'); From 478e25e77c4b903c227ec7d83664b6e98b7da7a8 Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Fri, 5 Mar 2021 00:30:03 +0100 Subject: [PATCH 3/8] UsersService: Polish methods Add test to createUser method to ensure an already used username triggers a AlreadyInDBError. Add debug entry if user is deleted. Add changeDisplayName method. Signed-off-by: Philip Molares --- src/users/users.service.ts | 43 ++++++++++++++++++++++++++++++-------- 1 file changed, 34 insertions(+), 9 deletions(-) diff --git a/src/users/users.service.ts b/src/users/users.service.ts index 1ec9de04c..7a10b8fac 100644 --- a/src/users/users.service.ts +++ b/src/users/users.service.ts @@ -31,21 +31,42 @@ export class UsersService { */ async createUser(userName: string, displayName: string): Promise { const user = User.create(userName, displayName); - return this.userRepository.save(user); + try { + return await this.userRepository.save(user); + } catch { + this.logger.debug( + `A user with the username '${userName}' already exists.`, + 'createUser', + ); + throw new AlreadyInDBError( + `A user with the username '${userName}' already exists.`, + ); + } } /** * @async * Delete the user with the specified userName - * @param userName - the username of the user to be delete + * @param {User} user - the username of the user to be delete * @throws {NotInDBError} the userName has no user associated with it. */ - async deleteUser(userName: string): Promise { - // TODO: Handle owned notes and edits - const user = await this.userRepository.findOne({ - where: { userName: userName }, - }); - await this.userRepository.delete(user); + async deleteUser(user: User): Promise { + await this.userRepository.remove(user); + this.logger.debug( + `Successfully deleted user with username ${user.userName}`, + 'deleteUser', + ); + } + + /** + * @async + * Change the displayName of the specified user + * @param {User} user - the user to be changed + * @param displayName - the new displayName + */ + async changeDisplayName(user: User, displayName: string): Promise { + user.displayName = displayName; + await this.userRepository.save(user); } /** @@ -56,9 +77,13 @@ export class UsersService { * @return {User} the specified user */ async getUserByUsername(userName: string, withTokens = false): Promise { + const relations: string[] = []; + if (withTokens) { + relations.push('authTokens'); + } const user = await this.userRepository.findOne({ where: { userName: userName }, - relations: withTokens ? ['authTokens'] : null, + relations: relations, }); if (user === undefined) { throw new NotInDBError(`User with username '${userName}' not found`); From 5f886b8a27ecf8a423b320d63338013dc2b4c545 Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Fri, 5 Mar 2021 00:30:19 +0100 Subject: [PATCH 4/8] UsersService: Add unit tests Signed-off-by: Philip Molares --- src/users/users.service.spec.ts | 122 ++++++++++++++++++++++++++++++-- 1 file changed, 117 insertions(+), 5 deletions(-) diff --git a/src/users/users.service.spec.ts b/src/users/users.service.spec.ts index 4bff57b39..db93576fa 100644 --- a/src/users/users.service.spec.ts +++ b/src/users/users.service.spec.ts @@ -9,11 +9,14 @@ import { getRepositoryToken } from '@nestjs/typeorm'; import { LoggerModule } from '../logger/logger.module'; import { User } from './user.entity'; import { UsersService } from './users.service'; +import { Repository } from 'typeorm'; +import { AlreadyInDBError, NotInDBError } from '../errors/errors'; import { ConfigModule } from '@nestjs/config'; import appConfigMock from '../config/mock/app.config.mock'; describe('UsersService', () => { let service: UsersService; + let userRepo: Repository; beforeEach(async () => { const module: TestingModule = await Test.createTestingModule({ @@ -21,7 +24,7 @@ describe('UsersService', () => { UsersService, { provide: getRepositoryToken(User), - useValue: {}, + useClass: Repository, }, ], imports: [ @@ -31,15 +34,124 @@ describe('UsersService', () => { }), LoggerModule, ], - }) - .overrideProvider(getRepositoryToken(User)) - .useValue({}) - .compile(); + }).compile(); service = module.get(UsersService); + userRepo = module.get>(getRepositoryToken(User)); }); it('should be defined', () => { expect(service).toBeDefined(); }); + + describe('createUser', () => { + const username = 'hardcoded'; + const displayname = 'Testy'; + beforeEach(() => { + jest + .spyOn(userRepo, 'save') + .mockImplementationOnce(async (user: User): Promise => user); + }); + it('works', async () => { + const user = await service.createUser(username, displayname); + expect(user.userName).toEqual(username); + expect(user.displayName).toEqual(displayname); + }); + it('fails if username is already taken', async () => { + jest.spyOn(userRepo, 'save').mockImplementationOnce(() => { + throw new Error(); + }); + // create first user with username + await service.createUser(username, displayname); + // attempt to create second user with username + await expect(service.createUser(username, displayname)).rejects.toThrow( + AlreadyInDBError, + ); + }); + }); + + describe('deleteUser', () => { + it('works', async () => { + const username = 'hardcoded'; + const displayname = 'Testy'; + const newUser = User.create(username, displayname) as User; + jest.spyOn(userRepo, 'remove').mockImplementationOnce( + // eslint-disable-next-line @typescript-eslint/require-await + async (user: User): Promise => { + expect(user).toEqual(newUser); + return user; + }, + ); + await service.deleteUser(newUser); + }); + }); + + describe('changedDisplayName', () => { + it('works', async () => { + const username = 'hardcoded'; + const displayname = 'Testy'; + const user = User.create(username, displayname) as User; + const newDisplayName = 'Testy2'; + jest.spyOn(userRepo, 'save').mockImplementationOnce( + // eslint-disable-next-line @typescript-eslint/require-await + async (user: User): Promise => { + expect(user.displayName).toEqual(newDisplayName); + return user; + }, + ); + await service.changeDisplayName(user, newDisplayName); + }); + }); + + describe('getUserByUsername', () => { + const username = 'hardcoded'; + const displayname = 'Testy'; + const user = User.create(username, displayname) as User; + it('works', async () => { + jest.spyOn(userRepo, 'findOne').mockResolvedValueOnce(user); + const getUser = await service.getUserByUsername(username); + expect(getUser.userName).toEqual(username); + expect(getUser.displayName).toEqual(displayname); + }); + it('fails when user does not exits', async () => { + jest.spyOn(userRepo, 'findOne').mockResolvedValueOnce(undefined); + await expect(service.getUserByUsername(username)).rejects.toThrow( + NotInDBError, + ); + }); + }); + + describe('getPhotoUrl', () => { + const username = 'hardcoded'; + const displayname = 'Testy'; + const user = User.create(username, displayname) as User; + it('works if a user has a photoUrl', () => { + const photo = 'testPhotoUrl'; + user.photo = photo; + const photoUrl = service.getPhotoUrl(user); + expect(photoUrl).toEqual(photo); + }); + it('works if a user no photoUrl', () => { + user.photo = undefined; + const photoUrl = service.getPhotoUrl(user); + expect(photoUrl).toEqual(''); + }); + }); + + describe('toUserDto', () => { + const username = 'hardcoded'; + const displayname = 'Testy'; + const user = User.create(username, displayname) as User; + it('works if a user is provided', () => { + const userDto = service.toUserDto(user); + expect(userDto.userName).toEqual(username); + expect(userDto.displayName).toEqual(displayname); + expect(userDto.photo).toEqual(''); + expect(userDto.email).toEqual(''); + }); + it('fails if no user is provided', () => { + expect(service.toUserDto(null)).toBeNull(); + expect(service.toUserDto(undefined)).toBeNull(); + }); + }); }); From 53d29c6e8add51bc96012eebf95c0586d0c84b3e Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Sat, 20 Mar 2021 18:58:59 +0100 Subject: [PATCH 5/8] MediaService: Change deleteFile The former deleteFile was moved to the public apis media controller and the actual deletion functionality was moved in a separate function to be called on user deletion. Signed-off-by: Philip Molares --- src/api/public/media/media.controller.ts | 18 ++++++++++++++- src/media/media.service.spec.ts | 27 +++-------------------- src/media/media.service.ts | 28 +++++------------------- 3 files changed, 25 insertions(+), 48 deletions(-) diff --git a/src/api/public/media/media.controller.ts b/src/api/public/media/media.controller.ts index 27f960c23..1472088b0 100644 --- a/src/api/public/media/media.controller.ts +++ b/src/api/public/media/media.controller.ts @@ -132,7 +132,23 @@ export class MediaController { ): Promise { const username = req.user.userName; try { - await this.mediaService.deleteFile(filename, username); + this.logger.debug( + `Deleting '${filename}' for user '${username}'`, + 'deleteFile', + ); + const mediaUpload = await this.mediaService.findUploadByFilename( + filename, + ); + if (mediaUpload.user.userName !== username) { + this.logger.warn( + `${username} tried to delete '${filename}', but is not the owner`, + 'deleteFile', + ); + throw new PermissionError( + `File '${filename}' is not owned by '${username}'`, + ); + } + await this.mediaService.deleteFile(mediaUpload); } catch (e) { if (e instanceof PermissionError) { throw new UnauthorizedException(e.message); diff --git a/src/media/media.service.spec.ts b/src/media/media.service.spec.ts index 9ee8481ab..e7acf22b0 100644 --- a/src/media/media.service.spec.ts +++ b/src/media/media.service.spec.ts @@ -24,7 +24,7 @@ import { BackendData, MediaUpload } from './media-upload.entity'; import { MediaService } from './media.service'; import { Repository } from 'typeorm'; import { promises as fs } from 'fs'; -import { ClientError, NotInDBError, PermissionError } from '../errors/errors'; +import { ClientError, NotInDBError } from '../errors/errors'; import { NoteGroupPermission } from '../permissions/note-group-permission.entity'; import { NoteUserPermission } from '../permissions/note-user-permission.entity'; import { Group } from '../groups/group.entity'; @@ -145,7 +145,6 @@ describe('MediaService', () => { describe('deleteFile', () => { it('works', async () => { - const testFileName = 'testFilename'; const mockMediaUploadEntry = { id: 'testMediaUpload', backendData: 'testBackendData', @@ -153,12 +152,9 @@ describe('MediaService', () => { userName: 'hardcoded', } as User, } as MediaUpload; - jest - .spyOn(mediaRepo, 'findOne') - .mockResolvedValueOnce(mockMediaUploadEntry); jest.spyOn(service.mediaBackend, 'deleteFile').mockImplementationOnce( async (fileName: string, backendData: BackendData): Promise => { - expect(fileName).toEqual(testFileName); + expect(fileName).toEqual(mockMediaUploadEntry.id); expect(backendData).toEqual(mockMediaUploadEntry.backendData); }, ); @@ -168,24 +164,7 @@ describe('MediaService', () => { expect(entry).toEqual(mockMediaUploadEntry); return entry; }); - await service.deleteFile(testFileName, 'hardcoded'); - }); - - it('fails: the mediaUpload is not owned by user', async () => { - const testFileName = 'testFilename'; - const mockMediaUploadEntry = { - id: 'testMediaUpload', - backendData: 'testBackendData', - user: { - userName: 'not-hardcoded', - } as User, - } as MediaUpload; - jest - .spyOn(mediaRepo, 'findOne') - .mockResolvedValueOnce(mockMediaUploadEntry); - await expect( - service.deleteFile(testFileName, 'hardcoded'), - ).rejects.toThrow(PermissionError); + await service.deleteFile(mockMediaUploadEntry); }); }); describe('findUploadByFilename', () => { diff --git a/src/media/media.service.ts b/src/media/media.service.ts index 7cb1b36d6..b0a25442d 100644 --- a/src/media/media.service.ts +++ b/src/media/media.service.ts @@ -10,7 +10,7 @@ import { InjectRepository } from '@nestjs/typeorm'; import * as FileType from 'file-type'; import { Repository } from 'typeorm'; import mediaConfiguration, { MediaConfig } from '../config/media.config'; -import { ClientError, NotInDBError, PermissionError } from '../errors/errors'; +import { ClientError, NotInDBError } from '../errors/errors'; import { ConsoleLoggerService } from '../logger/console-logger.service'; import { NotesService } from '../notes/notes.service'; import { UsersService } from '../users/users.service'; @@ -113,30 +113,12 @@ export class MediaService { /** * @async - * Try to delete the file specified by the filename with the user specified by the username. - * @param {string} filename - the name of the file to delete. - * @param {string} username - the username of the user who uploaded this file - * @return {string} the url of the saved file - * @throws {PermissionError} the user is not permitted to delete this file. - * @throws {NotInDBError} - the file entry specified is not in the database + * Try to delete the specified file. + * @param {MediaUpload} mediaUpload - the name of the file to delete. * @throws {MediaBackendError} - there was an error deleting the file */ - async deleteFile(filename: string, username: string): Promise { - this.logger.debug( - `Deleting '${filename}' for user '${username}'`, - 'deleteFile', - ); - const mediaUpload = await this.findUploadByFilename(filename); - if (mediaUpload.user.userName !== username) { - this.logger.warn( - `${username} tried to delete '${filename}', but is not the owner`, - 'deleteFile', - ); - throw new PermissionError( - `File '${filename}' is not owned by '${username}'`, - ); - } - await this.mediaBackend.deleteFile(filename, mediaUpload.backendData); + async deleteFile(mediaUpload: MediaUpload): Promise { + await this.mediaBackend.deleteFile(mediaUpload.id, mediaUpload.backendData); await this.mediaUploadRepository.remove(mediaUpload); } From 5758463b07f8e22ec4e873454051509c7922390e Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Fri, 5 Mar 2021 00:30:47 +0100 Subject: [PATCH 6/8] PrivateAPI: Add me controller Signed-off-by: Philip Molares --- src/api/private/me/me.controller.spec.ts | 83 ++++++++++++++++++++++++ src/api/private/me/me.controller.ts | 60 +++++++++++++++++ src/api/private/private-api.module.ts | 4 +- 3 files changed, 146 insertions(+), 1 deletion(-) create mode 100644 src/api/private/me/me.controller.spec.ts create mode 100644 src/api/private/me/me.controller.ts diff --git a/src/api/private/me/me.controller.spec.ts b/src/api/private/me/me.controller.spec.ts new file mode 100644 index 000000000..55a5e81d9 --- /dev/null +++ b/src/api/private/me/me.controller.spec.ts @@ -0,0 +1,83 @@ +/* + * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ + +import { Test, TestingModule } from '@nestjs/testing'; +import { MeController } from './me.controller'; +import { UsersModule } from '../../../users/users.module'; +import { LoggerModule } from '../../../logger/logger.module'; +import { getRepositoryToken } from '@nestjs/typeorm'; +import { User } from '../../../users/user.entity'; +import { Identity } from '../../../users/identity.entity'; +import { MediaModule } from '../../../media/media.module'; +import { AuthorColor } from '../../../notes/author-color.entity'; +import { NoteGroupPermission } from '../../../permissions/note-group-permission.entity'; +import { NoteUserPermission } from '../../../permissions/note-user-permission.entity'; +import { Authorship } from '../../../revisions/authorship.entity'; +import { ConfigModule } from '@nestjs/config'; +import appConfigMock from '../../../config/mock/app.config.mock'; +import authConfigMock from '../../../config/mock/auth.config.mock'; +import mediaConfigMock from '../../../config/mock/media.config.mock'; +import customizationConfigMock from '../../../config/mock/customization.config.mock'; +import externalServicesConfigMock from '../../../config/mock/external-services.config.mock'; +import { MediaUpload } from '../../../media/media-upload.entity'; +import { Note } from '../../../notes/note.entity'; +import { Tag } from '../../../notes/tag.entity'; +import { Revision } from '../../../revisions/revision.entity'; +import { Group } from '../../../groups/group.entity'; + +describe('MeController', () => { + let controller: MeController; + + beforeEach(async () => { + const module: TestingModule = await Test.createTestingModule({ + controllers: [MeController], + imports: [ + UsersModule, + LoggerModule, + MediaModule, + ConfigModule.forRoot({ + isGlobal: true, + load: [ + appConfigMock, + authConfigMock, + mediaConfigMock, + customizationConfigMock, + externalServicesConfigMock, + ], + }), + ], + }) + .overrideProvider(getRepositoryToken(User)) + .useValue({}) + .overrideProvider(getRepositoryToken(Identity)) + .useValue({}) + .overrideProvider(getRepositoryToken(Note)) + .useValue({}) + .overrideProvider(getRepositoryToken(Tag)) + .useValue({}) + .overrideProvider(getRepositoryToken(Revision)) + .useValue({}) + .overrideProvider(getRepositoryToken(Group)) + .useValue({}) + .overrideProvider(getRepositoryToken(AuthorColor)) + .useValue({}) + .overrideProvider(getRepositoryToken(NoteGroupPermission)) + .useValue({}) + .overrideProvider(getRepositoryToken(NoteUserPermission)) + .useValue({}) + .overrideProvider(getRepositoryToken(Authorship)) + .useValue({}) + .overrideProvider(getRepositoryToken(MediaUpload)) + .useValue({}) + .compile(); + + controller = module.get(MeController); + }); + + it('should be defined', () => { + expect(controller).toBeDefined(); + }); +}); diff --git a/src/api/private/me/me.controller.ts b/src/api/private/me/me.controller.ts new file mode 100644 index 000000000..3151a4fd7 --- /dev/null +++ b/src/api/private/me/me.controller.ts @@ -0,0 +1,60 @@ +/* + * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ + +import { Body, Controller, Delete, Get, HttpCode, Post } from '@nestjs/common'; +import { UserInfoDto } from '../../../users/user-info.dto'; +import { ConsoleLoggerService } from '../../../logger/console-logger.service'; +import { UsersService } from '../../../users/users.service'; +import { MediaService } from '../../../media/media.service'; +import { MediaUploadDto } from '../../../media/media-upload.dto'; + +@Controller('me') +export class MeController { + constructor( + private readonly logger: ConsoleLoggerService, + private userService: UsersService, + private mediaService: MediaService, + ) { + this.logger.setContext(MeController.name); + } + + @Get() + async getMe(): Promise { + // ToDo: use actual user here + const user = await this.userService.getUserByUsername('hardcoded'); + return this.userService.toUserDto(user); + } + + @Get('media') + async getMyMedia(): Promise { + // ToDo: use actual user here + const user = await this.userService.getUserByUsername('hardcoded'); + const media = await this.mediaService.listUploadsByUser(user); + return media.map((media) => this.mediaService.toMediaUploadDto(media)); + } + + @Delete() + @HttpCode(204) + async deleteUser(): Promise { + // ToDo: use actual user here + const user = await this.userService.getUserByUsername('hardcoded'); + const mediaUploads = await this.mediaService.listUploadsByUser(user); + for (const mediaUpload of mediaUploads) { + await this.mediaService.deleteFile(mediaUpload); + } + this.logger.debug(`Deleted all media uploads of ${user.userName}`); + await this.userService.deleteUser(user); + this.logger.debug(`Deleted ${user.userName}`); + } + + @Post('profile') + @HttpCode(200) + async updateDisplayName(@Body('name') newDisplayName: string): Promise { + // ToDo: use actual user here + const user = await this.userService.getUserByUsername('hardcoded'); + await this.userService.changeDisplayName(user, newDisplayName); + } +} diff --git a/src/api/private/private-api.module.ts b/src/api/private/private-api.module.ts index 729c65306..913920e81 100644 --- a/src/api/private/private-api.module.ts +++ b/src/api/private/private-api.module.ts @@ -9,6 +9,7 @@ import { TokensController } from './tokens/tokens.controller'; import { LoggerModule } from '../../logger/logger.module'; import { UsersModule } from '../../users/users.module'; import { AuthModule } from '../../auth/auth.module'; +import { MeController } from './me/me.controller'; import { ConfigController } from './config/config.controller'; import { FrontendConfigModule } from '../../frontend-config/frontend-config.module'; import { HistoryController } from './me/history/history.controller'; @@ -27,8 +28,8 @@ import { RevisionsModule } from '../../revisions/revisions.module'; AuthModule, FrontendConfigModule, HistoryModule, - NotesModule, PermissionsModule, + NotesModule, MediaModule, RevisionsModule, ], @@ -37,6 +38,7 @@ import { RevisionsModule } from '../../revisions/revisions.module'; ConfigController, MediaController, HistoryController, + MeController, NotesController, ], }) From 24ee95282de0b565f073ca1061fe57fad530bcd3 Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Thu, 11 Mar 2021 15:03:23 +0100 Subject: [PATCH 7/8] Entities: Add onDelete CASCADE to entities To better handle deletion of entities, all necessary other entities got the option onDelete CASCADE set. So everything that does not make any sense if something else is deleted will be deleted along side of it. Signed-off-by: Philip Molares --- src/auth/auth-token.entity.ts | 4 +++- src/groups/group.entity.ts | 1 - src/media/media-upload.entity.ts | 8 ++++++-- src/notes/note.entity.ts | 7 ++++++- src/users/identity.entity.ts | 4 +++- src/users/user.entity.ts | 4 ++++ 6 files changed, 22 insertions(+), 6 deletions(-) diff --git a/src/auth/auth-token.entity.ts b/src/auth/auth-token.entity.ts index 22f5a965f..79cb8b20f 100644 --- a/src/auth/auth-token.entity.ts +++ b/src/auth/auth-token.entity.ts @@ -21,7 +21,9 @@ export class AuthToken { @Column({ unique: true }) keyId: string; - @ManyToOne((_) => User, (user) => user.authTokens) + @ManyToOne((_) => User, (user) => user.authTokens, { + onDelete: 'CASCADE', // This deletes the AuthToken, when the associated User is deleted + }) user: User; @Column() diff --git a/src/groups/group.entity.ts b/src/groups/group.entity.ts index f3cdc6ad4..41f78e837 100644 --- a/src/groups/group.entity.ts +++ b/src/groups/group.entity.ts @@ -36,7 +36,6 @@ export class Group { @ManyToMany((_) => User, (user) => user.groups, { eager: true, - cascade: true, }) @JoinTable() members: User[]; diff --git a/src/media/media-upload.entity.ts b/src/media/media-upload.entity.ts index c9f253278..d25094d3b 100644 --- a/src/media/media-upload.entity.ts +++ b/src/media/media-upload.entity.ts @@ -23,10 +23,14 @@ export class MediaUpload { @PrimaryColumn() id: string; - @ManyToOne((_) => Note, { nullable: false }) + @ManyToOne((_) => Note, (note) => note.mediaUploads, { + nullable: false, + }) note: Note; - @ManyToOne((_) => User, { nullable: false }) + @ManyToOne((_) => User, (user) => user.mediaUploads, { + nullable: false, + }) user: User; @Column({ diff --git a/src/notes/note.entity.ts b/src/notes/note.entity.ts index 538af568b..5e4cf0ef9 100644 --- a/src/notes/note.entity.ts +++ b/src/notes/note.entity.ts @@ -21,6 +21,7 @@ import { User } from '../users/user.entity'; import { AuthorColor } from './author-color.entity'; import { Tag } from './tag.entity'; import { HistoryEntry } from '../history/history-entry.entity'; +import { MediaUpload } from '../media/media-upload.entity'; @Entity() export class Note { @@ -53,7 +54,9 @@ export class Note { default: 0, }) viewCount: number; - @ManyToOne((_) => User, (user) => user.ownedNotes, { onDelete: 'CASCADE' }) + @ManyToOne((_) => User, (user) => user.ownedNotes, { + onDelete: 'CASCADE', // This deletes the Note, when the associated User is deleted + }) owner: User; @OneToMany((_) => Revision, (revision) => revision.note, { cascade: true }) revisions: Promise; @@ -61,6 +64,8 @@ export class Note { authorColors: AuthorColor[]; @OneToMany((_) => HistoryEntry, (historyEntry) => historyEntry.user) historyEntries: HistoryEntry[]; + @OneToMany((_) => MediaUpload, (mediaUpload) => mediaUpload.note) + mediaUploads: MediaUpload[]; @Column({ nullable: true, diff --git a/src/users/identity.entity.ts b/src/users/identity.entity.ts index f8999bc82..462399d42 100644 --- a/src/users/identity.entity.ts +++ b/src/users/identity.entity.ts @@ -19,7 +19,9 @@ export class Identity { @PrimaryGeneratedColumn() id: number; - @ManyToOne((_) => User, (user) => user.identities) + @ManyToOne((_) => User, (user) => user.identities, { + onDelete: 'CASCADE', // This deletes the Identity, when the associated User is deleted + }) user: User; @Column() diff --git a/src/users/user.entity.ts b/src/users/user.entity.ts index 33e26114d..c04350821 100644 --- a/src/users/user.entity.ts +++ b/src/users/user.entity.ts @@ -17,6 +17,7 @@ import { AuthToken } from '../auth/auth-token.entity'; import { Identity } from './identity.entity'; import { Group } from '../groups/group.entity'; import { HistoryEntry } from '../history/history-entry.entity'; +import { MediaUpload } from '../media/media-upload.entity'; @Entity() export class User { @@ -62,6 +63,9 @@ export class User { @OneToMany((_) => HistoryEntry, (historyEntry) => historyEntry.user) historyEntries: HistoryEntry[]; + @OneToMany((_) => MediaUpload, (mediaUpload) => mediaUpload.user) + mediaUploads: MediaUpload[]; + // eslint-disable-next-line @typescript-eslint/no-empty-function private constructor() {} From c3047509aa15dda5c250a134f076f1e6d90c9b90 Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Sat, 20 Mar 2021 19:01:04 +0100 Subject: [PATCH 8/8] PrivateE2EMe: Add E2E test for private api /me routes Signed-off-by: Philip Molares --- test/private-api/me.e2e-spec.ts | 163 ++++++++++++++++++++++++++++++++ 1 file changed, 163 insertions(+) create mode 100644 test/private-api/me.e2e-spec.ts diff --git a/test/private-api/me.e2e-spec.ts b/test/private-api/me.e2e-spec.ts new file mode 100644 index 000000000..77245e9f0 --- /dev/null +++ b/test/private-api/me.e2e-spec.ts @@ -0,0 +1,163 @@ +/* + * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ + +/* eslint-disable +@typescript-eslint/no-unsafe-assignment, +@typescript-eslint/no-unsafe-member-access +*/ + +import { INestApplication } from '@nestjs/common'; +import { ConfigModule, ConfigService } from '@nestjs/config'; +import { Test } from '@nestjs/testing'; +import { TypeOrmModule } from '@nestjs/typeorm'; +import * as request from 'supertest'; +import appConfigMock from '../../src/config/mock/app.config.mock'; +import authConfigMock from '../../src/config/mock/auth.config.mock'; +import mediaConfigMock from '../../src/config/mock/media.config.mock'; +import customizationConfigMock from '../../src/config/mock/customization.config.mock'; +import externalServicesConfigMock from '../../src/config/mock/external-services.config.mock'; +import { GroupsModule } from '../../src/groups/groups.module'; +import { LoggerModule } from '../../src/logger/logger.module'; +import { NotesModule } from '../../src/notes/notes.module'; +import { PermissionsModule } from '../../src/permissions/permissions.module'; +import { AuthModule } from '../../src/auth/auth.module'; +import { UsersService } from '../../src/users/users.service'; +import { User } from '../../src/users/user.entity'; +import { UsersModule } from '../../src/users/users.module'; +import { PrivateApiModule } from '../../src/api/private/private-api.module'; +import { UserInfoDto } from '../../src/users/user-info.dto'; +import { MediaModule } from '../../src/media/media.module'; +import { HistoryModule } from '../../src/history/history.module'; +import { NotInDBError } from '../../src/errors/errors'; +import { promises as fs } from 'fs'; +import { Note } from '../../src/notes/note.entity'; +import { NotesService } from '../../src/notes/notes.service'; +import { MediaService } from '../../src/media/media.service'; + +describe('Me', () => { + let app: INestApplication; + let userService: UsersService; + let mediaService: MediaService; + let uploadPath: string; + let user: User; + let content: string; + let note1: Note; + let note2: Note; + + beforeAll(async () => { + const moduleRef = await Test.createTestingModule({ + imports: [ + ConfigModule.forRoot({ + isGlobal: true, + load: [ + appConfigMock, + authConfigMock, + mediaConfigMock, + customizationConfigMock, + externalServicesConfigMock, + ], + }), + PrivateApiModule, + NotesModule, + PermissionsModule, + GroupsModule, + TypeOrmModule.forRoot({ + type: 'sqlite', + database: './hedgedoc-e2e-private-me.sqlite', + autoLoadEntities: true, + synchronize: true, + dropSchema: true, + }), + LoggerModule, + AuthModule, + UsersModule, + MediaModule, + HistoryModule, + ], + }).compile(); + const config = moduleRef.get(ConfigService); + uploadPath = config.get('mediaConfig').backend.filesystem.uploadPath; + app = moduleRef.createNestApplication(); + await app.init(); + //historyService = moduleRef.get(); + userService = moduleRef.get(UsersService); + mediaService = moduleRef.get(MediaService); + user = await userService.createUser('hardcoded', 'Testy'); + const notesService = moduleRef.get(NotesService); + content = 'This is a test note.'; + note1 = await notesService.createNote(content, null, user); + note2 = await notesService.createNote(content, 'note2', user); + }); + + it('GET /me', async () => { + const userInfo = userService.toUserDto(user); + const response = await request(app.getHttpServer()) + .get('/me') + .expect('Content-Type', /json/) + .expect(200); + const gotUser = response.body as UserInfoDto; + expect(gotUser).toEqual(userInfo); + }); + + it('GET /me/media', async () => { + const httpServer = app.getHttpServer(); + const responseBefore = await request(httpServer) + .get('/me/media/') + .expect('Content-Type', /json/) + .expect(200); + expect(responseBefore.body).toHaveLength(0); + + const testImage = await fs.readFile('test/public-api/fixtures/test.png'); + const url0 = await mediaService.saveFile(testImage, 'hardcoded', note1.id); + const url1 = await mediaService.saveFile(testImage, 'hardcoded', note1.id); + const url2 = await mediaService.saveFile(testImage, 'hardcoded', note2.id); + const url3 = await mediaService.saveFile(testImage, 'hardcoded', note2.id); + + const response = await request(httpServer) + .get('/me/media/') + .expect('Content-Type', /json/) + .expect(200); + expect(response.body).toHaveLength(4); + expect(response.body[0].url).toEqual(url0); + expect(response.body[1].url).toEqual(url1); + expect(response.body[2].url).toEqual(url2); + expect(response.body[3].url).toEqual(url3); + const mediaUploads = await mediaService.listUploadsByUser(user); + for (const upload of mediaUploads) { + await mediaService.deleteFile(upload); + } + await fs.rmdir(uploadPath); + }); + + it('POST /me/profile', async () => { + const newDisplayName = 'Another name'; + expect(user.displayName).not.toEqual(newDisplayName); + await request(app.getHttpServer()) + .post('/me/profile') + .send({ + name: newDisplayName, + }) + .expect(200); + const dbUser = await userService.getUserByUsername('hardcoded'); + expect(dbUser.displayName).toEqual(newDisplayName); + }); + + it('DELETE /me', async () => { + const testImage = await fs.readFile('test/public-api/fixtures/test.png'); + const url0 = await mediaService.saveFile(testImage, 'hardcoded', note1.id); + const dbUser = await userService.getUserByUsername('hardcoded'); + expect(dbUser).toBeInstanceOf(User); + const mediaUploads = await mediaService.listUploadsByUser(dbUser); + expect(mediaUploads).toHaveLength(1); + expect(mediaUploads[0].fileUrl).toEqual(url0); + await request(app.getHttpServer()).delete('/me').expect(204); + await expect(userService.getUserByUsername('hardcoded')).rejects.toThrow( + NotInDBError, + ); + const mediaUploadsAfter = await mediaService.listUploadsByNote(note1); + expect(mediaUploadsAfter).toHaveLength(0); + }); +});