diff --git a/backend/src/api/private/media/media.controller.ts b/backend/src/api/private/media/media.controller.ts index 584946a9a..8eb1b04ac 100644 --- a/backend/src/api/private/media/media.controller.ts +++ b/backend/src/api/private/media/media.controller.ts @@ -24,6 +24,7 @@ import { MediaService } from '../../../media/media.service'; import { MulterFile } from '../../../media/multer-file.interface'; import { Note } from '../../../notes/note.entity'; import { Permission } from '../../../permissions/permissions.enum'; +import { PermissionsService } from '../../../permissions/permissions.service'; import { User } from '../../../users/user.entity'; import { NoteHeaderInterceptor } from '../../utils/note-header.interceptor'; import { OpenApi } from '../../utils/openapi.decorator'; @@ -40,6 +41,7 @@ export class MediaController { constructor( private readonly logger: ConsoleLoggerService, private mediaService: MediaService, + private permissionsService: PermissionsService, ) { this.logger.setContext(MediaController.name); } @@ -106,12 +108,11 @@ export class MediaController { @Param('filename') filename: string, ): Promise { const mediaUpload = await this.mediaService.findUploadByFilename(filename); - const mediaUploadOwner = await mediaUpload.user; - const mediaUploadNote = await mediaUpload.note; - const mediaUploadNoteOwner = await mediaUploadNote?.owner; if ( - mediaUploadOwner?.id === user.id || - user.id === mediaUploadNoteOwner?.id + await this.permissionsService.checkMediaDeletePermission( + user, + mediaUpload, + ) ) { this.logger.debug( `Deleting '${filename}' for user '${user.username}'`, @@ -123,10 +124,11 @@ export class MediaController { `${user.username} tried to delete '${filename}', but is not the owner of upload or connected note`, 'deleteMedia', ); + const mediaUploadNote = await mediaUpload.note; throw new PermissionError( `Neither file '${filename}' nor note '${ mediaUploadNote?.id ?? 'unknown' - }'is not owned by '${user.username}'`, + }'is owned by '${user.username}'`, ); } } diff --git a/backend/src/api/public/media/media.controller.ts b/backend/src/api/public/media/media.controller.ts index 9329f6aa6..91cca2862 100644 --- a/backend/src/api/public/media/media.controller.ts +++ b/backend/src/api/public/media/media.controller.ts @@ -30,6 +30,7 @@ import { MediaService } from '../../../media/media.service'; import { MulterFile } from '../../../media/multer-file.interface'; import { Note } from '../../../notes/note.entity'; import { Permission } from '../../../permissions/permissions.enum'; +import { PermissionsService } from '../../../permissions/permissions.service'; import { User } from '../../../users/user.entity'; import { NoteHeaderInterceptor } from '../../utils/note-header.interceptor'; import { OpenApi } from '../../utils/openapi.decorator'; @@ -47,6 +48,7 @@ export class MediaController { constructor( private readonly logger: ConsoleLoggerService, private mediaService: MediaService, + private permissionsService: PermissionsService, ) { this.logger.setContext(MediaController.name); } @@ -113,12 +115,11 @@ export class MediaController { @Param('filename') filename: string, ): Promise { const mediaUpload = await this.mediaService.findUploadByFilename(filename); - const mediaUploadOwner = await mediaUpload.user; - const mediaUploadNote = await mediaUpload.note; - const mediaUploadNoteOwner = await mediaUploadNote?.owner; if ( - mediaUploadOwner?.id === user.id || - user.id === mediaUploadNoteOwner?.id + await this.permissionsService.checkMediaDeletePermission( + user, + mediaUpload, + ) ) { this.logger.debug( `Deleting '${filename}' for user '${user.username}'`, @@ -130,10 +131,11 @@ export class MediaController { `${user.username} tried to delete '${filename}', but is not the owner of upload or connected note`, 'deleteMedia', ); + const mediaUploadNote = await mediaUpload.note; throw new PermissionError( `Neither file '${filename}' nor note '${ mediaUploadNote?.id ?? 'unknown' - }'is not owned by '${user.username}'`, + }'is owned by '${user.username}'`, ); } } diff --git a/backend/src/api/utils/permissions.guard.ts b/backend/src/api/utils/permissions.guard.ts index 620d671fa..5fd913d1f 100644 --- a/backend/src/api/utils/permissions.guard.ts +++ b/backend/src/api/utils/permissions.guard.ts @@ -54,14 +54,10 @@ export class PermissionsGuard implements CanActivate { if (noteIdOrAlias === undefined) noteIdOrAlias = request.headers['hedgedoc-note'] as string; const note = await getNote(this.noteService, noteIdOrAlias); - switch (permissions[0]) { - case Permission.READ: - return await this.permissionsService.mayRead(user, note); - case Permission.WRITE: - return await this.permissionsService.mayWrite(user, note); - case Permission.OWNER: - return await this.permissionsService.isOwner(user, note); - } - return false; + return await this.permissionsService.checkPermissionOnNote( + permissions[0], + user, + note, + ); } } diff --git a/backend/src/permissions/permissions.service.ts b/backend/src/permissions/permissions.service.ts index 6c4b3e34d..b5fd5d9ed 100644 --- a/backend/src/permissions/permissions.service.ts +++ b/backend/src/permissions/permissions.service.ts @@ -19,6 +19,7 @@ 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'; import { Note } from '../notes/note.entity'; import { User } from '../users/user.entity'; @@ -26,6 +27,7 @@ import { UsersService } from '../users/users.service'; import { checkArrayForDuplicates } from '../utils/arrayDuplicatCheck'; import { NoteGroupPermission } from './note-group-permission.entity'; import { NoteUserPermission } from './note-user-permission.entity'; +import { Permission } from './permissions.enum'; @Injectable() export class PermissionsService { @@ -38,6 +40,43 @@ 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 {Permission} 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: Permission, + user: User | null, + note: Note, + ): Promise { + switch (desiredPermission) { + case Permission.READ: + return await this.mayRead(user, note); + case Permission.WRITE: + return await this.mayWrite(user, note); + case Permission.OWNER: + return await this.isOwner(user, note); + } + return false; + } + + public async checkMediaDeletePermission( + user: User, + mediaUpload: MediaUpload, + ): Promise { + const mediaUploadNote = await mediaUpload.note; + const mediaUploadOwner = await mediaUpload.user; + + const owner = + !!mediaUploadNote && (await this.isOwner(user, mediaUploadNote)); + + return mediaUploadOwner?.id === user.id || owner; + } /** * Checks if the given {@link User} is allowed to read the given {@link Note}.