Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle S3 access error gracefully with 403 #4954

Merged
merged 1 commit into from
May 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -269,20 +269,21 @@ object FileRejection {

implicit final val fileRejectionHttpResponseFields: HttpResponseFields[FileRejection] =
HttpResponseFields.fromStatusAndHeaders {
case RevisionNotFound(_, _) => (StatusCodes.NotFound, Seq.empty)
case TagNotFound(_) => (StatusCodes.NotFound, Seq.empty)
case FileNotFound(_, _) => (StatusCodes.NotFound, Seq.empty)
case ResourceAlreadyExists(_, _) => (StatusCodes.Conflict, Seq.empty)
case IncorrectRev(_, _) => (StatusCodes.Conflict, Seq.empty)
case FileTooLarge(_) => (StatusCodes.PayloadTooLarge, Seq.empty)
case WrappedAkkaRejection(rej) => (rej.status, rej.headers)
case WrappedStorageRejection(rej) => (rej.status, rej.headers)
case RevisionNotFound(_, _) => (StatusCodes.NotFound, Seq.empty)
case TagNotFound(_) => (StatusCodes.NotFound, Seq.empty)
case FileNotFound(_, _) => (StatusCodes.NotFound, Seq.empty)
case ResourceAlreadyExists(_, _) => (StatusCodes.Conflict, Seq.empty)
case IncorrectRev(_, _) => (StatusCodes.Conflict, Seq.empty)
case FileTooLarge(_) => (StatusCodes.PayloadTooLarge, Seq.empty)
case WrappedAkkaRejection(rej) => (rej.status, rej.headers)
case WrappedStorageRejection(rej) => (rej.status, rej.headers)
// If this happens it signifies a system problem rather than the user having made a mistake
case FetchRejection(_, _, FetchFileRejection.FileNotFound(_)) => (StatusCodes.InternalServerError, Seq.empty)
case SaveRejection(_, _, SaveFileRejection.ResourceAlreadyExists(_)) => (StatusCodes.Conflict, Seq.empty)
case CopyRejection(_, _, _, rejection) => (rejection.status, Seq.empty)
case FetchRejection(_, _, _) => (StatusCodes.InternalServerError, Seq.empty)
case SaveRejection(_, _, _) => (StatusCodes.InternalServerError, Seq.empty)
case _ => (StatusCodes.BadRequest, Seq.empty)
case FetchRejection(_, _, FetchFileRejection.FileNotFound(_)) => (StatusCodes.InternalServerError, Seq.empty)
case SaveRejection(_, _, SaveFileRejection.ResourceAlreadyExists(_)) => (StatusCodes.Conflict, Seq.empty)
case SaveRejection(_, _, SaveFileRejection.BucketAccessDenied(_, _, _)) => (StatusCodes.Forbidden, Seq.empty)
case CopyRejection(_, _, _, rejection) => (rejection.status, Seq.empty)
case FetchRejection(_, _, _) => (StatusCodes.InternalServerError, Seq.empty)
case SaveRejection(_, _, _) => (StatusCodes.InternalServerError, Seq.empty)
case _ => (StatusCodes.BadRequest, Seq.empty)
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package ch.epfl.bluebrain.nexus.delta.plugins.storage.storages.operations

import akka.http.scaladsl.model.{StatusCodes, Uri}
import cats.data.NonEmptyList
import ch.epfl.bluebrain.nexus.delta.kernel.error.Rejection
import ch.epfl.bluebrain.nexus.delta.plugins.storage.storages.model.StorageType
import ch.epfl.bluebrain.nexus.delta.rdf.IriOrBNode.Iri
Expand Down Expand Up @@ -146,6 +145,8 @@ object StorageFileRejection {
s"File cannot be saved on path '$path' for unexpected reasons. Details '$details'"
)

final case class BucketAccessDenied(bucket: String, key: String, details: String)
extends SaveFileRejection(s"Access denied to bucket $bucket at key $key")
}

/**
Expand Down Expand Up @@ -196,9 +197,6 @@ object StorageFileRejection {
sealed abstract class RegisterFileRejection(loggedDetails: String) extends StorageFileRejection(loggedDetails)

object RegisterFileRejection {
final case class MissingS3Attributes(missingAttributes: NonEmptyList[String])
extends RegisterFileRejection(s"Missing attributes from S3: ${missingAttributes.toList.mkString(", ")}")

final case class InvalidContentType(received: String)
extends RegisterFileRejection(s"Invalid content type returned from S3: $received")

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ package ch.epfl.bluebrain.nexus.delta.plugins.storage.storages.operations.s3

import akka.NotUsed
import akka.actor.ActorSystem
import akka.http.scaladsl.model.{BodyPartEntity, Uri}
import akka.http.scaladsl.model.{BodyPartEntity, StatusCodes, Uri}
import akka.stream.scaladsl.Source
import akka.util.ByteString
import cats.effect.IO
Expand All @@ -18,6 +18,7 @@ import ch.epfl.bluebrain.nexus.delta.plugins.storage.storages.operations.s3.clie
import ch.epfl.bluebrain.nexus.delta.plugins.storage.storages.operations.s3.client.S3StorageClient.UploadMetadata
import ch.epfl.bluebrain.nexus.delta.sdk.stream.StreamConverter
import fs2.Stream
import software.amazon.awssdk.services.s3.model.S3Exception

import java.util.UUID

Expand Down Expand Up @@ -56,7 +57,11 @@ final class S3StorageSaveFile(s3StorageClient: S3StorageClient, locationGenerato
attr = fileMetadata(path, uuid, uploadMetadata)
} yield attr)
.onError(e => logger.error(e)("Unexpected error when storing file"))
.adaptError { err => UnexpectedSaveError(key, err.getMessage) }
.adaptError {
case e: S3Exception if e.statusCode() == StatusCodes.Forbidden.intValue =>
BucketAccessDenied(bucket, key, e.getMessage)
case e => UnexpectedSaveError(key, e.getMessage)
}
}

private def fileMetadata(
Expand Down