From 4a717551b9c59c0e0762749aeddcb4744bb3189b Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Sat, 30 Oct 2021 23:58:17 +0200 Subject: [PATCH 1/5] feat: add SpecialGroup enum Signed-off-by: Philip Molares --- src/groups/groups.special.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 src/groups/groups.special.ts diff --git a/src/groups/groups.special.ts b/src/groups/groups.special.ts new file mode 100644 index 000000000..f0ca6c79c --- /dev/null +++ b/src/groups/groups.special.ts @@ -0,0 +1,10 @@ +/* + * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ + +export enum SpecialGroup { + LOGGED_IN = '_LOGGED_IN', + EVERYONE = '_EVERYONE', +} From 371c7dfe111bbc921ce021c535bfd573fc3178ba Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Sun, 31 Oct 2021 00:04:12 +0200 Subject: [PATCH 2/5] feat: add createGroup function Signed-off-by: Philip Molares --- src/groups/groups.service.spec.ts | 29 ++++++++++++++++++++++++++++- src/groups/groups.service.ts | 31 ++++++++++++++++++++++++++++++- 2 files changed, 58 insertions(+), 2 deletions(-) diff --git a/src/groups/groups.service.spec.ts b/src/groups/groups.service.spec.ts index 5292d189b..62c0559c7 100644 --- a/src/groups/groups.service.spec.ts +++ b/src/groups/groups.service.spec.ts @@ -9,7 +9,7 @@ import { getRepositoryToken } from '@nestjs/typeorm'; import { Repository } from 'typeorm'; import appConfigMock from '../config/mock/app.config.mock'; -import { NotInDBError } from '../errors/errors'; +import { AlreadyInDBError, NotInDBError } from '../errors/errors'; import { LoggerModule } from '../logger/logger.module'; import { Group } from './group.entity'; import { GroupsService } from './groups.service'; @@ -46,6 +46,33 @@ describe('GroupsService', () => { expect(service).toBeDefined(); }); + describe('createGroup', () => { + const groupName = 'testGroup'; + const displayname = 'Group Test'; + beforeEach(() => { + jest + .spyOn(groupRepo, 'save') + .mockImplementationOnce(async (group: Group): Promise => group); + }); + it('successfully creates a group', async () => { + const user = await service.createGroup(groupName, displayname); + expect(user.name).toEqual(groupName); + expect(user.displayName).toEqual(displayname); + }); + it('fails if group name is already taken', async () => { + // add additional mock implementation for failure + jest.spyOn(groupRepo, 'save').mockImplementationOnce(() => { + throw new Error(); + }); + // create first group with group name + await service.createGroup(groupName, displayname); + // attempt to create second group with group name + await expect(service.createGroup(groupName, displayname)).rejects.toThrow( + AlreadyInDBError, + ); + }); + }); + describe('getGroupByName', () => { it('works', async () => { jest.spyOn(groupRepo, 'findOne').mockResolvedValueOnce(group); diff --git a/src/groups/groups.service.ts b/src/groups/groups.service.ts index a5d35bae8..9e762fd8b 100644 --- a/src/groups/groups.service.ts +++ b/src/groups/groups.service.ts @@ -7,7 +7,7 @@ import { Injectable } from '@nestjs/common'; import { InjectRepository } from '@nestjs/typeorm'; import { Repository } from 'typeorm'; -import { NotInDBError } from '../errors/errors'; +import { AlreadyInDBError, NotInDBError } from '../errors/errors'; import { ConsoleLoggerService } from '../logger/console-logger.service'; import { GroupInfoDto } from './group-info.dto'; import { Group } from './group.entity'; @@ -21,6 +21,35 @@ export class GroupsService { this.logger.setContext(GroupsService.name); } + /** + * @async + * Create a new group with a given name and displayName + * @param name - the group name the new group shall have + * @param displayName - the display name the new group shall have + * @param special - if the group is special or not + * @return {Group} the group + * @throws {AlreadyInDBError} the group name is already taken. + */ + async createGroup( + name: string, + displayName: string, + special = false, + ): Promise { + const group = Group.create(name, displayName); + group.special = special; + try { + return await this.groupRepository.save(group); + } catch { + this.logger.debug( + `A group with the name '${name}' already exists.`, + 'createGroup', + ); + throw new AlreadyInDBError( + `A group with the name '${name}' already exists.`, + ); + } + } + /** * @async * Get a group by their name. From b5ab3d830c0287a0f4a20c514bc9c5a202aab48a Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Sun, 31 Oct 2021 00:05:59 +0200 Subject: [PATCH 3/5] fix: permissions service use new SpecialGroup enum instead of random strings the permissions service now uses the SpecialGroup enum Signed-off-by: Philip Molares --- src/permissions/permissions.service.spec.ts | 37 ++++++++++++++++----- src/permissions/permissions.service.ts | 7 ++-- 2 files changed, 31 insertions(+), 13 deletions(-) diff --git a/src/permissions/permissions.service.spec.ts b/src/permissions/permissions.service.spec.ts index 0d9ef7d92..0ff6729ec 100644 --- a/src/permissions/permissions.service.spec.ts +++ b/src/permissions/permissions.service.spec.ts @@ -11,6 +11,7 @@ import { AuthToken } from '../auth/auth-token.entity'; import { Author } from '../authors/author.entity'; import appConfigMock from '../config/mock/app.config.mock'; import { Group } from '../groups/group.entity'; +import { SpecialGroup } from '../groups/groups.special'; import { Identity } from '../identity/identity.entity'; import { LoggerModule } from '../logger/logger.module'; import { Alias } from '../notes/alias.entity'; @@ -134,7 +135,7 @@ describe('PermissionsService', () => { note7.userPermissions.push(noteUserPermission2); const everybody = {} as Group; - everybody.name = 'everybody'; + everybody.name = SpecialGroup.EVERYONE; everybody.special = true; const noteEverybodyRead = createNote(user1); @@ -261,13 +262,19 @@ describe('PermissionsService', () => { function createGroups(): { [id: string]: Group } { const result: { [id: string]: Group } = {}; - const everybody: Group = Group.create('everybody', 'Everybody'); + const everybody: Group = Group.create( + SpecialGroup.EVERYONE, + SpecialGroup.EVERYONE, + ); everybody.special = true; - result['everybody'] = everybody; + result[SpecialGroup.EVERYONE] = everybody; - const loggedIn = Group.create('loggedIn', 'loggedIn'); + const loggedIn = Group.create( + SpecialGroup.LOGGED_IN, + SpecialGroup.LOGGED_IN, + ); loggedIn.special = true; - result['loggedIn'] = loggedIn; + result[SpecialGroup.LOGGED_IN] = loggedIn; const user1group = Group.create('user1group', 'user1group'); user1group.members = [user1]; @@ -304,11 +311,23 @@ describe('PermissionsService', () => { return NoteGroupPermission.create(group, write); } - const everybodyRead = createNoteGroupPermission(groups['everybody'], false); - const everybodyWrite = createNoteGroupPermission(groups['everybody'], true); + const everybodyRead = createNoteGroupPermission( + groups[SpecialGroup.EVERYONE], + false, + ); + const everybodyWrite = createNoteGroupPermission( + groups[SpecialGroup.EVERYONE], + true, + ); - const loggedInRead = createNoteGroupPermission(groups['loggedIn'], false); - const loggedInWrite = createNoteGroupPermission(groups['loggedIn'], true); + const loggedInRead = createNoteGroupPermission( + groups[SpecialGroup.LOGGED_IN], + false, + ); + const loggedInWrite = createNoteGroupPermission( + groups[SpecialGroup.LOGGED_IN], + true, + ); const user1groupRead = createNoteGroupPermission( groups['user1group'], diff --git a/src/permissions/permissions.service.ts b/src/permissions/permissions.service.ts index e04658659..c64c0356e 100644 --- a/src/permissions/permissions.service.ts +++ b/src/permissions/permissions.service.ts @@ -5,6 +5,7 @@ */ import { Injectable } from '@nestjs/common'; +import { SpecialGroup } from '../groups/groups.special'; import { Note } from '../notes/note.entity'; import { User } from '../users/user.entity'; @@ -102,16 +103,14 @@ export class PermissionsService { if (groupPermission.canEdit || !wantEdit) { // Handle special groups if (groupPermission.group.special) { - if (groupPermission.group.name == 'loggedIn') { - // TODO: Name of group for logged in users + if (groupPermission.group.name == SpecialGroup.LOGGED_IN) { return true; } if ( - groupPermission.group.name == 'everybody' && + groupPermission.group.name == SpecialGroup.EVERYONE && (groupPermission.canEdit || !wantEdit) && guestsAllowed ) { - // TODO: Name of group in which everybody even guests can edit return true; } } else { From 5d6863d03c220203cf86a9f9279c52250fe60d80 Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Sun, 31 Oct 2021 00:06:55 +0200 Subject: [PATCH 4/5] feat: setupSpecialGroups in bootstrap Signed-off-by: Philip Molares --- src/main.ts | 3 +++ src/utils/createSpecialGroups.ts | 34 ++++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+) create mode 100644 src/utils/createSpecialGroups.ts diff --git a/src/main.ts b/src/main.ts index 1fcc74a35..d04da4d99 100644 --- a/src/main.ts +++ b/src/main.ts @@ -14,6 +14,7 @@ import { AuthConfig } from './config/auth.config'; import { MediaConfig } from './config/media.config'; import { ConsoleLoggerService } from './logger/console-logger.service'; import { BackendType } from './media/backends/backend-type.enum'; +import { setupSpecialGroups } from './utils/createSpecialGroups'; import { setupFrontendProxy } from './utils/frontend-integration'; import { setupSessionMiddleware } from './utils/session'; import { setupValidationPipe } from './utils/setup-pipes'; @@ -51,6 +52,8 @@ async function bootstrap(): Promise { setupFrontendProxy(app, logger); } + await setupSpecialGroups(app); + setupSessionMiddleware(app, authConfig); app.enableCors({ diff --git a/src/utils/createSpecialGroups.ts b/src/utils/createSpecialGroups.ts new file mode 100644 index 000000000..fc2be167c --- /dev/null +++ b/src/utils/createSpecialGroups.ts @@ -0,0 +1,34 @@ +/* + * SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file) + * + * SPDX-License-Identifier: AGPL-3.0-only + */ +import { NestExpressApplication } from '@nestjs/platform-express'; + +import { AlreadyInDBError } from '../errors/errors'; +import { GroupsService } from '../groups/groups.service'; +import { SpecialGroup } from '../groups/groups.special'; + +export async function setupSpecialGroups( + app: NestExpressApplication, +): Promise { + const groupService = app.get(GroupsService); + try { + await groupService.createGroup( + SpecialGroup.EVERYONE, + SpecialGroup.EVERYONE, + true, + ); + await groupService.createGroup( + SpecialGroup.LOGGED_IN, + SpecialGroup.LOGGED_IN, + true, + ); + } catch (e) { + if (e instanceof AlreadyInDBError) { + // It's no problem if the special groups already exist + return; + } + throw e; + } +} From 3291b434231fdd29dbd9f3055661fe4e70fb9e02 Mon Sep 17 00:00:00 2001 From: Philip Molares Date: Sat, 6 Nov 2021 11:49:26 +0100 Subject: [PATCH 5/5] docs: fix createUser and test docs this ports the fixes applied to createGroup to this method as well Signed-off-by: Philip Molares --- src/users/users.service.spec.ts | 3 ++- src/users/users.service.ts | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/users/users.service.spec.ts b/src/users/users.service.spec.ts index a7f97659e..693187e74 100644 --- a/src/users/users.service.spec.ts +++ b/src/users/users.service.spec.ts @@ -52,12 +52,13 @@ describe('UsersService', () => { .spyOn(userRepo, 'save') .mockImplementationOnce(async (user: User): Promise => user); }); - it('works', async () => { + it('successfully creates a user', async () => { const user = await service.createUser(username, displayname); expect(user.username).toEqual(username); expect(user.displayName).toEqual(displayname); }); it('fails if username is already taken', async () => { + // add additional mock implementation for failure jest.spyOn(userRepo, 'save').mockImplementationOnce(() => { throw new Error(); }); diff --git a/src/users/users.service.ts b/src/users/users.service.ts index 75b9415b4..16fb5551f 100644 --- a/src/users/users.service.ts +++ b/src/users/users.service.ts @@ -26,7 +26,7 @@ export class UsersService { * @async * Create a new user with a given username and displayName * @param username - the username the new user shall have - * @param displayName - the display the new user shall have + * @param displayName - the display name the new user shall have * @return {User} the user * @throws {AlreadyInDBError} the username is already taken. */