Skip to content

Commit

Permalink
Handle S3 access error gracefully with 403 (#4954)
Browse files Browse the repository at this point in the history
  • Loading branch information
dantb authored May 11, 2024
1 parent 8a44f93 commit 17b2e7a
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 20 deletions.
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

0 comments on commit 17b2e7a

Please sign in to comment.