From d1c305865551e8f14644acc181daaa34569b36e7 Mon Sep 17 00:00:00 2001 From: David Mehren Date: Sun, 18 Sep 2022 18:24:27 +0200 Subject: [PATCH] fix(permissions): remove composite primary keys TypeORM promises to support composite primary keys, but that does not work in reality. This replaces the composite key used in the permission entities with a single generated primary key and a unique index on the relation columns. See https://github.com/typeorm/typeorm/issues/8513 Signed-off-by: David Mehren --- src/notes/notes.service.spec.ts | 20 +- src/notes/notes.service.ts | 20 +- .../note-group-permission.entity.ts | 33 ++-- .../note-user-permission.entity.ts | 31 ++- src/permissions/permissions.service.spec.ts | 176 +++++++++++------- src/permissions/permissions.service.ts | 56 +++--- test/public-api/notes.e2e-spec.ts | 6 +- 7 files changed, 199 insertions(+), 143 deletions(-) diff --git a/src/notes/notes.service.spec.ts b/src/notes/notes.service.spec.ts index 07189d81e..f17dcbb65 100644 --- a/src/notes/notes.service.spec.ts +++ b/src/notes/notes.service.spec.ts @@ -57,6 +57,8 @@ describe('NotesService', () => { const content = 'testContent'; jest .spyOn(noteRepo, 'save') + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore .mockImplementation(async (note: Note): Promise => note); const note = await service.createNote(content, null); const revisions = await note.revisions; @@ -94,19 +96,17 @@ describe('NotesService', () => { note.owner = Promise.resolve(user); note.userPermissions = Promise.resolve([ { - noteId: note.id, - note: note, - userId: user.id, - user: user, + id: 1, + note: Promise.resolve(note), + user: Promise.resolve(user), canEdit: true, }, ]); note.groupPermissions = Promise.resolve([ { - noteId: note.id, - note: note, - groupId: group.id, - group: group, + id: 1, + note: Promise.resolve(note), + group: Promise.resolve(group), canEdit: true, }, ]); @@ -291,6 +291,8 @@ describe('NotesService', () => { beforeEach(() => { jest .spyOn(noteRepo, 'save') + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore .mockImplementation(async (note: Note): Promise => note); }); it('without alias, without owner', async () => { @@ -368,6 +370,8 @@ describe('NotesService', () => { const content = 'testContent'; jest .spyOn(noteRepo, 'save') + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore .mockImplementation(async (note: Note): Promise => note); const newNote = await service.createNote(content, null); const revisions = await newNote.revisions; diff --git a/src/notes/notes.service.ts b/src/notes/notes.service.ts index c8b75366c..4d89e93b2 100644 --- a/src/notes/notes.service.ts +++ b/src/notes/notes.service.ts @@ -285,14 +285,18 @@ export class NotesService { const groupPermissions = await note.groupPermissions; return { owner: owner ? owner.username : null, - sharedToUsers: userPermissions.map((noteUserPermission) => ({ - username: noteUserPermission.user.username, - canEdit: noteUserPermission.canEdit, - })), - sharedToGroups: groupPermissions.map((noteGroupPermission) => ({ - groupName: noteGroupPermission.group.name, - canEdit: noteGroupPermission.canEdit, - })), + sharedToUsers: await Promise.all( + userPermissions.map(async (noteUserPermission) => ({ + username: (await noteUserPermission.user).username, + canEdit: noteUserPermission.canEdit, + })), + ), + sharedToGroups: await Promise.all( + groupPermissions.map(async (noteGroupPermission) => ({ + groupName: (await noteGroupPermission.group).name, + canEdit: noteGroupPermission.canEdit, + })), + ), }; } diff --git a/src/permissions/note-group-permission.entity.ts b/src/permissions/note-group-permission.entity.ts index 2fec661b6..08aef95a5 100644 --- a/src/permissions/note-group-permission.entity.ts +++ b/src/permissions/note-group-permission.entity.ts @@ -3,35 +3,34 @@ * * SPDX-License-Identifier: AGPL-3.0-only */ -import { Column, Entity, ManyToOne, PrimaryColumn } from 'typeorm'; +import { + Column, + Entity, + Index, + ManyToOne, + PrimaryGeneratedColumn, +} from 'typeorm'; import { Group } from '../groups/group.entity'; import { Note } from '../notes/note.entity'; @Entity() +@Index(['group', 'note'], { unique: true }) export class NoteGroupPermission { - /** - * The `group` and `note` properties cannot be converted to promises - * (to lazy-load them), as TypeORM gets confused about lazy composite - * primary keys. - * See https://github.com/typeorm/typeorm/issues/6908 - */ - - @PrimaryColumn() - groupId: number; + @PrimaryGeneratedColumn() + id: number; @ManyToOne((_) => Group, { onDelete: 'CASCADE', // This deletes the NoteGroupPermission, when the associated Group is deleted + orphanedRowAction: 'delete', // This ensures the whole row is deleted when the Permission stops being referenced }) - group: Group; - - @PrimaryColumn() - noteId: number; + group: Promise; @ManyToOne((_) => Note, (note) => note.groupPermissions, { onDelete: 'CASCADE', // This deletes the NoteGroupPermission, when the associated Note is deleted + orphanedRowAction: 'delete', // This ensures the whole row is deleted when the Permission stops being referenced }) - note: Note; + note: Promise; @Column() canEdit: boolean; @@ -45,8 +44,8 @@ export class NoteGroupPermission { canEdit: boolean, ): NoteGroupPermission { const groupPermission = new NoteGroupPermission(); - groupPermission.group = group; - groupPermission.note = note; + groupPermission.group = Promise.resolve(group); + groupPermission.note = Promise.resolve(note); groupPermission.canEdit = canEdit; return groupPermission; } diff --git a/src/permissions/note-user-permission.entity.ts b/src/permissions/note-user-permission.entity.ts index deb83ef49..1dd206205 100644 --- a/src/permissions/note-user-permission.entity.ts +++ b/src/permissions/note-user-permission.entity.ts @@ -3,37 +3,34 @@ * * SPDX-License-Identifier: AGPL-3.0-only */ -import { Column, Entity, ManyToOne, PrimaryColumn } from 'typeorm'; +import { + Column, + Entity, + Index, + ManyToOne, + PrimaryGeneratedColumn, +} from 'typeorm'; import { Note } from '../notes/note.entity'; import { User } from '../users/user.entity'; @Entity() +@Index(['user', 'note'], { unique: true }) export class NoteUserPermission { - /** - * The `user` and `note` properties cannot be converted to promises - * (to lazy-load them), as TypeORM gets confused about lazy composite - * primary keys. - * See https://github.com/typeorm/typeorm/issues/6908 - */ - - @PrimaryColumn() - userId: number; + @PrimaryGeneratedColumn() + id: number; @ManyToOne((_) => User, { onDelete: 'CASCADE', // This deletes the NoteUserPermission, when the associated Note is deleted orphanedRowAction: 'delete', // This ensures the whole row is deleted when the Permission stops being referenced }) - user: User; - - @PrimaryColumn() - noteId: number; + user: Promise; @ManyToOne((_) => Note, (note) => note.userPermissions, { onDelete: 'CASCADE', // This deletes the NoteUserPermission, when the associated Note is deleted orphanedRowAction: 'delete', // This ensures the whole row is deleted when the Permission stops being referenced }) - note: Note; + note: Promise; @Column() canEdit: boolean; @@ -47,8 +44,8 @@ export class NoteUserPermission { canEdit: boolean, ): NoteUserPermission { const userPermission = new NoteUserPermission(); - userPermission.user = user; - userPermission.note = note; + userPermission.user = Promise.resolve(user); + userPermission.note = Promise.resolve(note); userPermission.canEdit = canEdit; return userPermission; } diff --git a/src/permissions/permissions.service.spec.ts b/src/permissions/permissions.service.spec.ts index d510fb60b..13ac95932 100644 --- a/src/permissions/permissions.service.spec.ts +++ b/src/permissions/permissions.service.spec.ts @@ -168,14 +168,14 @@ describe('PermissionsService', () => { const note6 = createNote(user2); const note7 = createNote(user2); const noteUserPermission1 = {} as NoteUserPermission; - noteUserPermission1.user = user1; + noteUserPermission1.user = Promise.resolve(user1); const noteUserPermission2 = {} as NoteUserPermission; - noteUserPermission2.user = user2; + noteUserPermission2.user = Promise.resolve(user2); const noteUserPermission3 = {} as NoteUserPermission; - noteUserPermission3.user = user1; + noteUserPermission3.user = Promise.resolve(user1); noteUserPermission3.canEdit = true; const noteUserPermission4 = {} as NoteUserPermission; - noteUserPermission4.user = user2; + noteUserPermission4.user = Promise.resolve(user2); noteUserPermission4.canEdit = true; (await note1.userPermissions).push(noteUserPermission1); @@ -202,9 +202,9 @@ describe('PermissionsService', () => { const noteEverybodyRead = createNote(user1); const noteGroupPermissionRead = {} as NoteGroupPermission; - noteGroupPermissionRead.group = everybody; + noteGroupPermissionRead.group = Promise.resolve(everybody); noteGroupPermissionRead.canEdit = false; - noteGroupPermissionRead.note = noteEverybodyRead; + noteGroupPermissionRead.note = Promise.resolve(noteEverybodyRead); noteEverybodyRead.groupPermissions = Promise.resolve([ noteGroupPermissionRead, ]); @@ -212,9 +212,9 @@ describe('PermissionsService', () => { const noteEverybodyWrite = createNote(user1); const noteGroupPermissionWrite = {} as NoteGroupPermission; - noteGroupPermissionWrite.group = everybody; + noteGroupPermissionWrite.group = Promise.resolve(everybody); noteGroupPermissionWrite.canEdit = true; - noteGroupPermissionWrite.note = noteEverybodyWrite; + noteGroupPermissionWrite.note = Promise.resolve(noteEverybodyWrite); noteEverybodyWrite.groupPermissions = Promise.resolve([ noteGroupPermissionWrite, ]); @@ -568,7 +568,7 @@ describe('PermissionsService', () => { note.groupPermissions = Promise.resolve(permission.permissions); let permissionString = ''; for (const perm of permission.permissions) { - permissionString += ` ${perm.group.name}:${String(perm.canEdit)}`; + permissionString += ` ${perm.id}:${String(perm.canEdit)}`; } it(`mayWrite - test #${i}:${permissionString}`, async () => { service.guestPermission = guestPermission; @@ -642,6 +642,8 @@ describe('PermissionsService', () => { it('with empty GroupPermissions and with empty UserPermissions', async () => { jest .spyOn(noteRepo, 'save') + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore .mockImplementationOnce(async (entry: Note) => { return entry; }); @@ -655,6 +657,8 @@ describe('PermissionsService', () => { it('with empty GroupPermissions and with new UserPermissions', async () => { jest .spyOn(noteRepo, 'save') + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore .mockImplementationOnce(async (entry: Note) => { return entry; }); @@ -664,9 +668,9 @@ describe('PermissionsService', () => { sharedToGroups: [], }); expect(await savedNote.userPermissions).toHaveLength(1); - expect((await savedNote.userPermissions)[0].user.username).toEqual( - userPermissionUpdate.username, - ); + expect( + (await (await savedNote.userPermissions)[0].user).username, + ).toEqual(userPermissionUpdate.username); expect((await savedNote.userPermissions)[0].canEdit).toEqual( userPermissionUpdate.canEdit, ); @@ -676,15 +680,16 @@ describe('PermissionsService', () => { const noteWithPreexistingPermissions: Note = { ...note }; noteWithPreexistingPermissions.userPermissions = Promise.resolve([ { - noteId: 4711, - note: noteWithPreexistingPermissions, - userId: 4711, - user: user, + id: 1, + note: Promise.resolve(noteWithPreexistingPermissions), + user: Promise.resolve(user), canEdit: !userPermissionUpdate.canEdit, }, ]); jest .spyOn(noteRepo, 'save') + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore .mockImplementationOnce(async (entry: Note) => { return entry; }); @@ -694,9 +699,9 @@ describe('PermissionsService', () => { sharedToGroups: [], }); expect(await savedNote.userPermissions).toHaveLength(1); - expect((await savedNote.userPermissions)[0].user.username).toEqual( - userPermissionUpdate.username, - ); + expect( + (await (await savedNote.userPermissions)[0].user).username, + ).toEqual(userPermissionUpdate.username); expect((await savedNote.userPermissions)[0].canEdit).toEqual( userPermissionUpdate.canEdit, ); @@ -705,6 +710,8 @@ describe('PermissionsService', () => { it('with new GroupPermissions and with empty UserPermissions', async () => { jest .spyOn(noteRepo, 'save') + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore .mockImplementationOnce(async (entry: Note) => { return entry; }); @@ -714,9 +721,9 @@ describe('PermissionsService', () => { sharedToGroups: [groupPermissionUpdate], }); expect(await savedNote.userPermissions).toHaveLength(0); - expect((await savedNote.groupPermissions)[0].group.name).toEqual( - groupPermissionUpdate.groupName, - ); + expect( + (await (await savedNote.groupPermissions)[0].group).name, + ).toEqual(groupPermissionUpdate.groupName); expect((await savedNote.groupPermissions)[0].canEdit).toEqual( groupPermissionUpdate.canEdit, ); @@ -724,6 +731,8 @@ describe('PermissionsService', () => { it('with new GroupPermissions and with new UserPermissions', async () => { jest .spyOn(noteRepo, 'save') + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore .mockImplementationOnce(async (entry: Note) => { return entry; }); @@ -733,15 +742,15 @@ describe('PermissionsService', () => { sharedToUsers: [userPermissionUpdate], sharedToGroups: [groupPermissionUpdate], }); - expect((await savedNote.userPermissions)[0].user.username).toEqual( - userPermissionUpdate.username, - ); + expect( + (await (await savedNote.userPermissions)[0].user).username, + ).toEqual(userPermissionUpdate.username); expect((await savedNote.userPermissions)[0].canEdit).toEqual( userPermissionUpdate.canEdit, ); - expect((await savedNote.groupPermissions)[0].group.name).toEqual( - groupPermissionUpdate.groupName, - ); + expect( + (await (await savedNote.groupPermissions)[0].group).name, + ).toEqual(groupPermissionUpdate.groupName); expect((await savedNote.groupPermissions)[0].canEdit).toEqual( groupPermissionUpdate.canEdit, ); @@ -750,15 +759,16 @@ describe('PermissionsService', () => { const noteWithUserPermission: Note = { ...note }; noteWithUserPermission.userPermissions = Promise.resolve([ { - noteId: 4711, - note: noteWithUserPermission, - userId: 4711, - user: user, + id: 1, + note: Promise.resolve(noteWithUserPermission), + user: Promise.resolve(user), canEdit: !userPermissionUpdate.canEdit, }, ]); jest .spyOn(noteRepo, 'save') + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore .mockImplementationOnce(async (entry: Note) => { return entry; }); @@ -771,15 +781,15 @@ describe('PermissionsService', () => { sharedToGroups: [groupPermissionUpdate], }, ); - expect((await savedNote.userPermissions)[0].user.username).toEqual( - userPermissionUpdate.username, - ); + expect( + (await (await savedNote.userPermissions)[0].user).username, + ).toEqual(userPermissionUpdate.username); expect((await savedNote.userPermissions)[0].canEdit).toEqual( userPermissionUpdate.canEdit, ); - expect((await savedNote.groupPermissions)[0].group.name).toEqual( - groupPermissionUpdate.groupName, - ); + expect( + (await (await savedNote.groupPermissions)[0].group).name, + ).toEqual(groupPermissionUpdate.groupName); expect((await savedNote.groupPermissions)[0].canEdit).toEqual( groupPermissionUpdate.canEdit, ); @@ -788,16 +798,17 @@ describe('PermissionsService', () => { const noteWithPreexistingPermissions: Note = { ...note }; noteWithPreexistingPermissions.groupPermissions = Promise.resolve([ { - noteId: 4711, - note: noteWithPreexistingPermissions, - groupId: 0, - group: group, + id: 1, + note: Promise.resolve(noteWithPreexistingPermissions), + group: Promise.resolve(group), canEdit: !groupPermissionUpdate.canEdit, }, ]); jest.spyOn(groupRepo, 'findOne').mockResolvedValueOnce(group); jest .spyOn(noteRepo, 'save') + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore .mockImplementationOnce(async (entry: Note) => { return entry; }); @@ -809,9 +820,9 @@ describe('PermissionsService', () => { }, ); expect(await savedNote.userPermissions).toHaveLength(0); - expect((await savedNote.groupPermissions)[0].group.name).toEqual( - groupPermissionUpdate.groupName, - ); + expect( + (await (await savedNote.groupPermissions)[0].group).name, + ).toEqual(groupPermissionUpdate.groupName); expect((await savedNote.groupPermissions)[0].canEdit).toEqual( groupPermissionUpdate.canEdit, ); @@ -820,15 +831,16 @@ describe('PermissionsService', () => { const noteWithPreexistingPermissions: Note = { ...note }; noteWithPreexistingPermissions.groupPermissions = Promise.resolve([ { - noteId: 4711, - note: noteWithPreexistingPermissions, - groupId: 0, - group: group, + id: 1, + note: Promise.resolve(noteWithPreexistingPermissions), + group: Promise.resolve(group), canEdit: !groupPermissionUpdate.canEdit, }, ]); jest .spyOn(noteRepo, 'save') + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore .mockImplementationOnce(async (entry: Note) => { return entry; }); @@ -841,15 +853,15 @@ describe('PermissionsService', () => { sharedToGroups: [groupPermissionUpdate], }, ); - expect((await savedNote.userPermissions)[0].user.username).toEqual( - userPermissionUpdate.username, - ); + expect( + (await (await savedNote.userPermissions)[0].user).username, + ).toEqual(userPermissionUpdate.username); expect((await savedNote.userPermissions)[0].canEdit).toEqual( userPermissionUpdate.canEdit, ); - expect((await savedNote.groupPermissions)[0].group.name).toEqual( - groupPermissionUpdate.groupName, - ); + expect( + (await (await savedNote.groupPermissions)[0].group).name, + ).toEqual(groupPermissionUpdate.groupName); expect((await savedNote.groupPermissions)[0].canEdit).toEqual( groupPermissionUpdate.canEdit, ); @@ -858,24 +870,24 @@ describe('PermissionsService', () => { const noteWithPreexistingPermissions: Note = { ...note }; noteWithPreexistingPermissions.groupPermissions = Promise.resolve([ { - noteId: 4711, - note: noteWithPreexistingPermissions, - groupId: 0, - group: group, + id: 1, + note: Promise.resolve(noteWithPreexistingPermissions), + group: Promise.resolve(group), canEdit: !groupPermissionUpdate.canEdit, }, ]); noteWithPreexistingPermissions.userPermissions = Promise.resolve([ { - noteId: 4711, - note: noteWithPreexistingPermissions, - userId: 4711, - user: user, + id: 1, + note: Promise.resolve(noteWithPreexistingPermissions), + user: Promise.resolve(user), canEdit: !userPermissionUpdate.canEdit, }, ]); jest .spyOn(noteRepo, 'save') + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore .mockImplementationOnce(async (entry: Note) => { return entry; }); @@ -888,15 +900,15 @@ describe('PermissionsService', () => { sharedToGroups: [groupPermissionUpdate], }, ); - expect((await savedNote.userPermissions)[0].user.username).toEqual( - userPermissionUpdate.username, - ); + expect( + (await (await savedNote.userPermissions)[0].user).username, + ).toEqual(userPermissionUpdate.username); expect((await savedNote.userPermissions)[0].canEdit).toEqual( userPermissionUpdate.canEdit, ); - expect((await savedNote.groupPermissions)[0].group.name).toEqual( - groupPermissionUpdate.groupName, - ); + expect( + (await (await savedNote.groupPermissions)[0].group).name, + ).toEqual(groupPermissionUpdate.groupName); expect((await savedNote.groupPermissions)[0].canEdit).toEqual( groupPermissionUpdate.canEdit, ); @@ -937,6 +949,8 @@ describe('PermissionsService', () => { it('with user not added before and editable', async () => { jest .spyOn(noteRepo, 'save') + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore .mockImplementationOnce(async (entry: Note) => { return entry; }); @@ -951,6 +965,8 @@ describe('PermissionsService', () => { it('with user not added before and not editable', async () => { jest .spyOn(noteRepo, 'save') + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore .mockImplementationOnce(async (entry: Note) => { return entry; }); @@ -965,6 +981,8 @@ describe('PermissionsService', () => { it('with user added before and editable', async () => { jest .spyOn(noteRepo, 'save') + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore .mockImplementationOnce(async (entry: Note) => { return entry; }); @@ -983,6 +1001,8 @@ describe('PermissionsService', () => { it('with user added before and not editable', async () => { jest .spyOn(noteRepo, 'save') + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore .mockImplementationOnce(async (entry: Note) => { return entry; }); @@ -1005,6 +1025,8 @@ describe('PermissionsService', () => { it('with user added before and editable', async () => { jest .spyOn(noteRepo, 'save') + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore .mockImplementationOnce(async (entry: Note) => { return entry; }); @@ -1020,6 +1042,8 @@ describe('PermissionsService', () => { it('with user not added before and not editable', async () => { jest .spyOn(noteRepo, 'save') + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore .mockImplementationOnce(async (entry: Note) => { return entry; }); @@ -1039,6 +1063,8 @@ describe('PermissionsService', () => { it('with group not added before and editable', async () => { jest .spyOn(noteRepo, 'save') + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore .mockImplementationOnce(async (entry: Note) => { return entry; }); @@ -1057,6 +1083,8 @@ describe('PermissionsService', () => { it('with group not added before and not editable', async () => { jest .spyOn(noteRepo, 'save') + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore .mockImplementationOnce(async (entry: Note) => { return entry; }); @@ -1075,6 +1103,8 @@ describe('PermissionsService', () => { it('with group added before and editable', async () => { jest .spyOn(noteRepo, 'save') + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore .mockImplementationOnce(async (entry: Note) => { return entry; }); @@ -1097,6 +1127,8 @@ describe('PermissionsService', () => { it('with group added before and not editable', async () => { jest .spyOn(noteRepo, 'save') + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore .mockImplementationOnce(async (entry: Note) => { return entry; }); @@ -1123,6 +1155,8 @@ describe('PermissionsService', () => { it('with user added before and editable', async () => { jest .spyOn(noteRepo, 'save') + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore .mockImplementationOnce(async (entry: Note) => { return entry; }); @@ -1138,6 +1172,8 @@ describe('PermissionsService', () => { it('with user not added before and not editable', async () => { jest .spyOn(noteRepo, 'save') + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore .mockImplementationOnce(async (entry: Note) => { return entry; }); @@ -1158,6 +1194,8 @@ describe('PermissionsService', () => { const user = User.create('test', 'Testy') as User; jest .spyOn(noteRepo, 'save') + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore .mockImplementationOnce(async (entry: Note) => { return entry; }); diff --git a/src/permissions/permissions.service.ts b/src/permissions/permissions.service.ts index cd443eab7..1d21443f0 100644 --- a/src/permissions/permissions.service.ts +++ b/src/permissions/permissions.service.ts @@ -93,7 +93,7 @@ export class PermissionsService { } for (const userPermission of await note.userPermissions) { if ( - userPermission.user.id === user.id && + (await userPermission.user).id === user.id && (userPermission.canEdit || !wantEdit) ) { return true; @@ -121,12 +121,12 @@ export class PermissionsService { for (const groupPermission of await note.groupPermissions) { if (groupPermission.canEdit || !wantEdit) { // Handle special groups - if (groupPermission.group.special) { - if (groupPermission.group.name == SpecialGroup.LOGGED_IN) { + if ((await groupPermission.group).special) { + if ((await groupPermission.group).name == SpecialGroup.LOGGED_IN) { return true; } if ( - groupPermission.group.name == SpecialGroup.EVERYONE && + (await groupPermission.group).name == SpecialGroup.EVERYONE && (groupPermission.canEdit || !wantEdit) && guestsAllowed ) { @@ -135,7 +135,9 @@ export class PermissionsService { } else { // Handle normal groups if (user) { - for (const member of await groupPermission.group.members) { + for (const member of await ( + await groupPermission.group + ).members) { if (member.id === user.id) return true; } } @@ -189,7 +191,7 @@ export class PermissionsService { note, newUserPermission.canEdit, ); - createdPermission.note = note; + createdPermission.note = Promise.resolve(note); (await note.userPermissions).push(createdPermission); } @@ -203,7 +205,7 @@ export class PermissionsService { note, newGroupPermission.canEdit, ); - createdPermission.note = note; + createdPermission.note = Promise.resolve(note); (await note.groupPermissions).push(createdPermission); } @@ -225,9 +227,9 @@ export class PermissionsService { ): Promise { const permissions = await note.userPermissions; let permissionIndex = 0; - const permission = permissions.find((value, index) => { + const permission = permissions.find(async (value, index) => { permissionIndex = index; - return value.user.id == permissionUser.id; + return (await value.user).id == permissionUser.id; }); if (permission != undefined) { permission.canEdit = canEdit; @@ -252,10 +254,13 @@ export class PermissionsService { */ async removeUserPermission(note: Note, permissionUser: User): Promise { const permissions = await note.userPermissions; - const permissionsFiltered = permissions.filter( - (value) => value.user.id != permissionUser.id, - ); - note.userPermissions = Promise.resolve(permissionsFiltered); + const newPermissions = []; + for (const permission of permissions) { + if ((await permission.user).id != permissionUser.id) { + newPermissions.push(permission); + } + } + note.userPermissions = Promise.resolve(newPermissions); return await this.noteRepository.save(note); } @@ -272,16 +277,24 @@ export class PermissionsService { permissionGroup: Group, canEdit: boolean, ): Promise { + this.logger.debug( + `Setting group permission for group ${permissionGroup.name} on note ${note.id}`, + 'setGroupPermission', + ); const permissions = await note.groupPermissions; let permissionIndex = 0; - const permission = permissions.find((value, index) => { + const permission = permissions.find(async (value, index) => { permissionIndex = index; - return value.group.id == permissionGroup.id; + return (await value.group).id == permissionGroup.id; }); if (permission != undefined) { permission.canEdit = canEdit; permissions[permissionIndex] = permission; } else { + this.logger.debug( + `Permission does not exist yet, creating new one.`, + 'setGroupPermission', + ); const noteGroupPermission = NoteGroupPermission.create( permissionGroup, note, @@ -304,12 +317,13 @@ export class PermissionsService { permissionGroup: Group, ): Promise { const permissions = await note.groupPermissions; - const permissionsFiltered = permissions.filter( - (value: NoteGroupPermission) => { - return value.group.id != permissionGroup.id; - }, - ); - note.groupPermissions = Promise.resolve(permissionsFiltered); + const newPermissions = []; + for (const permission of permissions) { + if ((await permission.group).id != permissionGroup.id) { + newPermissions.push(permission); + } + } + note.groupPermissions = Promise.resolve(newPermissions); return await this.noteRepository.save(note); } diff --git a/test/public-api/notes.e2e-spec.ts b/test/public-api/notes.e2e-spec.ts index 43e264ebe..06239d63d 100644 --- a/test/public-api/notes.e2e-spec.ts +++ b/test/public-api/notes.e2e-spec.ts @@ -211,9 +211,9 @@ describe('Notes', () => { expect((await updatedNote.userPermissions)[0].canEdit).toEqual( updateNotePermission.sharedToUsers[0].canEdit, ); - expect((await updatedNote.userPermissions)[0].user.username).toEqual( - user.username, - ); + expect( + (await (await updatedNote.userPermissions)[0].user).username, + ).toEqual(user.username); expect(await updatedNote.groupPermissions).toHaveLength(0); await request(testSetup.app.getHttpServer()) .delete('/api/v2/notes/test3')