diff --git a/file-service/src/main/scala/org/apache/texera/service/resource/DatasetResource.scala b/file-service/src/main/scala/org/apache/texera/service/resource/DatasetResource.scala index ac9b5a9d62e..7dcf0574eff 100644 --- a/file-service/src/main/scala/org/apache/texera/service/resource/DatasetResource.scala +++ b/file-service/src/main/scala/org/apache/texera/service/resource/DatasetResource.scala @@ -36,11 +36,7 @@ import org.apache.texera.dao.jooq.generated.tables.Dataset.DATASET import org.apache.texera.dao.jooq.generated.tables.DatasetContributor.DATASET_CONTRIBUTOR import org.apache.texera.dao.jooq.generated.tables.DatasetVersion.DATASET_VERSION import org.apache.texera.dao.jooq.generated.tables.User.USER -import org.apache.texera.dao.jooq.generated.tables.daos.{ - DatasetDao, - DatasetUserAccessDao, - DatasetVersionDao -} +import org.apache.texera.dao.jooq.generated.tables.daos.{DatasetDao, DatasetUserAccessDao} import org.apache.texera.dao.jooq.generated.tables.pojos.{ Dataset, DatasetUserAccess, @@ -83,15 +79,18 @@ object DatasetResource { dataset } - /** - * Helper function to get the dataset version from DB using dvid + /** Scoped to `did`: the access check runs against `did`, so an unscoped lookup would + * resolve another dataset's version through this repository. */ private def getDatasetVersionByID( ctx: DSLContext, + did: Integer, dvid: Integer ): DatasetVersion = { - val datasetVersionDao = new DatasetVersionDao(ctx.configuration()) - val version = datasetVersionDao.fetchOneByDvid(dvid) + val version = ctx + .selectFrom(DATASET_VERSION) + .where(DATASET_VERSION.DVID.eq(dvid).and(DATASET_VERSION.DID.eq(did))) + .fetchOneInto(classOf[DatasetVersion]) if (version == null) { throw new NotFoundException("Dataset Version not found") } @@ -728,16 +727,16 @@ class DatasetResource extends LazyLogging { @Auth user: SessionUser ): Response = { val uid = user.getUid + // Validated before the getDatasetBy round-trip. + val operation = ResourceUploadService.multipartOperationOrThrow(operationType) val dataset: Dataset = getDatasetBy(ownerEmail, datasetName) - operationType.toLowerCase match { + operation match { case "list" => listMultipartUploads(dataset.getDid, uid) case "init" => initMultipartUpload(dataset.getDid, filePath, fileSizeBytes, partSizeBytes, restart, uid) case "finish" => finishMultipartUpload(dataset.getDid, filePath, uid) - case "abort" => abortMultipartUpload(dataset.getDid, filePath, uid) - case _ => - throw new BadRequestException("Invalid type parameter. Use 'init', 'finish', or 'abort'.") + case _ => abortMultipartUpload(dataset.getDid, filePath, uid) } } @@ -1008,7 +1007,7 @@ class DatasetResource extends LazyLogging { // Determine which version to retrieve val datasetVersion = if (dvid != null) { - getDatasetVersionByID(ctx, dvid) + getDatasetVersionByID(ctx, did, dvid) } else if (java.lang.Boolean.TRUE.equals(latest)) { getLatestDatasetVersion(ctx, did).getOrElse( throw new NotFoundException(ERR_DATASET_VERSION_NOT_FOUND_MESSAGE) @@ -1106,7 +1105,7 @@ class DatasetResource extends LazyLogging { uid: Option[Integer] ): DatasetVersionRootFileNodesResponse = { val dataset = getDashboardDataset(ctx, did, uid) - val datasetVersion = getDatasetVersionByID(ctx, dvid) + val datasetVersion = getDatasetVersionByID(ctx, did, dvid) val (nodes, size) = ResourceUploadService.versionRootFileNodes( resourceType, dataset.ownerEmail, diff --git a/file-service/src/main/scala/org/apache/texera/service/resource/ModelResource.scala b/file-service/src/main/scala/org/apache/texera/service/resource/ModelResource.scala index a9a6fa6f1d0..979f414b751 100644 --- a/file-service/src/main/scala/org/apache/texera/service/resource/ModelResource.scala +++ b/file-service/src/main/scala/org/apache/texera/service/resource/ModelResource.scala @@ -100,8 +100,6 @@ object ModelResource { // Matches model_version.name VARCHAR(128). private val MAX_VERSION_NAME_LENGTH = 128 - private val MULTIPART_OPERATIONS = Seq("list", "init", "finish", "abort") - private def context = SqlServer .getInstance() @@ -962,14 +960,8 @@ class ModelResource extends LazyLogging { ): Response = { val uid = user.getUid - // Optional query param: null when omitted, so validate before dereferencing and before - // the getModelBy round-trip. - val operation = Option(operationType).map(_.trim.toLowerCase).getOrElse("") - if (!MULTIPART_OPERATIONS.contains(operation)) { - throw new BadRequestException( - s"Invalid type parameter. Use ${MULTIPART_OPERATIONS.map(o => s"'$o'").mkString(", ")}." - ) - } + // Validated before the getModelBy round-trip. + val operation = ResourceUploadService.multipartOperationOrThrow(operationType) val model: Model = getModelBy(ownerEmail, modelName) diff --git a/file-service/src/main/scala/org/apache/texera/service/resource/ResourceUploadService.scala b/file-service/src/main/scala/org/apache/texera/service/resource/ResourceUploadService.scala index 8adf4f67958..2ed36dc022a 100644 --- a/file-service/src/main/scala/org/apache/texera/service/resource/ResourceUploadService.scala +++ b/file-service/src/main/scala/org/apache/texera/service/resource/ResourceUploadService.scala @@ -170,6 +170,19 @@ object ResourceStorage { */ object ResourceUploadService { + private val MultipartOperations = Seq("list", "init", "finish", "abort") + + /** The multipart `type` query param, lower-cased; a 400 when it is missing or unknown. */ + def multipartOperationOrThrow(operationType: String): String = { + val operation = Option(operationType).map(_.trim.toLowerCase).getOrElse("") + if (!MultipartOperations.contains(operation)) { + throw new BadRequestException( + s"Invalid type parameter. Use ${MultipartOperations.map(o => s"'$o'").mkString(", ")}." + ) + } + operation + } + private def context: DSLContext = SqlServer .getInstance() diff --git a/file-service/src/test/scala/org/apache/texera/service/resource/DatasetResourceSpec.scala b/file-service/src/test/scala/org/apache/texera/service/resource/DatasetResourceSpec.scala index 0024ec354ca..8acf3af06fb 100644 --- a/file-service/src/test/scala/org/apache/texera/service/resource/DatasetResourceSpec.scala +++ b/file-service/src/test/scala/org/apache/texera/service/resource/DatasetResourceSpec.scala @@ -2492,6 +2492,22 @@ class DatasetResourceSpec assertStatus(ex, 400) } + it should "reject a missing type parameter with a 400" in { + val ex = intercept[BadRequestException] { + datasetResource.multipartUpload( + null, + ownerUser.getEmail, + multipartDataset.getName, + urlEnc(uniqueFilePath("init-no-type")), + Optional.empty(), + Optional.empty(), + Optional.empty(), + multipartOwnerSessionUser + ) + } + assertStatus(ex, 400) + } + it should "reject init when caller lacks WRITE access" in { val filePath = uniqueFilePath("init-forbidden") val ex = intercept[ForbiddenException] { @@ -3854,7 +3870,37 @@ class DatasetResourceSpec .map(_.getName) shouldEqual List("logo.png") } - "retrieveDatasetVersionRootFileNodes" should "return the same tree to the dataset's owner" in { + it should "return 404 for a version of another dataset" in { + val (publicDataset, _) = seedDatasetWithVersion("public-host", Seq("a.txt" -> "a")) + val (_, otherVersion) = + seedDatasetWithVersion("private-other", Seq("b.txt" -> "b"), isPublic = false) + + assertThrows[NotFoundException] { + datasetResource.retrievePublicDatasetVersionRootFileNodes( + publicDataset.getDid, + otherVersion.getDvid + ) + } + } + + "retrieveDatasetVersionRootFileNodes" should "return 404 for a version of another dataset" in { + val (dataset, _) = seedDatasetWithVersion("owner-host", Seq("a.txt" -> "a"), isPublic = false) + val (_, otherVersion) = + seedDatasetWithVersion("owner-other", Seq("b.txt" -> "b"), isPublic = false) + + assertThrows[NotFoundException] { + datasetResource.retrieveDatasetVersionRootFileNodes( + dataset.getDid, + otherVersion.getDvid, + sessionUser + ) + } + assertThrows[NotFoundException] { + datasetResource.getDatasetVersionZip(dataset.getDid, otherVersion.getDvid, null, sessionUser) + } + } + + it should "return the same tree to the dataset's owner" in { val (dataset, version) = seedDatasetWithVersion("owner-nodes", Seq("a.txt" -> "1", "sub/b.txt" -> "2"))