From dd2667b523698594e7dbfb14c487e89dda7b0b41 Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Sat, 7 Aug 2021 21:54:42 +0200 Subject: [PATCH 01/12] chore: add alias dtos Signed-off-by: Philip Molares --- src/notes/alias-create.dto.ts | 23 +++++++++++++++++++++++ src/notes/alias-update.dto.ts | 16 ++++++++++++++++ src/notes/alias.dto.ts | 30 ++++++++++++++++++++++++++++++ 3 files changed, 69 insertions(+) create mode 100644 src/notes/alias-create.dto.ts create mode 100644 src/notes/alias-update.dto.ts create mode 100644 src/notes/alias.dto.ts diff --git a/src/notes/alias-create.dto.ts b/src/notes/alias-create.dto.ts new file mode 100644 index 000000000..2da4dd841 --- /dev/null +++ b/src/notes/alias-create.dto.ts @@ -0,0 +1,23 @@ +/* + * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ +import { ApiProperty } from '@nestjs/swagger'; +import { IsString } from 'class-validator'; + +export class AliasCreateDto { + /** + * The note id or alias, which identifies the note the alias should be added to + */ + @IsString() + @ApiProperty() + noteIdOrAlias: string; + + /** + * The new alias + */ + @IsString() + @ApiProperty() + newAlias: string; +} diff --git a/src/notes/alias-update.dto.ts b/src/notes/alias-update.dto.ts new file mode 100644 index 000000000..318c86680 --- /dev/null +++ b/src/notes/alias-update.dto.ts @@ -0,0 +1,16 @@ +/* + * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ +import { ApiProperty } from '@nestjs/swagger'; +import { IsBoolean } from 'class-validator'; + +export class AliasUpdateDto { + /** + * Whether the alias should become the primary alias or not + */ + @IsBoolean() + @ApiProperty() + primaryAlias: boolean; +} diff --git a/src/notes/alias.dto.ts b/src/notes/alias.dto.ts new file mode 100644 index 000000000..dccaddc23 --- /dev/null +++ b/src/notes/alias.dto.ts @@ -0,0 +1,30 @@ +/* + * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ +import { ApiProperty } from '@nestjs/swagger'; +import { IsBoolean, IsString } from 'class-validator'; + +export class AliasDto { + /** + * The name of the alias + */ + @IsString() + @ApiProperty() + name: string; + + /** + * Is the alias the primary alias or not + */ + @IsBoolean() + @ApiProperty() + primaryAlias: boolean; + + /** + * The public id of the note the alias is associated with + */ + @IsString() + @ApiProperty() + noteId: string; +} From 7dd4f97d64a056f15ffe2e987169767898bf5695 Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Sat, 7 Aug 2021 21:52:00 +0200 Subject: [PATCH 02/12] chore: add PrimaryAliasDeletionForbiddenError Signed-off-by: Philip Molares --- src/errors/errors.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/errors/errors.ts b/src/errors/errors.ts index 229f08c84..a42b6e31d 100644 --- a/src/errors/errors.ts +++ b/src/errors/errors.ts @@ -39,3 +39,7 @@ export class PermissionsUpdateInconsistentError extends Error { export class MediaBackendError extends Error { name = 'MediaBackendError'; } + +export class PrimaryAliasDeletionForbiddenError extends Error { + name = 'PrimaryAliasDeletionForbiddenError'; +} From aaef0f72baa6be4db5c812682c653d6a7a8cf1b4 Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Sun, 6 Jun 2021 17:46:32 +0200 Subject: [PATCH 03/12] feat: add list of aliases to note entity One of the aliases can be primary for each note, but all can be used to get information from the apis. Signed-off-by: Philip Molares --- docs/content/dev/db-schema.plantuml | 10 +++- src/notes/alias.entity.ts | 63 ++++++++++++++++++++++++++ src/notes/note-metadata.dto.ts | 16 +++++-- src/notes/note.entity.ts | 15 +++--- src/notes/notes.module.ts | 2 + src/notes/primary.value-transformer.ts | 22 +++++++++ 6 files changed, 115 insertions(+), 13 deletions(-) create mode 100644 src/notes/alias.entity.ts create mode 100644 src/notes/primary.value-transformer.ts diff --git a/docs/content/dev/db-schema.plantuml b/docs/content/dev/db-schema.plantuml index 7a9239184..f3a918e0d 100644 --- a/docs/content/dev/db-schema.plantuml +++ b/docs/content/dev/db-schema.plantuml @@ -6,13 +6,20 @@ entity "note" { *id : uuid <> -- publicId: text - alias : text *viewCount : number *ownerId : uuid <> description: text title: text } +entity "alias" { + *id: uuid <> + --- + name: text + ' If the alias is primary. Can be NULL, which means it's not primary + primary: boolean +} + entity "user" { *id : uuid <> -- @@ -169,6 +176,7 @@ media_upload "0..*" -- "1" note note "1" -d- "1..*" revision note "1" - "0..*" history_entry note "0..*" -l- "0..*" tag +note "1" - "0..*" alias note "0..*" -- "0..*" group user "1..*" -- "0..*" group diff --git a/src/notes/alias.entity.ts b/src/notes/alias.entity.ts new file mode 100644 index 000000000..eb7e746ce --- /dev/null +++ b/src/notes/alias.entity.ts @@ -0,0 +1,63 @@ +/* + * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ +import { + Column, + Entity, + ManyToOne, + PrimaryGeneratedColumn, + Unique, +} from 'typeorm'; + +import { Note } from './note.entity'; +import { PrimaryValueTransformer } from './primary.value-transformer'; + +@Entity() +@Unique('Only one primary alias per note', ['note', 'primary']) +export class Alias { + @PrimaryGeneratedColumn('uuid') + id: string; + + /** + * the actual alias + */ + @Column({ + nullable: false, + unique: true, + }) + name: string; + + /** + * Is this alias the primary alias, by which people access the note? + */ + @Column({ + /* + Because of the @Unique at the top of this entity, this field must be saved as null instead of false in the DB. + If a non-primary alias would be saved with `primary: false` it would only be possible to have one non-primary and one primary alias. + But a nullable field does not have such problems. + This way the DB keeps track that one note really only has one primary alias. + */ + comment: + 'This field tells you if this is the primary alias of the note. If this field is null, that means this alias is not primary.', + nullable: true, + transformer: new PrimaryValueTransformer(), + }) + primary: boolean; + + @ManyToOne((_) => Note, (note) => note.aliases, { + onDelete: 'CASCADE', // This deletes the Alias, when the associated Note is deleted + }) + note: Note; + + // eslint-disable-next-line @typescript-eslint/no-empty-function + private constructor() {} + + static create(name: string, primary = false): Alias { + const alias = new Alias(); + alias.name = name; + alias.primary = primary; + return alias; + } +} diff --git a/src/notes/note-metadata.dto.ts b/src/notes/note-metadata.dto.ts index 9a588ab5f..48af104fc 100644 --- a/src/notes/note-metadata.dto.ts +++ b/src/notes/note-metadata.dto.ts @@ -25,13 +25,19 @@ export class NoteMetadataDto { id: string; /** - * Alias of the note - * Can be null + * All aliases of the note (including the primaryAlias) + */ + @IsArray() + @IsString({ each: true }) + @ApiProperty() + aliases: string[]; + + /** + * The primary alias of the note */ @IsString() - @IsOptional() - @ApiPropertyOptional() - alias: string | null; + @ApiProperty() + primaryAlias: string | null; /** * Title of the note diff --git a/src/notes/note.entity.ts b/src/notes/note.entity.ts index 916a14c9c..23c964e78 100644 --- a/src/notes/note.entity.ts +++ b/src/notes/note.entity.ts @@ -19,6 +19,7 @@ import { NoteGroupPermission } from '../permissions/note-group-permission.entity import { NoteUserPermission } from '../permissions/note-user-permission.entity'; import { Revision } from '../revisions/revision.entity'; import { User } from '../users/user.entity'; +import { Alias } from './alias.entity'; import { Tag } from './tag.entity'; import { generatePublicId } from './utils'; @@ -28,12 +29,12 @@ export class Note { id: string; @Column({ type: 'text' }) publicId: string; - @Column({ - unique: true, - nullable: true, - type: 'text', - }) - alias: string | null; + @OneToMany( + (_) => Alias, + (alias) => alias.note, + { cascade: true }, // This ensures that embedded Aliases are automatically saved to the database + ) + aliases: Alias[]; @OneToMany( (_) => NoteGroupPermission, (groupPermission) => groupPermission.note, @@ -84,7 +85,7 @@ export class Note { public static create(owner?: User, alias?: string): Note { const newNote = new Note(); newNote.publicId = generatePublicId(); - newNote.alias = alias ?? null; + newNote.aliases = alias ? [Alias.create(alias, true)] : []; newNote.viewCount = 0; newNote.owner = owner ?? null; newNote.userPermissions = []; diff --git a/src/notes/notes.module.ts b/src/notes/notes.module.ts index 80e332288..0bb564f07 100644 --- a/src/notes/notes.module.ts +++ b/src/notes/notes.module.ts @@ -14,6 +14,7 @@ import { NoteUserPermission } from '../permissions/note-user-permission.entity'; import { RevisionsModule } from '../revisions/revisions.module'; import { User } from '../users/user.entity'; import { UsersModule } from '../users/users.module'; +import { Alias } from './alias.entity'; import { Note } from './note.entity'; import { NotesService } from './notes.service'; import { Tag } from './tag.entity'; @@ -26,6 +27,7 @@ import { Tag } from './tag.entity'; NoteGroupPermission, NoteUserPermission, User, + Alias, ]), forwardRef(() => RevisionsModule), UsersModule, diff --git a/src/notes/primary.value-transformer.ts b/src/notes/primary.value-transformer.ts new file mode 100644 index 000000000..21ff8f505 --- /dev/null +++ b/src/notes/primary.value-transformer.ts @@ -0,0 +1,22 @@ +/* + * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ +import { ValueTransformer } from 'typeorm'; + +export class PrimaryValueTransformer implements ValueTransformer { + from(value: boolean | null): boolean { + if (value === null) { + return false; + } + return value; + } + + to(value: boolean): boolean | null { + if (!value) { + return null; + } + return value; + } +} From 0db7a41d1ad869eb700178e0a1a3d9a21ddea39d Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Sat, 7 Aug 2021 21:53:54 +0200 Subject: [PATCH 04/12] feat: add alias service Signed-off-by: Philip Molares --- src/notes/alias.service.spec.ts | 267 ++++++++++++++++++++++++++++++++ src/notes/alias.service.ts | 186 ++++++++++++++++++++++ src/notes/notes.module.ts | 5 +- 3 files changed, 456 insertions(+), 2 deletions(-) create mode 100644 src/notes/alias.service.spec.ts create mode 100644 src/notes/alias.service.ts diff --git a/src/notes/alias.service.spec.ts b/src/notes/alias.service.spec.ts new file mode 100644 index 000000000..20bc41274 --- /dev/null +++ b/src/notes/alias.service.spec.ts @@ -0,0 +1,267 @@ +/* + * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ +import { ConfigModule, ConfigService } from '@nestjs/config'; +import { Test, TestingModule } from '@nestjs/testing'; +import { getRepositoryToken } from '@nestjs/typeorm'; +import { Repository } from 'typeorm'; + +import { AuthToken } from '../auth/auth-token.entity'; +import { Author } from '../authors/author.entity'; +import appConfigMock from '../config/mock/app.config.mock'; +import { + AlreadyInDBError, + ForbiddenIdError, + NotInDBError, + PrimaryAliasDeletionForbiddenError, +} from '../errors/errors'; +import { Group } from '../groups/group.entity'; +import { GroupsModule } from '../groups/groups.module'; +import { Identity } from '../identity/identity.entity'; +import { LoggerModule } from '../logger/logger.module'; +import { NoteGroupPermission } from '../permissions/note-group-permission.entity'; +import { NoteUserPermission } from '../permissions/note-user-permission.entity'; +import { Edit } from '../revisions/edit.entity'; +import { Revision } from '../revisions/revision.entity'; +import { RevisionsModule } from '../revisions/revisions.module'; +import { Session } from '../users/session.entity'; +import { User } from '../users/user.entity'; +import { UsersModule } from '../users/users.module'; +import { Alias } from './alias.entity'; +import { AliasService } from './alias.service'; +import { Note } from './note.entity'; +import { NotesService } from './notes.service'; +import { Tag } from './tag.entity'; + +describe('AliasService', () => { + let service: AliasService; + let noteRepo: Repository; + let aliasRepo: Repository; + let forbiddenNoteId: string; + beforeEach(async () => { + const module: TestingModule = await Test.createTestingModule({ + providers: [ + AliasService, + NotesService, + { + provide: getRepositoryToken(Note), + useClass: Repository, + }, + { + provide: getRepositoryToken(Alias), + useClass: Repository, + }, + { + provide: getRepositoryToken(Tag), + useClass: Repository, + }, + { + provide: getRepositoryToken(User), + useClass: Repository, + }, + ], + imports: [ + ConfigModule.forRoot({ + isGlobal: true, + load: [appConfigMock], + }), + LoggerModule, + UsersModule, + GroupsModule, + RevisionsModule, + ], + }) + .overrideProvider(getRepositoryToken(Note)) + .useClass(Repository) + .overrideProvider(getRepositoryToken(Tag)) + .useClass(Repository) + .overrideProvider(getRepositoryToken(Alias)) + .useClass(Repository) + .overrideProvider(getRepositoryToken(User)) + .useClass(Repository) + .overrideProvider(getRepositoryToken(AuthToken)) + .useValue({}) + .overrideProvider(getRepositoryToken(Identity)) + .useValue({}) + .overrideProvider(getRepositoryToken(Edit)) + .useValue({}) + .overrideProvider(getRepositoryToken(Revision)) + .useClass(Repository) + .overrideProvider(getRepositoryToken(NoteGroupPermission)) + .useValue({}) + .overrideProvider(getRepositoryToken(NoteUserPermission)) + .useValue({}) + .overrideProvider(getRepositoryToken(Group)) + .useClass(Repository) + .overrideProvider(getRepositoryToken(Session)) + .useValue({}) + .overrideProvider(getRepositoryToken(Author)) + .useValue({}) + .compile(); + + const config = module.get(ConfigService); + forbiddenNoteId = config.get('appConfig').forbiddenNoteIds[0]; + service = module.get(AliasService); + noteRepo = module.get>(getRepositoryToken(Note)); + aliasRepo = module.get>(getRepositoryToken(Alias)); + }); + describe('addAlias', () => { + const alias = 'testAlias'; + const alias2 = 'testAlias2'; + const user = User.create('hardcoded', 'Testy') as User; + describe('creates', () => { + it('an primary alias if no alias is already present', async () => { + const note = Note.create(user); + jest + .spyOn(noteRepo, 'save') + .mockImplementationOnce(async (note: Note): Promise => note); + jest.spyOn(noteRepo, 'findOne').mockResolvedValueOnce(undefined); + jest.spyOn(aliasRepo, 'findOne').mockResolvedValueOnce(undefined); + const savedAlias = await service.addAlias(note, alias); + expect(savedAlias.name).toEqual(alias); + expect(savedAlias.primary).toBeTruthy(); + }); + it('an non-primary alias if an primary alias is already present', async () => { + const note = Note.create(user, alias); + jest + .spyOn(noteRepo, 'save') + .mockImplementationOnce(async (note: Note): Promise => note); + jest.spyOn(noteRepo, 'findOne').mockResolvedValueOnce(undefined); + jest.spyOn(aliasRepo, 'findOne').mockResolvedValueOnce(undefined); + const savedAlias = await service.addAlias(note, alias2); + expect(savedAlias.name).toEqual(alias2); + expect(savedAlias.primary).toBeFalsy(); + }); + }); + describe('does not create an alias', () => { + const note = Note.create(user, alias2); + it('with an already used name', async () => { + jest + .spyOn(aliasRepo, 'findOne') + .mockResolvedValueOnce(Alias.create(alias2)); + await expect(service.addAlias(note, alias2)).rejects.toThrow( + AlreadyInDBError, + ); + }); + it('with a forbidden name', async () => { + await expect(service.addAlias(note, forbiddenNoteId)).rejects.toThrow( + ForbiddenIdError, + ); + }); + }); + }); + + describe('removeAlias', () => { + const alias = 'testAlias'; + const alias2 = 'testAlias2'; + const user = User.create('hardcoded', 'Testy') as User; + describe('removes one alias correctly', () => { + const note = Note.create(user, alias); + note.aliases.push(Alias.create(alias2)); + it('with two aliases', async () => { + jest + .spyOn(noteRepo, 'save') + .mockImplementationOnce(async (note: Note): Promise => note); + jest + .spyOn(aliasRepo, 'remove') + .mockImplementationOnce( + async (alias: Alias): Promise => alias, + ); + const savedNote = await service.removeAlias(note, alias2); + expect(savedNote.aliases).toHaveLength(1); + expect(savedNote.aliases[0].name).toEqual(alias); + expect(savedNote.aliases[0].primary).toBeTruthy(); + }); + it('with one alias, that is primary', async () => { + jest + .spyOn(noteRepo, 'save') + .mockImplementationOnce(async (note: Note): Promise => note); + jest + .spyOn(aliasRepo, 'remove') + .mockImplementationOnce( + async (alias: Alias): Promise => alias, + ); + const savedNote = await service.removeAlias(note, alias); + expect(savedNote.aliases).toHaveLength(0); + }); + }); + describe('does not remove one alias', () => { + const note = Note.create(user, alias); + note.aliases.push(Alias.create(alias2)); + it('if the alias is unknown', async () => { + await expect(service.removeAlias(note, 'non existent')).rejects.toThrow( + NotInDBError, + ); + }); + it('if it is primary and not the last one', async () => { + await expect(service.removeAlias(note, alias)).rejects.toThrow( + PrimaryAliasDeletionForbiddenError, + ); + }); + }); + }); + + describe('makeAliasPrimary', () => { + const alias = Alias.create('testAlias', true); + const alias2 = Alias.create('testAlias2'); + const user = User.create('hardcoded', 'Testy') as User; + const note = Note.create(user, alias.name); + note.aliases.push(alias2); + it('mark the alias as primary', async () => { + jest + .spyOn(aliasRepo, 'findOne') + .mockResolvedValueOnce(alias) + .mockResolvedValueOnce(alias2); + jest + .spyOn(aliasRepo, 'save') + .mockImplementationOnce(async (alias: Alias): Promise => alias) + .mockImplementationOnce(async (alias: Alias): Promise => alias); + const createQueryBuilder = { + leftJoinAndSelect: () => createQueryBuilder, + where: () => createQueryBuilder, + orWhere: () => createQueryBuilder, + setParameter: () => createQueryBuilder, + getOne: () => { + return { + ...note, + aliases: note.aliases.map((anAlias) => { + if (anAlias.primary) { + anAlias.primary = false; + } + if (anAlias.name === alias2.name) { + anAlias.primary = true; + } + return anAlias; + }), + }; + }, + }; + jest + .spyOn(noteRepo, 'createQueryBuilder') + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + .mockImplementation(() => createQueryBuilder); + const savedAlias = await service.makeAliasPrimary(note, alias2.name); + expect(savedAlias.name).toEqual(alias2.name); + expect(savedAlias.primary).toBeTruthy(); + }); + it('does not mark the alias as primary, if the alias does not exist', async () => { + await expect( + service.makeAliasPrimary(note, 'i_dont_exist'), + ).rejects.toThrow(NotInDBError); + }); + }); + + it('toAliasDto correctly creates an AliasDto', () => { + const aliasName = 'testAlias'; + const alias = Alias.create(aliasName, true); + const user = User.create('hardcoded', 'Testy') as User; + const note = Note.create(user, alias.name); + const aliasDto = service.toAliasDto(alias, note); + expect(aliasDto.name).toEqual(aliasName); + expect(aliasDto.primaryAlias).toBeTruthy(); + expect(aliasDto.noteId).toEqual(note.publicId); + }); +}); diff --git a/src/notes/alias.service.ts b/src/notes/alias.service.ts new file mode 100644 index 000000000..d507864c2 --- /dev/null +++ b/src/notes/alias.service.ts @@ -0,0 +1,186 @@ +/* + * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ +import { Inject, Injectable } from '@nestjs/common'; +import { InjectRepository } from '@nestjs/typeorm'; +import { Repository } from 'typeorm'; + +import { + AlreadyInDBError, + NotInDBError, + PrimaryAliasDeletionForbiddenError, +} from '../errors/errors'; +import { ConsoleLoggerService } from '../logger/console-logger.service'; +import { AliasDto } from './alias.dto'; +import { Alias } from './alias.entity'; +import { Note } from './note.entity'; +import { NotesService } from './notes.service'; +import { getPrimaryAlias } from './utils'; + +@Injectable() +export class AliasService { + constructor( + private readonly logger: ConsoleLoggerService, + @InjectRepository(Note) private noteRepository: Repository, + @InjectRepository(Alias) private aliasRepository: Repository, + @Inject(NotesService) private notesService: NotesService, + ) { + this.logger.setContext(AliasService.name); + } + + /** + * @async + * Add the specified alias to the note. + * @param {Note} note - the note to add the alias to + * @param {string} alias - the alias to add to the note + * @throws {AlreadyInDBError} the alias is already in use. + * @return {Alias} the new alias + */ + async addAlias(note: Note, alias: string): Promise { + this.notesService.checkNoteIdOrAlias(alias); + + const foundAlias = await this.aliasRepository.findOne({ + where: { name: alias }, + }); + if (foundAlias !== undefined) { + this.logger.debug(`The alias '${alias}' is already used.`, 'addAlias'); + throw new AlreadyInDBError(`The alias '${alias}' is already used.`); + } + + const foundNote = await this.noteRepository.findOne({ + where: { publicId: alias }, + }); + if (foundNote !== undefined) { + this.logger.debug( + `The alias '${alias}' is already a public id.`, + 'addAlias', + ); + throw new AlreadyInDBError( + `The alias '${alias}' is already a public id.`, + ); + } + let newAlias: Alias; + if (note.aliases.length === 0) { + // the first alias is automatically made the primary alias + newAlias = Alias.create(alias, true); + } else { + newAlias = Alias.create(alias); + } + note.aliases.push(newAlias); + + await this.noteRepository.save(note); + return newAlias; + } + + /** + * @async + * Set the specified alias as the primary alias of the note. + * @param {Note} note - the note to change the primary alias + * @param {string} alias - the alias to be the new primary alias of the note + * @throws {NotInDBError} the alias is not part of this note. + * @return {Alias} the new primary alias + */ + async makeAliasPrimary(note: Note, alias: string): Promise { + let newPrimaryFound = false; + let oldPrimaryId = ''; + let newPrimaryId = ''; + + for (const anAlias of note.aliases) { + // found old primary + if (anAlias.primary) { + oldPrimaryId = anAlias.id; + } + + // found new primary + if (anAlias.name === alias) { + newPrimaryFound = true; + newPrimaryId = anAlias.id; + } + } + + if (!newPrimaryFound) { + // the provided alias is not already an alias of this note + this.logger.debug( + `The alias '${alias}' is not used by this note.`, + 'makeAliasPrimary', + ); + throw new NotInDBError(`The alias '${alias}' is not used by this note.`); + } + + const oldPrimary = await this.aliasRepository.findOne(oldPrimaryId); + const newPrimary = await this.aliasRepository.findOne(newPrimaryId); + + if (!oldPrimary || !newPrimary) { + throw new Error('This should not happen!'); + } + + oldPrimary.primary = false; + newPrimary.primary = true; + + await this.aliasRepository.save(oldPrimary); + await this.aliasRepository.save(newPrimary); + + return newPrimary; + } + + /** + * @async + * Remove the specified alias from the note. + * @param {Note} note - the note to remove the alias from + * @param {string} alias - the alias to remove from the note + * @throws {NotInDBError} the alias is not part of this note. + * @throws {PrimaryAliasDeletionForbiddenError} the primary alias can only be deleted if it's the only alias + */ + async removeAlias(note: Note, alias: string): Promise { + const primaryAlias = getPrimaryAlias(note); + + if (primaryAlias === alias && note.aliases.length !== 1) { + this.logger.debug( + `The alias '${alias}' is the primary alias, which can only be removed if it's the only alias.`, + 'removeAlias', + ); + throw new PrimaryAliasDeletionForbiddenError( + `The alias '${alias}' is the primary alias, which can only be removed if it's the only alias.`, + ); + } + + const filteredAliases = note.aliases.filter( + (anAlias) => anAlias.name !== alias, + ); + if (note.aliases.length === filteredAliases.length) { + this.logger.debug( + `The alias '${alias}' is not used by this note or is the primary alias, which can't be removed.`, + 'removeAlias', + ); + throw new NotInDBError( + `The alias '${alias}' is not used by this note or is the primary alias, which can't be removed.`, + ); + } + const aliasToDelete = note.aliases.find( + (anAlias) => anAlias.name === alias, + ); + if (aliasToDelete !== undefined) { + await this.aliasRepository.remove(aliasToDelete); + } + note.aliases = filteredAliases; + return await this.noteRepository.save(note); + } + + /** + * @async + * Build AliasDto from a note. + * @param {Alias} alias - the alias to use + * @param {Note} note - the note to use + * @return {AliasDto} the built AliasDto + * @throws {NotInDBError} the specified alias does not exist + */ + toAliasDto(alias: Alias, note: Note): AliasDto { + return { + name: alias.name, + primaryAlias: alias.primary, + noteId: note.publicId, + }; + } +} diff --git a/src/notes/notes.module.ts b/src/notes/notes.module.ts index 0bb564f07..1e83c85ab 100644 --- a/src/notes/notes.module.ts +++ b/src/notes/notes.module.ts @@ -15,6 +15,7 @@ import { RevisionsModule } from '../revisions/revisions.module'; import { User } from '../users/user.entity'; import { UsersModule } from '../users/users.module'; import { Alias } from './alias.entity'; +import { AliasService } from './alias.service'; import { Note } from './note.entity'; import { NotesService } from './notes.service'; import { Tag } from './tag.entity'; @@ -36,7 +37,7 @@ import { Tag } from './tag.entity'; ConfigModule, ], controllers: [], - providers: [NotesService], - exports: [NotesService], + providers: [NotesService, AliasService], + exports: [NotesService, AliasService], }) export class NotesModule {} From 10ed40f9f104dceec06d86e7a17acc6b27d00551 Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Sat, 7 Aug 2021 21:51:47 +0200 Subject: [PATCH 05/12] chore: create getPrimaryAlias utility function Signed-off-by: Philip Molares --- src/notes/utils.spec.ts | 37 +++++++++++++++++++++++++++---------- src/notes/utils.ts | 17 +++++++++++++++++ 2 files changed, 44 insertions(+), 10 deletions(-) diff --git a/src/notes/utils.spec.ts b/src/notes/utils.spec.ts index 6fb90d12c..55bc5e302 100644 --- a/src/notes/utils.spec.ts +++ b/src/notes/utils.spec.ts @@ -5,19 +5,36 @@ */ import { randomBytes } from 'crypto'; -import { generatePublicId } from './utils'; +import { User } from '../users/user.entity'; +import { Alias } from './alias.entity'; +import { Note } from './note.entity'; +import { generatePublicId, getPrimaryAlias } from './utils'; jest.mock('crypto'); +const random128bitBuffer = Buffer.from([ + 0xe1, 0x75, 0x86, 0xb7, 0xc3, 0xfb, 0x03, 0xa9, 0x26, 0x9f, 0xc9, 0xd6, 0x8c, + 0x2d, 0x7b, 0x7b, +]); +const mockRandomBytes = randomBytes as jest.MockedFunction; +mockRandomBytes.mockImplementation((_) => random128bitBuffer); it('generatePublicId', () => { - const random128bitBuffer = Buffer.from([ - 0xe1, 0x75, 0x86, 0xb7, 0xc3, 0xfb, 0x03, 0xa9, 0x26, 0x9f, 0xc9, 0xd6, - 0x8c, 0x2d, 0x7b, 0x7b, - ]); - const mockRandomBytes = randomBytes as jest.MockedFunction< - typeof randomBytes - >; - mockRandomBytes.mockImplementationOnce((_) => random128bitBuffer); - expect(generatePublicId()).toEqual('w5trddy3zc1tj9mzs7b8rbbvfc'); }); + +describe('getPrimaryAlias', () => { + const alias = 'alias'; + let note: Note; + beforeEach(() => { + const user = User.create('hardcoded', 'Testy') as User; + note = Note.create(user, alias); + }); + it('finds correct primary alias', () => { + note.aliases.push(Alias.create('annother', false)); + expect(getPrimaryAlias(note)).toEqual(alias); + }); + it('returns undefined if there is no alias', () => { + note.aliases[0].primary = false; + expect(getPrimaryAlias(note)).toEqual(undefined); + }); +}); diff --git a/src/notes/utils.ts b/src/notes/utils.ts index 7d0e75c01..7f6229fe7 100644 --- a/src/notes/utils.ts +++ b/src/notes/utils.ts @@ -6,6 +6,9 @@ import base32Encode from 'base32-encode'; import { randomBytes } from 'crypto'; +import { Alias } from './alias.entity'; +import { Note } from './note.entity'; + /** * Generate publicId for a note. * This is a randomly generated 128-bit value encoded with base32-encode using the crockford variant and converted to lowercase. @@ -14,3 +17,17 @@ export function generatePublicId(): string { const randomId = randomBytes(16); return base32Encode(randomId, 'Crockford').toLowerCase(); } + +/** + * Extract the primary alias from a aliases of a note + * @param {Note} note - the note from which the primary alias should be extracted + */ +export function getPrimaryAlias(note: Note): string | undefined { + const listWithPrimaryAlias = note.aliases.filter( + (alias: Alias) => alias.primary, + ); + if (listWithPrimaryAlias.length !== 1) { + return undefined; + } + return listWithPrimaryAlias[0].name; +} From 794be4a5dc10783ce84fabe12718231da0bc60e4 Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Thu, 16 Sep 2021 23:53:29 +0200 Subject: [PATCH 06/12] chore: create getIdentifier utility function Signed-off-by: Philip Molares --- src/history/history.service.ts | 25 +++++++++++------------ src/history/utils.spec.ts | 36 ++++++++++++++++++++++++++++++++++ src/history/utils.ts | 18 +++++++++++++++++ 3 files changed, 65 insertions(+), 14 deletions(-) create mode 100644 src/history/utils.spec.ts create mode 100644 src/history/utils.ts diff --git a/src/history/history.service.ts b/src/history/history.service.ts index f61b8e93b..6e6832d65 100644 --- a/src/history/history.service.ts +++ b/src/history/history.service.ts @@ -17,6 +17,7 @@ import { HistoryEntryImportDto } from './history-entry-import.dto'; import { HistoryEntryUpdateDto } from './history-entry-update.dto'; import { HistoryEntryDto } from './history-entry.dto'; import { HistoryEntry } from './history-entry.entity'; +import { getIdentifier } from './utils'; @Injectable() export class HistoryService { @@ -41,7 +42,7 @@ export class HistoryService { async getEntriesByUser(user: User): Promise { return await this.historyEntryRepository.find({ where: { user: user }, - relations: ['note', 'user'], + relations: ['note', 'note.aliases', 'user'], }); } @@ -58,7 +59,7 @@ export class HistoryService { note: note, user: user, }, - relations: ['note', 'user'], + relations: ['note', 'note.aliases', 'user'], }); if (!entry) { throw new NotInDBError( @@ -150,23 +151,19 @@ export class HistoryService { await this.connection.transaction(async (manager) => { const currentHistory = await manager.find(HistoryEntry, { where: { user: user }, - relations: ['note', 'user'], + relations: ['note', 'note.aliases', 'user'], }); for (const entry of currentHistory) { await manager.remove(entry); } for (const historyEntry of history) { this.notesService.checkNoteIdOrAlias(historyEntry.note); - const note = await manager.findOne(Note, { - where: [ - { - id: historyEntry.note, - }, - { - alias: historyEntry.note, - }, - ], - }); + const note = await manager + .createQueryBuilder(Note, 'note') + .innerJoin('note.aliases', 'alias') + .where('note.id = :id', { id: historyEntry.note }) + .orWhere('alias.name = :id', { id: historyEntry.note }) + .getOne(); if (note === undefined) { this.logger.debug( `Could not find note '${historyEntry.note}'`, @@ -191,7 +188,7 @@ export class HistoryService { */ toHistoryEntryDto(entry: HistoryEntry): HistoryEntryDto { return { - identifier: entry.note.alias ? entry.note.alias : entry.note.id, + identifier: getIdentifier(entry), lastVisited: entry.updatedAt, tags: this.notesService.toTagList(entry.note), title: entry.note.title ?? '', diff --git a/src/history/utils.spec.ts b/src/history/utils.spec.ts new file mode 100644 index 000000000..d6465c27d --- /dev/null +++ b/src/history/utils.spec.ts @@ -0,0 +1,36 @@ +/* + * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ +import { Alias } from '../notes/alias.entity'; +import { Note } from '../notes/note.entity'; +import { User } from '../users/user.entity'; +import { HistoryEntry } from './history-entry.entity'; +import { getIdentifier } from './utils'; + +describe('getIdentifier', () => { + const alias = 'alias'; + let note: Note; + let entry: HistoryEntry; + beforeEach(() => { + const user = User.create('hardcoded', 'Testy') as User; + note = Note.create(user, alias); + entry = HistoryEntry.create(user, note); + }); + it('returns the publicId if there are no aliases', () => { + note.aliases = undefined as unknown as Alias[]; + expect(getIdentifier(entry)).toEqual(note.publicId); + }); + it('returns the publicId, if the alias array is empty', () => { + note.aliases = []; + expect(getIdentifier(entry)).toEqual(note.publicId); + }); + it('returns the publicId, if the only alias is not primary', () => { + note.aliases[0].primary = false; + expect(getIdentifier(entry)).toEqual(note.publicId); + }); + it('returns the primary alias, if one exists', () => { + expect(getIdentifier(entry)).toEqual(note.aliases[0].name); + }); +}); diff --git a/src/history/utils.ts b/src/history/utils.ts new file mode 100644 index 000000000..ea49cee63 --- /dev/null +++ b/src/history/utils.ts @@ -0,0 +1,18 @@ +/* + * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ +import { getPrimaryAlias } from '../notes/utils'; +import { HistoryEntry } from './history-entry.entity'; + +export function getIdentifier(entry: HistoryEntry): string { + if (!entry.note.aliases || entry.note.aliases.length === 0) { + return entry.note.publicId; + } + const primaryAlias = getPrimaryAlias(entry.note); + if (primaryAlias === undefined) { + return entry.note.publicId; + } + return primaryAlias; +} From 3b7a06913b09f641aba69e15c5347f2676a84c29 Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Sun, 6 Jun 2021 17:47:38 +0200 Subject: [PATCH 07/12] feat: add aliases to service files This commit makes it possible to identifier notes via any alias in the note and history service. Signed-off-by: Philip Molares --- src/history/history.service.ts | 1 + src/notes/notes.service.ts | 71 +++++++++++++++++++++------------- 2 files changed, 45 insertions(+), 27 deletions(-) diff --git a/src/history/history.service.ts b/src/history/history.service.ts index 6e6832d65..e6e26927e 100644 --- a/src/history/history.service.ts +++ b/src/history/history.service.ts @@ -11,6 +11,7 @@ import { NotInDBError } from '../errors/errors'; import { ConsoleLoggerService } from '../logger/console-logger.service'; import { Note } from '../notes/note.entity'; import { NotesService } from '../notes/notes.service'; +import { getPrimaryAlias } from '../notes/utils'; import { User } from '../users/user.entity'; import { UsersService } from '../users/users.service'; import { HistoryEntryImportDto } from './history-entry-import.dto'; diff --git a/src/notes/notes.service.ts b/src/notes/notes.service.ts index b0dfd0ab9..5611025fd 100644 --- a/src/notes/notes.service.ts +++ b/src/notes/notes.service.ts @@ -24,6 +24,7 @@ import { RevisionsService } from '../revisions/revisions.service'; import { User } from '../users/user.entity'; import { UsersService } from '../users/users.service'; import { checkArrayForDuplicates } from '../utils/arrayDuplicatCheck'; +import { Alias } from './alias.entity'; import { NoteMetadataDto } from './note-metadata.dto'; import { NotePermissionsDto, @@ -32,6 +33,7 @@ import { import { NoteDto } from './note.dto'; import { Note } from './note.entity'; import { Tag } from './tag.entity'; +import { getPrimaryAlias } from './utils'; @Injectable() export class NotesService { @@ -39,6 +41,7 @@ export class NotesService { private readonly logger: ConsoleLoggerService, @InjectRepository(Note) private noteRepository: Repository, @InjectRepository(Tag) private tagRepository: Repository, + @InjectRepository(Alias) private aliasRepository: Repository, @InjectRepository(User) private userRepository: Repository, @Inject(UsersService) private usersService: UsersService, @Inject(GroupsService) private groupsService: GroupsService, @@ -59,7 +62,13 @@ export class NotesService { async getUserNotes(user: User): Promise { const notes = await this.noteRepository.find({ where: { owner: user }, - relations: ['owner', 'userPermissions', 'groupPermissions', 'tags'], + relations: [ + 'owner', + 'userPermissions', + 'groupPermissions', + 'tags', + 'aliases', + ], }); if (notes === undefined) { return []; @@ -79,21 +88,19 @@ export class NotesService { */ async createNote( noteContent: string, - alias?: NoteMetadataDto['alias'], + alias?: string, owner?: User, ): Promise { - const newNote = Note.create(); + if (alias) { + this.checkNoteIdOrAlias(alias); + } + const newNote = Note.create(owner, alias); //TODO: Calculate patch newNote.revisions = Promise.resolve([ Revision.create(noteContent, noteContent), ]); - if (alias) { - newNote.alias = alias; - this.checkNoteIdOrAlias(alias); - } if (owner) { newNote.historyEntries = [HistoryEntry.create(owner)]; - newNote.owner = owner; } try { return await this.noteRepository.save(newNote); @@ -156,24 +163,33 @@ export class NotesService { 'getNoteByIdOrAlias', ); this.checkNoteIdOrAlias(noteIdOrAlias); - const note = await this.noteRepository.findOne({ - where: [ - { - publicId: noteIdOrAlias, - }, - { - alias: noteIdOrAlias, - }, - ], - relations: [ - 'owner', - 'groupPermissions', - 'groupPermissions.group', - 'userPermissions', - 'userPermissions.user', - 'tags', - ], - }); + + /** + * This query gets the note's aliases, owner, groupPermissions (and the groups), userPermissions (and the users) and tags and + * then only gets the note, that either has a publicId :noteIdOrAlias or has any alias with this name. + **/ + const note = await this.noteRepository + .createQueryBuilder('note') + .leftJoinAndSelect('note.aliases', 'alias') + .leftJoinAndSelect('note.owner', 'owner') + .leftJoinAndSelect('note.groupPermissions', 'group_permission') + .leftJoinAndSelect('group_permission.group', 'group') + .leftJoinAndSelect('note.userPermissions', 'user_permission') + .leftJoinAndSelect('user_permission.user', 'user') + .leftJoinAndSelect('note.tags', 'tag') + .where('note.publicId = :noteIdOrAlias') + .orWhere((queryBuilder) => { + const subQuery = queryBuilder + .subQuery() + .select('alias.noteId') + .from(Alias, 'alias') + .where('alias.name = :noteIdOrAlias') + .getQuery(); + return 'note.id IN ' + subQuery; + }) + .setParameter('noteIdOrAlias', noteIdOrAlias) + .getOne(); + if (note === undefined) { this.logger.debug( `Could not find note '${noteIdOrAlias}'`, @@ -369,7 +385,8 @@ export class NotesService { const updateUser = await this.calculateUpdateUser(note); return { id: note.publicId, - alias: note.alias ?? null, + aliases: note.aliases.map((alias) => alias.name), + primaryAlias: getPrimaryAlias(note) ?? null, title: note.title ?? '', createTime: (await this.getFirstRevision(note)).createdAt, description: note.description ?? '', From e9d4a8192231bdc68872e6b19eb54498802551e7 Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Sat, 7 Aug 2021 21:28:59 +0200 Subject: [PATCH 08/12] feat: add alias controller to private and public api Signed-off-by: Philip Molares --- .../private/alias/alias.controller.spec.ts | 123 ++++++++++++++ src/api/private/alias/alias.controller.ts | 140 +++++++++++++++ src/api/private/private-api.module.ts | 2 + src/api/public/alias/alias.controller.spec.ts | 123 ++++++++++++++ src/api/public/alias/alias.controller.ts | 159 ++++++++++++++++++ src/api/public/public-api.module.ts | 2 + 6 files changed, 549 insertions(+) create mode 100644 src/api/private/alias/alias.controller.spec.ts create mode 100644 src/api/private/alias/alias.controller.ts create mode 100644 src/api/public/alias/alias.controller.spec.ts create mode 100644 src/api/public/alias/alias.controller.ts diff --git a/src/api/private/alias/alias.controller.spec.ts b/src/api/private/alias/alias.controller.spec.ts new file mode 100644 index 000000000..c272c4298 --- /dev/null +++ b/src/api/private/alias/alias.controller.spec.ts @@ -0,0 +1,123 @@ +/* + * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ +import { ConfigModule } from '@nestjs/config'; +import { Test, TestingModule } from '@nestjs/testing'; +import { + getConnectionToken, + getRepositoryToken, + TypeOrmModule, +} from '@nestjs/typeorm'; + +import { AuthToken } from '../../../auth/auth-token.entity'; +import { Author } from '../../../authors/author.entity'; +import appConfigMock from '../../../config/mock/app.config.mock'; +import mediaConfigMock from '../../../config/mock/media.config.mock'; +import { Group } from '../../../groups/group.entity'; +import { GroupsModule } from '../../../groups/groups.module'; +import { HistoryEntry } from '../../../history/history-entry.entity'; +import { HistoryModule } from '../../../history/history.module'; +import { Identity } from '../../../identity/identity.entity'; +import { LoggerModule } from '../../../logger/logger.module'; +import { MediaUpload } from '../../../media/media-upload.entity'; +import { MediaModule } from '../../../media/media.module'; +import { Alias } from '../../../notes/alias.entity'; +import { AliasService } from '../../../notes/alias.service'; +import { Note } from '../../../notes/note.entity'; +import { NotesService } from '../../../notes/notes.service'; +import { Tag } from '../../../notes/tag.entity'; +import { NoteGroupPermission } from '../../../permissions/note-group-permission.entity'; +import { NoteUserPermission } from '../../../permissions/note-user-permission.entity'; +import { PermissionsModule } from '../../../permissions/permissions.module'; +import { Edit } from '../../../revisions/edit.entity'; +import { Revision } from '../../../revisions/revision.entity'; +import { RevisionsModule } from '../../../revisions/revisions.module'; +import { Session } from '../../../users/session.entity'; +import { User } from '../../../users/user.entity'; +import { UsersModule } from '../../../users/users.module'; +import { AliasController } from './alias.controller'; + +describe('AliasController', () => { + let controller: AliasController; + + beforeEach(async () => { + const module: TestingModule = await Test.createTestingModule({ + controllers: [AliasController], + providers: [ + AliasService, + NotesService, + { + provide: getRepositoryToken(Note), + useValue: {}, + }, + { + provide: getRepositoryToken(Tag), + useValue: {}, + }, + { + provide: getRepositoryToken(Alias), + useValue: {}, + }, + { + provide: getRepositoryToken(User), + useValue: {}, + }, + ], + imports: [ + RevisionsModule, + UsersModule, + GroupsModule, + LoggerModule, + PermissionsModule, + HistoryModule, + MediaModule, + ConfigModule.forRoot({ + isGlobal: true, + load: [appConfigMock, mediaConfigMock], + }), + TypeOrmModule.forRoot(), + ], + }) + .overrideProvider(getConnectionToken()) + .useValue({}) + .overrideProvider(getRepositoryToken(Revision)) + .useValue({}) + .overrideProvider(getRepositoryToken(Edit)) + .useValue({}) + .overrideProvider(getRepositoryToken(User)) + .useValue({}) + .overrideProvider(getRepositoryToken(AuthToken)) + .useValue({}) + .overrideProvider(getRepositoryToken(Identity)) + .useValue({}) + .overrideProvider(getRepositoryToken(Note)) + .useValue({}) + .overrideProvider(getRepositoryToken(Tag)) + .useValue({}) + .overrideProvider(getRepositoryToken(HistoryEntry)) + .useValue({}) + .overrideProvider(getRepositoryToken(NoteGroupPermission)) + .useValue({}) + .overrideProvider(getRepositoryToken(NoteUserPermission)) + .useValue({}) + .overrideProvider(getRepositoryToken(Group)) + .useValue({}) + .overrideProvider(getRepositoryToken(MediaUpload)) + .useValue({}) + .overrideProvider(getRepositoryToken(Alias)) + .useValue({}) + .overrideProvider(getRepositoryToken(Session)) + .useValue({}) + .overrideProvider(getRepositoryToken(Author)) + .useValue({}) + .compile(); + + controller = module.get(AliasController); + }); + + it('should be defined', () => { + expect(controller).toBeDefined(); + }); +}); diff --git a/src/api/private/alias/alias.controller.ts b/src/api/private/alias/alias.controller.ts new file mode 100644 index 000000000..a359728e0 --- /dev/null +++ b/src/api/private/alias/alias.controller.ts @@ -0,0 +1,140 @@ +/* + * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ +import { + BadRequestException, + Body, + Controller, + Delete, + HttpCode, + NotFoundException, + Param, + Post, + Put, + Req, + UnauthorizedException, +} from '@nestjs/common'; +import { Request } from 'express'; + +import { + AlreadyInDBError, + ForbiddenIdError, + NotInDBError, + PrimaryAliasDeletionForbiddenError, +} from '../../../errors/errors'; +import { ConsoleLoggerService } from '../../../logger/console-logger.service'; +import { AliasCreateDto } from '../../../notes/alias-create.dto'; +import { AliasUpdateDto } from '../../../notes/alias-update.dto'; +import { AliasDto } from '../../../notes/alias.dto'; +import { AliasService } from '../../../notes/alias.service'; +import { NotesService } from '../../../notes/notes.service'; +import { PermissionsService } from '../../../permissions/permissions.service'; +import { UsersService } from '../../../users/users.service'; + +@Controller('alias') +export class AliasController { + constructor( + private readonly logger: ConsoleLoggerService, + private aliasService: AliasService, + private noteService: NotesService, + private userService: UsersService, + private permissionsService: PermissionsService, + ) { + this.logger.setContext(AliasController.name); + } + + @Post() + async addAlias( + @Req() req: Request, + @Body() newAliasDto: AliasCreateDto, + ): Promise { + try { + // ToDo: use actual user here + const user = await this.userService.getUserByUsername('hardcoded'); + const note = await this.noteService.getNoteByIdOrAlias( + newAliasDto.noteIdOrAlias, + ); + if (!this.permissionsService.isOwner(user, note)) { + throw new UnauthorizedException('Reading note denied!'); + } + const updatedAlias = await this.aliasService.addAlias( + note, + newAliasDto.newAlias, + ); + return this.aliasService.toAliasDto(updatedAlias, note); + } catch (e) { + if (e instanceof AlreadyInDBError) { + throw new BadRequestException(e.message); + } + if (e instanceof ForbiddenIdError) { + throw new BadRequestException(e.message); + } + throw e; + } + } + + @Put(':alias') + async makeAliasPrimary( + @Req() req: Request, + @Param('alias') alias: string, + @Body() changeAliasDto: AliasUpdateDto, + ): Promise { + if (!changeAliasDto.primaryAlias) { + throw new BadRequestException( + `The field 'primaryAlias' must be set to 'true'.`, + ); + } + try { + // ToDo: use actual user here + const user = await this.userService.getUserByUsername('hardcoded'); + const note = await this.noteService.getNoteByIdOrAlias(alias); + if (!this.permissionsService.isOwner(user, note)) { + throw new UnauthorizedException('Reading note denied!'); + } + const updatedAlias = await this.aliasService.makeAliasPrimary( + note, + alias, + ); + return this.aliasService.toAliasDto(updatedAlias, note); + } catch (e) { + if (e instanceof NotInDBError) { + throw new NotFoundException(e.message); + } + if (e instanceof ForbiddenIdError) { + throw new BadRequestException(e.message); + } + throw e; + } + } + + @Delete(':alias') + @HttpCode(204) + async removeAlias( + @Req() req: Request, + @Param('alias') alias: string, + ): Promise { + try { + // ToDo: use actual user here + const user = await this.userService.getUserByUsername('hardcoded'); + const note = await this.noteService.getNoteByIdOrAlias(alias); + if (!this.permissionsService.isOwner(user, note)) { + throw new UnauthorizedException('Reading note denied!'); + } + await this.aliasService.removeAlias(note, alias); + return; + } catch (e) { + if (e instanceof NotInDBError) { + throw new NotFoundException(e.message); + } + if (e instanceof PrimaryAliasDeletionForbiddenError) { + throw new BadRequestException(e.message); + } + if (e instanceof ForbiddenIdError) { + throw new BadRequestException(e.message); + } + throw e; + } + } +} diff --git a/src/api/private/private-api.module.ts b/src/api/private/private-api.module.ts index 625cfbe13..9b8b2cb9d 100644 --- a/src/api/private/private-api.module.ts +++ b/src/api/private/private-api.module.ts @@ -15,6 +15,7 @@ import { NotesModule } from '../../notes/notes.module'; import { PermissionsModule } from '../../permissions/permissions.module'; import { RevisionsModule } from '../../revisions/revisions.module'; import { UsersModule } from '../../users/users.module'; +import { AliasController } from './alias/alias.controller'; import { AuthController } from './auth/auth.controller'; import { ConfigController } from './config/config.controller'; import { HistoryController } from './me/history/history.controller'; @@ -43,6 +44,7 @@ import { TokensController } from './tokens/tokens.controller'; HistoryController, MeController, NotesController, + AliasController, AuthController, ], }) diff --git a/src/api/public/alias/alias.controller.spec.ts b/src/api/public/alias/alias.controller.spec.ts new file mode 100644 index 000000000..c272c4298 --- /dev/null +++ b/src/api/public/alias/alias.controller.spec.ts @@ -0,0 +1,123 @@ +/* + * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ +import { ConfigModule } from '@nestjs/config'; +import { Test, TestingModule } from '@nestjs/testing'; +import { + getConnectionToken, + getRepositoryToken, + TypeOrmModule, +} from '@nestjs/typeorm'; + +import { AuthToken } from '../../../auth/auth-token.entity'; +import { Author } from '../../../authors/author.entity'; +import appConfigMock from '../../../config/mock/app.config.mock'; +import mediaConfigMock from '../../../config/mock/media.config.mock'; +import { Group } from '../../../groups/group.entity'; +import { GroupsModule } from '../../../groups/groups.module'; +import { HistoryEntry } from '../../../history/history-entry.entity'; +import { HistoryModule } from '../../../history/history.module'; +import { Identity } from '../../../identity/identity.entity'; +import { LoggerModule } from '../../../logger/logger.module'; +import { MediaUpload } from '../../../media/media-upload.entity'; +import { MediaModule } from '../../../media/media.module'; +import { Alias } from '../../../notes/alias.entity'; +import { AliasService } from '../../../notes/alias.service'; +import { Note } from '../../../notes/note.entity'; +import { NotesService } from '../../../notes/notes.service'; +import { Tag } from '../../../notes/tag.entity'; +import { NoteGroupPermission } from '../../../permissions/note-group-permission.entity'; +import { NoteUserPermission } from '../../../permissions/note-user-permission.entity'; +import { PermissionsModule } from '../../../permissions/permissions.module'; +import { Edit } from '../../../revisions/edit.entity'; +import { Revision } from '../../../revisions/revision.entity'; +import { RevisionsModule } from '../../../revisions/revisions.module'; +import { Session } from '../../../users/session.entity'; +import { User } from '../../../users/user.entity'; +import { UsersModule } from '../../../users/users.module'; +import { AliasController } from './alias.controller'; + +describe('AliasController', () => { + let controller: AliasController; + + beforeEach(async () => { + const module: TestingModule = await Test.createTestingModule({ + controllers: [AliasController], + providers: [ + AliasService, + NotesService, + { + provide: getRepositoryToken(Note), + useValue: {}, + }, + { + provide: getRepositoryToken(Tag), + useValue: {}, + }, + { + provide: getRepositoryToken(Alias), + useValue: {}, + }, + { + provide: getRepositoryToken(User), + useValue: {}, + }, + ], + imports: [ + RevisionsModule, + UsersModule, + GroupsModule, + LoggerModule, + PermissionsModule, + HistoryModule, + MediaModule, + ConfigModule.forRoot({ + isGlobal: true, + load: [appConfigMock, mediaConfigMock], + }), + TypeOrmModule.forRoot(), + ], + }) + .overrideProvider(getConnectionToken()) + .useValue({}) + .overrideProvider(getRepositoryToken(Revision)) + .useValue({}) + .overrideProvider(getRepositoryToken(Edit)) + .useValue({}) + .overrideProvider(getRepositoryToken(User)) + .useValue({}) + .overrideProvider(getRepositoryToken(AuthToken)) + .useValue({}) + .overrideProvider(getRepositoryToken(Identity)) + .useValue({}) + .overrideProvider(getRepositoryToken(Note)) + .useValue({}) + .overrideProvider(getRepositoryToken(Tag)) + .useValue({}) + .overrideProvider(getRepositoryToken(HistoryEntry)) + .useValue({}) + .overrideProvider(getRepositoryToken(NoteGroupPermission)) + .useValue({}) + .overrideProvider(getRepositoryToken(NoteUserPermission)) + .useValue({}) + .overrideProvider(getRepositoryToken(Group)) + .useValue({}) + .overrideProvider(getRepositoryToken(MediaUpload)) + .useValue({}) + .overrideProvider(getRepositoryToken(Alias)) + .useValue({}) + .overrideProvider(getRepositoryToken(Session)) + .useValue({}) + .overrideProvider(getRepositoryToken(Author)) + .useValue({}) + .compile(); + + controller = module.get(AliasController); + }); + + it('should be defined', () => { + expect(controller).toBeDefined(); + }); +}); diff --git a/src/api/public/alias/alias.controller.ts b/src/api/public/alias/alias.controller.ts new file mode 100644 index 000000000..35bf80676 --- /dev/null +++ b/src/api/public/alias/alias.controller.ts @@ -0,0 +1,159 @@ +/* + * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ +import { + BadRequestException, + Body, + Controller, + Delete, + HttpCode, + NotFoundException, + Param, + Post, + Put, + UnauthorizedException, + UseGuards, +} from '@nestjs/common'; +import { + ApiNoContentResponse, + ApiOkResponse, + ApiSecurity, + ApiTags, +} from '@nestjs/swagger'; + +import { TokenAuthGuard } from '../../../auth/token.strategy'; +import { + AlreadyInDBError, + ForbiddenIdError, + NotInDBError, + PrimaryAliasDeletionForbiddenError, +} from '../../../errors/errors'; +import { ConsoleLoggerService } from '../../../logger/console-logger.service'; +import { AliasCreateDto } from '../../../notes/alias-create.dto'; +import { AliasUpdateDto } from '../../../notes/alias-update.dto'; +import { AliasDto } from '../../../notes/alias.dto'; +import { AliasService } from '../../../notes/alias.service'; +import { NotesService } from '../../../notes/notes.service'; +import { PermissionsService } from '../../../permissions/permissions.service'; +import { User } from '../../../users/user.entity'; +import { FullApi } from '../../utils/fullapi-decorator'; +import { RequestUser } from '../../utils/request-user.decorator'; + +@ApiTags('alias') +@ApiSecurity('token') +@Controller('alias') +export class AliasController { + constructor( + private readonly logger: ConsoleLoggerService, + private aliasService: AliasService, + private noteService: NotesService, + private permissionsService: PermissionsService, + ) { + this.logger.setContext(AliasController.name); + } + + @UseGuards(TokenAuthGuard) + @Post() + @ApiOkResponse({ + description: 'The new alias', + type: AliasDto, + }) + @FullApi + async addAlias( + @RequestUser() user: User, + @Body() newAliasDto: AliasCreateDto, + ): Promise { + try { + const note = await this.noteService.getNoteByIdOrAlias( + newAliasDto.noteIdOrAlias, + ); + if (!this.permissionsService.isOwner(user, note)) { + throw new UnauthorizedException('Reading note denied!'); + } + const updatedAlias = await this.aliasService.addAlias( + note, + newAliasDto.newAlias, + ); + return this.aliasService.toAliasDto(updatedAlias, note); + } catch (e) { + if (e instanceof AlreadyInDBError) { + throw new BadRequestException(e.message); + } + if (e instanceof ForbiddenIdError) { + throw new BadRequestException(e.message); + } + throw e; + } + } + + @UseGuards(TokenAuthGuard) + @Put(':alias') + @ApiOkResponse({ + description: 'The updated alias', + type: AliasDto, + }) + @FullApi + async makeAliasPrimary( + @RequestUser() user: User, + @Param('alias') alias: string, + @Body() changeAliasDto: AliasUpdateDto, + ): Promise { + if (!changeAliasDto.primaryAlias) { + throw new BadRequestException( + `The field 'primaryAlias' must be set to 'true'.`, + ); + } + try { + const note = await this.noteService.getNoteByIdOrAlias(alias); + if (!this.permissionsService.isOwner(user, note)) { + throw new UnauthorizedException('Reading note denied!'); + } + const updatedAlias = await this.aliasService.makeAliasPrimary( + note, + alias, + ); + return this.aliasService.toAliasDto(updatedAlias, note); + } catch (e) { + if (e instanceof NotInDBError) { + throw new NotFoundException(e.message); + } + if (e instanceof ForbiddenIdError) { + throw new BadRequestException(e.message); + } + throw e; + } + } + + @UseGuards(TokenAuthGuard) + @Delete(':alias') + @HttpCode(204) + @ApiNoContentResponse({ + description: 'The alias was deleted', + }) + @FullApi + async removeAlias( + @RequestUser() user: User, + @Param('alias') alias: string, + ): Promise { + try { + const note = await this.noteService.getNoteByIdOrAlias(alias); + if (!this.permissionsService.isOwner(user, note)) { + throw new UnauthorizedException('Reading note denied!'); + } + await this.aliasService.removeAlias(note, alias); + } catch (e) { + if (e instanceof NotInDBError) { + throw new NotFoundException(e.message); + } + if (e instanceof PrimaryAliasDeletionForbiddenError) { + throw new BadRequestException(e.message); + } + if (e instanceof ForbiddenIdError) { + throw new BadRequestException(e.message); + } + throw e; + } + } +} diff --git a/src/api/public/public-api.module.ts b/src/api/public/public-api.module.ts index bafbaa794..8818bcca3 100644 --- a/src/api/public/public-api.module.ts +++ b/src/api/public/public-api.module.ts @@ -13,6 +13,7 @@ import { NotesModule } from '../../notes/notes.module'; import { PermissionsModule } from '../../permissions/permissions.module'; import { RevisionsModule } from '../../revisions/revisions.module'; import { UsersModule } from '../../users/users.module'; +import { AliasController } from './alias/alias.controller'; import { MeController } from './me/me.controller'; import { MediaController } from './media/media.controller'; import { MonitoringController } from './monitoring/monitoring.controller'; @@ -30,6 +31,7 @@ import { NotesController } from './notes/notes.controller'; PermissionsModule, ], controllers: [ + AliasController, MeController, NotesController, MediaController, From b95a6f56b6b4e3bccc6abea53bd639b7d35b7f57 Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Sun, 6 Jun 2021 17:53:07 +0200 Subject: [PATCH 09/12] test: fix service tests to handle the new aliases Signed-off-by: Philip Molares --- .../me/history/history.controller.spec.ts | 3 + src/api/private/me/me.controller.spec.ts | 3 + .../private/media/media.controller.spec.ts | 3 + .../private/notes/notes.controller.spec.ts | 7 ++ src/api/public/me/me.controller.spec.ts | 3 + src/api/public/media/media.controller.spec.ts | 3 + src/api/public/notes/notes.controller.spec.ts | 7 ++ src/history/history.service.spec.ts | 115 ++++++++++++------ src/media/media.service.spec.ts | 18 ++- src/notes/notes.service.spec.ts | 53 ++++++-- src/permissions/permissions.service.spec.ts | 3 + src/revisions/revisions.service.spec.ts | 3 + 12 files changed, 168 insertions(+), 53 deletions(-) diff --git a/src/api/private/me/history/history.controller.spec.ts b/src/api/private/me/history/history.controller.spec.ts index 410908181..d3ab85c98 100644 --- a/src/api/private/me/history/history.controller.spec.ts +++ b/src/api/private/me/history/history.controller.spec.ts @@ -19,6 +19,7 @@ import { HistoryEntry } from '../../../../history/history-entry.entity'; import { HistoryModule } from '../../../../history/history.module'; import { Identity } from '../../../../identity/identity.entity'; import { LoggerModule } from '../../../../logger/logger.module'; +import { Alias } from '../../../../notes/alias.entity'; import { Note } from '../../../../notes/note.entity'; import { NotesModule } from '../../../../notes/notes.module'; import { Tag } from '../../../../notes/tag.entity'; @@ -73,6 +74,8 @@ describe('HistoryController', () => { .useValue({}) .overrideProvider(getRepositoryToken(Group)) .useValue({}) + .overrideProvider(getRepositoryToken(Alias)) + .useValue({}) .overrideProvider(getRepositoryToken(Author)) .useValue({}) .overrideProvider(getRepositoryToken(Session)) diff --git a/src/api/private/me/me.controller.spec.ts b/src/api/private/me/me.controller.spec.ts index d03e7208b..38c2a367a 100644 --- a/src/api/private/me/me.controller.spec.ts +++ b/src/api/private/me/me.controller.spec.ts @@ -18,6 +18,7 @@ import { Identity } from '../../../identity/identity.entity'; import { LoggerModule } from '../../../logger/logger.module'; import { MediaUpload } from '../../../media/media-upload.entity'; import { MediaModule } from '../../../media/media.module'; +import { Alias } from '../../../notes/alias.entity'; import { Note } from '../../../notes/note.entity'; import { Tag } from '../../../notes/tag.entity'; import { NoteGroupPermission } from '../../../permissions/note-group-permission.entity'; @@ -71,6 +72,8 @@ describe('MeController', () => { .useValue({}) .overrideProvider(getRepositoryToken(MediaUpload)) .useValue({}) + .overrideProvider(getRepositoryToken(Alias)) + .useValue({}) .overrideProvider(getRepositoryToken(Session)) .useValue({}) .overrideProvider(getRepositoryToken(Author)) diff --git a/src/api/private/media/media.controller.spec.ts b/src/api/private/media/media.controller.spec.ts index ffb9a9b66..774f34881 100644 --- a/src/api/private/media/media.controller.spec.ts +++ b/src/api/private/media/media.controller.spec.ts @@ -19,6 +19,7 @@ import { Identity } from '../../../identity/identity.entity'; import { LoggerModule } from '../../../logger/logger.module'; import { MediaUpload } from '../../../media/media-upload.entity'; import { MediaModule } from '../../../media/media.module'; +import { Alias } from '../../../notes/alias.entity'; import { Note } from '../../../notes/note.entity'; import { NotesModule } from '../../../notes/notes.module'; import { Tag } from '../../../notes/tag.entity'; @@ -76,6 +77,8 @@ describe('MediaController', () => { .useValue({}) .overrideProvider(getRepositoryToken(Group)) .useValue({}) + .overrideProvider(getRepositoryToken(Alias)) + .useValue({}) .overrideProvider(getRepositoryToken(Session)) .useValue({}) .overrideProvider(getRepositoryToken(Author)) diff --git a/src/api/private/notes/notes.controller.spec.ts b/src/api/private/notes/notes.controller.spec.ts index eb908ff47..a47dda7ed 100644 --- a/src/api/private/notes/notes.controller.spec.ts +++ b/src/api/private/notes/notes.controller.spec.ts @@ -23,6 +23,7 @@ import { Identity } from '../../../identity/identity.entity'; import { LoggerModule } from '../../../logger/logger.module'; import { MediaUpload } from '../../../media/media-upload.entity'; import { MediaModule } from '../../../media/media.module'; +import { Alias } from '../../../notes/alias.entity'; import { Note } from '../../../notes/note.entity'; import { NotesService } from '../../../notes/notes.service'; import { Tag } from '../../../notes/tag.entity'; @@ -53,6 +54,10 @@ describe('NotesController', () => { provide: getRepositoryToken(Tag), useValue: {}, }, + { + provide: getRepositoryToken(Alias), + useValue: {}, + }, { provide: getRepositoryToken(User), useValue: {}, @@ -99,6 +104,8 @@ describe('NotesController', () => { .useValue({}) .overrideProvider(getRepositoryToken(MediaUpload)) .useValue({}) + .overrideProvider(getRepositoryToken(Alias)) + .useValue({}) .overrideProvider(getRepositoryToken(Session)) .useValue({}) .overrideProvider(getRepositoryToken(Author)) diff --git a/src/api/public/me/me.controller.spec.ts b/src/api/public/me/me.controller.spec.ts index dd5bf3750..13cc6cebd 100644 --- a/src/api/public/me/me.controller.spec.ts +++ b/src/api/public/me/me.controller.spec.ts @@ -22,6 +22,7 @@ import { Identity } from '../../../identity/identity.entity'; import { LoggerModule } from '../../../logger/logger.module'; import { MediaUpload } from '../../../media/media-upload.entity'; import { MediaModule } from '../../../media/media.module'; +import { Alias } from '../../../notes/alias.entity'; import { Note } from '../../../notes/note.entity'; import { NotesModule } from '../../../notes/notes.module'; import { Tag } from '../../../notes/tag.entity'; @@ -79,6 +80,8 @@ describe('Me Controller', () => { .useValue({}) .overrideProvider(getRepositoryToken(MediaUpload)) .useValue({}) + .overrideProvider(getRepositoryToken(Alias)) + .useValue({}) .overrideProvider(getRepositoryToken(Session)) .useValue({}) .overrideProvider(getRepositoryToken(Author)) diff --git a/src/api/public/media/media.controller.spec.ts b/src/api/public/media/media.controller.spec.ts index 028ccba75..4c75c7f6a 100644 --- a/src/api/public/media/media.controller.spec.ts +++ b/src/api/public/media/media.controller.spec.ts @@ -16,6 +16,7 @@ import { Identity } from '../../../identity/identity.entity'; import { LoggerModule } from '../../../logger/logger.module'; import { MediaUpload } from '../../../media/media-upload.entity'; import { MediaModule } from '../../../media/media.module'; +import { Alias } from '../../../notes/alias.entity'; import { Note } from '../../../notes/note.entity'; import { NotesModule } from '../../../notes/notes.module'; import { Tag } from '../../../notes/tag.entity'; @@ -65,6 +66,8 @@ describe('Media Controller', () => { .useValue({}) .overrideProvider(getRepositoryToken(Group)) .useValue({}) + .overrideProvider(getRepositoryToken(Alias)) + .useValue({}) .overrideProvider(getRepositoryToken(Session)) .useValue({}) .overrideProvider(getRepositoryToken(Author)) diff --git a/src/api/public/notes/notes.controller.spec.ts b/src/api/public/notes/notes.controller.spec.ts index d4ec8bbf0..77c281efc 100644 --- a/src/api/public/notes/notes.controller.spec.ts +++ b/src/api/public/notes/notes.controller.spec.ts @@ -23,6 +23,7 @@ import { Identity } from '../../../identity/identity.entity'; import { LoggerModule } from '../../../logger/logger.module'; import { MediaUpload } from '../../../media/media-upload.entity'; import { MediaModule } from '../../../media/media.module'; +import { Alias } from '../../../notes/alias.entity'; import { Note } from '../../../notes/note.entity'; import { NotesService } from '../../../notes/notes.service'; import { Tag } from '../../../notes/tag.entity'; @@ -53,6 +54,10 @@ describe('Notes Controller', () => { provide: getRepositoryToken(Tag), useValue: {}, }, + { + provide: getRepositoryToken(Alias), + useValue: {}, + }, { provide: getRepositoryToken(User), useValue: {}, @@ -101,6 +106,8 @@ describe('Notes Controller', () => { .useValue({}) .overrideProvider(getRepositoryToken(MediaUpload)) .useValue({}) + .overrideProvider(getRepositoryToken(Alias)) + .useValue({}) .overrideProvider(getRepositoryToken(Session)) .useValue({}) .overrideProvider(getRepositoryToken(Author)) diff --git a/src/history/history.service.spec.ts b/src/history/history.service.spec.ts index 559b2946b..c4b4c521f 100644 --- a/src/history/history.service.spec.ts +++ b/src/history/history.service.spec.ts @@ -15,6 +15,7 @@ import { NotInDBError } from '../errors/errors'; import { Group } from '../groups/group.entity'; import { Identity } from '../identity/identity.entity'; import { LoggerModule } from '../logger/logger.module'; +import { Alias } from '../notes/alias.entity'; import { Note } from '../notes/note.entity'; import { NotesModule } from '../notes/notes.module'; import { Tag } from '../notes/tag.entity'; @@ -34,6 +35,7 @@ describe('HistoryService', () => { let historyRepo: Repository; let connection; let noteRepo: Repository; + let aliasRepo: Repository; type MockConnection = { transaction: () => void; @@ -92,12 +94,15 @@ describe('HistoryService', () => { .useValue({}) .overrideProvider(getRepositoryToken(Author)) .useValue({}) + .overrideProvider(getRepositoryToken(Alias)) + .useClass(Repository) .compile(); service = module.get(HistoryService); historyRepo = module.get>( getRepositoryToken(HistoryEntry), ); + aliasRepo = module.get>(getRepositoryToken(Alias)); connection = module.get(Connection); noteRepo = module.get>(getRepositoryToken(Note)); }); @@ -151,7 +156,8 @@ describe('HistoryService', () => { Note.create(user, alias), user, ); - expect(createHistoryEntry.note.alias).toEqual(alias); + expect(createHistoryEntry.note.aliases).toHaveLength(1); + expect(createHistoryEntry.note.aliases[0].name).toEqual(alias); expect(createHistoryEntry.note.owner).toEqual(user); expect(createHistoryEntry.user).toEqual(user); expect(createHistoryEntry.pinStatus).toEqual(false); @@ -168,7 +174,8 @@ describe('HistoryService', () => { Note.create(user, alias), user, ); - expect(createHistoryEntry.note.alias).toEqual(alias); + expect(createHistoryEntry.note.aliases).toHaveLength(1); + expect(createHistoryEntry.note.aliases[0].name).toEqual(alias); expect(createHistoryEntry.note.owner).toEqual(user); expect(createHistoryEntry.user).toEqual(user); expect(createHistoryEntry.pinStatus).toEqual(false); @@ -180,14 +187,27 @@ describe('HistoryService', () => { }); describe('updateHistoryEntry', () => { + const user = {} as User; + const alias = 'alias'; + const note = Note.create(user, alias); + beforeEach(() => { + const createQueryBuilder = { + leftJoinAndSelect: () => createQueryBuilder, + where: () => createQueryBuilder, + orWhere: () => createQueryBuilder, + setParameter: () => createQueryBuilder, + getOne: () => note, + }; + jest + .spyOn(noteRepo, 'createQueryBuilder') + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + .mockImplementation(() => createQueryBuilder); + }); describe('works', () => { it('with an entry', async () => { - const user = {} as User; - const alias = 'alias'; - const note = Note.create(user, alias); const historyEntry = HistoryEntry.create(user, note); jest.spyOn(historyRepo, 'findOne').mockResolvedValueOnce(historyEntry); - jest.spyOn(noteRepo, 'findOne').mockResolvedValueOnce(note); jest .spyOn(historyRepo, 'save') .mockImplementation( @@ -200,18 +220,15 @@ describe('HistoryService', () => { pinStatus: true, }, ); - expect(updatedHistoryEntry.note.alias).toEqual(alias); + expect(updatedHistoryEntry.note.aliases).toHaveLength(1); + expect(updatedHistoryEntry.note.aliases[0].name).toEqual(alias); expect(updatedHistoryEntry.note.owner).toEqual(user); expect(updatedHistoryEntry.user).toEqual(user); expect(updatedHistoryEntry.pinStatus).toEqual(true); }); it('without an entry', async () => { - const user = {} as User; - const alias = 'alias'; - const note = Note.create(user, alias); jest.spyOn(historyRepo, 'findOne').mockResolvedValueOnce(undefined); - jest.spyOn(noteRepo, 'findOne').mockResolvedValueOnce(note); await expect( service.updateHistoryEntry(note, user, { pinStatus: true, @@ -278,7 +295,18 @@ describe('HistoryService', () => { const note = Note.create(user, alias); const historyEntry = HistoryEntry.create(user, note); jest.spyOn(historyRepo, 'findOne').mockResolvedValueOnce(historyEntry); - jest.spyOn(noteRepo, 'findOne').mockResolvedValueOnce(note); + const createQueryBuilder = { + leftJoinAndSelect: () => createQueryBuilder, + where: () => createQueryBuilder, + orWhere: () => createQueryBuilder, + setParameter: () => createQueryBuilder, + getOne: () => note, + }; + jest + .spyOn(noteRepo, 'createQueryBuilder') + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + .mockImplementation(() => createQueryBuilder); jest .spyOn(historyRepo, 'remove') .mockImplementation( @@ -295,8 +323,19 @@ describe('HistoryService', () => { const alias = 'alias'; it('without an entry', async () => { const note = Note.create(user, alias); + const createQueryBuilder = { + leftJoinAndSelect: () => createQueryBuilder, + where: () => createQueryBuilder, + orWhere: () => createQueryBuilder, + setParameter: () => createQueryBuilder, + getOne: () => note, + }; + jest + .spyOn(noteRepo, 'createQueryBuilder') + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + .mockImplementation(() => createQueryBuilder); jest.spyOn(historyRepo, 'findOne').mockResolvedValueOnce(undefined); - jest.spyOn(noteRepo, 'findOne').mockResolvedValueOnce(note); await expect(service.deleteHistoryEntry(note, user)).rejects.toThrow( NotInDBError, ); @@ -320,15 +359,24 @@ describe('HistoryService', () => { pinStatus: historyEntryImport.pinStatus, updatedAt: historyEntryImport.lastVisited, }; + const createQueryBuilder = { + innerJoin: () => createQueryBuilder, + where: () => createQueryBuilder, + orWhere: () => createQueryBuilder, + getOne: () => note, + }; const mockedManager = { find: jest.fn().mockResolvedValueOnce([historyEntry]), - findOne: jest.fn().mockResolvedValueOnce(note), + createQueryBuilder: () => createQueryBuilder, remove: jest.fn().mockImplementationOnce((entry: HistoryEntry) => { - expect(entry.note.alias).toEqual(alias); + expect(entry.note.aliases).toHaveLength(1); + expect(entry.note.aliases[0].name).toEqual(alias); expect(entry.pinStatus).toEqual(false); }), save: jest.fn().mockImplementationOnce((entry: HistoryEntry) => { - expect(entry.note.alias).toEqual(newlyCreatedHistoryEntry.note.alias); + expect(entry.note.aliases).toEqual( + newlyCreatedHistoryEntry.note.aliases, + ); expect(entry.pinStatus).toEqual(newlyCreatedHistoryEntry.pinStatus); expect(entry.updatedAt).toEqual(newlyCreatedHistoryEntry.updatedAt); }), @@ -358,36 +406,23 @@ describe('HistoryService', () => { }); const historyEntry = HistoryEntry.create(user, note); historyEntry.pinStatus = true; - jest.spyOn(noteRepo, 'findOne').mockResolvedValueOnce(note); + const createQueryBuilder = { + leftJoinAndSelect: () => createQueryBuilder, + where: () => createQueryBuilder, + orWhere: () => createQueryBuilder, + getOne: () => note, + }; + jest + .spyOn(noteRepo, 'createQueryBuilder') + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + .mockImplementation(() => createQueryBuilder); const historyEntryDto = service.toHistoryEntryDto(historyEntry); expect(historyEntryDto.pinStatus).toEqual(true); expect(historyEntryDto.identifier).toEqual(alias); expect(historyEntryDto.tags).toEqual(tags); expect(historyEntryDto.title).toEqual(title); }); - - it('with regular note', async () => { - const user = {} as User; - const title = 'title'; - const id = 'id'; - const tags = ['tag1', 'tag2']; - const note = Note.create(user); - note.title = title; - note.id = id; - note.tags = tags.map((tag) => { - const newTag = new Tag(); - newTag.name = tag; - return newTag; - }); - const historyEntry = HistoryEntry.create(user, note); - historyEntry.pinStatus = true; - jest.spyOn(noteRepo, 'findOne').mockResolvedValueOnce(note); - const historyEntryDto = service.toHistoryEntryDto(historyEntry); - expect(historyEntryDto.pinStatus).toEqual(true); - expect(historyEntryDto.identifier).toEqual(id); - expect(historyEntryDto.tags).toEqual(tags); - expect(historyEntryDto.title).toEqual(title); - }); }); }); }); diff --git a/src/media/media.service.spec.ts b/src/media/media.service.spec.ts index 76ff471a3..9aac642fe 100644 --- a/src/media/media.service.spec.ts +++ b/src/media/media.service.spec.ts @@ -17,6 +17,7 @@ import { ClientError, NotInDBError } from '../errors/errors'; import { Group } from '../groups/group.entity'; import { Identity } from '../identity/identity.entity'; import { LoggerModule } from '../logger/logger.module'; +import { Alias } from '../notes/alias.entity'; import { Note } from '../notes/note.entity'; import { NotesModule } from '../notes/notes.module'; import { Tag } from '../notes/tag.entity'; @@ -83,6 +84,8 @@ describe('MediaService', () => { .useValue({}) .overrideProvider(getRepositoryToken(Author)) .useValue({}) + .overrideProvider(getRepositoryToken(Alias)) + .useValue({}) .compile(); service = module.get(MediaService); @@ -105,7 +108,18 @@ describe('MediaService', () => { const alias = 'alias'; note = Note.create(user, alias); jest.spyOn(userRepo, 'findOne').mockResolvedValueOnce(user); - jest.spyOn(noteRepo, 'findOne').mockResolvedValueOnce(note); + const createQueryBuilder = { + leftJoinAndSelect: () => createQueryBuilder, + where: () => createQueryBuilder, + orWhere: () => createQueryBuilder, + setParameter: () => createQueryBuilder, + getOne: () => note, + }; + jest + .spyOn(noteRepo, 'createQueryBuilder') + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + .mockImplementation(() => createQueryBuilder); }); it('works', async () => { @@ -279,7 +293,7 @@ describe('MediaService', () => { id: 'testMediaUpload', backendData: 'testBackendData', note: { - alias: 'test', + aliases: [Alias.create('test', true)], } as Note, user: { userName: 'hardcoded', diff --git a/src/notes/notes.service.spec.ts b/src/notes/notes.service.spec.ts index 585a07b07..d61e36cc2 100644 --- a/src/notes/notes.service.spec.ts +++ b/src/notes/notes.service.spec.ts @@ -29,6 +29,7 @@ import { RevisionsModule } from '../revisions/revisions.module'; import { Session } from '../users/session.entity'; import { User } from '../users/user.entity'; import { UsersModule } from '../users/users.module'; +import { Alias } from './alias.entity'; import { NoteGroupPermissionUpdateDto, NoteUserPermissionUpdateDto, @@ -63,6 +64,10 @@ describe('NotesService', () => { provide: getRepositoryToken(Tag), useClass: Repository, }, + { + provide: getRepositoryToken(Alias), + useClass: Repository, + }, { provide: getRepositoryToken(User), useValue: userRepo, @@ -83,6 +88,8 @@ describe('NotesService', () => { .useClass(Repository) .overrideProvider(getRepositoryToken(Tag)) .useClass(Repository) + .overrideProvider(getRepositoryToken(Alias)) + .useClass(Repository) .overrideProvider(getRepositoryToken(User)) .useValue(userRepo) .overrideProvider(getRepositoryToken(AuthToken)) @@ -166,7 +173,7 @@ describe('NotesService', () => { expect(newNote.groupPermissions).toHaveLength(0); expect(newNote.tags).toHaveLength(0); expect(newNote.owner).toBeNull(); - expect(newNote.alias).toBeNull(); + expect(newNote.aliases).toHaveLength(0); }); it('without alias, with owner', async () => { const newNote = await service.createNote(content, undefined, user); @@ -179,7 +186,7 @@ describe('NotesService', () => { expect(newNote.groupPermissions).toHaveLength(0); expect(newNote.tags).toHaveLength(0); expect(newNote.owner).toEqual(user); - expect(newNote.alias).toBeNull(); + expect(newNote.aliases).toHaveLength(0); }); it('with alias, without owner', async () => { const newNote = await service.createNote(content, alias); @@ -191,7 +198,7 @@ describe('NotesService', () => { expect(newNote.groupPermissions).toHaveLength(0); expect(newNote.tags).toHaveLength(0); expect(newNote.owner).toBeNull(); - expect(newNote.alias).toEqual(alias); + expect(newNote.aliases).toHaveLength(1); }); it('with alias, with owner', async () => { const newNote = await service.createNote(content, alias, user); @@ -204,7 +211,8 @@ describe('NotesService', () => { expect(newNote.groupPermissions).toHaveLength(0); expect(newNote.tags).toHaveLength(0); expect(newNote.owner).toEqual(user); - expect(newNote.alias).toEqual(alias); + expect(newNote.aliases).toHaveLength(1); + expect(newNote.aliases[0].name).toEqual(alias); }); }); describe('fails:', () => { @@ -276,19 +284,40 @@ describe('NotesService', () => { it('works', async () => { const user = User.create('hardcoded', 'Testy') as User; const note = Note.create(user); - jest.spyOn(noteRepo, 'findOne').mockResolvedValueOnce(note); + const createQueryBuilder = { + leftJoinAndSelect: () => createQueryBuilder, + where: () => createQueryBuilder, + orWhere: () => createQueryBuilder, + setParameter: () => createQueryBuilder, + getOne: () => note, + }; + jest + .spyOn(noteRepo, 'createQueryBuilder') + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + .mockImplementation(() => createQueryBuilder); const foundNote = await service.getNoteByIdOrAlias('noteThatExists'); expect(foundNote).toEqual(note); }); describe('fails:', () => { it('no note found', async () => { - jest.spyOn(noteRepo, 'findOne').mockResolvedValueOnce(undefined); + const createQueryBuilder = { + leftJoinAndSelect: () => createQueryBuilder, + where: () => createQueryBuilder, + orWhere: () => createQueryBuilder, + setParameter: () => createQueryBuilder, + getOne: () => undefined, + }; + jest + .spyOn(noteRepo, 'createQueryBuilder') + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + .mockImplementation(() => createQueryBuilder); await expect( service.getNoteByIdOrAlias('noteThatDoesNoteExist'), ).rejects.toThrow(NotInDBError); }); it('id is forbidden', async () => { - jest.spyOn(noteRepo, 'findOne').mockResolvedValueOnce(undefined); await expect( service.getNoteByIdOrAlias(forbiddenNoteId), ).rejects.toThrow(ForbiddenIdError); @@ -709,7 +738,7 @@ describe('NotesService', () => { // @ts-ignore .mockImplementation(() => createQueryBuilder); note.publicId = 'testId'; - note.alias = 'testAlias'; + note.aliases = [Alias.create('testAlias', true)]; note.title = 'testTitle'; note.description = 'testDescription'; note.owner = user; @@ -737,7 +766,8 @@ describe('NotesService', () => { note.viewCount = 1337; const metadataDto = await service.toNoteMetadataDto(note); expect(metadataDto.id).toEqual(note.publicId); - expect(metadataDto.alias).toEqual(note.alias); + expect(metadataDto.aliases).toHaveLength(1); + expect(metadataDto.aliases[0]).toEqual(note.aliases[0].name); expect(metadataDto.title).toEqual(note.title); expect(metadataDto.createTime).toEqual(revisions[0].createdAt); expect(metadataDto.description).toEqual(note.description); @@ -808,7 +838,7 @@ describe('NotesService', () => { // @ts-ignore .mockImplementation(() => createQueryBuilder); note.publicId = 'testId'; - note.alias = 'testAlias'; + note.aliases = [Alias.create('testAlias', true)]; note.title = 'testTitle'; note.description = 'testDescription'; note.owner = user; @@ -836,7 +866,8 @@ describe('NotesService', () => { note.viewCount = 1337; const noteDto = await service.toNoteDto(note); expect(noteDto.metadata.id).toEqual(note.publicId); - expect(noteDto.metadata.alias).toEqual(note.alias); + expect(noteDto.metadata.aliases).toHaveLength(1); + expect(noteDto.metadata.aliases[0]).toEqual(note.aliases[0].name); expect(noteDto.metadata.title).toEqual(note.title); expect(noteDto.metadata.createTime).toEqual(revisions[0].createdAt); expect(noteDto.metadata.description).toEqual(note.description); diff --git a/src/permissions/permissions.service.spec.ts b/src/permissions/permissions.service.spec.ts index 9413a73f7..0d9ef7d92 100644 --- a/src/permissions/permissions.service.spec.ts +++ b/src/permissions/permissions.service.spec.ts @@ -13,6 +13,7 @@ import appConfigMock from '../config/mock/app.config.mock'; import { Group } from '../groups/group.entity'; import { Identity } from '../identity/identity.entity'; import { LoggerModule } from '../logger/logger.module'; +import { Alias } from '../notes/alias.entity'; import { Note } from '../notes/note.entity'; import { NotesModule } from '../notes/notes.module'; import { Tag } from '../notes/tag.entity'; @@ -67,6 +68,8 @@ describe('PermissionsService', () => { .useValue({}) .overrideProvider(getRepositoryToken(Author)) .useValue({}) + .overrideProvider(getRepositoryToken(Alias)) + .useValue({}) .compile(); permissionsService = module.get(PermissionsService); }); diff --git a/src/revisions/revisions.service.spec.ts b/src/revisions/revisions.service.spec.ts index cee143558..751721334 100644 --- a/src/revisions/revisions.service.spec.ts +++ b/src/revisions/revisions.service.spec.ts @@ -15,6 +15,7 @@ import { NotInDBError } from '../errors/errors'; import { Group } from '../groups/group.entity'; import { Identity } from '../identity/identity.entity'; import { LoggerModule } from '../logger/logger.module'; +import { Alias } from '../notes/alias.entity'; import { Note } from '../notes/note.entity'; import { NotesModule } from '../notes/notes.module'; import { Tag } from '../notes/tag.entity'; @@ -68,6 +69,8 @@ describe('RevisionsService', () => { .useValue({}) .overrideProvider(getRepositoryToken(Group)) .useValue({}) + .overrideProvider(getRepositoryToken(Alias)) + .useValue({}) .overrideProvider(getRepositoryToken(Session)) .useValue({}) .overrideProvider(getRepositoryToken(Author)) From 9f38b9036ca0e3f73a8d3dc5209cbe03d54d9f4a Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Sun, 6 Jun 2021 17:55:41 +0200 Subject: [PATCH 10/12] test: fix e2e tests to handle the new aliases Signed-off-by: Philip Molares --- test/private-api/alias.e2e-spec.ts | 219 +++++++++++++++++++++++++++ test/private-api/history.e2e-spec.ts | 20 ++- test/private-api/me.e2e-spec.ts | 6 +- test/public-api/alias.e2e-spec.ts | 215 ++++++++++++++++++++++++++ test/public-api/me.e2e-spec.ts | 24 ++- test/public-api/notes.e2e-spec.ts | 5 +- 6 files changed, 465 insertions(+), 24 deletions(-) create mode 100644 test/private-api/alias.e2e-spec.ts create mode 100644 test/public-api/alias.e2e-spec.ts diff --git a/test/private-api/alias.e2e-spec.ts b/test/private-api/alias.e2e-spec.ts new file mode 100644 index 000000000..9b620b04d --- /dev/null +++ b/test/private-api/alias.e2e-spec.ts @@ -0,0 +1,219 @@ +/* + * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ +import { INestApplication } from '@nestjs/common'; +import { ConfigModule, ConfigService } from '@nestjs/config'; +import { Test } from '@nestjs/testing'; +import { TypeOrmModule } from '@nestjs/typeorm'; +import request from 'supertest'; + +import { PrivateApiModule } from '../../src/api/private/private-api.module'; +import { AuthModule } from '../../src/auth/auth.module'; +import appConfigMock from '../../src/config/mock/app.config.mock'; +import authConfigMock from '../../src/config/mock/auth.config.mock'; +import customizationConfigMock from '../../src/config/mock/customization.config.mock'; +import externalConfigMock from '../../src/config/mock/external-services.config.mock'; +import mediaConfigMock from '../../src/config/mock/media.config.mock'; +import { GroupsModule } from '../../src/groups/groups.module'; +import { LoggerModule } from '../../src/logger/logger.module'; +import { AliasCreateDto } from '../../src/notes/alias-create.dto'; +import { AliasUpdateDto } from '../../src/notes/alias-update.dto'; +import { AliasService } from '../../src/notes/alias.service'; +import { NotesModule } from '../../src/notes/notes.module'; +import { NotesService } from '../../src/notes/notes.service'; +import { PermissionsModule } from '../../src/permissions/permissions.module'; +import { User } from '../../src/users/user.entity'; +import { UsersModule } from '../../src/users/users.module'; +import { UsersService } from '../../src/users/users.service'; + +describe('Alias', () => { + let app: INestApplication; + let aliasService: AliasService; + let notesService: NotesService; + let user: User; + let content: string; + let forbiddenNoteId: string; + + beforeAll(async () => { + const moduleRef = await Test.createTestingModule({ + imports: [ + ConfigModule.forRoot({ + isGlobal: true, + load: [ + mediaConfigMock, + appConfigMock, + authConfigMock, + customizationConfigMock, + externalConfigMock, + ], + }), + PrivateApiModule, + NotesModule, + PermissionsModule, + GroupsModule, + TypeOrmModule.forRoot({ + type: 'sqlite', + database: './hedgedoc-e2e-private-alias.sqlite', + autoLoadEntities: true, + synchronize: true, + dropSchema: true, + }), + LoggerModule, + AuthModule, + UsersModule, + ], + }).compile(); + + const config = moduleRef.get(ConfigService); + forbiddenNoteId = config.get('appConfig').forbiddenNoteIds[0]; + app = moduleRef.createNestApplication(); + await app.init(); + aliasService = moduleRef.get(AliasService); + notesService = moduleRef.get(NotesService); + const userService = moduleRef.get(UsersService); + user = await userService.createUser('hardcoded', 'Testy'); + content = 'This is a test note.'; + }); + + describe('POST /alias', () => { + const testAlias = 'aliasTest'; + const newAliasDto: AliasCreateDto = { + noteIdOrAlias: testAlias, + newAlias: '', + }; + let publicId = ''; + beforeAll(async () => { + const note = await notesService.createNote(content, testAlias, user); + publicId = note.publicId; + }); + + it('create with normal alias', async () => { + const newAlias = 'normalAlias'; + newAliasDto.newAlias = newAlias; + const metadata = await request(app.getHttpServer()) + .post(`/alias`) + .set('Content-Type', 'application/json') + .send(newAliasDto) + .expect(201); + expect(metadata.body.name).toEqual(newAlias); + expect(metadata.body.primaryAlias).toBeFalsy(); + expect(metadata.body.noteId).toEqual(publicId); + const note = await request(app.getHttpServer()) + .get(`/notes/${newAlias}`) + .expect(200); + expect(note.body.metadata.aliases).toContain(newAlias); + expect(note.body.metadata.primaryAlias).toBeTruthy(); + expect(note.body.metadata.id).toEqual(publicId); + }); + + describe('does not create an alias', () => { + it('because of a forbidden alias', async () => { + newAliasDto.newAlias = forbiddenNoteId; + await request(app.getHttpServer()) + .post(`/alias`) + .set('Content-Type', 'application/json') + .send(newAliasDto) + .expect(400); + }); + it('because of a alias that is a public id', async () => { + newAliasDto.newAlias = publicId; + await request(app.getHttpServer()) + .post(`/alias`) + .set('Content-Type', 'application/json') + .send(newAliasDto) + .expect(400); + }); + }); + }); + + describe('PUT /alias/{alias}', () => { + const testAlias = 'aliasTest2'; + const newAlias = 'normalAlias2'; + const changeAliasDto: AliasUpdateDto = { + primaryAlias: true, + }; + let publicId = ''; + beforeAll(async () => { + const note = await notesService.createNote(content, testAlias, user); + publicId = note.publicId; + await aliasService.addAlias(note, newAlias); + }); + + it('updates a note with a normal alias', async () => { + const metadata = await request(app.getHttpServer()) + .put(`/alias/${newAlias}`) + .set('Content-Type', 'application/json') + .send(changeAliasDto) + .expect(200); + expect(metadata.body.name).toEqual(newAlias); + expect(metadata.body.primaryAlias).toBeTruthy(); + expect(metadata.body.noteId).toEqual(publicId); + const note = await request(app.getHttpServer()) + .get(`/notes/${newAlias}`) + .expect(200); + expect(note.body.metadata.aliases).toContain(newAlias); + expect(note.body.metadata.primaryAlias).toBeTruthy(); + expect(note.body.metadata.id).toEqual(publicId); + }); + + describe('does not update', () => { + it('a note with unknown alias', async () => { + await request(app.getHttpServer()) + .put(`/alias/i_dont_exist`) + .set('Content-Type', 'application/json') + .send(changeAliasDto) + .expect(404); + }); + it('if the property primaryAlias is false', async () => { + changeAliasDto.primaryAlias = false; + await request(app.getHttpServer()) + .put(`/alias/${newAlias}`) + .set('Content-Type', 'application/json') + .send(changeAliasDto) + .expect(400); + }); + }); + }); + + describe('DELETE /alias/{alias}', () => { + const testAlias = 'aliasTest3'; + const newAlias = 'normalAlias3'; + beforeAll(async () => { + const note = await notesService.createNote(content, testAlias, user); + await aliasService.addAlias(note, newAlias); + }); + + it('deletes a normal alias', async () => { + await request(app.getHttpServer()) + .delete(`/alias/${newAlias}`) + .expect(204); + await request(app.getHttpServer()).get(`/notes/${newAlias}`).expect(404); + }); + + it('does not delete an unknown alias', async () => { + await request(app.getHttpServer()) + .delete(`/alias/i_dont_exist`) + .expect(404); + }); + + it('does not delete an primary alias (if it is not the only one)', async () => { + const note = await notesService.getNoteByIdOrAlias(testAlias); + await aliasService.addAlias(note, newAlias); + await request(app.getHttpServer()) + .delete(`/alias/${testAlias}`) + .expect(400); + await request(app.getHttpServer()).get(`/notes/${newAlias}`).expect(200); + }); + + it('deletes a primary alias (if it is the only one)', async () => { + await request(app.getHttpServer()) + .delete(`/alias/${newAlias}`) + .expect(204); + await request(app.getHttpServer()) + .delete(`/alias/${testAlias}`) + .expect(204); + }); + }); +}); diff --git a/test/private-api/history.e2e-spec.ts b/test/private-api/history.e2e-spec.ts index 1aae79ffe..4a6b2878a 100644 --- a/test/private-api/history.e2e-spec.ts +++ b/test/private-api/history.e2e-spec.ts @@ -77,7 +77,7 @@ describe('History', () => { const userService = moduleRef.get(UsersService); user = await userService.createUser('hardcoded', 'Testy'); const notesService = moduleRef.get(NotesService); - note = await notesService.createNote(content, null, user); + note = await notesService.createNote(content, 'note', user); note2 = await notesService.createNote(content, 'note2', user); }); @@ -109,7 +109,9 @@ describe('History', () => { const pinStatus = true; const lastVisited = new Date('2020-12-01 12:23:34'); const postEntryDto = new HistoryEntryImportDto(); - postEntryDto.note = note2.alias; + postEntryDto.note = note2.aliases.filter( + (alias) => alias.primary, + )[0].name; postEntryDto.pinStatus = pinStatus; postEntryDto.lastVisited = lastVisited; await request(app.getHttpServer()) @@ -119,7 +121,7 @@ describe('History', () => { .expect(201); const userEntries = await historyService.getEntriesByUser(user); expect(userEntries.length).toEqual(1); - expect(userEntries[0].note.alias).toEqual(note2.alias); + expect(userEntries[0].note.aliases).toEqual(note2.aliases); expect(userEntries[0].user.userName).toEqual(user.userName); expect(userEntries[0].pinStatus).toEqual(pinStatus); expect(userEntries[0].updatedAt).toEqual(lastVisited); @@ -136,7 +138,9 @@ describe('History', () => { pinStatus = !previousHistory[0].pinStatus; lastVisited = new Date('2020-12-01 23:34:45'); postEntryDto = new HistoryEntryImportDto(); - postEntryDto.note = note2.alias; + postEntryDto.note = note2.aliases.filter( + (alias) => alias.primary, + )[0].name; postEntryDto.pinStatus = pinStatus; postEntryDto.lastVisited = lastVisited; }); @@ -165,7 +169,7 @@ describe('History', () => { afterEach(async () => { const historyEntries = await historyService.getEntriesByUser(user); expect(historyEntries).toHaveLength(1); - expect(historyEntries[0].note.alias).toEqual(prevEntry.note.alias); + expect(historyEntries[0].note.aliases).toEqual(prevEntry.note.aliases); expect(historyEntries[0].user.userName).toEqual( prevEntry.user.userName, ); @@ -184,8 +188,9 @@ describe('History', () => { it('PUT /me/history/:note', async () => { const entry = await historyService.updateHistoryEntryTimestamp(note2, user); expect(entry.pinStatus).toBeFalsy(); + const alias = entry.note.aliases.filter((alias) => alias.primary)[0].name; await request(app.getHttpServer()) - .put(`/me/history/${entry.note.alias || 'undefined'}`) + .put(`/me/history/${alias || 'undefined'}`) .send({ pinStatus: true }) .expect(200); const userEntries = await historyService.getEntriesByUser(user); @@ -196,10 +201,11 @@ describe('History', () => { it('DELETE /me/history/:note', async () => { const entry = await historyService.updateHistoryEntryTimestamp(note2, user); + const alias = entry.note.aliases.filter((alias) => alias.primary)[0].name; const entry2 = await historyService.updateHistoryEntryTimestamp(note, user); const entryDto = historyService.toHistoryEntryDto(entry2); await request(app.getHttpServer()) - .delete(`/me/history/${entry.note.alias || 'undefined'}`) + .delete(`/me/history/${alias || 'undefined'}`) .expect(200); const userEntries = await historyService.getEntriesByUser(user); expect(userEntries.length).toEqual(1); diff --git a/test/private-api/me.e2e-spec.ts b/test/private-api/me.e2e-spec.ts index d0d35499e..b138bab47 100644 --- a/test/private-api/me.e2e-spec.ts +++ b/test/private-api/me.e2e-spec.ts @@ -45,6 +45,7 @@ describe('Me', () => { let user: User; let content: string; let note1: Note; + let alias2: string; let note2: Note; beforeAll(async () => { @@ -88,8 +89,9 @@ describe('Me', () => { 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); + alias2 = 'note2'; + note1 = await notesService.createNote(content, undefined, user); + note2 = await notesService.createNote(content, alias2, user); }); it('GET /me', async () => { diff --git a/test/public-api/alias.e2e-spec.ts b/test/public-api/alias.e2e-spec.ts new file mode 100644 index 000000000..5bc359c61 --- /dev/null +++ b/test/public-api/alias.e2e-spec.ts @@ -0,0 +1,215 @@ +/* + * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ +import { INestApplication } from '@nestjs/common'; +import { ConfigModule, ConfigService } from '@nestjs/config'; +import { Test } from '@nestjs/testing'; +import { TypeOrmModule } from '@nestjs/typeorm'; +import request from 'supertest'; + +import { PublicApiModule } from '../../src/api/public/public-api.module'; +import { AuthModule } from '../../src/auth/auth.module'; +import { MockAuthGuard } from '../../src/auth/mock-auth.guard'; +import { TokenAuthGuard } from '../../src/auth/token.strategy'; +import appConfigMock from '../../src/config/mock/app.config.mock'; +import mediaConfigMock from '../../src/config/mock/media.config.mock'; +import { GroupsModule } from '../../src/groups/groups.module'; +import { LoggerModule } from '../../src/logger/logger.module'; +import { AliasCreateDto } from '../../src/notes/alias-create.dto'; +import { AliasUpdateDto } from '../../src/notes/alias-update.dto'; +import { AliasService } from '../../src/notes/alias.service'; +import { NotesModule } from '../../src/notes/notes.module'; +import { NotesService } from '../../src/notes/notes.service'; +import { PermissionsModule } from '../../src/permissions/permissions.module'; +import { User } from '../../src/users/user.entity'; +import { UsersModule } from '../../src/users/users.module'; +import { UsersService } from '../../src/users/users.service'; + +describe('Notes', () => { + let app: INestApplication; + let notesService: NotesService; + let aliasService: AliasService; + let user: User; + let content: string; + let forbiddenNoteId: string; + + beforeAll(async () => { + const moduleRef = await Test.createTestingModule({ + imports: [ + ConfigModule.forRoot({ + isGlobal: true, + load: [mediaConfigMock, appConfigMock], + }), + PublicApiModule, + NotesModule, + PermissionsModule, + GroupsModule, + TypeOrmModule.forRoot({ + type: 'sqlite', + database: './hedgedoc-e2e-notes.sqlite', + autoLoadEntities: true, + synchronize: true, + dropSchema: true, + }), + LoggerModule, + AuthModule, + UsersModule, + ], + }) + .overrideGuard(TokenAuthGuard) + .useClass(MockAuthGuard) + .compile(); + + const config = moduleRef.get(ConfigService); + forbiddenNoteId = config.get('appConfig').forbiddenNoteIds[0]; + app = moduleRef.createNestApplication(); + await app.init(); + notesService = moduleRef.get(NotesService); + aliasService = moduleRef.get(AliasService); + const userService = moduleRef.get(UsersService); + user = await userService.createUser('hardcoded', 'Testy'); + content = 'This is a test note.'; + }); + + describe('POST /alias', () => { + const testAlias = 'aliasTest'; + const newAliasDto: AliasCreateDto = { + noteIdOrAlias: testAlias, + newAlias: '', + }; + let publicId = ''; + beforeAll(async () => { + const note = await notesService.createNote(content, testAlias, user); + publicId = note.publicId; + }); + + it('create with normal alias', async () => { + const newAlias = 'normalAlias'; + newAliasDto.newAlias = newAlias; + const metadata = await request(app.getHttpServer()) + .post(`/alias`) + .set('Content-Type', 'application/json') + .send(newAliasDto) + .expect(201); + expect(metadata.body.name).toEqual(newAlias); + expect(metadata.body.primaryAlias).toBeFalsy(); + expect(metadata.body.noteId).toEqual(publicId); + const note = await request(app.getHttpServer()) + .get(`/notes/${newAlias}`) + .expect(200); + expect(note.body.metadata.aliases).toContain(newAlias); + expect(note.body.metadata.primaryAlias).toBeTruthy(); + expect(note.body.metadata.id).toEqual(publicId); + }); + + describe('does not create an alias', () => { + it('because of a forbidden alias', async () => { + newAliasDto.newAlias = forbiddenNoteId; + await request(app.getHttpServer()) + .post(`/alias`) + .set('Content-Type', 'application/json') + .send(newAliasDto) + .expect(400); + }); + it('because of a alias that is a public id', async () => { + newAliasDto.newAlias = publicId; + await request(app.getHttpServer()) + .post(`/alias`) + .set('Content-Type', 'application/json') + .send(newAliasDto) + .expect(400); + }); + }); + }); + + describe('PUT /alias/{alias}', () => { + const testAlias = 'aliasTest2'; + const newAlias = 'normalAlias2'; + const changeAliasDto: AliasUpdateDto = { + primaryAlias: true, + }; + let publicId = ''; + beforeAll(async () => { + const note = await notesService.createNote(content, testAlias, user); + publicId = note.publicId; + await aliasService.addAlias(note, newAlias); + }); + + it('updates a note with a normal alias', async () => { + const metadata = await request(app.getHttpServer()) + .put(`/alias/${newAlias}`) + .set('Content-Type', 'application/json') + .send(changeAliasDto) + .expect(200); + expect(metadata.body.name).toEqual(newAlias); + expect(metadata.body.primaryAlias).toBeTruthy(); + expect(metadata.body.noteId).toEqual(publicId); + const note = await request(app.getHttpServer()) + .get(`/notes/${newAlias}`) + .expect(200); + expect(note.body.metadata.aliases).toContain(newAlias); + expect(note.body.metadata.primaryAlias).toBeTruthy(); + expect(note.body.metadata.id).toEqual(publicId); + }); + + describe('does not update', () => { + it('a note with unknown alias', async () => { + await request(app.getHttpServer()) + .put(`/alias/i_dont_exist`) + .set('Content-Type', 'application/json') + .send(changeAliasDto) + .expect(404); + }); + it('if the property primaryAlias is false', async () => { + changeAliasDto.primaryAlias = false; + await request(app.getHttpServer()) + .put(`/alias/${newAlias}`) + .set('Content-Type', 'application/json') + .send(changeAliasDto) + .expect(400); + }); + }); + }); + + describe('DELETE /alias/{alias}', () => { + const testAlias = 'aliasTest3'; + const newAlias = 'normalAlias3'; + beforeAll(async () => { + const note = await notesService.createNote(content, testAlias, user); + await aliasService.addAlias(note, newAlias); + }); + + it('deletes a normal alias', async () => { + await request(app.getHttpServer()) + .delete(`/alias/${newAlias}`) + .expect(204); + await request(app.getHttpServer()).get(`/notes/${newAlias}`).expect(404); + }); + + it('does not delete an unknown alias', async () => { + await request(app.getHttpServer()) + .delete(`/alias/i_dont_exist`) + .expect(404); + }); + + it('does not delete a primary alias (if it is not the only one)', async () => { + const note = await notesService.getNoteByIdOrAlias(testAlias); + await aliasService.addAlias(note, newAlias); + await request(app.getHttpServer()) + .delete(`/alias/${testAlias}`) + .expect(400); + await request(app.getHttpServer()).get(`/notes/${newAlias}`).expect(200); + }); + + it('deletes a primary alias (if it is the only one)', async () => { + await request(app.getHttpServer()) + .delete(`/alias/${newAlias}`) + .expect(204); + await request(app.getHttpServer()) + .delete(`/alias/${testAlias}`) + .expect(204); + }); + }); +}); diff --git a/test/public-api/me.e2e-spec.ts b/test/public-api/me.e2e-spec.ts index 1c89b2b3b..8a5c7a241 100644 --- a/test/public-api/me.e2e-spec.ts +++ b/test/public-api/me.e2e-spec.ts @@ -157,15 +157,15 @@ describe('Me', () => { .send(historyEntryUpdateDto) .expect(200); const history = await historyService.getEntriesByUser(user); - let historyEntry: HistoryEntryDto = response.body; + const historyEntry: HistoryEntryDto = response.body; expect(historyEntry.pinStatus).toEqual(true); - historyEntry = null; - for (const e of history) { - if (e.note.alias === noteName) { - historyEntry = historyService.toHistoryEntryDto(e); + let theEntry: HistoryEntryDto; + for (const entry of history) { + if (entry.note.aliases.find((element) => element.name === noteName)) { + theEntry = historyService.toHistoryEntryDto(entry); } } - expect(historyEntry.pinStatus).toEqual(true); + expect(theEntry.pinStatus).toEqual(true); }); it('fails with a non-existing note', async () => { await request(app.getHttpServer()) @@ -185,13 +185,11 @@ describe('Me', () => { .expect(204); expect(response.body).toEqual({}); const history = await historyService.getEntriesByUser(user); - let historyEntry: HistoryEntry = null; - for (const e of history) { - if (e.note.alias === noteName) { - historyEntry = e; + for (const entry of history) { + if (entry.note.aliases.find((element) => element.name === noteName)) { + throw new Error('Deleted history entry still in history'); } } - return expect(historyEntry).toBeNull(); }); describe('fails', () => { it('with a non-existing note', async () => { @@ -218,8 +216,8 @@ describe('Me', () => { .expect(200); const noteMetaDtos = response.body as NoteMetadataDto[]; expect(noteMetaDtos).toHaveLength(1); - expect(noteMetaDtos[0].alias).toEqual(noteName); - expect(noteMetaDtos[0].updateUser.userName).toEqual(user.userName); + expect(noteMetaDtos[0].primaryAlias).toEqual(noteName); + expect(noteMetaDtos[0].updateUser?.userName).toEqual(user.userName); }); it('GET /me/media', async () => { diff --git a/test/public-api/notes.e2e-spec.ts b/test/public-api/notes.e2e-spec.ts index 531a527a5..400f5d3c1 100644 --- a/test/public-api/notes.e2e-spec.ts +++ b/test/public-api/notes.e2e-spec.ts @@ -208,7 +208,7 @@ describe('Notes', () => { updateNotePermission.sharedToGroups = []; await notesService.updateNotePermissions(note, updateNotePermission); const updatedNote = await notesService.getNoteByIdOrAlias( - note.alias ?? '', + note.aliases.filter((alias) => alias.primary)[0].name, ); expect(updatedNote.userPermissions).toHaveLength(1); expect(updatedNote.userPermissions[0].canEdit).toEqual( @@ -274,7 +274,8 @@ describe('Notes', () => { .get('/notes/test5/metadata') .expect(200); expect(typeof metadata.body.id).toEqual('string'); - expect(metadata.body.alias).toEqual('test5'); + expect(metadata.body.aliases).toEqual(['test5']); + expect(metadata.body.primaryAlias).toEqual('test5'); expect(metadata.body.title).toEqual(''); expect(metadata.body.description).toEqual(''); expect(typeof metadata.body.createTime).toEqual('string'); From 7bb70649a01dc2264f6f42df8bf60c7822354ede Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Sun, 6 Jun 2021 17:56:12 +0200 Subject: [PATCH 11/12] fix: the seed command handles the new aliases Signed-off-by: Philip Molares --- src/seed.ts | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/seed.ts b/src/seed.ts index 3275640e1..49bf6c358 100644 --- a/src/seed.ts +++ b/src/seed.ts @@ -12,6 +12,7 @@ import { HistoryEntry } from './history/history-entry.entity'; import { Identity } from './identity/identity.entity'; import { ProviderType } from './identity/provider-type.enum'; import { MediaUpload } from './media/media-upload.entity'; +import { Alias } from './notes/alias.entity'; import { Note } from './notes/note.entity'; import { Tag } from './notes/tag.entity'; import { NoteGroupPermission } from './permissions/note-group-permission.entity'; @@ -43,6 +44,7 @@ createConnection({ Identity, Author, Session, + Alias, ], synchronize: true, logging: false, @@ -89,12 +91,14 @@ createConnection({ if (!foundUsers) { throw new Error('Could not find freshly seeded users. Aborting.'); } - const foundNotes = await connection.manager.find(Note); + const foundNotes = await connection.manager.find(Note, { + relations: ['aliases'], + }); if (!foundNotes) { throw new Error('Could not find freshly seeded notes. Aborting.'); } for (const note of foundNotes) { - if (!note.alias) { + if (!note.aliases[0]) { throw new Error( 'Could not find alias of freshly seeded notes. Aborting.', ); @@ -106,7 +110,7 @@ createConnection({ ); } for (const note of foundNotes) { - console.log(`Created Note '${note.alias ?? ''}'`); + console.log(`Created Note '${note.aliases[0].name ?? ''}'`); } for (const user of foundUsers) { for (const note of foundNotes) { @@ -114,7 +118,7 @@ createConnection({ await connection.manager.save(historyEntry); console.log( `Created HistoryEntry for user '${user.userName}' and note '${ - note.alias ?? '' + note.aliases[0].name ?? '' }'`, ); } From 49c1c8de341b53632b77892c3644031fe3e3cf38 Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Sun, 6 Jun 2021 18:07:53 +0200 Subject: [PATCH 12/12] docs: add primary aliases to the dev doc about notes Signed-off-by: Philip Molares --- docs/content/dev/notes.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/content/dev/notes.md b/docs/content/dev/notes.md index 141823dfb..655407705 100644 --- a/docs/content/dev/notes.md +++ b/docs/content/dev/notes.md @@ -18,7 +18,7 @@ Each note in HedgeDoc 2 contains the following information: The `publicId` is the default possibility of identifying a note. It will be a randomly generated 128-bit value encoded with [base32-encode](https://www.npmjs.com/package/base32-encode) using the crockford variant and converted to lowercase. This variant of base32 is used, because that results in ids that only use one case of alpha-numeric characters and other url safe characters. We convert the id to lowercase, because we want to minimize case confusion. -`aliases` are the other way of identifying a note. There can be any number of them, and the owner of the note is able to add or remove them. All aliases are just strings (especially to accommodate the old identifier from HedgeDoc 1 [see below](#conversion-of-hedgedoc-1-notes)), but new aliases added with HedgeDoc 2 will only allow characters matching this regex: `[a-z0-9\-_]`. This is done to once again prevent case confusion. +`aliases` are the other way of identifying a note. There can be any number of them, and the owner of the note is able to add or remove them. All aliases are just strings (especially to accommodate the old identifier from HedgeDoc 1 [see below](#conversion-of-hedgedoc-1-notes)), but new aliases added with HedgeDoc 2 will only allow characters matching this regex: `[a-z0-9\-_]`. This is done to once again prevent case confusion. One of the aliases can be set as the primary alias, which will be used as the identifier for the history entry. `groupPermissions` and `userPermissions` each hold a list of the appropriate permissions. Each permission holds a reference to a note and a user/group and specify what the user/group is allowed to do.