Skip to content

Commit e73ae93

Browse files
johavenjohaven
committed
fix(backend:spaces): apply MODIFY permission for PUT requests on existing files instead of ADD when the resource exists
1 parent fdeed09 commit e73ae93

File tree

3 files changed

+18
-8
lines changed

3 files changed

+18
-8
lines changed

‎backend/src/applications/files/files.controller.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -170,10 +170,10 @@ export class FilesController {
170170

171171
@Post(`${FILES_ROUTE.TASK_OPERATION}/${FILE_OPERATION.COMPRESS}/*`)
172172
@SkipSpacePermissionsCheck()
173-
// Compression could be used to download files, permission is checked later
173+
// Can be used to create or download an archive of files; permissions are checked later
174174
async compressAsTask(@Req() req: FastifySpaceRequest, @Body() compressFileDto: CompressFileDto): Promise<FileTask> {
175175
if (compressFileDto.compressInDirectory) {
176-
SpaceGuard.checkPermissions(req, this.logger)
176+
await SpaceGuard.checkPermissions(req, this.logger)
177177
}
178178
return this.filesTasksManager.createTask(FILE_OPERATION.COMPRESS, req.user, req.space, compressFileDto, this.filesMethods.compress.name)
179179
}

‎backend/src/applications/spaces/constants/spaces.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ export enum SPACE_ROLE {
3636

3737
export const SPACE_ALL_OPERATIONS: string = Object.values(SPACE_OPERATION).sort().join(SPACE_PERMS_SEP)
3838

39-
export const SPACE_HTTP_PERMISSION = {
39+
export const SPACE_HTTP_PERMISSION: Record<string, SPACE_OPERATION> = {
4040
GET: null,
4141
POST: SPACE_OPERATION.ADD,
4242
PUT: SPACE_OPERATION.ADD,
@@ -54,13 +54,13 @@ export const SPACE_PERSONAL = {
5454
id: 0,
5555
alias: SPACE_ALIAS.PERSONAL,
5656
name: SPACE_ALIAS.PERSONAL,
57-
permissions: '' // by default no rights are given on the space unless a resource is targeted
57+
permissions: '' // by default, no rights are given on the space unless a resource is targeted
5858
} as const
5959

6060
export const SPACE_SHARES = {
6161
// this space lists the shares
6262
id: 0,
6363
alias: SPACE_REPOSITORY.SHARES,
6464
name: SPACE_REPOSITORY.SHARES,
65-
permissions: '' // by default no rights are given on the share unless a resource is targeted
65+
permissions: '' // by default, no rights are given on the share unless a resource is targeted
6666
} as const

‎backend/src/applications/spaces/guards/space.guard.ts‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,10 @@
66

77
import { CanActivate, ExecutionContext, HttpException, HttpStatus, Injectable, Logger } from '@nestjs/common'
88
import { Reflector } from '@nestjs/core'
9+
import { HTTP_METHOD } from '../../applications.constants'
910
import { COLLABORA_CONTEXT } from '../../files/modules/collabora-online/collabora-online.constants'
1011
import { COLLABORA_ONLINE_TO_SPACE_SEGMENTS } from '../../files/modules/collabora-online/collabora-online.utils'
12+
import { isPathExists, isPathIsDir } from '../../files/utils/files'
1113
import { SYNC_CONTEXT } from '../../sync/decorators/sync-context.decorator'
1214
import { SYNC_PATH_TO_SPACE_SEGMENTS } from '../../sync/utils/routes'
1315
import { WEB_DAV_CONTEXT } from '../../webdav/decorators/webdav-context.decorator'
@@ -31,8 +33,16 @@ export class SpaceGuard implements CanActivate {
3133
private readonly spacesManager: SpacesManager
3234
) {}
3335

34-
static checkPermissions(req: FastifySpaceRequest, logger: Logger, overrideSpacePermission?: SPACE_OPERATION) {
35-
const permission = overrideSpacePermission || SPACE_HTTP_PERMISSION[req.method]
36+
static async checkPermissions(req: FastifySpaceRequest, logger: Logger, overrideSpacePermission?: SPACE_OPERATION) {
37+
let permission: SPACE_OPERATION
38+
if (req.method === HTTP_METHOD.PUT && (await isPathExists(req.space.realPath)) && !(await isPathIsDir(req.space.realPath))) {
39+
// PUT method may either create a new resource or replace an existing one.
40+
// Therefore, we must check whether the target resource already exists to apply the appropriate permission rules.
41+
permission = SPACE_OPERATION.MODIFY
42+
} else {
43+
// The override is applied for specific POST methods that update an existing file rather than creating it.
44+
permission = overrideSpacePermission || SPACE_HTTP_PERMISSION[req.method]
45+
}
3646
if (!haveSpaceEnvPermissions(req.space, permission)) {
3747
logger.warn(`is not allowed to ${req.method} on this space path : *${req.space.alias}* (${req.space.id}) : ${req.space.url}`)
3848
throw new HttpException('You are not allowed to do this action', HttpStatus.FORBIDDEN)
@@ -81,7 +91,7 @@ export class SpaceGuard implements CanActivate {
8191
const skipSpacePermissionsCheck = this.reflector.getAllAndOverride(SKIP_SPACE_PERMISSIONS_CHECK, [ctx.getHandler(), ctx.getClass()])
8292
if (skipSpacePermissionsCheck === undefined) {
8393
const overrideSpacePermission: SPACE_OPERATION = this.reflector.getAllAndOverride(OverrideSpacePermission, [ctx.getHandler(), ctx.getClass()])
84-
SpaceGuard.checkPermissions(req, this.logger, overrideSpacePermission)
94+
await SpaceGuard.checkPermissions(req, this.logger, overrideSpacePermission)
8595
}
8696
return true
8797
}

0 commit comments

Comments
 (0)