github/hedgedoc
1
0
Fork 0
mirror of https://github.com/hedgedoc/hedgedoc.git synced 2024-11-29 07:34:24 -05:00
Code Issues Projects Releases Packages Wiki Activity

fix(permissions): remove composite primary keys

Browse source 
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 <git@herrmehren.de>
This commit is contained in:
David Mehren 2022-09-18 18:24:27 +02:00
parent 2689f9f3dc
commit d1c3058655
7 changed files with 199 additions and 143 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

20
src/notes/notes.service.spec.ts
View file

@ -57,6 +57,8 @@ describe('NotesService', () => {
const content = 'testContent'; const content = 'testContent';
jest jest
.spyOn(noteRepo, 'save') .spyOn(noteRepo, 'save')
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
.mockImplementation(async (note: Note): Promise<Note> => note); .mockImplementation(async (note: Note): Promise<Note> => note);
const note = await service.createNote(content, null); const note = await service.createNote(content, null);
const revisions = await note.revisions; const revisions = await note.revisions;
@ -94,19 +96,17 @@ describe('NotesService', () => {
note.owner = Promise.resolve(user); note.owner = Promise.resolve(user);
note.userPermissions = Promise.resolve([ note.userPermissions = Promise.resolve([
{ {
noteId: note.id, id: 1,
note: note, note: Promise.resolve(note),
userId: user.id, user: Promise.resolve(user),
user: user,
canEdit: true, canEdit: true,
}, },
]); ]);
note.groupPermissions = Promise.resolve([ note.groupPermissions = Promise.resolve([
{ {
noteId: note.id, id: 1,
note: note, note: Promise.resolve(note),
groupId: group.id, group: Promise.resolve(group),
group: group,
canEdit: true, canEdit: true,
}, },
]); ]);
@ -291,6 +291,8 @@ describe('NotesService', () => {
beforeEach(() => { beforeEach(() => {
jest jest
.spyOn(noteRepo, 'save') .spyOn(noteRepo, 'save')
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
.mockImplementation(async (note: Note): Promise<Note> => note); .mockImplementation(async (note: Note): Promise<Note> => note);
}); });
it('without alias, without owner', async () => { it('without alias, without owner', async () => {
@ -368,6 +370,8 @@ describe('NotesService', () => {
const content = 'testContent'; const content = 'testContent';
jest jest
.spyOn(noteRepo, 'save') .spyOn(noteRepo, 'save')
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
.mockImplementation(async (note: Note): Promise<Note> => note); .mockImplementation(async (note: Note): Promise<Note> => note);
const newNote = await service.createNote(content, null); const newNote = await service.createNote(content, null);
const revisions = await newNote.revisions; const revisions = await newNote.revisions;

20
src/notes/notes.service.ts
View file

@ -285,14 +285,18 @@ export class NotesService {
const groupPermissions = await note.groupPermissions; const groupPermissions = await note.groupPermissions;
return { return {
owner: owner ? owner.username : null, owner: owner ? owner.username : null,
sharedToUsers: userPermissions.map((noteUserPermission) => ({ sharedToUsers: await Promise.all(
username: noteUserPermission.user.username, userPermissions.map(async (noteUserPermission) => ({
canEdit: noteUserPermission.canEdit, username: (await noteUserPermission.user).username,
})), canEdit: noteUserPermission.canEdit,
sharedToGroups: groupPermissions.map((noteGroupPermission) => ({ })),
groupName: noteGroupPermission.group.name, ),
canEdit: noteGroupPermission.canEdit, sharedToGroups: await Promise.all(
})), groupPermissions.map(async (noteGroupPermission) => ({
groupName: (await noteGroupPermission.group).name,
canEdit: noteGroupPermission.canEdit,
})),
),
}; };
} }

33
src/permissions/note-group-permission.entity.ts
View file

@ -3,35 +3,34 @@
* *
* SPDX-License-Identifier: AGPL-3.0-only * 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 { Group } from '../groups/group.entity';
import { Note } from '../notes/note.entity'; import { Note } from '../notes/note.entity';
@Entity() @Entity()
@Index(['group', 'note'], { unique: true })
export class NoteGroupPermission { export class NoteGroupPermission {
/** @PrimaryGeneratedColumn()
* The `group` and `note` properties cannot be converted to promises id: number;
* (to lazy-load them), as TypeORM gets confused about lazy composite
* primary keys.
* See https://github.com/typeorm/typeorm/issues/6908
*/
@PrimaryColumn()
groupId: number;
@ManyToOne((_) => Group, { @ManyToOne((_) => Group, {
onDelete: 'CASCADE', // This deletes the NoteGroupPermission, when the associated Group is deleted 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; group: Promise<Group>;
@PrimaryColumn()
noteId: number;
@ManyToOne((_) => Note, (note) => note.groupPermissions, { @ManyToOne((_) => Note, (note) => note.groupPermissions, {
onDelete: 'CASCADE', // This deletes the NoteGroupPermission, when the associated Note is deleted 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<Note>;
@Column() @Column()
canEdit: boolean; canEdit: boolean;
@ -45,8 +44,8 @@ export class NoteGroupPermission {
canEdit: boolean, canEdit: boolean,
): NoteGroupPermission { ): NoteGroupPermission {
const groupPermission = new NoteGroupPermission(); const groupPermission = new NoteGroupPermission();
groupPermission.group = group; groupPermission.group = Promise.resolve(group);
groupPermission.note = note; groupPermission.note = Promise.resolve(note);
groupPermission.canEdit = canEdit; groupPermission.canEdit = canEdit;
return groupPermission; return groupPermission;
} }

31
src/permissions/note-user-permission.entity.ts
View file

@ -3,37 +3,34 @@
* *
* SPDX-License-Identifier: AGPL-3.0-only * 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 { Note } from '../notes/note.entity';
import { User } from '../users/user.entity'; import { User } from '../users/user.entity';
@Entity() @Entity()
@Index(['user', 'note'], { unique: true })
export class NoteUserPermission { export class NoteUserPermission {
/** @PrimaryGeneratedColumn()
* The `user` and `note` properties cannot be converted to promises id: number;
* (to lazy-load them), as TypeORM gets confused about lazy composite
* primary keys.
* See https://github.com/typeorm/typeorm/issues/6908
*/
@PrimaryColumn()
userId: number;
@ManyToOne((_) => User, { @ManyToOne((_) => User, {
onDelete: 'CASCADE', // This deletes the NoteUserPermission, when the associated Note is deleted 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 orphanedRowAction: 'delete', // This ensures the whole row is deleted when the Permission stops being referenced
}) })
user: User; user: Promise<User>;
@PrimaryColumn()
noteId: number;
@ManyToOne((_) => Note, (note) => note.userPermissions, { @ManyToOne((_) => Note, (note) => note.userPermissions, {
onDelete: 'CASCADE', // This deletes the NoteUserPermission, when the associated Note is deleted 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 orphanedRowAction: 'delete', // This ensures the whole row is deleted when the Permission stops being referenced
}) })
note: Note; note: Promise<Note>;
@Column() @Column()
canEdit: boolean; canEdit: boolean;
@ -47,8 +44,8 @@ export class NoteUserPermission {
canEdit: boolean, canEdit: boolean,
): NoteUserPermission { ): NoteUserPermission {
const userPermission = new NoteUserPermission(); const userPermission = new NoteUserPermission();
userPermission.user = user; userPermission.user = Promise.resolve(user);
userPermission.note = note; userPermission.note = Promise.resolve(note);
userPermission.canEdit = canEdit; userPermission.canEdit = canEdit;
return userPermission; return userPermission;
} }

176
src/permissions/permissions.service.spec.ts
View file

@ -168,14 +168,14 @@ describe('PermissionsService', () => {
const note6 = createNote(user2); const note6 = createNote(user2);
const note7 = createNote(user2); const note7 = createNote(user2);
const noteUserPermission1 = {} as NoteUserPermission; const noteUserPermission1 = {} as NoteUserPermission;
noteUserPermission1.user = user1; noteUserPermission1.user = Promise.resolve(user1);
const noteUserPermission2 = {} as NoteUserPermission; const noteUserPermission2 = {} as NoteUserPermission;
noteUserPermission2.user = user2; noteUserPermission2.user = Promise.resolve(user2);
const noteUserPermission3 = {} as NoteUserPermission; const noteUserPermission3 = {} as NoteUserPermission;
noteUserPermission3.user = user1; noteUserPermission3.user = Promise.resolve(user1);
noteUserPermission3.canEdit = true; noteUserPermission3.canEdit = true;
const noteUserPermission4 = {} as NoteUserPermission; const noteUserPermission4 = {} as NoteUserPermission;
noteUserPermission4.user = user2; noteUserPermission4.user = Promise.resolve(user2);
noteUserPermission4.canEdit = true; noteUserPermission4.canEdit = true;
(await note1.userPermissions).push(noteUserPermission1); (await note1.userPermissions).push(noteUserPermission1);
@ -202,9 +202,9 @@ describe('PermissionsService', () => {
const noteEverybodyRead = createNote(user1); const noteEverybodyRead = createNote(user1);
const noteGroupPermissionRead = {} as NoteGroupPermission; const noteGroupPermissionRead = {} as NoteGroupPermission;
noteGroupPermissionRead.group = everybody; noteGroupPermissionRead.group = Promise.resolve(everybody);
noteGroupPermissionRead.canEdit = false; noteGroupPermissionRead.canEdit = false;
noteGroupPermissionRead.note = noteEverybodyRead; noteGroupPermissionRead.note = Promise.resolve(noteEverybodyRead);
noteEverybodyRead.groupPermissions = Promise.resolve([ noteEverybodyRead.groupPermissions = Promise.resolve([
noteGroupPermissionRead, noteGroupPermissionRead,
]); ]);
@ -212,9 +212,9 @@ describe('PermissionsService', () => {
const noteEverybodyWrite = createNote(user1); const noteEverybodyWrite = createNote(user1);
const noteGroupPermissionWrite = {} as NoteGroupPermission; const noteGroupPermissionWrite = {} as NoteGroupPermission;
noteGroupPermissionWrite.group = everybody; noteGroupPermissionWrite.group = Promise.resolve(everybody);
noteGroupPermissionWrite.canEdit = true; noteGroupPermissionWrite.canEdit = true;
noteGroupPermissionWrite.note = noteEverybodyWrite; noteGroupPermissionWrite.note = Promise.resolve(noteEverybodyWrite);
noteEverybodyWrite.groupPermissions = Promise.resolve([ noteEverybodyWrite.groupPermissions = Promise.resolve([
noteGroupPermissionWrite, noteGroupPermissionWrite,
]); ]);
@ -568,7 +568,7 @@ describe('PermissionsService', () => {
note.groupPermissions = Promise.resolve(permission.permissions); note.groupPermissions = Promise.resolve(permission.permissions);
let permissionString = ''; let permissionString = '';
for (const perm of permission.permissions) { 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 () => { it(`mayWrite - test #${i}:${permissionString}`, async () => {
service.guestPermission = guestPermission; service.guestPermission = guestPermission;
@ -642,6 +642,8 @@ describe('PermissionsService', () => {
it('with empty GroupPermissions and with empty UserPermissions', async () => { it('with empty GroupPermissions and with empty UserPermissions', async () => {
jest jest
.spyOn(noteRepo, 'save') .spyOn(noteRepo, 'save')
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
.mockImplementationOnce(async (entry: Note) => { .mockImplementationOnce(async (entry: Note) => {
return entry; return entry;
}); });
@ -655,6 +657,8 @@ describe('PermissionsService', () => {
it('with empty GroupPermissions and with new UserPermissions', async () => { it('with empty GroupPermissions and with new UserPermissions', async () => {
jest jest
.spyOn(noteRepo, 'save') .spyOn(noteRepo, 'save')
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
.mockImplementationOnce(async (entry: Note) => { .mockImplementationOnce(async (entry: Note) => {
return entry; return entry;
}); });
@ -664,9 +668,9 @@ describe('PermissionsService', () => {
sharedToGroups: [], sharedToGroups: [],
}); });
expect(await savedNote.userPermissions).toHaveLength(1); expect(await savedNote.userPermissions).toHaveLength(1);
expect((await savedNote.userPermissions)[0].user.username).toEqual( expect(
userPermissionUpdate.username, (await (await savedNote.userPermissions)[0].user).username,
); ).toEqual(userPermissionUpdate.username);
expect((await savedNote.userPermissions)[0].canEdit).toEqual( expect((await savedNote.userPermissions)[0].canEdit).toEqual(
userPermissionUpdate.canEdit, userPermissionUpdate.canEdit,
); );
@ -676,15 +680,16 @@ describe('PermissionsService', () => {
const noteWithPreexistingPermissions: Note = { ...note }; const noteWithPreexistingPermissions: Note = { ...note };
noteWithPreexistingPermissions.userPermissions = Promise.resolve([ noteWithPreexistingPermissions.userPermissions = Promise.resolve([
{ {
noteId: 4711, id: 1,
note: noteWithPreexistingPermissions, note: Promise.resolve(noteWithPreexistingPermissions),
userId: 4711, user: Promise.resolve(user),
user: user,
canEdit: !userPermissionUpdate.canEdit, canEdit: !userPermissionUpdate.canEdit,
}, },
]); ]);
jest jest
.spyOn(noteRepo, 'save') .spyOn(noteRepo, 'save')
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
.mockImplementationOnce(async (entry: Note) => { .mockImplementationOnce(async (entry: Note) => {
return entry; return entry;
}); });
@ -694,9 +699,9 @@ describe('PermissionsService', () => {
sharedToGroups: [], sharedToGroups: [],
}); });
expect(await savedNote.userPermissions).toHaveLength(1); expect(await savedNote.userPermissions).toHaveLength(1);
expect((await savedNote.userPermissions)[0].user.username).toEqual( expect(
userPermissionUpdate.username, (await (await savedNote.userPermissions)[0].user).username,
); ).toEqual(userPermissionUpdate.username);
expect((await savedNote.userPermissions)[0].canEdit).toEqual( expect((await savedNote.userPermissions)[0].canEdit).toEqual(
userPermissionUpdate.canEdit, userPermissionUpdate.canEdit,
); );
@ -705,6 +710,8 @@ describe('PermissionsService', () => {
it('with new GroupPermissions and with empty UserPermissions', async () => { it('with new GroupPermissions and with empty UserPermissions', async () => {
jest jest
.spyOn(noteRepo, 'save') .spyOn(noteRepo, 'save')
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
.mockImplementationOnce(async (entry: Note) => { .mockImplementationOnce(async (entry: Note) => {
return entry; return entry;
}); });
@ -714,9 +721,9 @@ describe('PermissionsService', () => {
sharedToGroups: [groupPermissionUpdate], sharedToGroups: [groupPermissionUpdate],
}); });
expect(await savedNote.userPermissions).toHaveLength(0); expect(await savedNote.userPermissions).toHaveLength(0);
expect((await savedNote.groupPermissions)[0].group.name).toEqual( expect(
groupPermissionUpdate.groupName, (await (await savedNote.groupPermissions)[0].group).name,
); ).toEqual(groupPermissionUpdate.groupName);
expect((await savedNote.groupPermissions)[0].canEdit).toEqual( expect((await savedNote.groupPermissions)[0].canEdit).toEqual(
groupPermissionUpdate.canEdit, groupPermissionUpdate.canEdit,
); );
@ -724,6 +731,8 @@ describe('PermissionsService', () => {
it('with new GroupPermissions and with new UserPermissions', async () => { it('with new GroupPermissions and with new UserPermissions', async () => {
jest jest
.spyOn(noteRepo, 'save') .spyOn(noteRepo, 'save')
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
.mockImplementationOnce(async (entry: Note) => { .mockImplementationOnce(async (entry: Note) => {
return entry; return entry;
}); });
@ -733,15 +742,15 @@ describe('PermissionsService', () => {
sharedToUsers: [userPermissionUpdate], sharedToUsers: [userPermissionUpdate],
sharedToGroups: [groupPermissionUpdate], sharedToGroups: [groupPermissionUpdate],
}); });
expect((await savedNote.userPermissions)[0].user.username).toEqual( expect(
userPermissionUpdate.username, (await (await savedNote.userPermissions)[0].user).username,
); ).toEqual(userPermissionUpdate.username);
expect((await savedNote.userPermissions)[0].canEdit).toEqual( expect((await savedNote.userPermissions)[0].canEdit).toEqual(
userPermissionUpdate.canEdit, userPermissionUpdate.canEdit,
); );
expect((await savedNote.groupPermissions)[0].group.name).toEqual( expect(
groupPermissionUpdate.groupName, (await (await savedNote.groupPermissions)[0].group).name,
); ).toEqual(groupPermissionUpdate.groupName);
expect((await savedNote.groupPermissions)[0].canEdit).toEqual( expect((await savedNote.groupPermissions)[0].canEdit).toEqual(
groupPermissionUpdate.canEdit, groupPermissionUpdate.canEdit,
); );
@ -750,15 +759,16 @@ describe('PermissionsService', () => {
const noteWithUserPermission: Note = { ...note }; const noteWithUserPermission: Note = { ...note };
noteWithUserPermission.userPermissions = Promise.resolve([ noteWithUserPermission.userPermissions = Promise.resolve([
{ {
noteId: 4711, id: 1,
note: noteWithUserPermission, note: Promise.resolve(noteWithUserPermission),
userId: 4711, user: Promise.resolve(user),
user: user,
canEdit: !userPermissionUpdate.canEdit, canEdit: !userPermissionUpdate.canEdit,
}, },
]); ]);
jest jest
.spyOn(noteRepo, 'save') .spyOn(noteRepo, 'save')
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
.mockImplementationOnce(async (entry: Note) => { .mockImplementationOnce(async (entry: Note) => {
return entry; return entry;
}); });
@ -771,15 +781,15 @@ describe('PermissionsService', () => {
sharedToGroups: [groupPermissionUpdate], sharedToGroups: [groupPermissionUpdate],
}, },
); );
expect((await savedNote.userPermissions)[0].user.username).toEqual( expect(
userPermissionUpdate.username, (await (await savedNote.userPermissions)[0].user).username,
); ).toEqual(userPermissionUpdate.username);
expect((await savedNote.userPermissions)[0].canEdit).toEqual( expect((await savedNote.userPermissions)[0].canEdit).toEqual(
userPermissionUpdate.canEdit, userPermissionUpdate.canEdit,
); );
expect((await savedNote.groupPermissions)[0].group.name).toEqual( expect(
groupPermissionUpdate.groupName, (await (await savedNote.groupPermissions)[0].group).name,
); ).toEqual(groupPermissionUpdate.groupName);
expect((await savedNote.groupPermissions)[0].canEdit).toEqual( expect((await savedNote.groupPermissions)[0].canEdit).toEqual(
groupPermissionUpdate.canEdit, groupPermissionUpdate.canEdit,
); );
@ -788,16 +798,17 @@ describe('PermissionsService', () => {
const noteWithPreexistingPermissions: Note = { ...note }; const noteWithPreexistingPermissions: Note = { ...note };
noteWithPreexistingPermissions.groupPermissions = Promise.resolve([ noteWithPreexistingPermissions.groupPermissions = Promise.resolve([
{ {
noteId: 4711, id: 1,
note: noteWithPreexistingPermissions, note: Promise.resolve(noteWithPreexistingPermissions),
groupId: 0, group: Promise.resolve(group),
group: group,
canEdit: !groupPermissionUpdate.canEdit, canEdit: !groupPermissionUpdate.canEdit,
}, },
]); ]);
jest.spyOn(groupRepo, 'findOne').mockResolvedValueOnce(group); jest.spyOn(groupRepo, 'findOne').mockResolvedValueOnce(group);
jest jest
.spyOn(noteRepo, 'save') .spyOn(noteRepo, 'save')
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
.mockImplementationOnce(async (entry: Note) => { .mockImplementationOnce(async (entry: Note) => {
return entry; return entry;
}); });
@ -809,9 +820,9 @@ describe('PermissionsService', () => {
}, },
); );
expect(await savedNote.userPermissions).toHaveLength(0); expect(await savedNote.userPermissions).toHaveLength(0);
expect((await savedNote.groupPermissions)[0].group.name).toEqual( expect(
groupPermissionUpdate.groupName, (await (await savedNote.groupPermissions)[0].group).name,
); ).toEqual(groupPermissionUpdate.groupName);
expect((await savedNote.groupPermissions)[0].canEdit).toEqual( expect((await savedNote.groupPermissions)[0].canEdit).toEqual(
groupPermissionUpdate.canEdit, groupPermissionUpdate.canEdit,
); );
@ -820,15 +831,16 @@ describe('PermissionsService', () => {
const noteWithPreexistingPermissions: Note = { ...note }; const noteWithPreexistingPermissions: Note = { ...note };
noteWithPreexistingPermissions.groupPermissions = Promise.resolve([ noteWithPreexistingPermissions.groupPermissions = Promise.resolve([
{ {
noteId: 4711, id: 1,
note: noteWithPreexistingPermissions, note: Promise.resolve(noteWithPreexistingPermissions),
groupId: 0, group: Promise.resolve(group),
group: group,
canEdit: !groupPermissionUpdate.canEdit, canEdit: !groupPermissionUpdate.canEdit,
}, },
]); ]);
jest jest
.spyOn(noteRepo, 'save') .spyOn(noteRepo, 'save')
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
.mockImplementationOnce(async (entry: Note) => { .mockImplementationOnce(async (entry: Note) => {
return entry; return entry;
}); });
@ -841,15 +853,15 @@ describe('PermissionsService', () => {
sharedToGroups: [groupPermissionUpdate], sharedToGroups: [groupPermissionUpdate],
}, },
); );
expect((await savedNote.userPermissions)[0].user.username).toEqual( expect(
userPermissionUpdate.username, (await (await savedNote.userPermissions)[0].user).username,
); ).toEqual(userPermissionUpdate.username);
expect((await savedNote.userPermissions)[0].canEdit).toEqual( expect((await savedNote.userPermissions)[0].canEdit).toEqual(
userPermissionUpdate.canEdit, userPermissionUpdate.canEdit,
); );
expect((await savedNote.groupPermissions)[0].group.name).toEqual( expect(
groupPermissionUpdate.groupName, (await (await savedNote.groupPermissions)[0].group).name,
); ).toEqual(groupPermissionUpdate.groupName);
expect((await savedNote.groupPermissions)[0].canEdit).toEqual( expect((await savedNote.groupPermissions)[0].canEdit).toEqual(
groupPermissionUpdate.canEdit, groupPermissionUpdate.canEdit,
); );
@ -858,24 +870,24 @@ describe('PermissionsService', () => {
const noteWithPreexistingPermissions: Note = { ...note }; const noteWithPreexistingPermissions: Note = { ...note };
noteWithPreexistingPermissions.groupPermissions = Promise.resolve([ noteWithPreexistingPermissions.groupPermissions = Promise.resolve([
{ {
noteId: 4711, id: 1,
note: noteWithPreexistingPermissions, note: Promise.resolve(noteWithPreexistingPermissions),
groupId: 0, group: Promise.resolve(group),
group: group,
canEdit: !groupPermissionUpdate.canEdit, canEdit: !groupPermissionUpdate.canEdit,
}, },
]); ]);
noteWithPreexistingPermissions.userPermissions = Promise.resolve([ noteWithPreexistingPermissions.userPermissions = Promise.resolve([
{ {
noteId: 4711, id: 1,
note: noteWithPreexistingPermissions, note: Promise.resolve(noteWithPreexistingPermissions),
userId: 4711, user: Promise.resolve(user),
user: user,
canEdit: !userPermissionUpdate.canEdit, canEdit: !userPermissionUpdate.canEdit,
}, },
]); ]);
jest jest
.spyOn(noteRepo, 'save') .spyOn(noteRepo, 'save')
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
.mockImplementationOnce(async (entry: Note) => { .mockImplementationOnce(async (entry: Note) => {
return entry; return entry;
}); });
@ -888,15 +900,15 @@ describe('PermissionsService', () => {
sharedToGroups: [groupPermissionUpdate], sharedToGroups: [groupPermissionUpdate],
}, },
); );
expect((await savedNote.userPermissions)[0].user.username).toEqual( expect(
userPermissionUpdate.username, (await (await savedNote.userPermissions)[0].user).username,
); ).toEqual(userPermissionUpdate.username);
expect((await savedNote.userPermissions)[0].canEdit).toEqual( expect((await savedNote.userPermissions)[0].canEdit).toEqual(
userPermissionUpdate.canEdit, userPermissionUpdate.canEdit,
); );
expect((await savedNote.groupPermissions)[0].group.name).toEqual( expect(
groupPermissionUpdate.groupName, (await (await savedNote.groupPermissions)[0].group).name,
); ).toEqual(groupPermissionUpdate.groupName);
expect((await savedNote.groupPermissions)[0].canEdit).toEqual( expect((await savedNote.groupPermissions)[0].canEdit).toEqual(
groupPermissionUpdate.canEdit, groupPermissionUpdate.canEdit,
); );
@ -937,6 +949,8 @@ describe('PermissionsService', () => {
it('with user not added before and editable', async () => { it('with user not added before and editable', async () => {
jest jest
.spyOn(noteRepo, 'save') .spyOn(noteRepo, 'save')
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
.mockImplementationOnce(async (entry: Note) => { .mockImplementationOnce(async (entry: Note) => {
return entry; return entry;
}); });
@ -951,6 +965,8 @@ describe('PermissionsService', () => {
it('with user not added before and not editable', async () => { it('with user not added before and not editable', async () => {
jest jest
.spyOn(noteRepo, 'save') .spyOn(noteRepo, 'save')
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
.mockImplementationOnce(async (entry: Note) => { .mockImplementationOnce(async (entry: Note) => {
return entry; return entry;
}); });
@ -965,6 +981,8 @@ describe('PermissionsService', () => {
it('with user added before and editable', async () => { it('with user added before and editable', async () => {
jest jest
.spyOn(noteRepo, 'save') .spyOn(noteRepo, 'save')
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
.mockImplementationOnce(async (entry: Note) => { .mockImplementationOnce(async (entry: Note) => {
return entry; return entry;
}); });
@ -983,6 +1001,8 @@ describe('PermissionsService', () => {
it('with user added before and not editable', async () => { it('with user added before and not editable', async () => {
jest jest
.spyOn(noteRepo, 'save') .spyOn(noteRepo, 'save')
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
.mockImplementationOnce(async (entry: Note) => { .mockImplementationOnce(async (entry: Note) => {
return entry; return entry;
}); });
@ -1005,6 +1025,8 @@ describe('PermissionsService', () => {
it('with user added before and editable', async () => { it('with user added before and editable', async () => {
jest jest
.spyOn(noteRepo, 'save') .spyOn(noteRepo, 'save')
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
.mockImplementationOnce(async (entry: Note) => { .mockImplementationOnce(async (entry: Note) => {
return entry; return entry;
}); });
@ -1020,6 +1042,8 @@ describe('PermissionsService', () => {
it('with user not added before and not editable', async () => { it('with user not added before and not editable', async () => {
jest jest
.spyOn(noteRepo, 'save') .spyOn(noteRepo, 'save')
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
.mockImplementationOnce(async (entry: Note) => { .mockImplementationOnce(async (entry: Note) => {
return entry; return entry;
}); });
@ -1039,6 +1063,8 @@ describe('PermissionsService', () => {
it('with group not added before and editable', async () => { it('with group not added before and editable', async () => {
jest jest
.spyOn(noteRepo, 'save') .spyOn(noteRepo, 'save')
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
.mockImplementationOnce(async (entry: Note) => { .mockImplementationOnce(async (entry: Note) => {
return entry; return entry;
}); });
@ -1057,6 +1083,8 @@ describe('PermissionsService', () => {
it('with group not added before and not editable', async () => { it('with group not added before and not editable', async () => {
jest jest
.spyOn(noteRepo, 'save') .spyOn(noteRepo, 'save')
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
.mockImplementationOnce(async (entry: Note) => { .mockImplementationOnce(async (entry: Note) => {
return entry; return entry;
}); });
@ -1075,6 +1103,8 @@ describe('PermissionsService', () => {
it('with group added before and editable', async () => { it('with group added before and editable', async () => {
jest jest
.spyOn(noteRepo, 'save') .spyOn(noteRepo, 'save')
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
.mockImplementationOnce(async (entry: Note) => { .mockImplementationOnce(async (entry: Note) => {
return entry; return entry;
}); });
@ -1097,6 +1127,8 @@ describe('PermissionsService', () => {
it('with group added before and not editable', async () => { it('with group added before and not editable', async () => {
jest jest
.spyOn(noteRepo, 'save') .spyOn(noteRepo, 'save')
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
.mockImplementationOnce(async (entry: Note) => { .mockImplementationOnce(async (entry: Note) => {
return entry; return entry;
}); });
@ -1123,6 +1155,8 @@ describe('PermissionsService', () => {
it('with user added before and editable', async () => { it('with user added before and editable', async () => {
jest jest
.spyOn(noteRepo, 'save') .spyOn(noteRepo, 'save')
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
.mockImplementationOnce(async (entry: Note) => { .mockImplementationOnce(async (entry: Note) => {
return entry; return entry;
}); });
@ -1138,6 +1172,8 @@ describe('PermissionsService', () => {
it('with user not added before and not editable', async () => { it('with user not added before and not editable', async () => {
jest jest
.spyOn(noteRepo, 'save') .spyOn(noteRepo, 'save')
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
.mockImplementationOnce(async (entry: Note) => { .mockImplementationOnce(async (entry: Note) => {
return entry; return entry;
}); });
@ -1158,6 +1194,8 @@ describe('PermissionsService', () => {
const user = User.create('test', 'Testy') as User; const user = User.create('test', 'Testy') as User;
jest jest
.spyOn(noteRepo, 'save') .spyOn(noteRepo, 'save')
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
.mockImplementationOnce(async (entry: Note) => { .mockImplementationOnce(async (entry: Note) => {
return entry; return entry;
}); });

56
src/permissions/permissions.service.ts
View file

@ -93,7 +93,7 @@ export class PermissionsService {
} }
for (const userPermission of await note.userPermissions) { for (const userPermission of await note.userPermissions) {
if ( if (
userPermission.user.id === user.id && (await userPermission.user).id === user.id &&
(userPermission.canEdit || !wantEdit) (userPermission.canEdit || !wantEdit)
) { ) {
return true; return true;
@ -121,12 +121,12 @@ export class PermissionsService {
for (const groupPermission of await note.groupPermissions) { for (const groupPermission of await note.groupPermissions) {
if (groupPermission.canEdit || !wantEdit) { if (groupPermission.canEdit || !wantEdit) {
// Handle special groups // Handle special groups
if (groupPermission.group.special) { if ((await groupPermission.group).special) {
if (groupPermission.group.name == SpecialGroup.LOGGED_IN) { if ((await groupPermission.group).name == SpecialGroup.LOGGED_IN) {
return true; return true;
} }
if ( if (
groupPermission.group.name == SpecialGroup.EVERYONE && (await groupPermission.group).name == SpecialGroup.EVERYONE &&
(groupPermission.canEdit || !wantEdit) && (groupPermission.canEdit || !wantEdit) &&
guestsAllowed guestsAllowed
) { ) {
@ -135,7 +135,9 @@ export class PermissionsService {
} else { } else {
// Handle normal groups // Handle normal groups
if (user) { 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; if (member.id === user.id) return true;
} }
} }
@ -189,7 +191,7 @@ export class PermissionsService {
note, note,
newUserPermission.canEdit, newUserPermission.canEdit,
); );
createdPermission.note = note; createdPermission.note = Promise.resolve(note);
(await note.userPermissions).push(createdPermission); (await note.userPermissions).push(createdPermission);
} }
@ -203,7 +205,7 @@ export class PermissionsService {
note, note,
newGroupPermission.canEdit, newGroupPermission.canEdit,
); );
createdPermission.note = note; createdPermission.note = Promise.resolve(note);
(await note.groupPermissions).push(createdPermission); (await note.groupPermissions).push(createdPermission);
} }
@ -225,9 +227,9 @@ export class PermissionsService {
): Promise<Note> { ): Promise<Note> {
const permissions = await note.userPermissions; const permissions = await note.userPermissions;
let permissionIndex = 0; let permissionIndex = 0;
const permission = permissions.find((value, index) => { const permission = permissions.find(async (value, index) => {
permissionIndex = index; permissionIndex = index;
return value.user.id == permissionUser.id; return (await value.user).id == permissionUser.id;
}); });
if (permission != undefined) { if (permission != undefined) {
permission.canEdit = canEdit; permission.canEdit = canEdit;
@ -252,10 +254,13 @@ export class PermissionsService {
*/ */
async removeUserPermission(note: Note, permissionUser: User): Promise<Note> { async removeUserPermission(note: Note, permissionUser: User): Promise<Note> {
const permissions = await note.userPermissions; const permissions = await note.userPermissions;
const permissionsFiltered = permissions.filter( const newPermissions = [];
(value) => value.user.id != permissionUser.id, for (const permission of permissions) {
); if ((await permission.user).id != permissionUser.id) {
note.userPermissions = Promise.resolve(permissionsFiltered); newPermissions.push(permission);
}
}
note.userPermissions = Promise.resolve(newPermissions);
return await this.noteRepository.save(note); return await this.noteRepository.save(note);
} }
@ -272,16 +277,24 @@ export class PermissionsService {
permissionGroup: Group, permissionGroup: Group,
canEdit: boolean, canEdit: boolean,
): Promise<Note> { ): Promise<Note> {
this.logger.debug(
`Setting group permission for group ${permissionGroup.name} on note ${note.id}`,
'setGroupPermission',
);
const permissions = await note.groupPermissions; const permissions = await note.groupPermissions;
let permissionIndex = 0; let permissionIndex = 0;
const permission = permissions.find((value, index) => { const permission = permissions.find(async (value, index) => {
permissionIndex = index; permissionIndex = index;
return value.group.id == permissionGroup.id; return (await value.group).id == permissionGroup.id;
}); });
if (permission != undefined) { if (permission != undefined) {
permission.canEdit = canEdit; permission.canEdit = canEdit;
permissions[permissionIndex] = permission; permissions[permissionIndex] = permission;
} else { } else {
this.logger.debug(
`Permission does not exist yet, creating new one.`,
'setGroupPermission',
);
const noteGroupPermission = NoteGroupPermission.create( const noteGroupPermission = NoteGroupPermission.create(
permissionGroup, permissionGroup,
note, note,
@ -304,12 +317,13 @@ export class PermissionsService {
permissionGroup: Group, permissionGroup: Group,
): Promise<Note> { ): Promise<Note> {
const permissions = await note.groupPermissions; const permissions = await note.groupPermissions;
const permissionsFiltered = permissions.filter( const newPermissions = [];
(value: NoteGroupPermission) => { for (const permission of permissions) {
return value.group.id != permissionGroup.id; if ((await permission.group).id != permissionGroup.id) {
}, newPermissions.push(permission);
); }
note.groupPermissions = Promise.resolve(permissionsFiltered); }
note.groupPermissions = Promise.resolve(newPermissions);
return await this.noteRepository.save(note); return await this.noteRepository.save(note);
} }

6
test/public-api/notes.e2e-spec.ts
View file

@ -211,9 +211,9 @@ describe('Notes', () => {
expect((await updatedNote.userPermissions)[0].canEdit).toEqual( expect((await updatedNote.userPermissions)[0].canEdit).toEqual(
updateNotePermission.sharedToUsers[0].canEdit, updateNotePermission.sharedToUsers[0].canEdit,
); );
expect((await updatedNote.userPermissions)[0].user.username).toEqual( expect(
user.username, (await (await updatedNote.userPermissions)[0].user).username,
); ).toEqual(user.username);
expect(await updatedNote.groupPermissions).toHaveLength(0); expect(await updatedNote.groupPermissions).toHaveLength(0);
await request(testSetup.app.getHttpServer()) await request(testSetup.app.getHttpServer())
.delete('/api/v2/notes/test3') .delete('/api/v2/notes/test3')