From fd949a77b87cb2025e7e537ba9f42029cd0121c7 Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Sun, 28 Aug 2022 23:51:15 +0200 Subject: [PATCH] feat(permission): use new HD_GUEST_ACCESS config Co-authored-by: Tilman Vatteroth Signed-off-by: Tilman Vatteroth Signed-off-by: Philip Molares Signed-off-by: Tilman Vatteroth Signed-off-by: Philip Molares --- src/config/guest_access.enum.ts | 4 +- src/config/mock/note.config.mock.ts | 4 +- src/frontend-config/frontend-config.dto.ts | 7 +- .../frontend-config.service.spec.ts | 7 +- .../frontend-config.service.ts | 2 +- src/notes/notes.service.spec.ts | 94 ++++--- src/notes/notes.service.ts | 10 +- src/permissions/permissions.service.spec.ts | 245 ++++++++++++------ src/permissions/permissions.service.ts | 69 ++--- 9 files changed, 264 insertions(+), 178 deletions(-) diff --git a/src/config/guest_access.enum.ts b/src/config/guest_access.enum.ts index 79f0066ef..f1724dc25 100644 --- a/src/config/guest_access.enum.ts +++ b/src/config/guest_access.enum.ts @@ -11,9 +11,7 @@ export enum GuestAccess { CREATE = 'create', } -export function getGuestAccessOrdinal( - guestAccess: GuestAccess, -): number { +export function getGuestAccessOrdinal(guestAccess: GuestAccess): number { switch (guestAccess) { case GuestAccess.DENY: return 0; diff --git a/src/config/mock/note.config.mock.ts b/src/config/mock/note.config.mock.ts index 487dd711d..594bf59a7 100644 --- a/src/config/mock/note.config.mock.ts +++ b/src/config/mock/note.config.mock.ts @@ -7,9 +7,8 @@ import { ConfigFactoryKeyHost, registerAs } from '@nestjs/config'; import { ConfigFactory } from '@nestjs/config/dist/interfaces'; import { DefaultAccessPermission } from '../default-access-permission.enum'; -import { DefaultAccessPermission } from '../default-access-permission.enum'; -import { NoteConfig } from '../note.config'; import { GuestAccess } from '../guest_access.enum'; +import { NoteConfig } from '../note.config'; export function createDefaultMockNoteConfig(): NoteConfig { return { @@ -21,6 +20,7 @@ export function createDefaultMockNoteConfig(): NoteConfig { loggedIn: DefaultAccessPermission.WRITE, }, }, + guestAccess: GuestAccess.CREATE, }; } diff --git a/src/frontend-config/frontend-config.dto.ts b/src/frontend-config/frontend-config.dto.ts index 201890342..458caaa4f 100644 --- a/src/frontend-config/frontend-config.dto.ts +++ b/src/frontend-config/frontend-config.dto.ts @@ -14,6 +14,7 @@ import { } from 'class-validator'; import { URL } from 'url'; +import { GuestAccess } from '../config/guest_access.enum'; import { ServerVersion } from '../monitoring/server-status.dto'; import { BaseDto } from '../utils/base.dto.'; @@ -140,10 +141,10 @@ export class IframeCommunicationDto extends BaseDto { export class FrontendConfigDto extends BaseDto { /** - * Is anonymous usage of the instance allowed? + * Maximum access level for guest users */ - @IsBoolean() - allowAnonymous: boolean; + @IsString() + guestAccess: GuestAccess; /** * Are users allowed to register on this instance? diff --git a/src/frontend-config/frontend-config.service.spec.ts b/src/frontend-config/frontend-config.service.spec.ts index 1b3844120..c87c9815b 100644 --- a/src/frontend-config/frontend-config.service.spec.ts +++ b/src/frontend-config/frontend-config.service.spec.ts @@ -13,6 +13,7 @@ import { CustomizationConfig } from '../config/customization.config'; import { DefaultAccessPermission } from '../config/default-access-permission.enum'; import { ExternalServicesConfig } from '../config/external-services.config'; import { GitlabScope, GitlabVersion } from '../config/gitlab.enum'; +import { GuestAccess } from '../config/guest_access.enum'; import { Loglevel } from '../config/loglevel.enum'; import { NoteConfig } from '../config/note.config'; import { LoggerModule } from '../logger/logger.module'; @@ -192,7 +193,7 @@ describe('FrontendConfigService', () => { return { forbiddenNoteIds: [], maxDocumentLength: 200, - guestAccess: true, + guestAccess: GuestAccess.CREATE, permissions: { default: { everyone: DefaultAccessPermission.READ, @@ -358,7 +359,7 @@ describe('FrontendConfigService', () => { const noteConfig: NoteConfig = { forbiddenNoteIds: [], maxDocumentLength: maxDocumentLength, - guestAccess: true, + guestAccess: GuestAccess.CREATE, permissions: { default: { everyone: DefaultAccessPermission.READ, @@ -392,7 +393,7 @@ describe('FrontendConfigService', () => { const service = module.get(FrontendConfigService); const config = await service.getFrontendConfig(); expect(config.allowRegister).toEqual(enableRegister); - expect(config.allowAnonymous).toEqual(noteConfig.guestAccess); + expect(config.guestAccess).toEqual(noteConfig.guestAccess); expect(config.branding.name).toEqual(customName); expect(config.branding.logo).toEqual( customLogo ? new URL(customLogo) : undefined, diff --git a/src/frontend-config/frontend-config.service.ts b/src/frontend-config/frontend-config.service.ts index fe815b8fe..d4e1cf31c 100644 --- a/src/frontend-config/frontend-config.service.ts +++ b/src/frontend-config/frontend-config.service.ts @@ -46,7 +46,7 @@ export class FrontendConfigService { async getFrontendConfig(): Promise { return { - allowAnonymous: this.noteConfig.guestAccess, + guestAccess: this.noteConfig.guestAccess, allowRegister: this.authConfig.local.enableRegister, authProviders: this.getAuthProviders(), branding: this.getBranding(), diff --git a/src/notes/notes.service.spec.ts b/src/notes/notes.service.spec.ts index f439a14a6..57e19f361 100644 --- a/src/notes/notes.service.spec.ts +++ b/src/notes/notes.service.spec.ts @@ -6,7 +6,12 @@ import { ConfigModule, ConfigService } from '@nestjs/config'; import { Test, TestingModule } from '@nestjs/testing'; import { getRepositoryToken } from '@nestjs/typeorm'; -import { DataSource, EntityManager, Repository } from 'typeorm'; +import { + DataSource, + EntityManager, + FindOptionsWhere, + Repository, +} from 'typeorm'; import { AuthToken } from '../auth/auth-token.entity'; import { Author } from '../authors/author.entity'; @@ -80,10 +85,7 @@ describe('NotesService', () => { // eslint-disable-next-line @typescript-eslint/ban-ts-comment // @ts-ignore .mockImplementation(async (note: Note): Promise => note); - jest - .spyOn(groupRepo, 'findOne') - .mockResolvedValueOnce(everyone as Group) - .mockResolvedValueOnce(loggedin as Group); + mockGroupRepo(); const note = await service.createNote(content, null); const revisions = await note.revisions; revisions[0].edits = Promise.resolve([ @@ -146,6 +148,20 @@ describe('NotesService', () => { return [note, user, group]; } + function mockGroupRepo() { + jest.spyOn(groupRepo, 'findOne').mockReset(); + jest.spyOn(groupRepo, 'findOne').mockImplementation((args) => { + const groupName = (args.where as FindOptionsWhere).name; + if (groupName === loggedin.name) { + return Promise.resolve(loggedin as Group); + } else if (groupName === everyone.name) { + return Promise.resolve(everyone as Group); + } else { + return Promise.resolve(null); + } + }); + } + beforeEach(async () => { /** * We need to have *one* userRepo for both the providers array and @@ -255,10 +271,8 @@ describe('NotesService', () => { const config = module.get(ConfigService); const noteConfig = config.get('noteConfig') as NoteConfig; forbiddenNoteId = noteConfig.forbiddenNoteIds[0]; - everyoneDefaultAccessPermission = - noteConfig.permissions.accessDefault.everyone; - loggedinDefaultAccessPermission = - noteConfig.permissions.accessDefault.loggedIn; + everyoneDefaultAccessPermission = noteConfig.permissions.default.everyone; + loggedinDefaultAccessPermission = noteConfig.permissions.default.loggedIn; service = module.get(NotesService); noteRepo = module.get>(getRepositoryToken(Note)); revisionRepo = module.get>( @@ -340,10 +354,7 @@ describe('NotesService', () => { // eslint-disable-next-line @typescript-eslint/ban-ts-comment // @ts-ignore .mockImplementation(async (note: Note): Promise => note); - jest - .spyOn(groupRepo, 'findOne') - .mockResolvedValueOnce(everyone as Group) - .mockResolvedValueOnce(loggedin as Group); + mockGroupRepo(); }); it('without alias, without owner', async () => { const newNote = await service.createNote(content, null); @@ -355,13 +366,17 @@ describe('NotesService', () => { const groupPermissions = await newNote.groupPermissions; expect(groupPermissions).toHaveLength(2); expect(groupPermissions[0].canEdit).toEqual( - everyoneDefaultAccessPermission === DefaultAccessPermission.WRITE, + everyoneDefaultAccessPermission !== DefaultAccessPermission.WRITE, + ); + expect((await groupPermissions[0].group).name).toEqual( + SpecialGroup.EVERYONE, ); - expect(groupPermissions[0].group.name).toEqual(SpecialGroup.EVERYONE); expect(groupPermissions[1].canEdit).toEqual( loggedinDefaultAccessPermission === DefaultAccessPermission.WRITE, ); - expect(groupPermissions[1].group.name).toEqual(SpecialGroup.LOGGED_IN); + expect((await groupPermissions[1].group).name).toEqual( + SpecialGroup.LOGGED_IN, + ); expect(await newNote.tags).toHaveLength(0); expect(await newNote.owner).toBeNull(); expect(await newNote.aliases).toHaveLength(0); @@ -379,11 +394,15 @@ describe('NotesService', () => { expect(groupPermissions[0].canEdit).toEqual( everyoneDefaultAccessPermission === DefaultAccessPermission.WRITE, ); - expect(groupPermissions[0].group.name).toEqual(SpecialGroup.EVERYONE); + expect((await groupPermissions[0].group).name).toEqual( + SpecialGroup.EVERYONE, + ); expect(groupPermissions[1].canEdit).toEqual( loggedinDefaultAccessPermission === DefaultAccessPermission.WRITE, ); - expect(groupPermissions[1].group.name).toEqual(SpecialGroup.LOGGED_IN); + expect((await groupPermissions[1].group).name).toEqual( + SpecialGroup.LOGGED_IN, + ); expect(await newNote.tags).toHaveLength(0); expect(await newNote.owner).toEqual(user); expect(await newNote.aliases).toHaveLength(0); @@ -398,13 +417,17 @@ describe('NotesService', () => { const groupPermissions = await newNote.groupPermissions; expect(groupPermissions).toHaveLength(2); expect(groupPermissions[0].canEdit).toEqual( - everyoneDefaultAccessPermission === DefaultAccessPermission.WRITE, + everyoneDefaultAccessPermission !== DefaultAccessPermission.WRITE, + ); + expect((await groupPermissions[0].group).name).toEqual( + SpecialGroup.EVERYONE, ); - expect(groupPermissions[0].group.name).toEqual(SpecialGroup.EVERYONE); expect(groupPermissions[1].canEdit).toEqual( loggedinDefaultAccessPermission === DefaultAccessPermission.WRITE, ); - expect(groupPermissions[1].group.name).toEqual(SpecialGroup.LOGGED_IN); + expect((await groupPermissions[1].group).name).toEqual( + SpecialGroup.LOGGED_IN, + ); expect(await newNote.tags).toHaveLength(0); expect(await newNote.owner).toBeNull(); expect(await newNote.aliases).toHaveLength(1); @@ -422,11 +445,15 @@ describe('NotesService', () => { expect(groupPermissions[0].canEdit).toEqual( everyoneDefaultAccessPermission === DefaultAccessPermission.WRITE, ); - expect(groupPermissions[0].group.name).toEqual(SpecialGroup.EVERYONE); + expect((await groupPermissions[0].group).name).toEqual( + SpecialGroup.EVERYONE, + ); expect(groupPermissions[1].canEdit).toEqual( loggedinDefaultAccessPermission === DefaultAccessPermission.WRITE, ); - expect(groupPermissions[1].group.name).toEqual(SpecialGroup.LOGGED_IN); + expect((await groupPermissions[1].group).name).toEqual( + SpecialGroup.LOGGED_IN, + ); expect(await newNote.tags).toHaveLength(0); expect(await newNote.owner).toEqual(user); expect(await newNote.aliases).toHaveLength(1); @@ -435,27 +462,24 @@ describe('NotesService', () => { describe('with other', () => { beforeEach( () => - (noteMockConfig.permissions.accessDefault.everyone = + (noteMockConfig.permissions.default.everyone = DefaultAccessPermission.NONE), ); it('default permissions', async () => { - jest.spyOn(groupRepo, 'findOne').mockReset(); - jest - .spyOn(groupRepo, 'findOne') - .mockResolvedValueOnce(loggedin as Group); + mockGroupRepo(); const newNote = await service.createNote(content, user, alias); const revisions = await newNote.revisions; expect(revisions).toHaveLength(1); expect(revisions[0].content).toEqual(content); expect(await newNote.historyEntries).toHaveLength(1); - expect((await newNote.historyEntries)[0].user).toEqual(user); + expect(await (await newNote.historyEntries)[0].user).toEqual(user); expect(await newNote.userPermissions).toHaveLength(0); const groupPermissions = await newNote.groupPermissions; expect(groupPermissions).toHaveLength(1); expect(groupPermissions[0].canEdit).toEqual( loggedinDefaultAccessPermission === DefaultAccessPermission.WRITE, ); - expect(groupPermissions[0].group.name).toEqual( + expect((await groupPermissions[0].group).name).toEqual( SpecialGroup.LOGGED_IN, ); expect(await newNote.tags).toHaveLength(0); @@ -473,10 +497,7 @@ describe('NotesService', () => { }); it('alias is already used', async () => { - jest - .spyOn(groupRepo, 'findOne') - .mockResolvedValueOnce(everyone as Group) - .mockResolvedValueOnce(loggedin as Group); + mockGroupRepo(); jest.spyOn(noteRepo, 'save').mockImplementationOnce(async () => { throw new Error(); }); @@ -495,10 +516,7 @@ describe('NotesService', () => { // eslint-disable-next-line @typescript-eslint/ban-ts-comment // @ts-ignore .mockImplementation(async (note: Note): Promise => note); - jest - .spyOn(groupRepo, 'findOne') - .mockResolvedValueOnce(everyone as Group) - .mockResolvedValueOnce(loggedin as Group); + mockGroupRepo(); const newNote = await service.createNote(content, null); const revisions = await newNote.revisions; jest.spyOn(revisionRepo, 'findOne').mockResolvedValueOnce(revisions[0]); diff --git a/src/notes/notes.service.ts b/src/notes/notes.service.ts index 48b8a1ef4..d8fb8ceb3 100644 --- a/src/notes/notes.service.ts +++ b/src/notes/notes.service.ts @@ -3,6 +3,7 @@ * * SPDX-License-Identifier: AGPL-3.0-only */ +import { Optional } from '@mrdrogdrog/optional'; import { forwardRef, Inject, Injectable } from '@nestjs/common'; import { InjectRepository } from '@nestjs/typeorm'; import { Repository } from 'typeorm'; @@ -14,8 +15,8 @@ import { ForbiddenIdError, NotInDBError, } from '../errors/errors'; +import { Group } from '../groups/group.entity'; import { GroupsService } from '../groups/groups.service'; -import { SpecialGroup } from '../groups/groups.special'; import { HistoryEntry } from '../history/history-entry.entity'; import { ConsoleLoggerService } from '../logger/console-logger.service'; import { NoteGroupPermission } from '../permissions/note-group-permission.entity'; @@ -63,13 +64,6 @@ export class NotesService { async getUserNotes(user: User): Promise { const notes = await this.noteRepository .createQueryBuilder('note') - .leftJoinAndSelect('note.aliases', 'alias') - .leftJoinAndSelect('note.owner', 'owner') - .leftJoinAndSelect('note.groupPermissions', 'group_permission') - .leftJoinAndSelect('group_permission.group', 'group') - .leftJoinAndSelect('note.userPermissions', 'user_permission') - .leftJoinAndSelect('user_permission.user', 'user') - .leftJoinAndSelect('note.tags', 'tag') .where('note.owner = :user', { user: user.id }) .getMany(); if (notes === null) { diff --git a/src/permissions/permissions.service.spec.ts b/src/permissions/permissions.service.spec.ts index 13ac95932..caabf2fca 100644 --- a/src/permissions/permissions.service.spec.ts +++ b/src/permissions/permissions.service.spec.ts @@ -10,10 +10,16 @@ import { DataSource, EntityManager, Repository } from 'typeorm'; import { AuthToken } from '../auth/auth-token.entity'; import { Author } from '../authors/author.entity'; +import { DefaultAccessPermission } from '../config/default-access-permission.enum'; +import { GuestAccess } from '../config/guest_access.enum'; import appConfigMock from '../config/mock/app.config.mock'; import authConfigMock from '../config/mock/auth.config.mock'; import databaseConfigMock from '../config/mock/database.config.mock'; -import noteConfigMock from '../config/mock/note.config.mock'; +import { + createDefaultMockNoteConfig, + registerNoteConfig, +} from '../config/mock/note.config.mock'; +import { NoteConfig } from '../config/note.config'; import { PermissionsUpdateInconsistentError } from '../errors/errors'; import { Group } from '../groups/group.entity'; import { GroupsModule } from '../groups/groups.module'; @@ -36,7 +42,7 @@ import { UsersModule } from '../users/users.module'; import { NoteGroupPermission } from './note-group-permission.entity'; import { NoteUserPermission } from './note-user-permission.entity'; import { PermissionsModule } from './permissions.module'; -import { GuestPermission, PermissionsService } from './permissions.service'; +import { PermissionsService } from './permissions.service'; describe('PermissionsService', () => { let service: PermissionsService; @@ -44,6 +50,7 @@ describe('PermissionsService', () => { let noteRepo: Repository; let userRepo: Repository; let groupRepo: Repository; + const noteMockConfig: NoteConfig = createDefaultMockNoteConfig(); beforeAll(async () => { /** @@ -98,7 +105,7 @@ describe('PermissionsService', () => { appConfigMock, databaseConfigMock, authConfigMock, - noteConfigMock, + registerNoteConfig(noteMockConfig), ], }), GroupsModule, @@ -199,8 +206,11 @@ describe('PermissionsService', () => { const everybody = {} as Group; everybody.name = SpecialGroup.EVERYONE; everybody.special = true; - const noteEverybodyRead = createNote(user1); + const noteEverybodyNone = createNote(user1); + noteEverybodyNone.groupPermissions = Promise.resolve([]); + + const noteEverybodyRead = createNote(user1); const noteGroupPermissionRead = {} as NoteGroupPermission; noteGroupPermissionRead.group = Promise.resolve(everybody); noteGroupPermissionRead.canEdit = false; @@ -210,7 +220,6 @@ describe('PermissionsService', () => { ]); const noteEverybodyWrite = createNote(user1); - const noteGroupPermissionWrite = {} as NoteGroupPermission; noteGroupPermissionWrite.group = Promise.resolve(everybody); noteGroupPermissionWrite.canEdit = true; @@ -230,23 +239,21 @@ describe('PermissionsService', () => { note7, noteEverybodyRead, noteEverybodyWrite, + noteEverybodyNone, ]; } describe('mayRead works with', () => { it('Owner', async () => { - service.guestPermission = GuestPermission.DENY; expect(await service.mayRead(user1, notes[0])).toBeTruthy(); expect(await service.mayRead(user1, notes[7])).toBeFalsy(); }); it('userPermission read', async () => { - service.guestPermission = GuestPermission.DENY; 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 () => { - service.guestPermission = GuestPermission.DENY; expect(await service.mayRead(user1, notes[4])).toBeTruthy(); expect(await service.mayRead(user1, notes[5])).toBeTruthy(); expect(await service.mayRead(user1, notes[6])).toBeTruthy(); @@ -254,59 +261,153 @@ describe('PermissionsService', () => { }); describe('guest permission', () => { - it('CREATE_ALIAS', async () => { - service.guestPermission = GuestPermission.CREATE_ALIAS; - expect(await service.mayRead(null, notes[8])).toBeTruthy(); + beforeEach(() => { + noteMockConfig.permissions.default.loggedIn = + DefaultAccessPermission.WRITE; + noteMockConfig.permissions.default.everyone = + DefaultAccessPermission.WRITE; }); - it('CREATE', async () => { - service.guestPermission = GuestPermission.CREATE; - expect(await service.mayRead(null, notes[8])).toBeTruthy(); + 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(); + }); }); - it('WRITE', async () => { - service.guestPermission = GuestPermission.WRITE; - expect(await service.mayRead(null, notes[8])).toBeTruthy(); + 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(); + }); }); - it('READ', async () => { - service.guestPermission = GuestPermission.READ; - expect(await service.mayRead(null, notes[8])).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 () => { - service.guestPermission = GuestPermission.DENY; expect(await service.mayWrite(user1, notes[0])).toBeTruthy(); expect(await service.mayWrite(user1, notes[7])).toBeFalsy(); }); it('userPermission read', async () => { - service.guestPermission = GuestPermission.DENY; 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 () => { - service.guestPermission = GuestPermission.DENY; 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', () => { - it('CREATE_ALIAS', async () => { - service.guestPermission = GuestPermission.CREATE_ALIAS; - expect(await service.mayWrite(null, notes[9])).toBeTruthy(); + beforeEach(() => { + noteMockConfig.permissions.default.loggedIn = + DefaultAccessPermission.WRITE; + noteMockConfig.permissions.default.everyone = + DefaultAccessPermission.WRITE; }); - it('CREATE', async () => { - service.guestPermission = GuestPermission.CREATE; - expect(await service.mayWrite(null, notes[9])).toBeTruthy(); + + 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(); + }); }); - it('WRITE', async () => { - service.guestPermission = GuestPermission.WRITE; - expect(await service.mayWrite(null, notes[9])).toBeTruthy(); + + 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(); + }); }); - it('READ', async () => { - service.guestPermission = GuestPermission.READ; - 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(); + }); }); }); }); @@ -326,19 +427,17 @@ describe('PermissionsService', () => { function createGroups(): { [id: string]: Group } { const result: { [id: string]: Group } = {}; - const everybody: Group = Group.create( + result[SpecialGroup.EVERYONE] = Group.create( SpecialGroup.EVERYONE, SpecialGroup.EVERYONE, true, ) as Group; - result[SpecialGroup.EVERYONE] = everybody; - const loggedIn = Group.create( + result[SpecialGroup.LOGGED_IN] = Group.create( SpecialGroup.LOGGED_IN, SpecialGroup.LOGGED_IN, true, ) as Group; - result[SpecialGroup.LOGGED_IN] = loggedIn; const user1group = Group.create('user1group', 'user1group', false) as Group; user1group.members = Promise.resolve([user1]); @@ -370,7 +469,7 @@ describe('PermissionsService', () => { /* * Create all GroupPermissions: For each group two GroupPermissions are created one with read permission and one with write permission. */ - function createAllNoteGroupPermissions(): NoteGroupPermission[][] { + function createAllNoteGroupPermissions(): (NoteGroupPermission | null)[][] { const groups = createGroups(); /* @@ -450,7 +549,7 @@ describe('PermissionsService', () => { * creates the matrix multiplication of group0 to group4 of createAllNoteGroupPermissions */ function createNoteGroupPermissionsCombinations( - guestPermission: GuestPermission, + everyoneDefaultPermission: DefaultAccessPermission, ): NoteGroupPermissionWithResultForUser[] { // for logged in users const noteGroupPermissions = createAllNoteGroupPermissions(); @@ -476,16 +575,15 @@ describe('PermissionsService', () => { } if (group2 !== null) { - // everybody group TODO config options - switch (guestPermission) { - case GuestPermission.CREATE_ALIAS: - case GuestPermission.CREATE: - case GuestPermission.WRITE: - writePermission = writePermission || group2.canEdit; - readPermission = true; - break; - case GuestPermission.READ: - readPermission = true; + if ( + everyoneDefaultPermission === DefaultAccessPermission.WRITE + ) { + writePermission = writePermission || group2.canEdit; + readPermission = true; + } else if ( + everyoneDefaultPermission === DefaultAccessPermission.READ + ) { + readPermission = true; } insert.push(group2); } @@ -558,9 +656,9 @@ describe('PermissionsService', () => { } describe('check if groups work with', () => { - const guestPermission = GuestPermission.WRITE; - const rawPermissions = - createNoteGroupPermissionsCombinations(guestPermission); + const rawPermissions = createNoteGroupPermissionsCombinations( + DefaultAccessPermission.WRITE, + ); const permissions = permuteNoteGroupPermissions(rawPermissions); let i = 0; for (const permission of permissions) { @@ -571,13 +669,11 @@ describe('PermissionsService', () => { permissionString += ` ${perm.id}:${String(perm.canEdit)}`; } it(`mayWrite - test #${i}:${permissionString}`, async () => { - service.guestPermission = guestPermission; expect(await service.mayWrite(user1, note)).toEqual( permission.allowsWrite, ); }); it(`mayRead - test #${i}:${permissionString}`, async () => { - service.guestPermission = guestPermission; expect(await service.mayRead(user1, note)).toEqual( permission.allowsRead, ); @@ -586,40 +682,33 @@ describe('PermissionsService', () => { } }); - describe('mayCreate works for', () => { - it('logged in', () => { - service.guestPermission = GuestPermission.DENY; + describe('mayCreate', () => { + it('allows creation for logged in', () => { expect(service.mayCreate(user1)).toBeTruthy(); }); - it('guest denied', () => { - service.guestPermission = GuestPermission.DENY; - expect(service.mayCreate(null)).toBeFalsy(); - }); - it('guest read', () => { - service.guestPermission = GuestPermission.READ; - expect(service.mayCreate(null)).toBeFalsy(); - }); - it('guest write', () => { - service.guestPermission = GuestPermission.WRITE; - expect(service.mayCreate(null)).toBeFalsy(); - }); - it('guest create', () => { - service.guestPermission = GuestPermission.CREATE; + it('allows creation of notes for guests with permission', () => { + noteMockConfig.guestAccess = GuestAccess.CREATE; + noteMockConfig.permissions.default.loggedIn = + DefaultAccessPermission.WRITE; + noteMockConfig.permissions.default.everyone = + DefaultAccessPermission.WRITE; expect(service.mayCreate(null)).toBeTruthy(); }); - it('guest create alias', () => { - service.guestPermission = GuestPermission.CREATE_ALIAS; - expect(service.mayCreate(null)).toBeTruthy(); + it('denies creation of notes for guests without permission', () => { + noteMockConfig.guestAccess = GuestAccess.WRITE; + noteMockConfig.permissions.default.loggedIn = + DefaultAccessPermission.WRITE; + noteMockConfig.permissions.default.everyone = + DefaultAccessPermission.WRITE; + expect(service.mayCreate(null)).toBeFalsy(); }); }); describe('isOwner works', () => { it('for positive case', async () => { - service.guestPermission = GuestPermission.DENY; expect(await service.isOwner(user1, notes[0])).toBeTruthy(); }); it('for negative case', async () => { - service.guestPermission = GuestPermission.DENY; expect(await service.isOwner(user1, notes[1])).toBeFalsy(); }); }); diff --git a/src/permissions/permissions.service.ts b/src/permissions/permissions.service.ts index a28d7873d..6518f767f 100644 --- a/src/permissions/permissions.service.ts +++ b/src/permissions/permissions.service.ts @@ -3,10 +3,15 @@ * * SPDX-License-Identifier: AGPL-3.0-only */ -import { Injectable } from '@nestjs/common'; +import { Inject, Injectable } from '@nestjs/common'; import { InjectRepository } from '@nestjs/typeorm'; import { Repository } from 'typeorm'; +import { + getGuestAccessOrdinal, + GuestAccess, +} from '../config/guest_access.enum'; +import noteConfiguration, { NoteConfig } from '../config/note.config'; import { PermissionsUpdateInconsistentError } from '../errors/errors'; import { Group } from '../groups/group.entity'; import { GroupsService } from '../groups/groups.service'; @@ -20,26 +25,17 @@ import { checkArrayForDuplicates } from '../utils/arrayDuplicatCheck'; import { NoteGroupPermission } from './note-group-permission.entity'; import { NoteUserPermission } from './note-user-permission.entity'; -// TODO move to config or remove -export enum GuestPermission { - DENY = 'deny', - READ = 'read', - WRITE = 'write', - CREATE = 'create', - CREATE_ALIAS = 'createAlias', -} - @Injectable() export class PermissionsService { constructor( - public usersService: UsersService, - public groupsService: GroupsService, + private usersService: UsersService, + private groupsService: GroupsService, @InjectRepository(Note) private noteRepository: Repository, private readonly logger: ConsoleLoggerService, + @Inject(noteConfiguration.KEY) + private noteConfig: NoteConfig, ) {} - public guestPermission: GuestPermission; // TODO change to configOption - /** * Checks if the given {@link User} is allowed to read the given {@link Note}. * @@ -80,18 +76,7 @@ export class PermissionsService { * @return if the user is allowed to create notes */ public mayCreate(user: User | null): boolean { - if (user) { - return true; - } else { - if ( - this.guestPermission == GuestPermission.CREATE || - this.guestPermission == GuestPermission.CREATE_ALIAS - ) { - // TODO change to guestPermission to config option - return true; - } - } - return false; + return user !== null || this.noteConfig.guestAccess === GuestAccess.CREATE; } async isOwner(user: User | null, note: Note): Promise { @@ -101,6 +86,7 @@ export class PermissionsService { return owner.id === user.id; } + // noinspection JSMethodCanBeStatic private async hasPermissionUser( user: User | null, note: Note, @@ -125,28 +111,27 @@ export class PermissionsService { note: Note, wantEdit: boolean, ): Promise { - // TODO: Get real config value - let guestsAllowed = false; - switch (this.guestPermission) { - case GuestPermission.CREATE_ALIAS: - case GuestPermission.CREATE: - case GuestPermission.WRITE: - guestsAllowed = true; - break; - case GuestPermission.READ: - guestsAllowed = !wantEdit; + if (user === null) { + if ( + (!wantEdit && + getGuestAccessOrdinal(this.noteConfig.guestAccess) < + getGuestAccessOrdinal(GuestAccess.READ)) || + (wantEdit && + getGuestAccessOrdinal(this.noteConfig.guestAccess) < + getGuestAccessOrdinal(GuestAccess.WRITE)) + ) { + return false; + } } + for (const groupPermission of await note.groupPermissions) { if (groupPermission.canEdit || !wantEdit) { // Handle special groups if ((await groupPermission.group).special) { - if ((await groupPermission.group).name == SpecialGroup.LOGGED_IN) { - return true; - } if ( - (await groupPermission.group).name == SpecialGroup.EVERYONE && - (groupPermission.canEdit || !wantEdit) && - guestsAllowed + (user && + (await groupPermission.group).name == SpecialGroup.LOGGED_IN) || + (await groupPermission.group).name == SpecialGroup.EVERYONE ) { return true; }