From d4b270e527df44ca6b3b3a3a89b242abcd79c8c7 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Thu, 25 Aug 2022 01:24:54 +1200 Subject: [PATCH] Better storage permission checks --- app/controllers/api/storage.php | 121 +++++++++++++------------------- 1 file changed, 49 insertions(+), 72 deletions(-) diff --git a/app/controllers/api/storage.php b/app/controllers/api/storage.php index e9bec53340..40e9248094 100644 --- a/app/controllers/api/storage.php +++ b/app/controllers/api/storage.php @@ -364,7 +364,7 @@ App::post('/v1/storage/buckets/:bucketId/files') throw new Exception(Exception::STORAGE_BUCKET_NOT_FOUND); } - $validator = new Authorization('create'); + $validator = new Authorization(Database::PERMISSION_CREATE); if (!$validator->isValid($bucket->getCreate())) { throw new Exception(Exception::USER_UNAUTHORIZED); } @@ -409,14 +409,6 @@ App::post('/v1/storage/buckets/:bucketId/files') } } - $file = $request->getFiles('file'); - - /** - * Validators - */ - $allowedFileExtensions = $bucket->getAttribute('allowedFileExtensions', []); - $fileExt = new FileExt($allowedFileExtensions); - $maximumFileSize = $bucket->getAttribute('maximumFileSize', 0); if ($maximumFileSize > (int) App::getEnv('_APP_STORAGE_LIMIT', 0)) { throw new Exception(Exception::GENERAL_SERVER_ERROR, 'Maximum bucket file size is larger than _APP_STORAGE_LIMIT'); @@ -683,8 +675,10 @@ App::get('/v1/storage/buckets/:bucketId/files') throw new Exception(Exception::STORAGE_BUCKET_NOT_FOUND); } - $validator = new Authorization('read'); - if (!$validator->isValid($bucket->getRead())) { + $fileSecurity = $bucket->getAttribute('fileSecurity', false); + $validator = new Authorization(Database::PERMISSION_READ); + $valid = $validator->isValid($bucket->getRead()); + if (!$fileSecurity && !$valid) { throw new Exception(Exception::USER_UNAUTHORIZED); } @@ -699,7 +693,9 @@ App::get('/v1/storage/buckets/:bucketId/files') $queries[] = Query::offset($offset); $queries[] = $orderType === Database::ORDER_ASC ? Query::orderAsc('') : Query::orderDesc(''); if (!empty($cursor)) { - $cursorDocument = $dbForProject->getDocument('bucket_' . $bucket->getInternalId(), $cursor); + $cursorDocument = $fileSecurity + ? $dbForProject->getDocument('bucket_' . $bucket->getInternalId(), $cursor) + : Authorization::skip(fn() => $dbForProject->getDocument('bucket_' . $bucket->getInternalId(), $cursor)); if ($cursorDocument->isEmpty()) { throw new Exception(Exception::GENERAL_CURSOR_NOT_FOUND, "File '{$cursor}' for the 'cursor' value not found."); @@ -708,7 +704,7 @@ App::get('/v1/storage/buckets/:bucketId/files') $queries[] = $cursorDirection === Database::CURSOR_AFTER ? Query::cursorAfter($cursorDocument) : Query::cursorBefore($cursorDocument); } - if ($bucket->getAttribute('fileSecurity', false)) { + if ($fileSecurity && !$valid) { $files = $dbForProject->find('bucket_' . $bucket->getInternalId(), \array_merge($filterQueries, $queries)); $total = $dbForProject->count('bucket_' . $bucket->getInternalId(), $filterQueries, APP_LIMIT_COUNT); } else { @@ -754,25 +750,22 @@ App::get('/v1/storage/buckets/:bucketId/files/:fileId') } $fileSecurity = $bucket->getAttribute('fileSecurity', false); - $validator = new Authorization('read'); + $validator = new Authorization(Database::PERMISSION_READ); $valid = $validator->isValid($bucket->getRead()); if (!$valid && !$fileSecurity) { throw new Exception(Exception::USER_UNAUTHORIZED); } - $file = $dbForProject->getDocument('bucket_' . $bucket->getInternalId(), $fileId); + if ($fileSecurity && !$valid) { + $file = $dbForProject->getDocument('bucket_' . $bucket->getInternalId(), $fileId); + } else { + $file = Authorization::skip(fn() => $dbForProject->getDocument('bucket_' . $bucket->getInternalId(), $fileId)); + } if ($file->isEmpty() || $file->getAttribute('bucketId') !== $bucketId) { throw new Exception(Exception::STORAGE_FILE_NOT_FOUND); } - if ($fileSecurity) { - $valid = $validator->isValid($file->getRead()); - if (!$valid) { - throw new Exception(Exception::USER_UNAUTHORIZED); - } - } - $usage ->setParam('storage.files.read', 1) ->setParam('bucketId', $bucketId) @@ -829,9 +822,9 @@ App::get('/v1/storage/buckets/:bucketId/files/:fileId/preview') } $fileSecurity = $bucket->getAttribute('fileSecurity', false); - $validator = new Authorization('read'); + $validator = new Authorization(Database::PERMISSION_READ); $valid = $validator->isValid($bucket->getRead()); - if (!$valid && !$fileSecurity) { + if (!$fileSecurity && !$valid) { throw new Exception(Exception::USER_UNAUTHORIZED); } @@ -846,19 +839,16 @@ App::get('/v1/storage/buckets/:bucketId/files/:fileId/preview') $date = \date('D, d M Y H:i:s', \time() + (60 * 60 * 24 * 45)) . ' GMT'; // 45 days cache $key = \md5($fileId . $width . $height . $gravity . $quality . $borderWidth . $borderColor . $borderRadius . $opacity . $rotation . $background . $output); - $file = Authorization::skip(fn() => $dbForProject->getDocument('bucket_' . $bucket->getInternalId(), $fileId)); + if ($fileSecurity && !$valid) { + $file = $dbForProject->getDocument('bucket_' . $bucket->getInternalId(), $fileId); + } else { + $file = Authorization::skip(fn() => $dbForProject->getDocument('bucket_' . $bucket->getInternalId(), $fileId)); + } if ($file->isEmpty() || $file->getAttribute('bucketId') !== $bucketId) { throw new Exception(Exception::STORAGE_FILE_NOT_FOUND); } - if ($fileSecurity) { - $valid |= $validator->isValid($file->getRead()); - if (!$valid) { - throw new Exception(Exception::USER_UNAUTHORIZED); - } - } - $path = $file->getAttribute('path'); $type = \strtolower(\pathinfo($path, PATHINFO_EXTENSION)); $algorithm = $file->getAttribute('algorithm'); @@ -980,25 +970,22 @@ App::get('/v1/storage/buckets/:bucketId/files/:fileId/download') } $fileSecurity = $bucket->getAttribute('fileSecurity', false); - $validator = new Authorization('read'); + $validator = new Authorization(Database::PERMISSION_READ); $valid = $validator->isValid($bucket->getRead()); - if (!$valid && !$fileSecurity) { + if (!$fileSecurity && !$valid) { throw new Exception(Exception::USER_UNAUTHORIZED); } - $file = $dbForProject->getDocument('bucket_' . $bucket->getInternalId(), $fileId); + if ($fileSecurity && !$valid) { + $file = $dbForProject->getDocument('bucket_' . $bucket->getInternalId(), $fileId); + } else { + $file = Authorization::skip(fn() => $dbForProject->getDocument('bucket_' . $bucket->getInternalId(), $fileId)); + } if ($file->isEmpty() || $file->getAttribute('bucketId') !== $bucketId) { throw new Exception(Exception::STORAGE_FILE_NOT_FOUND); } - if ($fileSecurity) { - $valid |= $validator->isValid($file->getRead()); - if (!$valid) { - throw new Exception(Exception::USER_UNAUTHORIZED); - } - } - $path = $file->getAttribute('path', ''); if (!$deviceFiles->exists($path)) { @@ -1118,25 +1105,22 @@ App::get('/v1/storage/buckets/:bucketId/files/:fileId/view') } $fileSecurity = $bucket->getAttribute('fileSecurity', false); - $validator = new Authorization('read'); + $validator = new Authorization(Database::PERMISSION_READ); $valid = $validator->isValid($bucket->getRead()); - if (!$valid && !$fileSecurity) { + if (!$fileSecurity && !$valid) { throw new Exception(Exception::USER_UNAUTHORIZED); } - $file = $dbForProject->getDocument('bucket_' . $bucket->getInternalId(), $fileId); + if ($fileSecurity && !$valid) { + $file = $dbForProject->getDocument('bucket_' . $bucket->getInternalId(), $fileId); + } else { + $file = Authorization::skip(fn() => $dbForProject->getDocument('bucket_' . $bucket->getInternalId(), $fileId)); + } if ($file->isEmpty() || $file->getAttribute('bucketId') !== $bucketId) { throw new Exception(Exception::STORAGE_FILE_NOT_FOUND); } - if ($fileSecurity) { - $valid |= !$validator->isValid($file->getRead()); - if (!$valid) { - throw new Exception(Exception::USER_UNAUTHORIZED); - } - } - $mimes = Config::getParam('storage-mimes'); $path = $file->getAttribute('path', ''); @@ -1270,26 +1254,22 @@ App::put('/v1/storage/buckets/:bucketId/files/:fileId') } $fileSecurity = $bucket->getAttributes('fileSecurity', false); - $validator = new Authorization('update'); + $validator = new Authorization(Database::PERMISSION_UPDATE); $valid = $validator->isValid($bucket->getUpdate()); - if (!$valid && !$fileSecurity) { + if (!$fileSecurity && !$valid) { throw new Exception(Exception::USER_UNAUTHORIZED); } - $file = $dbForProject->getDocument('bucket_' . $bucket->getInternalId(), $fileId); + if ($fileSecurity && !$valid) { + $file = $dbForProject->getDocument('bucket_' . $bucket->getInternalId(), $fileId); + } else { + $file = Authorization::skip(fn() => $dbForProject->getDocument('bucket_' . $bucket->getInternalId(), $fileId)); + } if ($file->isEmpty() || $file->getAttribute('bucketId') !== $bucketId) { throw new Exception(Exception::STORAGE_FILE_NOT_FOUND); } - if ($fileSecurity) { - $valid |= $validator->isValid($file->getUpdate()); - if (!$valid) { - throw new Exception(Exception::USER_UNAUTHORIZED); - } - } - - // Users can only manage their own roles, API keys and Admin users can manage any // Users can only manage their own roles, API keys and Admin users can manage any $roles = Authorization::getRoles(); if (!Auth::isAppUser($roles) && !Auth::isPrivilegedUser($roles)) { @@ -1368,23 +1348,20 @@ App::delete('/v1/storage/buckets/:bucketId/files/:fileId') $fileSecurity = $bucket->getAttributes('fileSecurity', false); $validator = new Authorization('delete'); $valid = $validator->isValid($bucket->getDelete()); - if (!$valid && !$fileSecurity) { + if (!$fileSecurity && !$valid) { throw new Exception(Exception::USER_UNAUTHORIZED); } - $file = $dbForProject->getDocument('bucket_' . $bucket->getInternalId(), $fileId); + if ($fileSecurity && !$valid) { + $file = $dbForProject->getDocument('bucket_' . $bucket->getInternalId(), $fileId); + } else { + $file = Authorization::skip(fn() => $dbForProject->getDocument('bucket_' . $bucket->getInternalId(), $fileId)); + } if ($file->isEmpty() || $file->getAttribute('bucketId') !== $bucketId) { throw new Exception(Exception::STORAGE_FILE_NOT_FOUND); } - if ($fileSecurity) { - $valid |= $validator->isValid($file->getDelete()); - if (!$valid) { - throw new Exception(Exception::USER_UNAUTHORIZED); - } - } - $deviceDeleted = false; if ($file->getAttribute('chunksTotal') !== $file->getAttribute('chunksUploaded')) { $deviceDeleted = $deviceFiles->abort(