diff --git a/backend/src/permissions/note-permission.enum.spec.ts b/backend/src/permissions/note-permission.enum.spec.ts new file mode 100644 index 000000000..303b14832 --- /dev/null +++ b/backend/src/permissions/note-permission.enum.spec.ts @@ -0,0 +1,32 @@ +/* + * SPDX-FileCopyrightText: 2023 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ +import { + getNotePermissionDisplayName, + NotePermission, +} from './note-permission.enum'; + +describe('note permission order', () => { + it('DENY is less than READ', () => { + expect(NotePermission.DENY < NotePermission.READ).toBeTruthy(); + }); + it('READ is less than WRITE', () => { + expect(NotePermission.READ < NotePermission.WRITE).toBeTruthy(); + }); + it('WRITE is less than OWNER', () => { + expect(NotePermission.WRITE < NotePermission.OWNER).toBeTruthy(); + }); +}); + +describe('getNotePermissionDisplayName', () => { + it.each([ + ['deny', NotePermission.DENY], + ['read', NotePermission.READ], + ['write', NotePermission.WRITE], + ['owner', NotePermission.OWNER], + ])('displays %s correctly', (displayName, permission) => { + expect(getNotePermissionDisplayName(permission)).toBe(displayName); + }); +}); diff --git a/backend/src/permissions/note-permission.enum.ts b/backend/src/permissions/note-permission.enum.ts new file mode 100644 index 000000000..76ae51307 --- /dev/null +++ b/backend/src/permissions/note-permission.enum.ts @@ -0,0 +1,34 @@ +/* + * SPDX-FileCopyrightText: 2023 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ + +/** + * Defines if a user can access a note and if yes how much power they have. + */ +export enum NotePermission { + DENY = 0, + READ = 1, + WRITE = 2, + OWNER = 3, +} + +/** + * Returns the display name for the given {@link NotePermission}. + * + * @param {NotePermission} value the note permission to display + * @return {string} The display name + */ +export function getNotePermissionDisplayName(value: NotePermission): string { + switch (value) { + case NotePermission.DENY: + return 'deny'; + case NotePermission.READ: + return 'read'; + case NotePermission.WRITE: + return 'write'; + case NotePermission.OWNER: + return 'owner'; + } +} diff --git a/backend/src/permissions/permissions.guard.ts b/backend/src/permissions/permissions.guard.ts index a76c41c24..5952ab826 100644 --- a/backend/src/permissions/permissions.guard.ts +++ b/backend/src/permissions/permissions.guard.ts @@ -10,6 +10,7 @@ import { extractNoteFromRequest } from '../api/utils/extract-note-from-request'; import { CompleteRequest } from '../api/utils/request.type'; import { ConsoleLoggerService } from '../logger/console-logger.service'; import { NotesService } from '../notes/notes.service'; +import { NotePermission } from './note-permission.enum'; import { PermissionsService } from './permissions.service'; import { PERMISSION_METADATA_KEY } from './require-permission.decorator'; import { RequiredPermission } from './required-permission.enum'; @@ -32,24 +33,18 @@ export class PermissionsGuard implements CanActivate { } async canActivate(context: ExecutionContext): Promise { - const permission = this.reflector.get( - PERMISSION_METADATA_KEY, - context.getHandler(), - ); - // If no permissions are set this is probably an error and this guard should not let the request pass - if (!permission) { - this.logger.error( - 'Could not find permission metadata. This should never happen. If you see this, please open an issue at https://github.com/hedgedoc/hedgedoc/issues', - ); + const requiredAccessLevel = this.extractRequiredPermission(context); + if (requiredAccessLevel === undefined) { return false; } const request: CompleteRequest = context.switchToHttp().getRequest(); const user = request.user ?? null; - // handle CREATE permissions, as this does not need any note - if (permission === RequiredPermission.CREATE) { + + // handle CREATE requiredAccessLevel, as this does not need any note + if (requiredAccessLevel === RequiredPermission.CREATE) { return this.permissionsService.mayCreate(user); } - // Attention: This gets the note an additional time if used in conjunction with GetNoteInterceptor or NoteHeaderInterceptor + const note = await extractNoteFromRequest(request, this.noteService); if (note === undefined) { this.logger.error( @@ -57,10 +52,41 @@ export class PermissionsGuard implements CanActivate { ); return false; } - return await this.permissionsService.checkPermissionOnNote( - permission, - user, - note, + + return this.isNotePermissionFulfillingRequiredAccessLevel( + requiredAccessLevel, + await this.permissionsService.determinePermission(user, note), ); } + + private extractRequiredPermission( + context: ExecutionContext, + ): RequiredPermission | undefined { + const requiredPermission = this.reflector.get( + PERMISSION_METADATA_KEY, + context.getHandler(), + ); + // If no requiredPermission are set this is probably an error and this guard should not let the request pass + if (!requiredPermission) { + this.logger.error( + 'Could not find requiredPermission metadata. This should never happen. If you see this, please open an issue at https://github.com/hedgedoc/hedgedoc/issues', + ); + return undefined; + } + return requiredPermission; + } + + private isNotePermissionFulfillingRequiredAccessLevel( + requiredAccessLevel: Exclude, + actualNotePermission: NotePermission, + ): boolean { + switch (requiredAccessLevel) { + case RequiredPermission.READ: + return actualNotePermission >= NotePermission.READ; + case RequiredPermission.WRITE: + return actualNotePermission >= NotePermission.WRITE; + case RequiredPermission.OWNER: + return actualNotePermission >= NotePermission.OWNER; + } + } } diff --git a/backend/src/permissions/permissions.service.spec.ts b/backend/src/permissions/permissions.service.spec.ts index ba2ae2e0a..83af24699 100644 --- a/backend/src/permissions/permissions.service.spec.ts +++ b/backend/src/permissions/permissions.service.spec.ts @@ -7,6 +7,7 @@ import { ConfigModule } from '@nestjs/config'; import { EventEmitter2, EventEmitterModule } from '@nestjs/event-emitter'; import { Test, TestingModule } from '@nestjs/testing'; import { getRepositoryToken } from '@nestjs/typeorm'; +import { Mock } from 'ts-mockery'; import { DataSource, EntityManager, Repository } from 'typeorm'; import { AuthToken } from '../auth/auth-token.entity'; @@ -25,7 +26,7 @@ import { PermissionsUpdateInconsistentError } from '../errors/errors'; import { eventModuleConfig, NoteEvent } from '../events'; import { Group } from '../groups/group.entity'; import { GroupsModule } from '../groups/groups.module'; -import { SpecialGroup } from '../groups/groups.special'; +import { GroupsService } from '../groups/groups.service'; import { Identity } from '../identity/identity.entity'; import { LoggerModule } from '../logger/logger.module'; import { MediaUpload } from '../media/media-upload.entity'; @@ -43,10 +44,32 @@ import { Session } from '../users/session.entity'; import { User } from '../users/user.entity'; import { UsersModule } from '../users/users.module'; import { NoteGroupPermission } from './note-group-permission.entity'; +import { + getNotePermissionDisplayName, + NotePermission, +} from './note-permission.enum'; import { NoteUserPermission } from './note-user-permission.entity'; import { PermissionsModule } from './permissions.module'; import { PermissionsService } from './permissions.service'; -import { RequiredPermission } from './required-permission.enum'; +import { convertGuestAccessToNotePermission } from './utils/convert-guest-access-to-note-permission'; +import * as FindHighestNotePermissionByGroupModule from './utils/find-highest-note-permission-by-group'; +import * as FindHighestNotePermissionByUserModule from './utils/find-highest-note-permission-by-user'; + +jest.mock( + './utils/find-highest-note-permission-by-user', + () => + jest.requireActual( + './utils/find-highest-note-permission-by-user', + ) as unknown, +); + +jest.mock( + './utils/find-highest-note-permission-by-group', + () => + jest.requireActual( + './utils/find-highest-note-permission-by-group', + ) as unknown, +); function mockedEventEmitter(eventEmitter: EventEmitter2) { return jest.spyOn(eventEmitter, 'emit').mockImplementationOnce((event) => { @@ -67,7 +90,7 @@ function mockNoteRepo(noteRepo: Repository) { describe('PermissionsService', () => { let service: PermissionsService; - let notes: Note[]; + let groupService: GroupsService; let noteRepo: Repository; let userRepo: Repository; let groupRepo: Repository; @@ -165,570 +188,42 @@ describe('PermissionsService', () => { .useValue({}) .compile(); service = module.get(PermissionsService); - notes = await createNoteUserPermissionNotes(); + groupService = module.get(GroupsService); groupRepo = module.get>(getRepositoryToken(Group)); noteRepo = module.get>(getRepositoryToken(Note)); eventEmitter = module.get(EventEmitter2); }); - function mockMayReadTrue(): void { - jest.spyOn(service, 'mayRead').mockImplementationOnce(async () => { - return true; - }); - } - - function mockMayWriteTrue(): void { - jest.spyOn(service, 'mayWrite').mockImplementationOnce(async () => { - return true; - }); - } - - function mockIsOwner(isOwner: boolean): void { - jest.spyOn(service, 'isOwner').mockImplementationOnce(async () => { - return isOwner; - }); - } - beforeEach(() => { mockNoteRepo(noteRepo); eventEmitterEmitSpy = mockedEventEmitter(eventEmitter); }); + afterEach(() => { - jest.clearAllMocks(); + jest.resetModules(); + jest.restoreAllMocks(); }); // The two users we test with: - const user2 = {} as User; - user2.id = 2; - const user1 = {} as User; - user1.id = 1; + const user1 = Mock.of({ id: 1 }); + const user2 = Mock.of({ id: 2 }); + + function mockNote( + owner: User, + userPermissions: NoteUserPermission[] = [], + groupPermissions: NoteGroupPermission[] = [], + ): Note { + return Mock.of({ + owner: Promise.resolve(owner), + userPermissions: Promise.resolve(userPermissions), + groupPermissions: Promise.resolve(groupPermissions), + }); + } it('should be defined', () => { expect(service).toBeDefined(); }); - function createNote(owner: User): Note { - const note = {} as Note; - note.userPermissions = Promise.resolve([]); - note.groupPermissions = Promise.resolve([]); - note.owner = Promise.resolve(owner); - return note; - } - - /* - * Creates the permission objects for UserPermission for two users with write and with out write permission - */ - async function createNoteUserPermissionNotes(): Promise { - const note0 = createNote(user1); - const note1 = createNote(user2); - const note2 = createNote(user2); - const note3 = createNote(user2); - const note4 = createNote(user2); - const note5 = createNote(user2); - const note6 = createNote(user2); - const note7 = createNote(user2); - const noteUserPermission1 = {} as NoteUserPermission; - noteUserPermission1.user = Promise.resolve(user1); - const noteUserPermission2 = {} as NoteUserPermission; - noteUserPermission2.user = Promise.resolve(user2); - const noteUserPermission3 = {} as NoteUserPermission; - noteUserPermission3.user = Promise.resolve(user1); - noteUserPermission3.canEdit = true; - const noteUserPermission4 = {} as NoteUserPermission; - noteUserPermission4.user = Promise.resolve(user2); - noteUserPermission4.canEdit = true; - - (await note1.userPermissions).push(noteUserPermission1); - - (await note2.userPermissions).push(noteUserPermission1); - (await note2.userPermissions).push(noteUserPermission2); - - (await note3.userPermissions).push(noteUserPermission2); - (await note3.userPermissions).push(noteUserPermission1); - - (await note4.userPermissions).push(noteUserPermission3); - - (await note5.userPermissions).push(noteUserPermission3); - (await note5.userPermissions).push(noteUserPermission4); - - (await note6.userPermissions).push(noteUserPermission4); - (await note6.userPermissions).push(noteUserPermission3); - - (await note7.userPermissions).push(noteUserPermission2); - - const everybody = {} as Group; - everybody.name = SpecialGroup.EVERYONE; - everybody.special = true; - - const noteEverybodyNone = createNote(user1); - noteEverybodyNone.groupPermissions = Promise.resolve([]); - - const noteEverybodyRead = createNote(user1); - const noteGroupPermissionRead = {} as NoteGroupPermission; - noteGroupPermissionRead.group = Promise.resolve(everybody); - noteGroupPermissionRead.canEdit = false; - noteGroupPermissionRead.note = Promise.resolve(noteEverybodyRead); - noteEverybodyRead.groupPermissions = Promise.resolve([ - noteGroupPermissionRead, - ]); - - const noteEverybodyWrite = createNote(user1); - const noteGroupPermissionWrite = {} as NoteGroupPermission; - noteGroupPermissionWrite.group = Promise.resolve(everybody); - noteGroupPermissionWrite.canEdit = true; - noteGroupPermissionWrite.note = Promise.resolve(noteEverybodyWrite); - noteEverybodyWrite.groupPermissions = Promise.resolve([ - noteGroupPermissionWrite, - ]); - - return [ - note0, - note1, - note2, - note3, - note4, - note5, - note6, - note7, - noteEverybodyRead, - noteEverybodyWrite, - noteEverybodyNone, - ]; - } - - describe('mayRead works with', () => { - it('Owner', async () => { - expect(await service.mayRead(user1, notes[0])).toBeTruthy(); - expect(await service.mayRead(user1, notes[7])).toBeFalsy(); - }); - it('userPermission read', async () => { - expect(await service.mayRead(user1, notes[1])).toBeTruthy(); - expect(await service.mayRead(user1, notes[2])).toBeTruthy(); - expect(await service.mayRead(user1, notes[3])).toBeTruthy(); - }); - it('userPermission write', async () => { - expect(await service.mayRead(user1, notes[4])).toBeTruthy(); - expect(await service.mayRead(user1, notes[5])).toBeTruthy(); - expect(await service.mayRead(user1, notes[6])).toBeTruthy(); - expect(await service.mayRead(user1, notes[7])).toBeFalsy(); - }); - - describe('guest permission', () => { - beforeEach(() => { - noteMockConfig.permissions.default.loggedIn = DefaultAccessLevel.WRITE; - noteMockConfig.permissions.default.everyone = DefaultAccessLevel.WRITE; - }); - describe('with guest access deny', () => { - beforeEach(() => { - noteMockConfig.guestAccess = GuestAccess.DENY; - }); - it('guest permission none', async () => { - expect(await service.mayRead(null, notes[10])).toBeFalsy(); - }); - it('guest permission read', async () => { - expect(await service.mayRead(null, notes[8])).toBeFalsy(); - }); - it('guest permission write', async () => { - expect(await service.mayRead(null, notes[9])).toBeFalsy(); - }); - }); - describe('with guest access read', () => { - beforeEach(() => { - noteMockConfig.guestAccess = GuestAccess.READ; - }); - it('guest permission none', async () => { - expect(await service.mayRead(null, notes[10])).toBeFalsy(); - }); - it('guest permission read', async () => { - expect(await service.mayRead(null, notes[8])).toBeTruthy(); - }); - it('guest permission write', async () => { - expect(await service.mayRead(null, notes[9])).toBeTruthy(); - }); - }); - describe('with guest access write', () => { - beforeEach(() => { - noteMockConfig.guestAccess = GuestAccess.WRITE; - }); - it('guest permission none', async () => { - expect(await service.mayRead(null, notes[10])).toBeFalsy(); - }); - it('guest permission read', async () => { - expect(await service.mayRead(null, notes[8])).toBeTruthy(); - }); - it('guest permission write', async () => { - expect(await service.mayRead(null, notes[9])).toBeTruthy(); - }); - }); - describe('with guest access create', () => { - beforeEach(() => { - noteMockConfig.guestAccess = GuestAccess.CREATE; - }); - it('guest permission none', async () => { - expect(await service.mayRead(null, notes[10])).toBeFalsy(); - }); - it('guest permission read', async () => { - expect(await service.mayRead(null, notes[8])).toBeTruthy(); - }); - it('guest permission write', async () => { - expect(await service.mayRead(null, notes[9])).toBeTruthy(); - }); - }); - }); - }); - - describe('mayWrite works with', () => { - it('Owner', async () => { - expect(await service.mayWrite(user1, notes[0])).toBeTruthy(); - expect(await service.mayWrite(user1, notes[7])).toBeFalsy(); - }); - it('userPermission read', async () => { - expect(await service.mayWrite(user1, notes[1])).toBeFalsy(); - expect(await service.mayWrite(user1, notes[2])).toBeFalsy(); - expect(await service.mayWrite(user1, notes[3])).toBeFalsy(); - }); - it('userPermission write', async () => { - expect(await service.mayWrite(user1, notes[4])).toBeTruthy(); - expect(await service.mayWrite(user1, notes[5])).toBeTruthy(); - expect(await service.mayWrite(user1, notes[6])).toBeTruthy(); - expect(await service.mayWrite(user1, notes[7])).toBeFalsy(); - }); - describe('guest permission', () => { - beforeEach(() => { - noteMockConfig.permissions.default.loggedIn = DefaultAccessLevel.WRITE; - noteMockConfig.permissions.default.everyone = DefaultAccessLevel.WRITE; - }); - - describe('with guest access deny', () => { - beforeEach(() => { - noteMockConfig.guestAccess = GuestAccess.DENY; - }); - it('guest permission none', async () => { - expect(await service.mayWrite(null, notes[10])).toBeFalsy(); - }); - it('guest permission read', async () => { - expect(await service.mayWrite(null, notes[8])).toBeFalsy(); - }); - it('guest permission write', async () => { - expect(await service.mayWrite(null, notes[9])).toBeFalsy(); - }); - }); - - describe('with guest access read', () => { - beforeEach(() => { - noteMockConfig.guestAccess = GuestAccess.READ; - }); - it('guest permission none', async () => { - expect(await service.mayWrite(null, notes[10])).toBeFalsy(); - }); - it('guest permission read', async () => { - expect(await service.mayWrite(null, notes[8])).toBeFalsy(); - }); - it('guest permission write', async () => { - expect(await service.mayWrite(null, notes[9])).toBeFalsy(); - }); - }); - - describe('with guest access write', () => { - beforeEach(() => { - noteMockConfig.guestAccess = GuestAccess.WRITE; - }); - it('guest permission none', async () => { - expect(await service.mayWrite(null, notes[10])).toBeFalsy(); - }); - it('guest permission read', async () => { - expect(await service.mayWrite(null, notes[8])).toBeFalsy(); - }); - it('guest permission write', async () => { - expect(await service.mayWrite(null, notes[9])).toBeTruthy(); - }); - }); - - describe('with guest access create', () => { - beforeEach(() => { - noteMockConfig.guestAccess = GuestAccess.CREATE; - }); - it('guest permission none', async () => { - expect(await service.mayWrite(null, notes[10])).toBeFalsy(); - }); - it('guest permission read', async () => { - expect(await service.mayWrite(null, notes[8])).toBeFalsy(); - }); - it('guest permission write', async () => { - expect(await service.mayWrite(null, notes[9])).toBeTruthy(); - }); - }); - }); - }); - - /* - * Helper Object that arranges a list of GroupPermissions and if they allow a user to read or write a particular note. - */ - class NoteGroupPermissionWithResultForUser { - permissions: NoteGroupPermission[]; - allowsRead: boolean; - allowsWrite: boolean; - } - - /* - * Setup function to create all the groups we use in the tests. - */ - function createGroups(): { [id: string]: Group } { - const result: { [id: string]: Group } = {}; - - result[SpecialGroup.EVERYONE] = Group.create( - SpecialGroup.EVERYONE, - SpecialGroup.EVERYONE, - true, - ) as Group; - - result[SpecialGroup.LOGGED_IN] = Group.create( - SpecialGroup.LOGGED_IN, - SpecialGroup.LOGGED_IN, - true, - ) as Group; - - const user1group = Group.create('user1group', 'user1group', false) as Group; - user1group.members = Promise.resolve([user1]); - result['user1group'] = user1group; - - const user2group = Group.create('user2group', 'user2group', false) as Group; - user2group.members = Promise.resolve([user2]); - result['user2group'] = user2group; - - const user1and2group = Group.create( - 'user1and2group', - 'user1and2group', - false, - ) as Group; - user1and2group.members = Promise.resolve([user1, user2]); - result['user1and2group'] = user1and2group; - - const user2and1group = Group.create( - 'user2and1group', - 'user2and1group', - false, - ) as Group; - user2and1group.members = Promise.resolve([user2, user1]); - result['user2and1group'] = user2and1group; - - return result; - } - - /* - * Create all GroupPermissions: For each group two GroupPermissions are created one with read permission and one with write permission. - */ - function createAllNoteGroupPermissions(): (NoteGroupPermission | null)[][] { - const groups = createGroups(); - - /* - * Helper function for creating GroupPermissions - */ - function createNoteGroupPermission( - group: Group, - write: boolean, - ): NoteGroupPermission { - return NoteGroupPermission.create(group, {} as Note, write); - } - - const everybodyRead = createNoteGroupPermission( - groups[SpecialGroup.EVERYONE], - false, - ); - const everybodyWrite = createNoteGroupPermission( - groups[SpecialGroup.EVERYONE], - true, - ); - - const loggedInRead = createNoteGroupPermission( - groups[SpecialGroup.LOGGED_IN], - false, - ); - const loggedInWrite = createNoteGroupPermission( - groups[SpecialGroup.LOGGED_IN], - true, - ); - - const user1groupRead = createNoteGroupPermission( - groups['user1group'], - false, - ); - const user1groupWrite = createNoteGroupPermission( - groups['user1group'], - true, - ); - - const user2groupRead = createNoteGroupPermission( - groups['user2group'], - false, - ); - const user2groupWrite = createNoteGroupPermission( - groups['user2group'], - true, - ); - - const user1and2groupRead = createNoteGroupPermission( - groups['user1and2group'], - false, - ); - const user1and2groupWrite = createNoteGroupPermission( - groups['user1and2group'], - true, - ); - - const user2and1groupRead = createNoteGroupPermission( - groups['user2and1group'], - false, - ); - const user2and1groupWrite = createNoteGroupPermission( - groups['user2and1group'], - true, - ); - - return [ - [user1groupRead, user1and2groupRead, user2and1groupRead, null], // group0: allow user1 to read via group - [user2and1groupWrite, user1and2groupWrite, user1groupWrite, null], // group1: allow user1 to write via group - [everybodyRead, everybodyWrite, null], // group2: permissions of the special group everybody - [loggedInRead, loggedInWrite, null], // group3: permissions of the special group loggedIn - [user2groupWrite, user2groupRead, null], // group4: don't allow user1 to read or write via group - ]; - } - - /* - * creates the matrix multiplication of group0 to group4 of createAllNoteGroupPermissions - */ - function createNoteGroupPermissionsCombinations( - everyoneDefaultPermission: DefaultAccessLevel, - ): NoteGroupPermissionWithResultForUser[] { - // for logged in users - const noteGroupPermissions = createAllNoteGroupPermissions(); - const result: NoteGroupPermissionWithResultForUser[] = []; - for (const group0 of noteGroupPermissions[0]) { - for (const group1 of noteGroupPermissions[1]) { - for (const group2 of noteGroupPermissions[2]) { - for (const group3 of noteGroupPermissions[3]) { - for (const group4 of noteGroupPermissions[4]) { - const insert: NoteGroupPermission[] = []; - let readPermission = false; - let writePermission = false; - if (group0 !== null) { - // user1 in ReadGroups - readPermission = true; - insert.push(group0); - } - if (group1 !== null) { - // user1 in WriteGroups - readPermission = true; - writePermission = true; - insert.push(group1); - } - - if (group2 !== null) { - if (everyoneDefaultPermission === DefaultAccessLevel.WRITE) { - writePermission = writePermission || group2.canEdit; - readPermission = true; - } else if ( - everyoneDefaultPermission === DefaultAccessLevel.READ - ) { - readPermission = true; - } - insert.push(group2); - } - if (group3 !== null) { - // loggedIn users - readPermission = true; - writePermission = writePermission || group3.canEdit; - insert.push(group3); - } - if (group4 !== null) { - // user not in group - insert.push(group4); - } - result.push({ - permissions: insert, - allowsRead: readPermission, - allowsWrite: writePermission, - }); - } - } - } - } - } - return result; - } - - // inspired by https://stackoverflow.com/questions/9960908/permutations-in-javascript - function permutator( - inputArr: NoteGroupPermission[], - ): NoteGroupPermission[][] { - const results: NoteGroupPermission[][] = []; - - function permute( - arr: NoteGroupPermission[], - memo: NoteGroupPermission[], - ): NoteGroupPermission[][] { - let cur: NoteGroupPermission[]; - - for (let i = 0; i < arr.length; i++) { - cur = arr.splice(i, 1); - if (arr.length === 0) { - results.push(memo.concat(cur)); - } - permute(arr.slice(), memo.concat(cur)); - arr.splice(i, 0, cur[0]); - } - - return results; - } - - return permute(inputArr, []); - } - - // takes each set of permissions from createNoteGroupPermissionsCombinations, permute them and add them to the list - function permuteNoteGroupPermissions( - noteGroupPermissions: NoteGroupPermissionWithResultForUser[], - ): NoteGroupPermissionWithResultForUser[] { - const result: NoteGroupPermissionWithResultForUser[] = []; - for (const permission of noteGroupPermissions) { - const permutations = permutator(permission.permissions); - for (const permutation of permutations) { - result.push({ - permissions: permutation, - allowsRead: permission.allowsRead, - allowsWrite: permission.allowsWrite, - }); - } - } - return result; - } - - describe('check if groups work with', () => { - const rawPermissions = createNoteGroupPermissionsCombinations( - DefaultAccessLevel.WRITE, - ); - const permissions = permuteNoteGroupPermissions(rawPermissions); - let i = 0; - for (const permission of permissions) { - const note = createNote(user2); - note.groupPermissions = Promise.resolve(permission.permissions); - let permissionString = ''; - for (const perm of permission.permissions) { - permissionString += ` ${perm.id}:${String(perm.canEdit)}`; - } - it(`mayWrite - test #${i}:${permissionString}`, async () => { - expect(await service.mayWrite(user1, note)).toEqual( - permission.allowsWrite, - ); - }); - it(`mayRead - test #${i}:${permissionString}`, async () => { - expect(await service.mayRead(user1, note)).toEqual( - permission.allowsRead, - ); - }); - i++; - } - }); - describe('mayCreate', () => { it('allows creation for logged in', () => { expect(service.mayCreate(user1)).toBeTruthy(); @@ -747,20 +242,33 @@ describe('PermissionsService', () => { }); }); - describe('isOwner works', () => { - it('for positive case', async () => { - expect(await service.isOwner(user1, notes[0])).toBeTruthy(); + describe('isOwner', () => { + it('works correctly if user is owner', async () => { + const note = mockNote(user1); + expect(await service.isOwner(user1, note)).toBeTruthy(); }); - it('for negative case', async () => { - expect(await service.isOwner(user1, notes[1])).toBeFalsy(); + it("works correctly if user isn't the owner", async () => { + const note = mockNote(user2); + expect(await service.isOwner(user1, note)).toBeFalsy(); + }); + it('works correctly if no user is provided', async () => { + const note = mockNote(user2); + expect(await service.isOwner(null, note)).toBeFalsy(); }); }); describe('checkMediaDeletePermission', () => { + const noteUserPermission1 = Mock.of({ + user: Promise.resolve(user1), + canEdit: false, + }); + + const noteOfUser2 = mockNote(user2, [noteUserPermission1]); + describe('accepts', () => { it('for media owner', async () => { const mediaUpload = {} as MediaUpload; - mediaUpload.note = Promise.resolve(notes[1]); + mediaUpload.note = Promise.resolve(noteOfUser2); mediaUpload.user = Promise.resolve(user1); expect( service.checkMediaDeletePermission(user1, mediaUpload), @@ -768,7 +276,7 @@ describe('PermissionsService', () => { }); it('for note owner', async () => { const mediaUpload = {} as MediaUpload; - mediaUpload.note = Promise.resolve(notes[1]); + mediaUpload.note = Promise.resolve(noteOfUser2); mediaUpload.user = Promise.resolve(user1); expect( service.checkMediaDeletePermission(user2, mediaUpload), @@ -777,10 +285,9 @@ describe('PermissionsService', () => { }); describe('denies', () => { it('for not owner', async () => { - const user3 = {} as User; - user3.id = 3; - const mediaUpload = {} as MediaUpload; - mediaUpload.note = Promise.resolve(notes[1]); + const user3 = Mock.of({ id: 3 }); + const mediaUpload = Mock.of(); + mediaUpload.note = Promise.resolve(noteOfUser2); mediaUpload.user = Promise.resolve(user1); expect( await service.checkMediaDeletePermission(user3, mediaUpload), @@ -789,51 +296,166 @@ describe('PermissionsService', () => { }); }); - describe('checkPermissionOnNote', () => { - describe('accepts', () => { - it('with mayRead', async () => { - mockMayReadTrue(); - expect( - await service.checkPermissionOnNote( - RequiredPermission.READ, - user1, - notes[0], - ), - ).toBeTruthy(); + describe('determinePermission', () => { + const everyoneGroup = Mock.of({ id: 99 }); + const loggedInGroup = Mock.of({ id: 98 }); + + beforeEach(() => { + jest + .spyOn(groupService, 'getEveryoneGroup') + .mockImplementation(() => Promise.resolve(everyoneGroup)); + jest + .spyOn(groupService, 'getLoggedInGroup') + .mockImplementation(() => Promise.resolve(loggedInGroup)); + }); + + describe('with guest user', () => { + const loggedInReadPermission = Mock.of({ + canEdit: false, + group: Promise.resolve(loggedInGroup), }); - it('with mayWrite', async () => { - mockMayWriteTrue(); - expect( - await service.checkPermissionOnNote( - RequiredPermission.WRITE, - user1, - notes[0], - ), - ).toBeTruthy(); + + it(`with no everyone permission will deny`, async () => { + const note = mockNote(user1, [], [loggedInReadPermission]); + const foundPermission = await service.determinePermission(null, note); + expect(foundPermission).toBe(NotePermission.DENY); }); - it('with isOwner', async () => { - mockIsOwner(true); - expect( - await service.checkPermissionOnNote( - RequiredPermission.OWNER, - user1, - notes[0], - ), - ).toBeTruthy(); + + describe.each([ + GuestAccess.DENY, + GuestAccess.READ, + GuestAccess.WRITE, + GuestAccess.CREATE, + ])('with configured guest access %s', (guestAccess) => { + beforeEach(() => { + noteMockConfig.guestAccess = guestAccess; + }); + + const guestAccessNotePermission = + convertGuestAccessToNotePermission(guestAccess); + + describe.each([false, true])( + 'with everybody group permission with edit set to %s', + (canEdit) => { + const editPermission = canEdit + ? NotePermission.WRITE + : NotePermission.READ; + const expectedLimitedPermission = + guestAccessNotePermission >= editPermission + ? editPermission + : guestAccessNotePermission; + + const permissionDisplayName = getNotePermissionDisplayName( + expectedLimitedPermission, + ); + it(`will ${permissionDisplayName}`, async () => { + const everybodyPermission = Mock.of({ + group: Promise.resolve(everyoneGroup), + canEdit: canEdit, + }); + + const note = mockNote( + user1, + [], + [everybodyPermission, loggedInReadPermission], + ); + + const foundPermission = await service.determinePermission( + null, + note, + ); + expect(foundPermission).toBe(expectedLimitedPermission); + }); + }, + ); }); }); - describe('denies', () => { - it('with no permission', async () => { - mockMayReadTrue(); - mockMayWriteTrue(); - mockIsOwner(false); - expect( - await service.checkPermissionOnNote( - RequiredPermission.OWNER, + + describe('with logged in user', () => { + describe('as owner will be OWNER permission', () => { + it('without other permissions', async () => { + const note = mockNote(user1); + + const foundPermission = await service.determinePermission( user1, - notes[0], - ), - ).toBeFalsy(); + note, + ); + + expect(foundPermission).toBe(NotePermission.OWNER); + }); + it('with other lower permissions', async () => { + const userPermission = Mock.of({ + user: Promise.resolve(user1), + canEdit: true, + }); + + const group1 = Mock.of({ + name: 'mockGroup', + id: 99, + members: Promise.resolve([user1]), + }); + + const groupPermission = Mock.of({ + group: Promise.resolve(group1), + canEdit: true, + }); + + const note = mockNote(user1, [userPermission], [groupPermission]); + + const foundPermission = await service.determinePermission( + user1, + note, + ); + + expect(foundPermission).toBe(NotePermission.OWNER); + }); + }); + describe('as non owner', () => { + it('with user permission higher than group permission', async () => { + jest + .spyOn( + FindHighestNotePermissionByUserModule, + 'findHighestNotePermissionByUser', + ) + .mockReturnValue(Promise.resolve(NotePermission.DENY)); + jest + .spyOn( + FindHighestNotePermissionByGroupModule, + 'findHighestNotePermissionByGroup', + ) + .mockReturnValue(Promise.resolve(NotePermission.WRITE)); + + const note = mockNote(user2); + + const foundPermission = await service.determinePermission( + user1, + note, + ); + expect(foundPermission).toBe(NotePermission.WRITE); + }); + + it('with group permission higher than user permission', async () => { + jest + .spyOn( + FindHighestNotePermissionByUserModule, + 'findHighestNotePermissionByUser', + ) + .mockReturnValue(Promise.resolve(NotePermission.WRITE)); + jest + .spyOn( + FindHighestNotePermissionByGroupModule, + 'findHighestNotePermissionByGroup', + ) + .mockReturnValue(Promise.resolve(NotePermission.DENY)); + + const note = mockNote(user2); + + const foundPermission = await service.determinePermission( + user1, + note, + ); + expect(foundPermission).toBe(NotePermission.WRITE); + }); }); }); }); @@ -1183,24 +805,49 @@ describe('PermissionsService', () => { expect(eventEmitterEmitSpy).toHaveBeenCalled(); }); describe('works', () => { - it('with user added before and editable', async () => { - const note = Note.create(null) as Note; - const user = User.create('test', 'Testy') as User; - note.userPermissions = Promise.resolve([ - NoteUserPermission.create(user, note, true), - ]); + const note = Note.create(null) as Note; + const user1 = Mock.of({ id: 1 }); + const user2 = Mock.of({ id: 2 }); - const resultNote = await service.removeUserPermission(note, user); - expect((await resultNote.userPermissions).length).toStrictEqual(0); - }); - it('with user not added before and not editable', async () => { - const note = Note.create(null) as Note; - const user = User.create('test', 'Testy') as User; + it('with user added before and editable', async () => { + const noteUserPermission1 = NoteUserPermission.create( + user1, + note, + true, + ); + const noteUserPermission2 = NoteUserPermission.create( + user2, + note, + true, + ); note.userPermissions = Promise.resolve([ - NoteUserPermission.create(user, note, false), + noteUserPermission1, + noteUserPermission2, + ]); + const resultNote = await service.removeUserPermission(note, user1); + expect(await resultNote.userPermissions).toStrictEqual([ + noteUserPermission2, + ]); + }); + it('with user added before and not editable', async () => { + const noteUserPermission1 = NoteUserPermission.create( + user1, + note, + false, + ); + const noteUserPermission2 = NoteUserPermission.create( + user2, + note, + false, + ); + note.userPermissions = Promise.resolve([ + noteUserPermission1, + noteUserPermission2, + ]); + const resultNote = await service.removeUserPermission(note, user1); + expect(await resultNote.userPermissions).toStrictEqual([ + noteUserPermission2, ]); - const resultNote = await service.removeUserPermission(note, user); - expect((await resultNote.userPermissions).length).toStrictEqual(0); }); }); }); @@ -1290,24 +937,50 @@ describe('PermissionsService', () => { expect(eventEmitterEmitSpy).toHaveBeenCalled(); }); describe('works', () => { - it('with user added before and editable', async () => { - const note = Note.create(null) as Note; - const group = Group.create('test', 'Testy', false) as Group; + const note = Note.create(null) as Note; + const group1 = Mock.of({ id: 1 }); + const group2 = Mock.of({ id: 2 }); + + it('with editable group', async () => { + const noteGroupPermission1 = NoteGroupPermission.create( + group1, + note, + true, + ); + const noteGroupPermission2 = NoteGroupPermission.create( + group2, + note, + true, + ); note.groupPermissions = Promise.resolve([ - NoteGroupPermission.create(group, note, true), + noteGroupPermission1, + noteGroupPermission2, ]); - const resultNote = await service.removeGroupPermission(note, group); - expect((await resultNote.groupPermissions).length).toStrictEqual(0); - }); - it('with user not added before and not editable', async () => { - const note = Note.create(null) as Note; - const group = Group.create('test', 'Testy', false) as Group; - note.groupPermissions = Promise.resolve([ - NoteGroupPermission.create(group, note, false), + const resultNote = await service.removeGroupPermission(note, group1); + expect(await resultNote.groupPermissions).toStrictEqual([ + noteGroupPermission2, + ]); + }); + it('with not editable group', async () => { + const noteGroupPermission1 = NoteGroupPermission.create( + group1, + note, + false, + ); + const noteGroupPermission2 = NoteGroupPermission.create( + group2, + note, + false, + ); + note.groupPermissions = Promise.resolve([ + noteGroupPermission1, + noteGroupPermission2, + ]); + const resultNote = await service.removeGroupPermission(note, group1); + expect(await resultNote.groupPermissions).toStrictEqual([ + noteGroupPermission2, ]); - const resultNote = await service.removeGroupPermission(note, group); - expect((await resultNote.groupPermissions).length).toStrictEqual(0); }); }); }); diff --git a/backend/src/permissions/permissions.service.ts b/backend/src/permissions/permissions.service.ts index 84706500b..f2f7a79a5 100644 --- a/backend/src/permissions/permissions.service.ts +++ b/backend/src/permissions/permissions.service.ts @@ -8,16 +8,12 @@ import { EventEmitter2 } from '@nestjs/event-emitter'; import { InjectRepository } from '@nestjs/typeorm'; import { Repository } from 'typeorm'; -import { - getGuestAccessOrdinal, - GuestAccess, -} from '../config/guest_access.enum'; +import { GuestAccess } from '../config/guest_access.enum'; import noteConfiguration, { NoteConfig } from '../config/note.config'; import { PermissionsUpdateInconsistentError } from '../errors/errors'; import { NoteEvent, NoteEventMap } from '../events'; import { Group } from '../groups/group.entity'; import { GroupsService } from '../groups/groups.service'; -import { SpecialGroup } from '../groups/groups.special'; import { ConsoleLoggerService } from '../logger/console-logger.service'; import { MediaUpload } from '../media/media-upload.entity'; import { NotePermissionsUpdateDto } from '../notes/note-permissions.dto'; @@ -26,8 +22,11 @@ import { User } from '../users/user.entity'; import { UsersService } from '../users/users.service'; import { checkArrayForDuplicates } from '../utils/arrayDuplicatCheck'; import { NoteGroupPermission } from './note-group-permission.entity'; +import { NotePermission } from './note-permission.enum'; import { NoteUserPermission } from './note-user-permission.entity'; -import { RequiredPermission } from './required-permission.enum'; +import { convertGuestAccessToNotePermission } from './utils/convert-guest-access-to-note-permission'; +import { findHighestNotePermissionByGroup } from './utils/find-highest-note-permission-by-group'; +import { findHighestNotePermissionByUser } from './utils/find-highest-note-permission-by-user'; @Injectable() export class PermissionsService { @@ -40,29 +39,6 @@ export class PermissionsService { private noteConfig: NoteConfig, private eventEmitter: EventEmitter2, ) {} - /** - * Checks if the given {@link User} is has the in {@link desiredPermission} specified permission on {@link Note}. - * - * @async - * @param {RequiredPermission} desiredPermission - permission level to check for - * @param {User} user - The user whose permission should be checked. Value is null if guest access should be checked - * @param {Note} note - The note for which the permission should be checked - * @return if the user has the specified permission on the note - */ - public async checkPermissionOnNote( - desiredPermission: Exclude, - user: User | null, - note: Note, - ): Promise { - switch (desiredPermission) { - case RequiredPermission.READ: - return await this.mayRead(user, note); - case RequiredPermission.WRITE: - return await this.mayWrite(user, note); - case RequiredPermission.OWNER: - return await this.isOwner(user, note); - } - } public async checkMediaDeletePermission( user: User, @@ -77,38 +53,6 @@ export class PermissionsService { return mediaUploadOwner?.id === user.id || owner; } - /** - * Checks if the given {@link User} is allowed to read the given {@link Note}. - * - * @async - * @param {User} user - The user whose permission should be checked. Value is null if guest access should be checked - * @param {Note} note - The note for which the permission should be checked - * @return if the user is allowed to read the note - */ - public async mayRead(user: User | null, note: Note): Promise { - return ( - (await this.isOwner(user, note)) || - (await this.hasPermissionUser(user, note, false)) || - (await this.hasPermissionGroup(user, note, false)) - ); - } - - /** - * Checks if the given {@link User} is allowed to edit the given {@link Note}. - * - * @async - * @param {User} user - The user whose permission should be checked - * @param {Note} note - The note for which the permission should be checked. Value is null if guest access should be checked - * @return if the user is allowed to edit the note - */ - public async mayWrite(user: User | null, note: Note): Promise { - return ( - (await this.isOwner(user, note)) || - (await this.hasPermissionUser(user, note, true)) || - (await this.hasPermissionGroup(user, note, true)) - ); - } - /** * Checks if the given {@link User} is allowed to create notes. * @@ -121,74 +65,77 @@ export class PermissionsService { } async isOwner(user: User | null, note: Note): Promise { - if (!user) return false; - const owner = await note.owner; - if (!owner) return false; - return owner.id === user.id; - } - - // noinspection JSMethodCanBeStatic - private async hasPermissionUser( - user: User | null, - note: Note, - wantEdit: boolean, - ): Promise { if (!user) { return false; } - for (const userPermission of await note.userPermissions) { - if ( - (await userPermission.user).id === user.id && - (userPermission.canEdit || !wantEdit) - ) { - return true; - } + const owner = await note.owner; + if (!owner) { + return false; } - return false; + return owner.id === user.id; } - private async hasPermissionGroup( + /** + * Determines the {@link NotePermission permission} of the user on the given {@link Note}. + * + * @param {User | null} user The user whose permission should be checked + * @param {Note} note The note that is accessed by the given user + * @return {Promise} The determined permission + */ + public async determinePermission( user: User | null, note: Note, - wantEdit: boolean, - ): Promise { + ): Promise { if (user === null) { - if ( - (!wantEdit && - getGuestAccessOrdinal(this.noteConfig.guestAccess) < - getGuestAccessOrdinal(GuestAccess.READ)) || - (wantEdit && - getGuestAccessOrdinal(this.noteConfig.guestAccess) < - getGuestAccessOrdinal(GuestAccess.WRITE)) - ) { - return false; - } + return await this.findGuestNotePermission(await note.groupPermissions); } - for (const groupPermission of await note.groupPermissions) { - if (groupPermission.canEdit || !wantEdit) { - // Handle special groups - if ((await groupPermission.group).special) { - if ( - (user && - (await groupPermission.group).name == SpecialGroup.LOGGED_IN) || - (await groupPermission.group).name == SpecialGroup.EVERYONE - ) { - return true; - } - } else { - // Handle normal groups - if (user) { - for (const member of await ( - await groupPermission.group - ).members) { - if (member.id === user.id) return true; - } - } - } - } + if (await this.isOwner(user, note)) { + return NotePermission.OWNER; } - return false; + const userPermission = await findHighestNotePermissionByUser( + user, + await note.userPermissions, + ); + if (userPermission === NotePermission.WRITE) { + return userPermission; + } + const groupPermission = await findHighestNotePermissionByGroup( + user, + await note.groupPermissions, + ); + return groupPermission > userPermission ? groupPermission : userPermission; + } + + private async findGuestNotePermission( + groupPermissions: NoteGroupPermission[], + ): Promise { + if (this.noteConfig.guestAccess === GuestAccess.DENY) { + return NotePermission.DENY; + } + + const everyonePermission = await this.findPermissionForGroup( + groupPermissions, + await this.groupsService.getEveryoneGroup(), + ); + if (everyonePermission === undefined) { + return NotePermission.DENY; + } + const notePermission = everyonePermission.canEdit + ? NotePermission.WRITE + : NotePermission.READ; + return this.limitNotePermissionToGuestAccessLevel(notePermission); + } + + private limitNotePermissionToGuestAccessLevel( + notePermission: NotePermission, + ): NotePermission { + const configuredGuestNotePermission = convertGuestAccessToNotePermission( + this.noteConfig.guestAccess, + ); + return configuredGuestNotePermission < notePermission + ? configuredGuestNotePermission + : notePermission; } private notifyOthers(note: Note): void { diff --git a/backend/src/permissions/utils/convert-guest-access-to-note-permission.spec.ts b/backend/src/permissions/utils/convert-guest-access-to-note-permission.spec.ts new file mode 100644 index 000000000..04bef2ef7 --- /dev/null +++ b/backend/src/permissions/utils/convert-guest-access-to-note-permission.spec.ts @@ -0,0 +1,34 @@ +/* + * SPDX-FileCopyrightText: 2023 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ +import { GuestAccess } from '../../config/guest_access.enum'; +import { NotePermission } from '../note-permission.enum'; +import { convertGuestAccessToNotePermission } from './convert-guest-access-to-note-permission'; + +describe('convert guest access to note permission', () => { + it('no guest access means no note access', () => { + expect(convertGuestAccessToNotePermission(GuestAccess.DENY)).toBe( + NotePermission.DENY, + ); + }); + + it('translates read access to read permission', () => { + expect(convertGuestAccessToNotePermission(GuestAccess.READ)).toBe( + NotePermission.READ, + ); + }); + + it('translates write access to write permission', () => { + expect(convertGuestAccessToNotePermission(GuestAccess.WRITE)).toBe( + NotePermission.WRITE, + ); + }); + + it('translates create access to write permission', () => { + expect(convertGuestAccessToNotePermission(GuestAccess.CREATE)).toBe( + NotePermission.WRITE, + ); + }); +}); diff --git a/backend/src/permissions/utils/convert-guest-access-to-note-permission.ts b/backend/src/permissions/utils/convert-guest-access-to-note-permission.ts new file mode 100644 index 000000000..39dc7dd35 --- /dev/null +++ b/backend/src/permissions/utils/convert-guest-access-to-note-permission.ts @@ -0,0 +1,28 @@ +/* + * SPDX-FileCopyrightText: 2023 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ +import { GuestAccess } from '../../config/guest_access.enum'; +import { NotePermission } from '../note-permission.enum'; + +/** + * Converts the given guest access level to the highest possible {@link NotePermission}. + * + * @param guestAccess the guest access level to should be converted + * @return the {@link NotePermission} representation + */ +export function convertGuestAccessToNotePermission( + guestAccess: GuestAccess, +): NotePermission.READ | NotePermission.WRITE | NotePermission.DENY { + switch (guestAccess) { + case GuestAccess.DENY: + return NotePermission.DENY; + case GuestAccess.READ: + return NotePermission.READ; + case GuestAccess.WRITE: + return NotePermission.WRITE; + case GuestAccess.CREATE: + return NotePermission.WRITE; + } +} diff --git a/backend/src/permissions/utils/find-highest-note-permission-by-group.spec.ts b/backend/src/permissions/utils/find-highest-note-permission-by-group.spec.ts new file mode 100644 index 000000000..6d5533d62 --- /dev/null +++ b/backend/src/permissions/utils/find-highest-note-permission-by-group.spec.ts @@ -0,0 +1,194 @@ +/* + * SPDX-FileCopyrightText: 2023 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ +import { Mock } from 'ts-mockery'; + +import { Group } from '../../groups/group.entity'; +import { SpecialGroup } from '../../groups/groups.special'; +import { User } from '../../users/user.entity'; +import { NoteGroupPermission } from '../note-group-permission.entity'; +import { NotePermission } from '../note-permission.enum'; +import { findHighestNotePermissionByGroup } from './find-highest-note-permission-by-group'; + +describe('find highest note permission by group', () => { + const user1 = Mock.of({ id: 0 }); + const user2 = Mock.of({ id: 1 }); + const user3 = Mock.of({ id: 2 }); + const group2 = Mock.of({ + id: 1, + special: false, + members: Promise.resolve([user2]), + }); + const group3 = Mock.of({ + id: 2, + special: false, + members: Promise.resolve([user3]), + }); + + const permissionGroup2Read = Mock.of({ + group: Promise.resolve(group2), + canEdit: false, + }); + + const permissionGroup3Read = Mock.of({ + group: Promise.resolve(group3), + canEdit: false, + }); + + const permissionGroup3Write = Mock.of({ + group: Promise.resolve(group3), + canEdit: true, + }); + + describe('normal groups', () => { + it('will fallback to NONE if no permission for the user could be found', async () => { + const result = await findHighestNotePermissionByGroup(user1, [ + permissionGroup2Read, + permissionGroup3Write, + ]); + expect(result).toBe(NotePermission.DENY); + }); + + it('can extract a READ permission for the correct user', async () => { + const result = await findHighestNotePermissionByGroup(user2, [ + permissionGroup2Read, + permissionGroup3Write, + ]); + expect(result).toBe(NotePermission.READ); + }); + + it('can extract a WRITE permission for the correct user', async () => { + const result = await findHighestNotePermissionByGroup(user3, [ + permissionGroup2Read, + permissionGroup3Write, + ]); + expect(result).toBe(NotePermission.WRITE); + }); + + it('can extract a WRITE permission for the correct user if read and write are defined', async () => { + const result = await findHighestNotePermissionByGroup(user3, [ + permissionGroup2Read, + permissionGroup3Read, + permissionGroup3Write, + ]); + expect(result).toBe(NotePermission.WRITE); + }); + }); + + describe('special group', () => { + const groupEveryone = Mock.of({ + id: 3, + special: true, + name: SpecialGroup.EVERYONE, + }); + const groupLoggedIn = Mock.of({ + id: 4, + special: true, + name: SpecialGroup.LOGGED_IN, + }); + const permissionGroupEveryoneRead = Mock.of({ + group: Promise.resolve(groupEveryone), + canEdit: false, + }); + const permissionGroupLoggedInRead = Mock.of({ + group: Promise.resolve(groupLoggedIn), + canEdit: false, + }); + const permissionGroupEveryoneWrite = Mock.of({ + group: Promise.resolve(groupEveryone), + canEdit: true, + }); + const permissionGroupLoggedInWrite = Mock.of({ + group: Promise.resolve(groupLoggedIn), + canEdit: true, + }); + + it('will ignore unknown special groups', async () => { + const nonsenseSpecialGroup = Mock.of({ + id: 99, + special: true, + name: 'Unknown Special Group', + members: Promise.resolve([]), + }); + + const permissionUnknownSpecialGroup = Mock.of({ + group: Promise.resolve(nonsenseSpecialGroup), + canEdit: false, + }); + + const result = await findHighestNotePermissionByGroup(user1, [ + permissionUnknownSpecialGroup, + ]); + expect(result).toBe(NotePermission.DENY); + }); + + it('can extract the READ permission for logged in users', async () => { + const result = await findHighestNotePermissionByGroup(user1, [ + permissionGroupLoggedInRead, + ]); + expect(result).toBe(NotePermission.READ); + }); + + it('can extract the READ permission for everyone', async () => { + const result = await findHighestNotePermissionByGroup(user1, [ + permissionGroupEveryoneRead, + ]); + expect(result).toBe(NotePermission.READ); + }); + it('can extract the WRITE permission for logged in users', async () => { + const result = await findHighestNotePermissionByGroup(user1, [ + permissionGroupLoggedInWrite, + ]); + expect(result).toBe(NotePermission.WRITE); + }); + + it('can extract the WRITE permission for everyone', async () => { + const result = await findHighestNotePermissionByGroup(user1, [ + permissionGroupEveryoneWrite, + ]); + expect(result).toBe(NotePermission.WRITE); + }); + + it('can prefer everyone over logged in if necessary', async () => { + const result = await findHighestNotePermissionByGroup(user1, [ + permissionGroupEveryoneWrite, + permissionGroupLoggedInRead, + ]); + expect(result).toBe(NotePermission.WRITE); + }); + + it('can prefer normal groups over logged in if necessary', async () => { + const result = await findHighestNotePermissionByGroup(user3, [ + permissionGroup3Write, + permissionGroupLoggedInRead, + ]); + expect(result).toBe(NotePermission.WRITE); + }); + + it('can prefer normal groups over everyone if necessary', async () => { + const result = await findHighestNotePermissionByGroup(user3, [ + permissionGroup3Write, + permissionGroupEveryoneRead, + ]); + expect(result).toBe(NotePermission.WRITE); + }); + + it('can prefer logged in over normal groups if necessary', async () => { + const result = await findHighestNotePermissionByGroup(user3, [ + permissionGroup3Read, + permissionGroupLoggedInWrite, + ]); + expect(result).toBe(NotePermission.WRITE); + }); + + it('can prefer everyone over normal groups if necessary', async () => { + const result = await findHighestNotePermissionByGroup(user3, [ + permissionGroup3Read, + permissionGroupEveryoneWrite, + ]); + expect(result).toBe(NotePermission.WRITE); + }); + }); +}); diff --git a/backend/src/permissions/utils/find-highest-note-permission-by-group.ts b/backend/src/permissions/utils/find-highest-note-permission-by-group.ts new file mode 100644 index 000000000..310f14868 --- /dev/null +++ b/backend/src/permissions/utils/find-highest-note-permission-by-group.ts @@ -0,0 +1,62 @@ +/* + * SPDX-FileCopyrightText: 2023 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ +import { Group } from '../../groups/group.entity'; +import { SpecialGroup } from '../../groups/groups.special'; +import { User } from '../../users/user.entity'; +import { NoteGroupPermission } from '../note-group-permission.entity'; +import { NotePermission } from '../note-permission.enum'; + +/** + * Inspects the given note permissions and finds the highest {@link NoteGroupPermission} for the given {@link Group}. + * + * @param user The group whose permissions should be determined + * @param groupPermissions The search basis + * @return The found permission or {@link NotePermission.DENY} if no permission could be found. + * @async + */ +export async function findHighestNotePermissionByGroup( + user: User, + groupPermissions: NoteGroupPermission[], +): Promise { + let highestGroupPermission = NotePermission.DENY; + for (const groupPermission of groupPermissions) { + const permission = await findNotePermissionByGroup(user, groupPermission); + if (permission === NotePermission.WRITE) { + return NotePermission.WRITE; + } + highestGroupPermission = + highestGroupPermission > permission ? highestGroupPermission : permission; + } + return highestGroupPermission; +} + +async function findNotePermissionByGroup( + user: User, + groupPermission: NoteGroupPermission, +): Promise { + const group = await groupPermission.group; + if (!isSpecialGroup(group) && !(await isUserInGroup(user, group))) { + return NotePermission.DENY; + } + return groupPermission.canEdit ? NotePermission.WRITE : NotePermission.READ; +} + +function isSpecialGroup(group: Group): boolean { + return ( + group.special && + (group.name === SpecialGroup.LOGGED_IN || + group.name === SpecialGroup.EVERYONE) + ); +} + +async function isUserInGroup(user: User, group: Group): Promise { + for (const member of await group.members) { + if (member.id === user.id) { + return true; + } + } + return false; +} diff --git a/backend/src/permissions/utils/find-highest-note-permission-by-user.spec.ts b/backend/src/permissions/utils/find-highest-note-permission-by-user.spec.ts new file mode 100644 index 000000000..c59f5b7c7 --- /dev/null +++ b/backend/src/permissions/utils/find-highest-note-permission-by-user.spec.ts @@ -0,0 +1,65 @@ +/* + * SPDX-FileCopyrightText: 2023 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ +import { Mock } from 'ts-mockery'; + +import { User } from '../../users/user.entity'; +import { NotePermission } from '../note-permission.enum'; +import { NoteUserPermission } from '../note-user-permission.entity'; +import { findHighestNotePermissionByUser } from './find-highest-note-permission-by-user'; + +describe('find highest note permission by user', () => { + const user1 = Mock.of({ id: 0 }); + const user2 = Mock.of({ id: 1 }); + const user3 = Mock.of({ id: 2 }); + + const permissionUser2Read = Mock.of({ + user: Promise.resolve(user2), + canEdit: false, + }); + + const permissionUser3Read = Mock.of({ + user: Promise.resolve(user3), + canEdit: false, + }); + + const permissionUser3Write = Mock.of({ + user: Promise.resolve(user3), + canEdit: true, + }); + + it('will fallback to NONE if no permission for the user could be found', async () => { + const result = await findHighestNotePermissionByUser(user1, [ + permissionUser2Read, + permissionUser3Write, + ]); + expect(result).toBe(NotePermission.DENY); + }); + + it('can extract a READ permission for the correct user', async () => { + const result = await findHighestNotePermissionByUser(user2, [ + permissionUser2Read, + permissionUser3Write, + ]); + expect(result).toBe(NotePermission.READ); + }); + + it('can extract a WRITE permission for the correct user', async () => { + const result = await findHighestNotePermissionByUser(user3, [ + permissionUser2Read, + permissionUser3Write, + ]); + expect(result).toBe(NotePermission.WRITE); + }); + + it('can extract a WRITE permission for the correct user if read and write are defined', async () => { + const result = await findHighestNotePermissionByUser(user3, [ + permissionUser2Read, + permissionUser3Read, + permissionUser3Write, + ]); + expect(result).toBe(NotePermission.WRITE); + }); +}); diff --git a/backend/src/permissions/utils/find-highest-note-permission-by-user.ts b/backend/src/permissions/utils/find-highest-note-permission-by-user.ts new file mode 100644 index 000000000..67bb9a799 --- /dev/null +++ b/backend/src/permissions/utils/find-highest-note-permission-by-user.ts @@ -0,0 +1,35 @@ +/* + * SPDX-FileCopyrightText: 2023 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ +import { User } from '../../users/user.entity'; +import { NotePermission } from '../note-permission.enum'; +import { NoteUserPermission } from '../note-user-permission.entity'; + +/** + * Inspects the given note permissions and finds the highest {@link NoteUserPermission} for the given {@link User}. + * + * @param user The user whose permissions should be determined + * @param userPermissions The search basis + * @return The found permission or {@link NotePermission.DENY} if no permission could be found. + * @async + */ +export async function findHighestNotePermissionByUser( + user: User, + userPermissions: NoteUserPermission[], +): Promise { + let hasReadPermission = false; + for (const userPermission of userPermissions) { + if ((await userPermission.user).id !== user.id) { + continue; + } + + if (userPermission.canEdit) { + return NotePermission.WRITE; + } + + hasReadPermission = true; + } + return hasReadPermission ? NotePermission.READ : NotePermission.DENY; +} diff --git a/backend/src/realtime/realtime-note/realtime-note.service.spec.ts b/backend/src/realtime/realtime-note/realtime-note.service.spec.ts index 723362269..2595bb55d 100644 --- a/backend/src/realtime/realtime-note/realtime-note.service.spec.ts +++ b/backend/src/realtime/realtime-note/realtime-note.service.spec.ts @@ -9,6 +9,7 @@ import { Mock } from 'ts-mockery'; import { AppConfig } from '../../config/app.config'; import { ConsoleLoggerService } from '../../logger/console-logger.service'; import { Note } from '../../notes/note.entity'; +import { NotePermission } from '../../permissions/note-permission.enum'; import { PermissionsService } from '../../permissions/permissions.service'; import { Revision } from '../../revisions/revision.entity'; import { RevisionsService } from '../../revisions/revisions.service'; @@ -91,9 +92,15 @@ describe('RealtimeNoteService', () => { mockedAppConfig = Mock.of({ persistInterval: 0 }); mockedPermissionService = Mock.of({ - mayRead: async (user: User) => - [readWriteUsername, onlyReadUsername].includes(user?.username), - mayWrite: async (user: User) => user?.username === readWriteUsername, + determinePermission: async (user: User | null) => { + if (user?.username === readWriteUsername) { + return NotePermission.WRITE; + } else if (user?.username === onlyReadUsername) { + return NotePermission.READ; + } else { + return NotePermission.DENY; + } + }, }); const schedulerRegistry = Mock.of({ diff --git a/backend/src/realtime/realtime-note/realtime-note.service.ts b/backend/src/realtime/realtime-note/realtime-note.service.ts index ad13139fe..d7900dd3b 100644 --- a/backend/src/realtime/realtime-note/realtime-note.service.ts +++ b/backend/src/realtime/realtime-note/realtime-note.service.ts @@ -12,6 +12,7 @@ import appConfiguration, { AppConfig } from '../../config/app.config'; import { NoteEvent } from '../../events'; import { ConsoleLoggerService } from '../../logger/console-logger.service'; import { Note } from '../../notes/note.entity'; +import { NotePermission } from '../../permissions/note-permission.enum'; import { PermissionsService } from '../../permissions/permissions.service'; import { RevisionsService } from '../../revisions/revisions.service'; import { RealtimeConnection } from './realtime-connection'; @@ -126,24 +127,18 @@ export class RealtimeNoteService implements BeforeApplicationShutdown { note: Note, ): Promise { for (const connection of connections) { - if (await this.connectionCanRead(connection, note)) { - connection.acceptEdits = await this.permissionService.mayWrite( - connection.getUser(), - note, - ); - } else { + const permission = await this.permissionService.determinePermission( + connection.getUser(), + note, + ); + if (permission === NotePermission.DENY) { connection.getTransporter().disconnect(); + } else { + connection.acceptEdits = permission > NotePermission.READ; } } } - private async connectionCanRead( - connection: RealtimeConnection, - note: Note, - ): Promise { - return await this.permissionService.mayRead(connection.getUser(), note); - } - @OnEvent(NoteEvent.DELETION) public handleNoteDeleted(noteId: Note['id']): void { const realtimeNote = this.realtimeNoteStore.find(noteId); diff --git a/backend/src/realtime/websocket/websocket.gateway.spec.ts b/backend/src/realtime/websocket/websocket.gateway.spec.ts index dafaafa48..9545e22c3 100644 --- a/backend/src/realtime/websocket/websocket.gateway.spec.ts +++ b/backend/src/realtime/websocket/websocket.gateway.spec.ts @@ -29,6 +29,7 @@ import { NotesModule } from '../../notes/notes.module'; import { NotesService } from '../../notes/notes.service'; import { Tag } from '../../notes/tag.entity'; import { NoteGroupPermission } from '../../permissions/note-group-permission.entity'; +import { NotePermission } from '../../permissions/note-permission.enum'; import { NoteUserPermission } from '../../permissions/note-user-permission.entity'; import { PermissionsModule } from '../../permissions/permissions.module'; import { PermissionsService } from '../../permissions/permissions.service'; @@ -221,15 +222,15 @@ describe('Websocket gateway', () => { }); jest - .spyOn(permissionsService, 'mayRead') + .spyOn(permissionsService, 'determinePermission') .mockImplementation( - (user: User | null, note: Note): Promise => - Promise.resolve( - (user === mockUser && - note === mockedNote && - userHasReadPermissions) || - (user === null && note === mockedGuestNote), - ), + async (user: User | null, note: Note): Promise => + (user === mockUser && + note === mockedNote && + userHasReadPermissions) || + (user === null && note === mockedGuestNote) + ? NotePermission.READ + : NotePermission.DENY, ); const mockedRealtimeNote = Mock.of({ diff --git a/backend/src/realtime/websocket/websocket.gateway.ts b/backend/src/realtime/websocket/websocket.gateway.ts index 5226f5130..7a55e5279 100644 --- a/backend/src/realtime/websocket/websocket.gateway.ts +++ b/backend/src/realtime/websocket/websocket.gateway.ts @@ -14,6 +14,7 @@ import WebSocket from 'ws'; import { ConsoleLoggerService } from '../../logger/console-logger.service'; import { NotesService } from '../../notes/notes.service'; +import { NotePermission } from '../../permissions/note-permission.enum'; import { PermissionsService } from '../../permissions/permissions.service'; import { SessionService } from '../../session/session.service'; import { User } from '../../users/user.entity'; @@ -59,7 +60,11 @@ export class WebsocketGateway implements OnGatewayConnection { const username = user?.username ?? 'guest'; - if (!(await this.permissionsService.mayRead(user, note))) { + const notePermission = await this.permissionsService.determinePermission( + user, + note, + ); + if (notePermission < NotePermission.READ) { //TODO: [mrdrogdrog] inform client about reason of disconnect. this.logger.log( `Access denied to note '${note.id}' for user '${username}'`,