aicam commented on code in PR #7922:
URL: https://github.com/apache/texera/pull/7922#discussion_r3859121094
##########
file-service/src/main/scala/org/apache/texera/service/resource/ModelResource.scala:
##########
@@ -408,4 +474,513 @@ class ModelResource extends LazyLogging {
): DashboardModel = {
withTransaction(context)(ctx => getDashboardModel(ctx, mid, None))
}
+
+ @GET
+ @RolesAllowed(Array("REGULAR", "ADMIN"))
+ @Path("/{mid}/versionZip")
+ def getModelVersionZip(
+ @PathParam("mid") mid: Integer,
+ @QueryParam("mvid") mvid: Integer,
+ @QueryParam("latest") latest: java.lang.Boolean,
+ @Auth user: SessionUser
+ ): Response =
+ withTransaction(context) { ctx =>
+ if ((mvid != null && latest != null) || (mvid == null && latest ==
null)) {
+ throw new BadRequestException("Specify exactly one: mvid=<ID> OR
latest=true")
+ }
+
+ val uid = user.getUid
+ if (!userHasReadAccess(ctx, mid, uid)) {
+ throw new ForbiddenException(ERR_USER_HAS_NO_ACCESS_TO_MODEL_MESSAGE)
+ }
+
+ val model = getModelByID(ctx, mid)
+ // Non-owners may download only while the owner leaves the model
downloadable.
+ if (!userOwnModel(ctx, mid, uid) && !model.getIsDownloadable) {
+ throw new ForbiddenException("Model download is not allowed")
+ }
+
+ val modelVersion =
+ if (mvid != null) getModelVersionByID(ctx, mvid)
+ else
+ getLatestModelVersion(ctx, mid).getOrElse(
+ throw new NotFoundException(ERR_MODEL_VERSION_NOT_FOUND_MESSAGE)
+ )
+
+ ResourceUploadService.versionZipResponse(
+ model.getRepositoryName,
+ modelVersion.getVersionHash,
+ model.getName,
+ modelVersion.getName
+ )
+ }
+
+ /** Owner facet for the model list page. */
+ @GET
+ @RolesAllowed(Array("REGULAR", "ADMIN"))
+ @Path("/user-model-owners")
+ def retrieveOwners(@Auth user: SessionUser): java.util.List[String] =
+ withTransaction(context)(ctx =>
+ ResourceAccess.ownerEmailsVisibleTo(ctx, MODEL_RESOURCE, user.getUid)
+ )
+
+ //
===========================================================================
+ // Staged changes
+ //
===========================================================================
+
+ @GET
+ @RolesAllowed(Array("REGULAR", "ADMIN"))
+ @Path("/{mid}/diff")
+ def getModelDiff(
+ @PathParam("mid") mid: Integer,
+ @Auth user: SessionUser
+ ): List[Diff] =
+ ResourceUploadService.stagedChanges(ResourceStorage.Model, mid,
user.getUid)
+
+ @PUT
+ @RolesAllowed(Array("REGULAR", "ADMIN"))
+ @Path("/{mid}/diff")
+ def resetModelFileDiff(
+ @PathParam("mid") mid: Integer,
+ @QueryParam("filePath") encodedFilePath: String,
+ @Auth user: SessionUser
+ ): Response =
+ ResourceUploadService.resetStagedChange(
+ ResourceStorage.Model,
+ mid,
+ encodedFilePath,
+ user.getUid
+ )
+
+ @POST
+ @RolesAllowed(Array("REGULAR", "ADMIN"))
+ @Path("/{mid}/existing-upload-files")
+ @Consumes(Array(MediaType.APPLICATION_JSON))
+ def findExistingUploadFiles(
+ @PathParam("mid") mid: Integer,
+ request: ExistingUploadFilesRequest,
+ @Auth user: SessionUser
+ ): Response =
+ ResourceUploadService.matchExistingUploads(
+ ResourceStorage.Model,
+ mid,
+ user.getUid,
+ request,
+ ctx => getLatestModelVersion(ctx, mid).map(_.getVersionHash)
+ )
+
+ //
===========================================================================
+ // Presigned downloads
+ //
===========================================================================
+
+ /**
+ * Resolves a presign request against the model tables and wraps the signed
URL.
+ * The resolution itself is shared with datasets; only the descriptor
differs.
+ */
+ /** Size of a model's LakeFS repository, or 0 if LakeFS cannot answer. */
+ private def repositorySizeOrZero(model: Model): Long = {
+ try {
+ LakeFSStorageClient.retrieveRepositorySize(model.getRepositoryName)
+ } catch {
+ case e: io.lakefs.clients.sdk.ApiException =>
+ logger.error(
+ s"LakeFS ApiException for model repository
'${model.getRepositoryName}': ${e.getMessage}",
+ e
+ )
+ 0L
+ }
+ }
+
+ private def generatePresignedResponse(
+ encodedUrl: String,
+ repositoryName: String,
+ commitHash: String,
+ uid: Integer
+ ): Response =
+ ResourceUploadService.presignedUrlResponse(
+ ResourceStorage.Model,
+ encodedUrl,
+ repositoryName,
+ commitHash,
+ uid
+ )
+
+ @GET
+ @RolesAllowed(Array("REGULAR", "ADMIN"))
+ @Path("/presign-download")
+ def getPresignedUrl(
+ @QueryParam("filePath") encodedUrl: String,
+ @QueryParam("repositoryName") repositoryName: String,
+ @QueryParam("commitHash") commitHash: String,
+ @Auth user: SessionUser
+ ): Response =
+ generatePresignedResponse(encodedUrl, repositoryName, commitHash,
user.getUid)
+
+ @GET
+ @RolesAllowed(Array("REGULAR", "ADMIN"))
+ @Path("/presign-download-s3")
+ def getPresignedUrlWithS3(
+ @QueryParam("filePath") encodedUrl: String,
+ @QueryParam("repositoryName") repositoryName: String,
+ @QueryParam("commitHash") commitHash: String,
+ @Auth user: SessionUser
+ ): Response =
+ generatePresignedResponse(encodedUrl, repositoryName, commitHash,
user.getUid)
+
+ @GET
+ @PermitAll
+ @Path("/public-presign-download")
+ def getPublicPresignedUrl(
Review Comment:
The `is_downloadable` gate enforced in `getModelVersionZip` is not applied
here. `ResourceUploadService.presignedUrlResponse` only calls
`requireReadAccessToRepository`, so for a public-but-not-downloadable model an
anonymous caller can still presign the raw files:
```
GET
/model/public-presign-download?filePath=/model/[email protected]/resnet/v1/weights.bin
```
A READ-grantee has an easier route: `/model/{mid}/version/list` gives
`versionHash`, then
`presign-download?repositoryName=model-N&commitHash=<hash>&filePath=...` for
every file. As it stands the download restriction is not enforceable.
##########
file-service/src/main/scala/org/apache/texera/service/resource/ModelResource.scala:
##########
@@ -408,4 +474,513 @@ class ModelResource extends LazyLogging {
): DashboardModel = {
withTransaction(context)(ctx => getDashboardModel(ctx, mid, None))
}
+
+ @GET
+ @RolesAllowed(Array("REGULAR", "ADMIN"))
+ @Path("/{mid}/versionZip")
+ def getModelVersionZip(
+ @PathParam("mid") mid: Integer,
+ @QueryParam("mvid") mvid: Integer,
+ @QueryParam("latest") latest: java.lang.Boolean,
+ @Auth user: SessionUser
+ ): Response =
+ withTransaction(context) { ctx =>
+ if ((mvid != null && latest != null) || (mvid == null && latest ==
null)) {
+ throw new BadRequestException("Specify exactly one: mvid=<ID> OR
latest=true")
+ }
+
+ val uid = user.getUid
+ if (!userHasReadAccess(ctx, mid, uid)) {
+ throw new ForbiddenException(ERR_USER_HAS_NO_ACCESS_TO_MODEL_MESSAGE)
+ }
+
+ val model = getModelByID(ctx, mid)
+ // Non-owners may download only while the owner leaves the model
downloadable.
+ if (!userOwnModel(ctx, mid, uid) && !model.getIsDownloadable) {
+ throw new ForbiddenException("Model download is not allowed")
+ }
+
+ val modelVersion =
+ if (mvid != null) getModelVersionByID(ctx, mvid)
Review Comment:
`getModelVersionByID` fetches by primary key only, so `mvid` is never
checked to belong to `mid`. `GET /model/1/versionZip?mvid=<version of model 2>`
passes the access check on model 1, then asks LakeFS for repo `model-1` at a
commit that only exists in `model-2` — unhandled `ApiException` → 500 instead
of 400/404.
Same issue in `fetchModelVersionRootFileNodes` (~L933). Filtering the query
by `MODEL_VERSION.MID`, or asserting `version.getMid == mid`, fixes both.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
