Merge pull request #1844 from hedgedoc/feat/permissionsGuard

This commit is contained in:
Yannick Bungers 2021-12-02 21:13:43 +01:00 committed by GitHub
commit 85aa3324f4
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 204 additions and 126 deletions

View file

@ -24,9 +24,9 @@ import { HistoryEntryDto } from '../../../../history/history-entry.dto';
import { HistoryService } from '../../../../history/history.service';
import { SessionGuard } from '../../../../identity/session.guard';
import { ConsoleLoggerService } from '../../../../logger/console-logger.service';
import { GetNotePipe } from '../../../../notes/get-note.pipe';
import { Note } from '../../../../notes/note.entity';
import { User } from '../../../../users/user.entity';
import { GetNotePipe } from '../../../utils/get-note.pipe';
import { RequestUser } from '../../../utils/request-user.decorator';
@UseGuards(SessionGuard)

View file

@ -13,7 +13,6 @@ import {
NotFoundException,
Param,
Post,
UnauthorizedException,
UseGuards,
} from '@nestjs/common';
@ -27,18 +26,20 @@ import { SessionGuard } from '../../../identity/session.guard';
import { ConsoleLoggerService } from '../../../logger/console-logger.service';
import { MediaUploadDto } from '../../../media/media-upload.dto';
import { MediaService } from '../../../media/media.service';
import { GetNotePipe } from '../../../notes/get-note.pipe';
import { NoteDto } from '../../../notes/note.dto';
import { Note } from '../../../notes/note.entity';
import { NoteMediaDeletionDto } from '../../../notes/note.media-deletion.dto';
import { NotesService } from '../../../notes/notes.service';
import { PermissionsService } from '../../../permissions/permissions.service';
import { Permissions } from '../../../permissions/permissions.decorator';
import { Permission } from '../../../permissions/permissions.enum';
import { RevisionMetadataDto } from '../../../revisions/revision-metadata.dto';
import { RevisionDto } from '../../../revisions/revision.dto';
import { RevisionsService } from '../../../revisions/revisions.service';
import { User } from '../../../users/user.entity';
import { UsersService } from '../../../users/users.service';
import { GetNotePipe } from '../../utils/get-note.pipe';
import { MarkdownBody } from '../../utils/markdownbody-decorator';
import { PermissionsGuard } from '../../utils/permissions.guard';
import { RequestUser } from '../../utils/request-user.decorator';
@UseGuards(SessionGuard)
@ -47,7 +48,6 @@ export class NotesController {
constructor(
private readonly logger: ConsoleLoggerService,
private noteService: NotesService,
private permissionsService: PermissionsService,
private historyService: HistoryService,
private userService: UsersService,
private mediaService: MediaService,
@ -57,38 +57,34 @@ export class NotesController {
}
@Get(':noteIdOrAlias')
@Permissions(Permission.READ)
@UseGuards(PermissionsGuard)
async getNote(
@RequestUser() user: User,
@Param('noteIdOrAlias', GetNotePipe) note: Note,
): Promise<NoteDto> {
if (!this.permissionsService.mayRead(user, note)) {
throw new UnauthorizedException('Reading note denied!');
}
await this.historyService.updateHistoryEntryTimestamp(note, user);
return await this.noteService.toNoteDto(note);
}
@Get(':noteIdOrAlias/media')
@Permissions(Permission.READ)
@UseGuards(PermissionsGuard)
async getNotesMedia(
@Param('noteIdOrAlias', GetNotePipe) note: Note,
@RequestUser() user: User,
): Promise<MediaUploadDto[]> {
if (!this.permissionsService.mayRead(user, note)) {
throw new UnauthorizedException('Reading note denied!');
}
const media = await this.mediaService.listUploadsByNote(note);
return media.map((media) => this.mediaService.toMediaUploadDto(media));
}
@Post()
@HttpCode(201)
@Permissions(Permission.CREATE)
@UseGuards(PermissionsGuard)
async createNote(
@RequestUser() user: User,
@MarkdownBody() text: string,
): Promise<NoteDto> {
if (!this.permissionsService.mayCreate(user)) {
throw new UnauthorizedException('Creating note denied!');
}
this.logger.debug('Got raw markdown:\n' + text);
return await this.noteService.toNoteDto(
await this.noteService.createNote(text, user),
@ -97,14 +93,13 @@ export class NotesController {
@Post(':noteAlias')
@HttpCode(201)
@Permissions(Permission.CREATE)
@UseGuards(PermissionsGuard)
async createNamedNote(
@RequestUser() user: User,
@Param('noteAlias') noteAlias: string,
@MarkdownBody() text: string,
): Promise<NoteDto> {
if (!this.permissionsService.mayCreate(user)) {
throw new UnauthorizedException('Creating note denied!');
}
this.logger.debug('Got raw markdown:\n' + text, 'createNamedNote');
try {
return await this.noteService.toNoteDto(
@ -123,14 +118,13 @@ export class NotesController {
@Delete(':noteIdOrAlias')
@HttpCode(204)
@Permissions(Permission.OWNER)
@UseGuards(PermissionsGuard)
async deleteNote(
@RequestUser() user: User,
@Param('noteIdOrAlias', GetNotePipe) note: Note,
@Body() noteMediaDeletionDto: NoteMediaDeletionDto,
): Promise<void> {
if (!this.permissionsService.isOwner(user, note)) {
throw new UnauthorizedException('Deleting note denied!');
}
const mediaUploads = await this.mediaService.listUploadsByNote(note);
for (const mediaUpload of mediaUploads) {
if (!noteMediaDeletionDto.keepMedia) {
@ -146,13 +140,12 @@ export class NotesController {
}
@Get(':noteIdOrAlias/revisions')
@Permissions(Permission.READ)
@UseGuards(PermissionsGuard)
async getNoteRevisions(
@RequestUser() user: User,
@Param('noteIdOrAlias', GetNotePipe) note: Note,
): Promise<RevisionMetadataDto[]> {
if (!this.permissionsService.mayRead(user, note)) {
throw new UnauthorizedException('Reading note denied!');
}
const revisions = await this.revisionsService.getAllRevisions(note);
return await Promise.all(
revisions.map((revision) =>
@ -163,13 +156,12 @@ export class NotesController {
@Delete(':noteIdOrAlias/revisions')
@HttpCode(204)
@Permissions(Permission.READ)
@UseGuards(PermissionsGuard)
async purgeNoteRevisions(
@RequestUser() user: User,
@Param('noteIdOrAlias', GetNotePipe) note: Note,
): Promise<void> {
if (!this.permissionsService.mayRead(user, note)) {
throw new UnauthorizedException('Reading note denied!');
}
this.logger.debug(
'Purging history of note: ' + note.id,
'purgeNoteRevisions',
@ -183,15 +175,14 @@ export class NotesController {
}
@Get(':noteIdOrAlias/revisions/:revisionId')
@Permissions(Permission.READ)
@UseGuards(PermissionsGuard)
async getNoteRevision(
@RequestUser() user: User,
@Param('noteIdOrAlias', GetNotePipe) note: Note,
@Param('revisionId') revisionId: number,
): Promise<RevisionDto> {
try {
if (!this.permissionsService.mayRead(user, note)) {
throw new UnauthorizedException('Reading note denied!');
}
return this.revisionsService.toRevisionDto(
await this.revisionsService.getRevision(note, revisionId),
);

View file

@ -31,7 +31,6 @@ import { HistoryService } from '../../../history/history.service';
import { ConsoleLoggerService } from '../../../logger/console-logger.service';
import { MediaUploadDto } from '../../../media/media-upload.dto';
import { MediaService } from '../../../media/media.service';
import { GetNotePipe } from '../../../notes/get-note.pipe';
import { NoteMetadataDto } from '../../../notes/note-metadata.dto';
import { Note } from '../../../notes/note.entity';
import { NotesService } from '../../../notes/notes.service';
@ -43,6 +42,7 @@ import {
successfullyDeletedDescription,
unauthorizedDescription,
} from '../../utils/descriptions';
import { GetNotePipe } from '../../utils/get-note.pipe';
import { RequestUser } from '../../utils/request-user.decorator';
@ApiTags('me')

View file

@ -15,7 +15,6 @@ import {
Param,
Post,
Put,
UnauthorizedException,
UseGuards,
} from '@nestjs/common';
import {
@ -39,7 +38,6 @@ import { HistoryService } from '../../../history/history.service';
import { ConsoleLoggerService } from '../../../logger/console-logger.service';
import { MediaUploadDto } from '../../../media/media-upload.dto';
import { MediaService } from '../../../media/media.service';
import { GetNotePipe } from '../../../notes/get-note.pipe';
import { NoteMetadataDto } from '../../../notes/note-metadata.dto';
import {
NotePermissionsDto,
@ -49,7 +47,8 @@ import { NoteDto } from '../../../notes/note.dto';
import { Note } from '../../../notes/note.entity';
import { NoteMediaDeletionDto } from '../../../notes/note.media-deletion.dto';
import { NotesService } from '../../../notes/notes.service';
import { PermissionsService } from '../../../permissions/permissions.service';
import { Permissions } from '../../../permissions/permissions.decorator';
import { Permission } from '../../../permissions/permissions.enum';
import { RevisionMetadataDto } from '../../../revisions/revision-metadata.dto';
import { RevisionDto } from '../../../revisions/revision.dto';
import { RevisionsService } from '../../../revisions/revisions.service';
@ -60,7 +59,9 @@ import {
unauthorizedDescription,
} from '../../utils/descriptions';
import { FullApi } from '../../utils/fullapi-decorator';
import { GetNotePipe } from '../../utils/get-note.pipe';
import { MarkdownBody } from '../../utils/markdownbody-decorator';
import { PermissionsGuard } from '../../utils/permissions.guard';
import { RequestUser } from '../../utils/request-user.decorator';
@ApiTags('notes')
@ -71,14 +72,14 @@ export class NotesController {
private readonly logger: ConsoleLoggerService,
private noteService: NotesService,
private revisionsService: RevisionsService,
private permissionsService: PermissionsService,
private historyService: HistoryService,
private mediaService: MediaService,
) {
this.logger.setContext(NotesController.name);
}
@UseGuards(TokenAuthGuard)
@Permissions(Permission.CREATE)
@UseGuards(TokenAuthGuard, PermissionsGuard)
@Post()
@HttpCode(201)
@ApiUnauthorizedResponse({ description: unauthorizedDescription })
@ -87,17 +88,14 @@ export class NotesController {
@RequestUser() user: User,
@MarkdownBody() text: string,
): Promise<NoteDto> {
// ToDo: provide user for createNoteDto
if (!this.permissionsService.mayCreate(user)) {
throw new UnauthorizedException('Creating note denied!');
}
this.logger.debug('Got raw markdown:\n' + text);
return await this.noteService.toNoteDto(
await this.noteService.createNote(text, user),
);
}
@UseGuards(TokenAuthGuard)
@Permissions(Permission.READ)
@UseGuards(TokenAuthGuard, PermissionsGuard)
@Get(':noteIdOrAlias')
@ApiOkResponse({
description: 'Get information about the newly created note',
@ -108,14 +106,12 @@ export class NotesController {
@RequestUser() user: User,
@Param('noteIdOrAlias', GetNotePipe) note: Note,
): Promise<NoteDto> {
if (!this.permissionsService.mayRead(user, note)) {
throw new UnauthorizedException('Reading note denied!');
}
await this.historyService.updateHistoryEntryTimestamp(note, user);
return await this.noteService.toNoteDto(note);
}
@UseGuards(TokenAuthGuard)
@Permissions(Permission.CREATE)
@UseGuards(TokenAuthGuard, PermissionsGuard)
@Post(':noteAlias')
@HttpCode(201)
@ApiCreatedResponse({
@ -129,9 +125,6 @@ export class NotesController {
@Param('noteAlias') noteAlias: string,
@MarkdownBody() text: string,
): Promise<NoteDto> {
if (!this.permissionsService.mayCreate(user)) {
throw new UnauthorizedException('Creating note denied!');
}
this.logger.debug('Got raw markdown:\n' + text, 'createNamedNote');
try {
return await this.noteService.toNoteDto(
@ -148,7 +141,8 @@ export class NotesController {
}
}
@UseGuards(TokenAuthGuard)
@Permissions(Permission.OWNER)
@UseGuards(TokenAuthGuard, PermissionsGuard)
@Delete(':noteIdOrAlias')
@HttpCode(204)
@ApiNoContentResponse({ description: successfullyDeletedDescription })
@ -158,9 +152,6 @@ export class NotesController {
@Param('noteIdOrAlias', GetNotePipe) note: Note,
@Body() noteMediaDeletionDto: NoteMediaDeletionDto,
): Promise<void> {
if (!this.permissionsService.isOwner(user, note)) {
throw new UnauthorizedException('Deleting note denied!');
}
const mediaUploads = await this.mediaService.listUploadsByNote(note);
for (const mediaUpload of mediaUploads) {
if (!noteMediaDeletionDto.keepMedia) {
@ -175,7 +166,8 @@ export class NotesController {
return;
}
@UseGuards(TokenAuthGuard)
@Permissions(Permission.WRITE)
@UseGuards(TokenAuthGuard, PermissionsGuard)
@Put(':noteIdOrAlias')
@ApiOkResponse({
description: 'The new, changed note',
@ -187,16 +179,14 @@ export class NotesController {
@Param('noteIdOrAlias', GetNotePipe) note: Note,
@MarkdownBody() text: string,
): Promise<NoteDto> {
if (!this.permissionsService.mayWrite(user, note)) {
throw new UnauthorizedException('Updating note denied!');
}
this.logger.debug('Got raw markdown:\n' + text, 'updateNote');
return await this.noteService.toNoteDto(
await this.noteService.updateNote(note, text),
);
}
@UseGuards(TokenAuthGuard)
@Permissions(Permission.READ)
@UseGuards(TokenAuthGuard, PermissionsGuard)
@Get(':noteIdOrAlias/content')
@ApiProduces('text/markdown')
@ApiOkResponse({
@ -208,13 +198,11 @@ export class NotesController {
@RequestUser() user: User,
@Param('noteIdOrAlias', GetNotePipe) note: Note,
): Promise<string> {
if (!this.permissionsService.mayRead(user, note)) {
throw new UnauthorizedException('Reading note denied!');
}
return await this.noteService.getNoteContent(note);
}
@UseGuards(TokenAuthGuard)
@Permissions(Permission.READ)
@UseGuards(TokenAuthGuard, PermissionsGuard)
@Get(':noteIdOrAlias/metadata')
@ApiOkResponse({
description: 'The metadata of the note',
@ -225,13 +213,11 @@ export class NotesController {
@RequestUser() user: User,
@Param('noteIdOrAlias', GetNotePipe) note: Note,
): Promise<NoteMetadataDto> {
if (!this.permissionsService.mayRead(user, note)) {
throw new UnauthorizedException('Reading note denied!');
}
return await this.noteService.toNoteMetadataDto(note);
}
@UseGuards(TokenAuthGuard)
@Permissions(Permission.OWNER)
@UseGuards(TokenAuthGuard, PermissionsGuard)
@Put(':noteIdOrAlias/metadata/permissions')
@ApiOkResponse({
description: 'The updated permissions of the note',
@ -243,15 +229,13 @@ export class NotesController {
@Param('noteIdOrAlias', GetNotePipe) note: Note,
@Body() updateDto: NotePermissionsUpdateDto,
): Promise<NotePermissionsDto> {
if (!this.permissionsService.isOwner(user, note)) {
throw new UnauthorizedException('Updating note denied!');
}
return this.noteService.toNotePermissionsDto(
await this.noteService.updateNotePermissions(note, updateDto),
);
}
@UseGuards(TokenAuthGuard)
@Permissions(Permission.READ)
@UseGuards(TokenAuthGuard, PermissionsGuard)
@Get(':noteIdOrAlias/revisions')
@ApiOkResponse({
description: 'Revisions of the note',
@ -263,9 +247,6 @@ export class NotesController {
@RequestUser() user: User,
@Param('noteIdOrAlias', GetNotePipe) note: Note,
): Promise<RevisionMetadataDto[]> {
if (!this.permissionsService.mayRead(user, note)) {
throw new UnauthorizedException('Reading note denied!');
}
const revisions = await this.revisionsService.getAllRevisions(note);
return await Promise.all(
revisions.map((revision) =>
@ -274,7 +255,8 @@ export class NotesController {
);
}
@UseGuards(TokenAuthGuard)
@Permissions(Permission.READ)
@UseGuards(TokenAuthGuard, PermissionsGuard)
@Get(':noteIdOrAlias/revisions/:revisionId')
@ApiOkResponse({
description: 'Revision of the note for the given id or alias',
@ -286,9 +268,6 @@ export class NotesController {
@Param('noteIdOrAlias', GetNotePipe) note: Note,
@Param('revisionId') revisionId: number,
): Promise<RevisionDto> {
if (!this.permissionsService.mayRead(user, note)) {
throw new UnauthorizedException('Reading note denied!');
}
try {
return this.revisionsService.toRevisionDto(
await this.revisionsService.getRevision(note, revisionId),
@ -301,7 +280,8 @@ export class NotesController {
}
}
@UseGuards(TokenAuthGuard)
@Permissions(Permission.READ)
@UseGuards(TokenAuthGuard, PermissionsGuard)
@Get(':noteIdOrAlias/media')
@ApiOkResponse({
description: 'All media uploads of the note',
@ -313,9 +293,6 @@ export class NotesController {
@RequestUser() user: User,
@Param('noteIdOrAlias', GetNotePipe) note: Note,
): Promise<MediaUploadDto[]> {
if (!this.permissionsService.mayRead(user, note)) {
throw new UnauthorizedException('Reading note denied!');
}
const media = await this.mediaService.listUploadsByNote(note);
return media.map((media) => this.mediaService.toMediaUploadDto(media));
}

View file

@ -0,0 +1,50 @@
/*
* SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file)
*
* SPDX-License-Identifier: AGPL-3.0-only
*/
import {
ArgumentMetadata,
BadRequestException,
Injectable,
NotFoundException,
PipeTransform,
} from '@nestjs/common';
import { ForbiddenIdError, NotInDBError } from '../../errors/errors';
import { ConsoleLoggerService } from '../../logger/console-logger.service';
import { Note } from '../../notes/note.entity';
import { NotesService } from '../../notes/notes.service';
@Injectable()
export class GetNotePipe implements PipeTransform<string, Promise<Note>> {
constructor(
private readonly logger: ConsoleLoggerService,
private noteService: NotesService,
) {
this.logger.setContext(GetNotePipe.name);
}
async transform(noteIdOrAlias: string, _: ArgumentMetadata): Promise<Note> {
return await getNote(this.noteService, noteIdOrAlias);
}
}
export async function getNote(
noteService: NotesService,
noteIdOrAlias: string,
): Promise<Note> {
let note: Note;
try {
note = await noteService.getNoteByIdOrAlias(noteIdOrAlias);
} catch (e) {
if (e instanceof NotInDBError) {
throw new NotFoundException(e.message);
}
if (e instanceof ForbiddenIdError) {
throw new BadRequestException(e.message);
}
throw e;
}
return note;
}

View file

@ -0,0 +1,66 @@
/*
* SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file)
*
* SPDX-License-Identifier: AGPL-3.0-only
*/
import { CanActivate, ExecutionContext, Injectable } from '@nestjs/common';
import { Reflector } from '@nestjs/core';
import { Request } from 'express';
import { ConsoleLoggerService } from '../../logger/console-logger.service';
import { NotesService } from '../../notes/notes.service';
import { Permission } from '../../permissions/permissions.enum';
import { PermissionsService } from '../../permissions/permissions.service';
import { User } from '../../users/user.entity';
import { getNote } from './get-note.pipe';
/**
* This guards controller methods from access, if the user has not the appropriate permissions.
* The permissions are set via the {@link Permissions} decorator in addition to this guard.
*/
@Injectable()
export class PermissionsGuard implements CanActivate {
constructor(
private readonly logger: ConsoleLoggerService,
private reflector: Reflector,
private permissionsService: PermissionsService,
private noteService: NotesService,
) {
this.logger.setContext(PermissionsGuard.name);
}
async canActivate(context: ExecutionContext): Promise<boolean> {
const permissions = this.reflector.get<Permission[]>(
'permissions',
context.getHandler(),
);
// If no permissions are set this is probably an error and this guard should not let the request pass
if (!permissions) {
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',
);
return false;
}
const request: Request & { user: User } = context
.switchToHttp()
.getRequest();
const user = request.user;
// handle CREATE permissions, as this does not need any note
if (permissions[0] === Permission.CREATE) {
return this.permissionsService.mayCreate(user);
}
// Get the note from the parameter noteIdOrAlias
// Attention: This gets the note an additional time if used in conjunction with GetNotePipe
const noteIdOrAlias = request.params['noteIdOrAlias'];
const note = await getNote(this.noteService, noteIdOrAlias);
switch (permissions[0]) {
case Permission.READ:
return this.permissionsService.mayRead(user, note);
case Permission.WRITE:
return this.permissionsService.mayWrite(user, note);
case Permission.OWNER:
return this.permissionsService.isOwner(user, note);
}
return false;
}
}

View file

@ -1,43 +0,0 @@
/*
* SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file)
*
* SPDX-License-Identifier: AGPL-3.0-only
*/
import {
ArgumentMetadata,
BadRequestException,
Injectable,
NotFoundException,
PipeTransform,
} from '@nestjs/common';
import { ForbiddenIdError, NotInDBError } from '../errors/errors';
import { ConsoleLoggerService } from '../logger/console-logger.service';
import { Note } from './note.entity';
import { NotesService } from './notes.service';
@Injectable()
export class GetNotePipe implements PipeTransform<string, Promise<Note>> {
constructor(
private readonly logger: ConsoleLoggerService,
private noteService: NotesService,
) {
this.logger.setContext(GetNotePipe.name);
}
async transform(noteIdOrAlias: string, _: ArgumentMetadata): Promise<Note> {
let note: Note;
try {
note = await this.noteService.getNoteByIdOrAlias(noteIdOrAlias);
} catch (e) {
if (e instanceof NotInDBError) {
throw new NotFoundException(e.message);
}
if (e instanceof ForbiddenIdError) {
throw new BadRequestException(e.message);
}
throw e;
}
return note;
}
}

View file

@ -0,0 +1,17 @@
/*
* SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file)
*
* SPDX-License-Identifier: AGPL-3.0-only
*/
import { CustomDecorator, SetMetadata } from '@nestjs/common';
import { Permission } from './permissions.enum';
/**
* This decorator gathers the {@link Permission Permission} a user must hold for the {@link PermissionsGuard}
* @param permissions - an array of permissions. In practice this should always contain exactly one {@link Permission}
* @constructor
*/
// eslint-disable-next-line func-style,@typescript-eslint/naming-convention
export const Permissions = (...permissions: Permission[]): CustomDecorator =>
SetMetadata('permissions', permissions);

View file

@ -0,0 +1,21 @@
/*
* SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file)
*
* SPDX-License-Identifier: AGPL-3.0-only
*/
/*
* SPDX-FileCopyrightText: 2021 The HedgeDoc developers (see AUTHORS file)
*
* SPDX-License-Identifier: AGPL-3.0-only
*/
/**
* Represents the Permissions a user may hold in a request
*/
export enum Permission {
READ = 'read',
WRITE = 'write',
CREATE = 'create',
OWNER = 'owner',
}

View file

@ -349,7 +349,7 @@ describe('Notes', () => {
await agent
.get(`/api/private/notes/${alias}/media/`)
.expect('Content-Type', /json/)
.expect(401);
.expect(403);
});
});

View file

@ -396,7 +396,6 @@ describe('Notes', () => {
// check if a missing note correctly returns 404
await request(testSetup.app.getHttpServer())
.get('/api/v2/notes/i_dont_exist/content')
.expect('Content-Type', /text\/markdown/)
.expect(404);
});
});
@ -464,7 +463,7 @@ describe('Notes', () => {
await request(testSetup.app.getHttpServer())
.get(`/api/v2/notes/${alias}/media/`)
.expect('Content-Type', /json/)
.expect(401);
.expect(403);
});
});