diff --git a/jest-e2e.json b/jest-e2e.json index 7594b2068..4dccfdafa 100644 --- a/jest-e2e.json +++ b/jest-e2e.json @@ -13,5 +13,10 @@ "^.+\\.(t|j)s$": "ts-jest" }, "coverageDirectory": "./coverage-e2e", - "testTimeout": 10000 + "testTimeout": 10000, + "globals": { + "ts-jest": { + "tsconfig": "tsconfig.test.json" + } + } } diff --git a/package.json b/package.json index 23cd9d452..5d73538cc 100644 --- a/package.json +++ b/package.json @@ -98,6 +98,11 @@ "^.+\\.(t|j)s$": "ts-jest" }, "coverageDirectory": "../coverage", - "testEnvironment": "node" + "testEnvironment": "node", + "globals": { + "ts-jest": { + "tsconfig": "tsconfig.test.json" + } + } } } diff --git a/src/api/public/me/me.controller.ts b/src/api/public/me/me.controller.ts index 3bc2ae167..2ba887108 100644 --- a/src/api/public/me/me.controller.ts +++ b/src/api/public/me/me.controller.ts @@ -15,6 +15,7 @@ import { Put, UseGuards, Req, + InternalServerErrorException, } from '@nestjs/common'; import { HistoryEntryUpdateDto } from '../../../history/history-entry-update.dto'; import { HistoryService } from '../../../history/history.service'; @@ -65,6 +66,10 @@ export class MeController { }) @ApiUnauthorizedResponse({ description: unauthorizedDescription }) async getMe(@Req() req: Request): Promise { + if (!req.user) { + // We should never reach this, as the TokenAuthGuard handles missing user info + throw new InternalServerErrorException('Request did not specify user'); + } return this.usersService.toUserDto( await this.usersService.getUserByUsername(req.user.userName), ); @@ -79,6 +84,10 @@ export class MeController { }) @ApiUnauthorizedResponse({ description: unauthorizedDescription }) async getUserHistory(@Req() req: Request): Promise { + if (!req.user) { + // We should never reach this, as the TokenAuthGuard handles missing user info + throw new InternalServerErrorException('Request did not specify user'); + } const foundEntries = await this.historyService.getEntriesByUser(req.user); return await Promise.all( foundEntries.map((entry) => this.historyService.toHistoryEntryDto(entry)), @@ -97,6 +106,10 @@ export class MeController { @Req() req: Request, @Param('note') note: string, ): Promise { + if (!req.user) { + // We should never reach this, as the TokenAuthGuard handles missing user info + throw new InternalServerErrorException('Request did not specify user'); + } try { const foundEntry = await this.historyService.getEntryByNoteIdOrAlias( note, @@ -124,6 +137,10 @@ export class MeController { @Param('note') note: string, @Body() entryUpdateDto: HistoryEntryUpdateDto, ): Promise { + if (!req.user) { + // We should never reach this, as the TokenAuthGuard handles missing user info + throw new InternalServerErrorException('Request did not specify user'); + } // ToDo: Check if user is allowed to pin this history entry try { return this.historyService.toHistoryEntryDto( @@ -151,6 +168,10 @@ export class MeController { @Req() req: Request, @Param('note') note: string, ): Promise { + if (!req.user) { + // We should never reach this, as the TokenAuthGuard handles missing user info + throw new InternalServerErrorException('Request did not specify user'); + } // ToDo: Check if user is allowed to delete note try { await this.historyService.deleteHistoryEntry(note, req.user); @@ -171,6 +192,10 @@ export class MeController { }) @ApiUnauthorizedResponse({ description: unauthorizedDescription }) async getMyNotes(@Req() req: Request): Promise { + if (!req.user) { + // We should never reach this, as the TokenAuthGuard handles missing user info + throw new InternalServerErrorException('Request did not specify user'); + } const notes = this.notesService.getUserNotes(req.user); return await Promise.all( (await notes).map((note) => this.notesService.toNoteMetadataDto(note)), @@ -186,6 +211,10 @@ export class MeController { }) @ApiUnauthorizedResponse({ description: unauthorizedDescription }) async getMyMedia(@Req() req: Request): Promise { + if (!req.user) { + // We should never reach this, as the TokenAuthGuard handles missing user info + throw new InternalServerErrorException('Request did not specify user'); + } const media = await this.mediaService.listUploadsByUser(req.user); return media.map((media) => this.mediaService.toMediaUploadDto(media)); } diff --git a/src/api/public/media/media.controller.ts b/src/api/public/media/media.controller.ts index 1472088b0..3436a1dcc 100644 --- a/src/api/public/media/media.controller.ts +++ b/src/api/public/media/media.controller.ts @@ -94,6 +94,10 @@ export class MediaController { @UploadedFile() file: MulterFile, @Headers('HedgeDoc-Note') noteId: string, ): Promise { + if (!req.user) { + // We should never reach this, as the TokenAuthGuard handles missing user info + throw new InternalServerErrorException('Request did not specify user'); + } const username = req.user.userName; this.logger.debug( `Recieved filename '${file.originalname}' for note '${noteId}' from user '${username}'`, @@ -130,6 +134,10 @@ export class MediaController { @Req() req: Request, @Param('filename') filename: string, ): Promise { + if (!req.user) { + // We should never reach this, as the TokenAuthGuard handles missing user info + throw new InternalServerErrorException('Request did not specify user'); + } const username = req.user.userName; try { this.logger.debug( diff --git a/src/api/public/notes/notes.controller.ts b/src/api/public/notes/notes.controller.ts index f32ba7d02..c87957850 100644 --- a/src/api/public/notes/notes.controller.ts +++ b/src/api/public/notes/notes.controller.ts @@ -12,6 +12,7 @@ import { Get, Header, HttpCode, + InternalServerErrorException, NotFoundException, Param, Post, @@ -88,6 +89,10 @@ export class NotesController { @Req() req: Request, @MarkdownBody() text: string, ): Promise { + if (!req.user) { + // We should never reach this, as the TokenAuthGuard handles missing user info + throw new InternalServerErrorException('Request did not specify user'); + } // ToDo: provide user for createNoteDto if (!this.permissionsService.mayCreate(req.user)) { throw new UnauthorizedException('Creating note denied!'); @@ -111,6 +116,10 @@ export class NotesController { @Req() req: Request, @Param('noteIdOrAlias') noteIdOrAlias: string, ): Promise { + if (!req.user) { + // We should never reach this, as the TokenAuthGuard handles missing user info + throw new InternalServerErrorException('Request did not specify user'); + } let note: Note; try { note = await this.noteService.getNoteByIdOrAlias(noteIdOrAlias); @@ -144,6 +153,10 @@ export class NotesController { @Param('noteAlias') noteAlias: string, @MarkdownBody() text: string, ): Promise { + if (!req.user) { + // We should never reach this, as the TokenAuthGuard handles missing user info + throw new InternalServerErrorException('Request did not specify user'); + } if (!this.permissionsService.mayCreate(req.user)) { throw new UnauthorizedException('Creating note denied!'); } @@ -175,6 +188,10 @@ export class NotesController { @Param('noteIdOrAlias') noteIdOrAlias: string, @Body() noteMediaDeletionDto: NoteMediaDeletionDto, ): Promise { + if (!req.user) { + // We should never reach this, as the TokenAuthGuard handles missing user info + throw new InternalServerErrorException('Request did not specify user'); + } try { const note = await this.noteService.getNoteByIdOrAlias(noteIdOrAlias); if (!this.permissionsService.isOwner(req.user, note)) { @@ -217,6 +234,10 @@ export class NotesController { @Param('noteIdOrAlias') noteIdOrAlias: string, @MarkdownBody() text: string, ): Promise { + if (!req.user) { + // We should never reach this, as the TokenAuthGuard handles missing user info + throw new InternalServerErrorException('Request did not specify user'); + } try { const note = await this.noteService.getNoteByIdOrAlias(noteIdOrAlias); if (!this.permissionsService.mayWrite(req.user, note)) { @@ -251,6 +272,10 @@ export class NotesController { @Req() req: Request, @Param('noteIdOrAlias') noteIdOrAlias: string, ): Promise { + if (!req.user) { + // We should never reach this, as the TokenAuthGuard handles missing user info + throw new InternalServerErrorException('Request did not specify user'); + } try { const note = await this.noteService.getNoteByIdOrAlias(noteIdOrAlias); if (!this.permissionsService.mayRead(req.user, note)) { @@ -281,6 +306,10 @@ export class NotesController { @Req() req: Request, @Param('noteIdOrAlias') noteIdOrAlias: string, ): Promise { + if (!req.user) { + // We should never reach this, as the TokenAuthGuard handles missing user info + throw new InternalServerErrorException('Request did not specify user'); + } try { const note = await this.noteService.getNoteByIdOrAlias(noteIdOrAlias); if (!this.permissionsService.mayRead(req.user, note)) { @@ -315,6 +344,10 @@ export class NotesController { @Param('noteIdOrAlias') noteIdOrAlias: string, @Body() updateDto: NotePermissionsUpdateDto, ): Promise { + if (!req.user) { + // We should never reach this, as the TokenAuthGuard handles missing user info + throw new InternalServerErrorException('Request did not specify user'); + } try { const note = await this.noteService.getNoteByIdOrAlias(noteIdOrAlias); if (!this.permissionsService.isOwner(req.user, note)) { @@ -348,6 +381,10 @@ export class NotesController { @Req() req: Request, @Param('noteIdOrAlias') noteIdOrAlias: string, ): Promise { + if (!req.user) { + // We should never reach this, as the TokenAuthGuard handles missing user info + throw new InternalServerErrorException('Request did not specify user'); + } try { const note = await this.noteService.getNoteByIdOrAlias(noteIdOrAlias); if (!this.permissionsService.mayRead(req.user, note)) { @@ -384,6 +421,10 @@ export class NotesController { @Param('noteIdOrAlias') noteIdOrAlias: string, @Param('revisionId') revisionId: number, ): Promise { + if (!req.user) { + // We should never reach this, as the TokenAuthGuard handles missing user info + throw new InternalServerErrorException('Request did not specify user'); + } try { const note = await this.noteService.getNoteByIdOrAlias(noteIdOrAlias); if (!this.permissionsService.mayRead(req.user, note)) { @@ -415,6 +456,10 @@ export class NotesController { @Req() req: Request, @Param('noteIdOrAlias') noteIdOrAlias: string, ): Promise { + if (!req.user) { + // We should never reach this, as the TokenAuthGuard handles missing user info + throw new InternalServerErrorException('Request did not specify user'); + } try { const note = await this.noteService.getNoteByIdOrAlias(noteIdOrAlias); if (!this.permissionsService.mayRead(req.user, note)) { diff --git a/src/api/utils/markdownbody-decorator.ts b/src/api/utils/markdownbody-decorator.ts index 4c55256a4..7614d3f30 100644 --- a/src/api/utils/markdownbody-decorator.ts +++ b/src/api/utils/markdownbody-decorator.ts @@ -42,15 +42,20 @@ export const MarkdownBody = createParamDecorator( }, [ (target, key): void => { - ApiConsumes('text/markdown')( + const ownPropertyDescriptor = Object.getOwnPropertyDescriptor( target, key, - Object.getOwnPropertyDescriptor(target, key), ); + if (!ownPropertyDescriptor) { + throw new Error( + `Could not get property descriptor for target ${target.toString()} and key ${key.toString()}`, + ); + } + ApiConsumes('text/markdown')(target, key, ownPropertyDescriptor); ApiBody({ required: true, schema: { example: '# Markdown Body' }, - })(target, key, Object.getOwnPropertyDescriptor(target, key)); + })(target, key, ownPropertyDescriptor); }, ], ); diff --git a/src/auth/auth-token.dto.ts b/src/auth/auth-token.dto.ts index 46af7d6be..bd3ef8970 100644 --- a/src/auth/auth-token.dto.ts +++ b/src/auth/auth-token.dto.ts @@ -15,8 +15,8 @@ export class AuthTokenDto { createdAt: Date; @IsDate() @IsOptional() - validUntil: Date; + validUntil: Date | null; @IsDate() @IsOptional() - lastUsed: Date; + lastUsed: Date | null; } diff --git a/src/auth/auth-token.entity.ts b/src/auth/auth-token.entity.ts index 79cb8b20f..996199fed 100644 --- a/src/auth/auth-token.entity.ts +++ b/src/auth/auth-token.entity.ts @@ -37,13 +37,15 @@ export class AuthToken { @Column({ nullable: true, + type: 'date', }) - validUntil: Date; + validUntil: Date | null; @Column({ nullable: true, + type: 'date', }) - lastUsed: Date; + lastUsed: Date | null; public static create( user: User, @@ -62,6 +64,7 @@ export class AuthToken { newToken.accessTokenHash = accessToken; newToken.createdAt = new Date(); newToken.validUntil = validUntil; + newToken.lastUsed = null; return newToken; } } diff --git a/src/auth/auth.service.spec.ts b/src/auth/auth.service.spec.ts index 9e10f418a..ab13f1141 100644 --- a/src/auth/auth.service.spec.ts +++ b/src/auth/auth.service.spec.ts @@ -168,6 +168,12 @@ describe('AuthService', () => { ); await service.setLastUsedToken(authToken.keyId); }); + it('throws if the token is not in the database', async () => { + jest.spyOn(authTokenRepo, 'findOne').mockResolvedValueOnce(undefined); + await expect(service.setLastUsedToken(authToken.keyId)).rejects.toThrow( + NotInDBError, + ); + }); }); describe('validateToken', () => { @@ -227,6 +233,12 @@ describe('AuthService', () => { ); await service.removeToken(authToken.keyId); }); + it('throws if the token is not in the database', async () => { + jest.spyOn(authTokenRepo, 'findOne').mockResolvedValueOnce(undefined); + await expect(service.removeToken(authToken.keyId)).rejects.toThrow( + NotInDBError, + ); + }); }); describe('createTokenForUser', () => { @@ -239,7 +251,7 @@ describe('AuthService', () => { }); jest.spyOn(authTokenRepo, 'save').mockImplementationOnce( async (authTokenSaved: AuthToken, _): Promise => { - expect(authTokenSaved.lastUsed).toBeUndefined(); + expect(authTokenSaved.lastUsed).toBeNull(); return authTokenSaved; }, ); @@ -263,7 +275,7 @@ describe('AuthService', () => { }); jest.spyOn(authTokenRepo, 'save').mockImplementationOnce( async (authTokenSaved: AuthToken, _): Promise => { - expect(authTokenSaved.lastUsed).toBeUndefined(); + expect(authTokenSaved.lastUsed).toBeNull(); return authTokenSaved; }, ); @@ -307,4 +319,10 @@ describe('AuthService', () => { ); }); }); + describe('randomString', () => { + it('throws on invalid lenght parameter', () => { + expect(() => service.randomString(0)).toThrow(); + expect(() => service.randomString(-1)).toThrow(); + }); + }); }); diff --git a/src/auth/auth.service.ts b/src/auth/auth.service.ts index 833c24232..c018f4d6c 100644 --- a/src/auth/auth.service.ts +++ b/src/auth/auth.service.ts @@ -64,7 +64,7 @@ export class AuthService { randomString(length: number): Buffer { if (length <= 0) { - return null; + throw new Error('randomString cannot have a length < 1'); } return randomBytes(length); } @@ -127,6 +127,9 @@ export class AuthService { const accessToken = await this.authTokenRepository.findOne({ where: { keyId: keyId }, }); + if (accessToken === undefined) { + throw new NotInDBError(`AuthToken for key '${keyId}' not found`); + } accessToken.lastUsed = new Date(); await this.authTokenRepository.save(accessToken); } @@ -170,23 +173,19 @@ export class AuthService { const token = await this.authTokenRepository.findOne({ where: { keyId: keyId }, }); + if (token === undefined) { + throw new NotInDBError(`AuthToken for key '${keyId}' not found`); + } await this.authTokenRepository.remove(token); } - toAuthTokenDto(authToken: AuthToken): AuthTokenDto | null { - if (!authToken) { - this.logger.warn( - `Recieved ${String(authToken)} argument!`, - 'toAuthTokenDto', - ); - return null; - } + toAuthTokenDto(authToken: AuthToken): AuthTokenDto { const tokenDto: AuthTokenDto = { - lastUsed: null, - validUntil: null, label: authToken.label, keyId: authToken.keyId, createdAt: authToken.createdAt, + validUntil: null, + lastUsed: null, }; if (authToken.validUntil) { @@ -201,9 +200,9 @@ export class AuthService { } toAuthTokenWithSecretDto( - authToken: AuthToken | null | undefined, + authToken: AuthToken, secret: string, - ): AuthTokenWithSecretDto | null { + ): AuthTokenWithSecretDto { const tokenDto = this.toAuthTokenDto(authToken); return { ...tokenDto, diff --git a/src/config/app.config.ts b/src/config/app.config.ts index 52e593032..c12c2e171 100644 --- a/src/config/app.config.ts +++ b/src/config/app.config.ts @@ -7,7 +7,7 @@ import { registerAs } from '@nestjs/config'; import * as Joi from 'joi'; import { Loglevel } from './loglevel.enum'; -import { buildErrorMessage, toArrayConfig } from './utils'; +import { buildErrorMessage, parseOptionalInt, toArrayConfig } from './utils'; export interface AppConfig { domain: string; @@ -46,11 +46,10 @@ export default registerAs('appConfig', () => { { domain: process.env.HD_DOMAIN, rendererOrigin: process.env.HD_RENDERER_ORIGIN, - port: parseInt(process.env.PORT) || undefined, + port: parseOptionalInt(process.env.PORT), loglevel: process.env.HD_LOGLEVEL, forbiddenNoteIds: toArrayConfig(process.env.HD_FORBIDDEN_NOTE_IDS, ','), - maxDocumentLength: - parseInt(process.env.HD_MAX_DOCUMENT_LENGTH) || undefined, + maxDocumentLength: parseOptionalInt(process.env.HD_MAX_DOCUMENT_LENGTH), }, { abortEarly: false, diff --git a/src/config/database.config.ts b/src/config/database.config.ts index da437faf5..94b8a5802 100644 --- a/src/config/database.config.ts +++ b/src/config/database.config.ts @@ -7,7 +7,7 @@ import * as Joi from 'joi'; import { DatabaseDialect } from './database-dialect.enum'; import { registerAs } from '@nestjs/config'; -import { buildErrorMessage } from './utils'; +import { buildErrorMessage, parseOptionalInt } from './utils'; export interface DatabaseConfig { username: string; @@ -62,7 +62,7 @@ export default registerAs('databaseConfig', () => { password: process.env.HD_DATABASE_PASS, database: process.env.HD_DATABASE_NAME, host: process.env.HD_DATABASE_HOST, - port: parseInt(process.env.HD_DATABASE_PORT) || undefined, + port: parseOptionalInt(process.env.HD_DATABASE_PORT), storage: process.env.HD_DATABASE_STORAGE, dialect: process.env.HD_DATABASE_DIALECT, }, diff --git a/src/config/hsts.config.ts b/src/config/hsts.config.ts index 0a1aa6916..de66729fc 100644 --- a/src/config/hsts.config.ts +++ b/src/config/hsts.config.ts @@ -6,7 +6,7 @@ import * as Joi from 'joi'; import { registerAs } from '@nestjs/config'; -import { buildErrorMessage } from './utils'; +import { buildErrorMessage, parseOptionalInt } from './utils'; export interface HstsConfig { enable: boolean; @@ -32,7 +32,7 @@ export default registerAs('hstsConfig', () => { const hstsConfig = hstsSchema.validate( { enable: process.env.HD_HSTS_ENABLE, - maxAgeSeconds: parseInt(process.env.HD_HSTS_MAX_AGE) || undefined, + maxAgeSeconds: parseOptionalInt(process.env.HD_HSTS_MAX_AGE), includeSubdomains: process.env.HD_HSTS_INCLUDE_SUBDOMAINS, preload: process.env.HD_HSTS_PRELOAD, }, diff --git a/src/config/utils.spec.ts b/src/config/utils.spec.ts index 551aed0c5..6e24a37e2 100644 --- a/src/config/utils.spec.ts +++ b/src/config/utils.spec.ts @@ -6,6 +6,7 @@ import { needToLog, + parseOptionalInt, replaceAuthErrorsWithEnvironmentVariables, toArrayConfig, } from './utils'; @@ -84,4 +85,12 @@ describe('config utils', () => { expect(needToLog(currentLevel, Loglevel.TRACE)).toBeTruthy(); }); }); + describe('parseOptionalInt', () => { + it('returns undefined on undefined parameter', () => { + expect(parseOptionalInt(undefined)).toEqual(undefined); + }); + it('correctly parses a string', () => { + expect(parseOptionalInt('42')).toEqual(42); + }); + }); }); diff --git a/src/config/utils.ts b/src/config/utils.ts index e02ca8ded..f9f935af9 100644 --- a/src/config/utils.ts +++ b/src/config/utils.ts @@ -6,7 +6,7 @@ import { Loglevel } from './loglevel.enum'; -export function toArrayConfig(configValue: string, separator = ','): string[] { +export function toArrayConfig(configValue?: string, separator = ','): string[] { if (!configValue) { return []; } @@ -113,3 +113,10 @@ function transformLoglevelToInt(loglevel: Loglevel): number { return 1; } } + +export function parseOptionalInt(value?: string): number | undefined { + if (value === undefined) { + return undefined; + } + return parseInt(value); +} diff --git a/src/frontend-config/frontend-config.dto.ts b/src/frontend-config/frontend-config.dto.ts index 057dacb32..e2b54ac07 100644 --- a/src/frontend-config/frontend-config.dto.ts +++ b/src/frontend-config/frontend-config.dto.ts @@ -84,7 +84,7 @@ export class BrandingDto { */ @IsString() @IsOptional() - name: string; + name?: string; /** * The logo to be displayed next to the HedgeDoc logo @@ -92,7 +92,7 @@ export class BrandingDto { */ @IsUrl() @IsOptional() - logo: URL; + logo?: URL; } export class CustomAuthEntry { @@ -148,7 +148,7 @@ export class SpecialUrlsDto { */ @IsUrl() @IsOptional() - privacy: URL; + privacy?: URL; /** * A link to the terms of use @@ -156,7 +156,7 @@ export class SpecialUrlsDto { */ @IsUrl() @IsOptional() - termsOfUse: URL; + termsOfUse?: URL; /** * A link to the imprint @@ -164,7 +164,7 @@ export class SpecialUrlsDto { */ @IsUrl() @IsOptional() - imprint: URL; + imprint?: URL; } export class IframeCommunicationDto { @@ -174,7 +174,7 @@ export class IframeCommunicationDto { */ @IsUrl() @IsOptional() - editorOrigin: URL; + editorOrigin?: URL; /** * The origin under which the renderer page will be served @@ -182,7 +182,7 @@ export class IframeCommunicationDto { */ @IsUrl() @IsOptional() - rendererOrigin: URL; + rendererOrigin?: URL; } export class FrontendConfigDto { @@ -239,7 +239,7 @@ export class FrontendConfigDto { */ @IsUrl() @IsOptional() - plantUmlServer: URL; + plantUmlServer?: URL; /** * The maximal length of each document diff --git a/src/groups/groups.service.spec.ts b/src/groups/groups.service.spec.ts index 16a16097e..bd7d1edad 100644 --- a/src/groups/groups.service.spec.ts +++ b/src/groups/groups.service.spec.ts @@ -69,13 +69,5 @@ describe('GroupsService', () => { expect(groupDto.name).toEqual(group.name); expect(groupDto.special).toBeFalsy(); }); - it('fails with null parameter', () => { - const groupDto = service.toGroupDto(null); - expect(groupDto).toBeNull(); - }); - it('fails with undefined parameter', () => { - const groupDto = service.toGroupDto(undefined); - expect(groupDto).toBeNull(); - }); }); }); diff --git a/src/groups/groups.service.ts b/src/groups/groups.service.ts index ea2e7fd0d..56878effa 100644 --- a/src/groups/groups.service.ts +++ b/src/groups/groups.service.ts @@ -43,11 +43,7 @@ export class GroupsService { * @param {Group} group - the group to use * @return {GroupInfoDto} the built GroupInfoDto */ - toGroupDto(group: Group | null | undefined): GroupInfoDto | null { - if (!group) { - this.logger.warn(`Recieved ${String(group)} argument!`, 'toGroupDto'); - return null; - } + toGroupDto(group: Group): GroupInfoDto { return { name: group.name, displayName: group.displayName, diff --git a/src/history/history.service.ts b/src/history/history.service.ts index fa4aed431..fd733f732 100644 --- a/src/history/history.service.ts +++ b/src/history/history.service.ts @@ -69,13 +69,19 @@ export class HistoryService { * @return {HistoryEntry} the requested history entry */ private async getEntryByNote(note: Note, user: User): Promise { - return await this.historyEntryRepository.findOne({ + const entry = await this.historyEntryRepository.findOne({ where: { note: note, user: user, }, relations: ['note', 'user'], }); + if (!entry) { + throw new NotInDBError( + `User '${user.userName}' has no HistoryEntry for Note with id '${note.id}'`, + ); + } + return entry; } /** @@ -89,13 +95,17 @@ export class HistoryService { note: Note, user: User, ): Promise { - let entry = await this.getEntryByNote(note, user); - if (!entry) { - entry = HistoryEntry.create(user, note); - } else { + try { + const entry = await this.getEntryByNote(note, user); entry.updatedAt = new Date(); + return await this.historyEntryRepository.save(entry); + } catch (e) { + if (e instanceof NotInDBError) { + const entry = HistoryEntry.create(user, note); + return await this.historyEntryRepository.save(entry); + } + throw e; } - return await this.historyEntryRepository.save(entry); } /** @@ -112,11 +122,6 @@ export class HistoryService { updateDto: HistoryEntryUpdateDto, ): Promise { const entry = await this.getEntryByNoteIdOrAlias(noteIdOrAlias, user); - if (!entry) { - throw new NotInDBError( - `User '${user.userName}' has no HistoryEntry for Note with id '${noteIdOrAlias}'`, - ); - } entry.pinStatus = updateDto.pinStatus; return await this.historyEntryRepository.save(entry); } @@ -130,11 +135,6 @@ export class HistoryService { */ async deleteHistoryEntry(noteIdOrAlias: string, user: User): Promise { const entry = await this.getEntryByNoteIdOrAlias(noteIdOrAlias, user); - if (!entry) { - throw new NotInDBError( - `User '${user.userName}' has no HistoryEntry for Note with id '${noteIdOrAlias}'`, - ); - } await this.historyEntryRepository.remove(entry); return; } diff --git a/src/logger/console-logger.service.ts b/src/logger/console-logger.service.ts index cc8e6a392..c87460bce 100644 --- a/src/logger/console-logger.service.ts +++ b/src/logger/console-logger.service.ts @@ -14,7 +14,7 @@ import { needToLog } from '../config/utils'; @Injectable({ scope: Scope.TRANSIENT }) export class ConsoleLoggerService { - private classContext: string; + private classContext: string | undefined; private lastTimestamp: number; constructor( @@ -83,7 +83,7 @@ export class ConsoleLoggerService { } } - private makeContextString(functionContext: string): string { + private makeContextString(functionContext?: string): string { let context = this.classContext; if (!context) { context = 'HedgeDoc'; diff --git a/src/main.ts b/src/main.ts index 589b01696..d2b75f3a9 100644 --- a/src/main.ts +++ b/src/main.ts @@ -26,6 +26,11 @@ async function bootstrap(): Promise { const appConfig = configService.get('appConfig'); const mediaConfig = configService.get('mediaConfig'); + if (!appConfig || !mediaConfig) { + logger.error('Could not initialize config, aborting.', 'AppBootstrap'); + process.exit(1); + } + setupPublicApiDocs(app); logger.log( `Serving OpenAPI docs for public api under '/apidoc'`, diff --git a/src/media/media-upload.dto.ts b/src/media/media-upload.dto.ts index 0ba14cf33..8bf462371 100644 --- a/src/media/media-upload.dto.ts +++ b/src/media/media-upload.dto.ts @@ -4,7 +4,7 @@ * SPDX-License-Identifier: AGPL-3.0-only */ -import { IsDate, IsString } from 'class-validator'; +import { IsDate, IsOptional, IsString } from 'class-validator'; import { ApiProperty } from '@nestjs/swagger'; export class MediaUploadDto { @@ -21,8 +21,9 @@ export class MediaUploadDto { * @example "noteId" TODO how looks a note id? */ @IsString() + @IsOptional() @ApiProperty() - noteId: string; + noteId: string | null; /** * The date when the upload objects was created. diff --git a/src/media/media-upload.entity.ts b/src/media/media-upload.entity.ts index 341ad52ad..64918074b 100644 --- a/src/media/media-upload.entity.ts +++ b/src/media/media-upload.entity.ts @@ -26,7 +26,7 @@ export class MediaUpload { @ManyToOne((_) => Note, (note) => note.mediaUploads, { nullable: true, }) - note: Note; + note: Note | null; @ManyToOne((_) => User, (user) => user.mediaUploads, { nullable: false, @@ -43,8 +43,9 @@ export class MediaUpload { @Column({ nullable: true, + type: 'text', }) - backendData: BackendData; + backendData: BackendData | null; @CreateDateColumn() createdAt: Date; diff --git a/src/media/media.service.ts b/src/media/media.service.ts index 259c94829..2e68c4642 100644 --- a/src/media/media.service.ts +++ b/src/media/media.service.ts @@ -202,6 +202,10 @@ export class MediaService { return BackendType.S3; case 'webdav': return BackendType.WEBDAV; + default: + throw new Error( + `Unexpected media backend ${this.mediaConfig.backend.use}`, + ); } } @@ -223,7 +227,7 @@ export class MediaService { toMediaUploadDto(mediaUpload: MediaUpload): MediaUploadDto { return { url: mediaUpload.fileUrl, - noteId: mediaUpload.note.id, + noteId: mediaUpload.note?.id ?? null, createdAt: mediaUpload.createdAt, userName: mediaUpload.user.userName, }; diff --git a/src/notes/note-metadata.dto.ts b/src/notes/note-metadata.dto.ts index 03f23011e..f28621340 100644 --- a/src/notes/note-metadata.dto.ts +++ b/src/notes/note-metadata.dto.ts @@ -31,7 +31,7 @@ export class NoteMetadataDto { @IsString() @IsOptional() @ApiPropertyOptional() - alias: string; + alias: string | null; /** * Title of the note @@ -72,8 +72,9 @@ export class NoteMetadataDto { * User that last edited the note */ @ValidateNested() - @ApiProperty({ type: UserInfoDto }) - updateUser: UserInfoDto; + @ApiPropertyOptional({ type: UserInfoDto }) + @IsOptional() + updateUser: UserInfoDto | null; /** * Counts how many times the published note has been viewed diff --git a/src/notes/note-permissions.dto.ts b/src/notes/note-permissions.dto.ts index 11c7d3c62..a9a0d9389 100644 --- a/src/notes/note-permissions.dto.ts +++ b/src/notes/note-permissions.dto.ts @@ -4,10 +4,16 @@ * SPDX-License-Identifier: AGPL-3.0-only */ -import { IsArray, IsBoolean, IsString, ValidateNested } from 'class-validator'; +import { + IsArray, + IsBoolean, + IsOptional, + IsString, + ValidateNested, +} from 'class-validator'; import { UserInfoDto } from '../users/user-info.dto'; import { GroupInfoDto } from '../groups/group-info.dto'; -import { ApiProperty } from '@nestjs/swagger'; +import { ApiProperty, ApiPropertyOptional } from '@nestjs/swagger'; export class NoteUserPermissionEntryDto { /** @@ -84,8 +90,9 @@ export class NotePermissionsDto { * User this permission applies to */ @ValidateNested() - @ApiProperty({ type: UserInfoDto }) - owner: UserInfoDto; + @ApiPropertyOptional({ type: UserInfoDto }) + @IsOptional() + owner: UserInfoDto | null; /** * List of users the note is shared with diff --git a/src/notes/note.entity.ts b/src/notes/note.entity.ts index 5e4cf0ef9..db53f546e 100644 --- a/src/notes/note.entity.ts +++ b/src/notes/note.entity.ts @@ -35,8 +35,9 @@ export class Note { @Column({ unique: true, nullable: true, + type: 'text', }) - alias?: string; + alias: string | null; @OneToMany( (_) => NoteGroupPermission, (groupPermission) => groupPermission.note, @@ -56,8 +57,9 @@ export class Note { viewCount: number; @ManyToOne((_) => User, (user) => user.ownedNotes, { onDelete: 'CASCADE', // This deletes the Note, when the associated User is deleted + nullable: true, }) - owner: User; + owner: User | null; @OneToMany((_) => Revision, (revision) => revision.note, { cascade: true }) revisions: Promise; @OneToMany((_) => AuthorColor, (authorColor) => authorColor.note) @@ -69,12 +71,14 @@ export class Note { @Column({ nullable: true, + type: 'text', }) - description?: string; + description: string | null; @Column({ nullable: true, + type: 'text', }) - title?: string; + title: string | null; @ManyToMany((_) => Tag, (tag) => tag.notes, { eager: true, cascade: true }) @JoinTable() @@ -89,9 +93,9 @@ export class Note { } const newNote = new Note(); newNote.shortid = shortid; - newNote.alias = alias; + newNote.alias = alias ?? null; newNote.viewCount = 0; - newNote.owner = owner; + newNote.owner = owner ?? null; newNote.authorColors = []; newNote.userPermissions = []; newNote.groupPermissions = []; diff --git a/src/notes/notes.service.spec.ts b/src/notes/notes.service.spec.ts index 7192b5d96..ab852a9e9 100644 --- a/src/notes/notes.service.spec.ts +++ b/src/notes/notes.service.spec.ts @@ -152,8 +152,8 @@ describe('NotesService', () => { expect(newNote.userPermissions).toHaveLength(0); expect(newNote.groupPermissions).toHaveLength(0); expect(newNote.tags).toHaveLength(0); - expect(newNote.owner).toBeUndefined(); - expect(newNote.alias).toBeUndefined(); + expect(newNote.owner).toBeNull(); + expect(newNote.alias).toBeNull(); }); it('without alias, with owner', async () => { const newNote = await service.createNote(content, undefined, user); @@ -166,7 +166,7 @@ describe('NotesService', () => { expect(newNote.groupPermissions).toHaveLength(0); expect(newNote.tags).toHaveLength(0); expect(newNote.owner).toEqual(user); - expect(newNote.alias).toBeUndefined(); + expect(newNote.alias).toBeNull(); }); it('with alias, without owner', async () => { const newNote = await service.createNote(content, alias); @@ -177,7 +177,7 @@ describe('NotesService', () => { expect(newNote.userPermissions).toHaveLength(0); expect(newNote.groupPermissions).toHaveLength(0); expect(newNote.tags).toHaveLength(0); - expect(newNote.owner).toBeUndefined(); + expect(newNote.owner).toBeNull(); expect(newNote.alias).toEqual(alias); }); it('with alias, with owner', async () => { diff --git a/src/notes/notes.service.ts b/src/notes/notes.service.ts index 05e82ce7d..481700186 100644 --- a/src/notes/notes.service.ts +++ b/src/notes/notes.service.ts @@ -102,14 +102,18 @@ export class NotesService { } try { return await this.noteRepository.save(newNote); - } catch { - this.logger.debug( - `A note with the alias '${alias}' already exists.`, - 'createNote', - ); - throw new AlreadyInDBError( - `A note with the alias '${alias}' already exists.`, - ); + } catch (e) { + if (alias) { + this.logger.debug( + `A note with the alias '${alias}' already exists.`, + 'createNote', + ); + throw new AlreadyInDBError( + `A note with the alias '${alias}' already exists.`, + ); + } else { + throw e; + } } } @@ -304,7 +308,7 @@ export class NotesService { * @param {Note} note - the note to use * @return {User} user to be used as updateUser in the NoteDto */ - async calculateUpdateUser(note: Note): Promise { + async calculateUpdateUser(note: Note): Promise { const lastRevision = await this.getLatestRevision(note); if (lastRevision && lastRevision.authorships) { // Sort the last Revisions Authorships by their updatedAt Date to get the latest one @@ -333,7 +337,7 @@ export class NotesService { */ toNotePermissionsDto(note: Note): NotePermissionsDto { return { - owner: this.usersService.toUserDto(note.owner), + owner: note.owner ? this.usersService.toUserDto(note.owner) : null, sharedToUsers: note.userPermissions.map((noteUserPermission) => ({ user: this.usersService.toUserDto(noteUserPermission.user), canEdit: noteUserPermission.canEdit, @@ -352,10 +356,11 @@ export class NotesService { * @return {NoteMetadataDto} the built NoteMetadataDto */ async toNoteMetadataDto(note: Note): Promise { + const updateUser = await this.calculateUpdateUser(note); return { // TODO: Convert DB UUID to base64 id: note.id, - alias: note.alias, + alias: note.alias ?? null, title: note.title ?? '', createTime: (await this.getFirstRevision(note)).createdAt, description: note.description ?? '', @@ -365,9 +370,7 @@ export class NotesService { permissions: this.toNotePermissionsDto(note), tags: this.toTagList(note), updateTime: (await this.getLatestRevision(note)).createdAt, - updateUser: this.usersService.toUserDto( - await this.calculateUpdateUser(note), - ), + updateUser: updateUser ? this.usersService.toUserDto(updateUser) : null, viewCount: note.viewCount, }; } diff --git a/src/revisions/revisions.service.spec.ts b/src/revisions/revisions.service.spec.ts index c509d5fd7..2d1226856 100644 --- a/src/revisions/revisions.service.spec.ts +++ b/src/revisions/revisions.service.spec.ts @@ -6,6 +6,8 @@ import { Test, TestingModule } from '@nestjs/testing'; import { getRepositoryToken } from '@nestjs/typeorm'; +import { Repository } from 'typeorm'; +import { NotInDBError } from '../errors/errors'; import { LoggerModule } from '../logger/logger.module'; import { AuthorColor } from '../notes/author-color.entity'; import { Note } from '../notes/note.entity'; @@ -25,6 +27,7 @@ import appConfigMock from '../config/mock/app.config.mock'; describe('RevisionsService', () => { let service: RevisionsService; + let revisionRepo: Repository; beforeEach(async () => { const module: TestingModule = await Test.createTestingModule({ @@ -32,7 +35,7 @@ describe('RevisionsService', () => { RevisionsService, { provide: getRepositoryToken(Revision), - useValue: {}, + useClass: Repository, }, ], imports: [ @@ -57,7 +60,7 @@ describe('RevisionsService', () => { .overrideProvider(getRepositoryToken(Note)) .useValue({}) .overrideProvider(getRepositoryToken(Revision)) - .useValue({}) + .useClass(Repository) .overrideProvider(getRepositoryToken(Tag)) .useValue({}) .overrideProvider(getRepositoryToken(NoteGroupPermission)) @@ -69,9 +72,26 @@ describe('RevisionsService', () => { .compile(); service = module.get(RevisionsService); + revisionRepo = module.get>( + getRepositoryToken(Revision), + ); }); it('should be defined', () => { expect(service).toBeDefined(); }); + + describe('getRevision', () => { + it('returns a revision', async () => { + const revision = Revision.create('', ''); + jest.spyOn(revisionRepo, 'findOne').mockResolvedValueOnce(revision); + expect(await service.getRevision({} as Note, 1)).toEqual(revision); + }); + it('throws if the revision is not in the databse', async () => { + jest.spyOn(revisionRepo, 'findOne').mockResolvedValueOnce(undefined); + await expect(service.getRevision({} as Note, 1)).rejects.toThrow( + NotInDBError, + ); + }); + }); }); diff --git a/src/revisions/revisions.service.ts b/src/revisions/revisions.service.ts index 22774b945..0655a441a 100644 --- a/src/revisions/revisions.service.ts +++ b/src/revisions/revisions.service.ts @@ -13,6 +13,7 @@ import { RevisionMetadataDto } from './revision-metadata.dto'; import { RevisionDto } from './revision.dto'; import { Revision } from './revision.entity'; import { Note } from '../notes/note.entity'; +import { NotInDBError } from '../errors/errors'; @Injectable() export class RevisionsService { @@ -34,16 +35,22 @@ export class RevisionsService { } async getRevision(note: Note, revisionId: number): Promise { - return await this.revisionRepository.findOne({ + const revision = await this.revisionRepository.findOne({ where: { id: revisionId, note: note, }, }); + if (revision === undefined) { + throw new NotInDBError( + `Revision with ID ${revisionId} for note ${note.id} not found.`, + ); + } + return revision; } async getLatestRevision(noteId: string): Promise { - return await this.revisionRepository.findOne({ + const revision = await this.revisionRepository.findOne({ where: { note: noteId, }, @@ -52,10 +59,14 @@ export class RevisionsService { id: 'DESC', }, }); + if (revision === undefined) { + throw new NotInDBError(`Revision for note ${noteId} not found.`); + } + return revision; } async getFirstRevision(noteId: string): Promise { - return await this.revisionRepository.findOne({ + const revision = await this.revisionRepository.findOne({ where: { note: noteId, }, @@ -63,6 +74,10 @@ export class RevisionsService { createdAt: 'ASC', }, }); + if (revision === undefined) { + throw new NotInDBError(`Revision for note ${noteId} not found.`); + } + return revision; } toRevisionMetadataDto(revision: Revision): RevisionMetadataDto { diff --git a/src/seed.ts b/src/seed.ts index 0589a041f..b9f4b23bc 100644 --- a/src/seed.ts +++ b/src/seed.ts @@ -56,7 +56,16 @@ createConnection({ user.ownedNotes = [note]; await connection.manager.save([user, note, revision]); const foundUser = await connection.manager.findOne(User); + if (!foundUser) { + throw new Error('Could not find freshly seeded user. Aborting.'); + } const foundNote = await connection.manager.findOne(Note); + if (!foundNote) { + throw new Error('Could not find freshly seeded note. Aborting.'); + } + if (!foundNote.alias) { + throw new Error('Could not find alias of freshly seeded note. Aborting.'); + } const historyEntry = HistoryEntry.create(foundUser, foundNote); await connection.manager.save(historyEntry); console.log(`Created User '${foundUser.userName}'`); diff --git a/src/users/identity.entity.ts b/src/users/identity.entity.ts index 462399d42..8cbe0f230 100644 --- a/src/users/identity.entity.ts +++ b/src/users/identity.entity.ts @@ -38,16 +38,19 @@ export class Identity { @Column({ nullable: true, + type: 'text', }) - providerUserId?: string; + providerUserId: string | null; @Column({ nullable: true, + type: 'text', }) - oAuthAccessToken?: string; + oAuthAccessToken: string | null; @Column({ nullable: true, + type: 'text', }) - passwordHash?: string; + passwordHash: string | null; } diff --git a/src/users/user.entity.ts b/src/users/user.entity.ts index c04350821..b3267db57 100644 --- a/src/users/user.entity.ts +++ b/src/users/user.entity.ts @@ -40,13 +40,15 @@ export class User { @Column({ nullable: true, + type: 'text', }) - photo?: string; + photo: string | null; @Column({ nullable: true, + type: 'text', }) - email?: string; + email: string | null; @OneToMany((_) => Note, (note) => note.owner) ownedNotes: Note[]; diff --git a/src/users/users.service.spec.ts b/src/users/users.service.spec.ts index db93576fa..da9e3d755 100644 --- a/src/users/users.service.spec.ts +++ b/src/users/users.service.spec.ts @@ -149,9 +149,5 @@ describe('UsersService', () => { 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(); - }); }); }); diff --git a/src/users/users.service.ts b/src/users/users.service.ts index 7a10b8fac..f7ce84217 100644 --- a/src/users/users.service.ts +++ b/src/users/users.service.ts @@ -108,13 +108,9 @@ export class UsersService { /** * Build UserInfoDto from a user. * @param {User=} user - the user to use - * @return {(UserInfoDto|null)} the built UserInfoDto + * @return {(UserInfoDto)} the built UserInfoDto */ - toUserDto(user: User | null | undefined): UserInfoDto | null { - if (!user) { - this.logger.warn(`Recieved ${String(user)} argument!`, 'toUserDto'); - return null; - } + toUserDto(user: User): UserInfoDto { return { userName: user.userName, displayName: user.displayName, diff --git a/test/private-api/history.e2e-spec.ts b/test/private-api/history.e2e-spec.ts index 6e5b29890..79eb8d0e9 100644 --- a/test/private-api/history.e2e-spec.ts +++ b/test/private-api/history.e2e-spec.ts @@ -185,7 +185,7 @@ describe('History', () => { const entry = await historyService.createOrUpdateHistoryEntry(note2, user); expect(entry.pinStatus).toBeFalsy(); await request(app.getHttpServer()) - .put(`/me/history/${entry.note.alias}`) + .put(`/me/history/${entry.note.alias || 'undefined'}`) .send({ pinStatus: true }) .expect(200); const userEntries = await historyService.getEntriesByUser(user); @@ -199,7 +199,7 @@ describe('History', () => { const entry2 = await historyService.createOrUpdateHistoryEntry(note, user); const entryDto = historyService.toHistoryEntryDto(entry2); await request(app.getHttpServer()) - .delete(`/me/history/${entry.note.alias}`) + .delete(`/me/history/${entry.note.alias || 'undefined'}`) .expect(200); const userEntries = await historyService.getEntriesByUser(user); expect(userEntries.length).toEqual(1); diff --git a/test/public-api/media.e2e-spec.ts b/test/public-api/media.e2e-spec.ts index b2dc81b8a..af6f558e9 100644 --- a/test/public-api/media.e2e-spec.ts +++ b/test/public-api/media.e2e-spec.ts @@ -134,7 +134,7 @@ describe('Media', () => { 'hardcoded', 'test_upload_media', ); - const filename = url.split('/').pop(); + const filename = url.split('/').pop() || ''; await request(app.getHttpServer()) .delete('/media/' + filename) .expect(204); diff --git a/tsconfig.json b/tsconfig.json index c925859f0..03bcdca01 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -14,6 +14,7 @@ "./types", "./node_modules/@types" ], - "strict": false + "strict": true, + "strictPropertyInitialization": false } } diff --git a/tsconfig.test.json b/tsconfig.test.json new file mode 100644 index 000000000..5787af45b --- /dev/null +++ b/tsconfig.test.json @@ -0,0 +1,6 @@ +{ + "extends": "./tsconfig.json", + "compilerOptions": { + "strict": false + } +} diff --git a/tsconfig.test.json.license b/tsconfig.test.json.license new file mode 100644 index 000000000..078e5a9ac --- /dev/null +++ b/tsconfig.test.json.license @@ -0,0 +1,3 @@ +SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file) + +SPDX-License-Identifier: CC0-1.0