Skip to content

Commit

Permalink
Add Vary header for resource/files fetch operations (#4337)
Browse files Browse the repository at this point in the history
  • Loading branch information
olivergrabinski authored Oct 6, 2023
1 parent c1d5d65 commit d24ff40
Show file tree
Hide file tree
Showing 7 changed files with 74 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ final class ResourcesRoutes(
}
},
// Fetch a resource
(get & idSegmentRef(id)) { id =>
(get & idSegmentRef(id) & varyAcceptHeaders) { id =>
emitOrFusionRedirect(
ref,
id,
Expand Down Expand Up @@ -173,7 +173,7 @@ final class ResourcesRoutes(
}
},
// Fetch a resource original source
(pathPrefix("source") & get & pathEndOrSingleSlash & idSegmentRef(id)) { id =>
(pathPrefix("source") & get & pathEndOrSingleSlash & idSegmentRef(id) & varyAcceptHeaders) { id =>
authorizeFor(ref, Read).apply {
parameter("annotate".as[Boolean].withDefault(false)) { annotate =>
implicit val source: Printer = sourcePrinter
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package ch.epfl.bluebrain.nexus.delta.routes

import akka.http.scaladsl.model.MediaTypes.`text/html`
import akka.http.scaladsl.model.headers.{Accept, Location, OAuth2BearerToken}
import akka.http.scaladsl.model.headers.{Accept, Location, OAuth2BearerToken, RawHeader}
import akka.http.scaladsl.model.{StatusCodes, Uri}
import akka.http.scaladsl.server.Route
import cats.effect.IO
Expand Down Expand Up @@ -119,6 +119,8 @@ class ResourcesRoutesSpec extends BaseRouteSpec with IOFromMap {

private val payloadUpdatedWithMetdata = payloadWithMetadata deepMerge json"""{"name": "Alice", "address": null}"""

private val varyHeader = RawHeader("Vary", "Accept,Accept-Encoding")

"A resource route" should {

"fail to create a resource without resources/write permission" in {
Expand Down Expand Up @@ -359,6 +361,7 @@ class ResourcesRoutesSpec extends BaseRouteSpec with IOFromMap {
status shouldEqual StatusCodes.OK
val meta = resourceMetadata(projectRef, myId, schemas.resources, "Custom", deprecated = true, rev = 10)
response.asJson shouldEqual payloadUpdated.dropNullValues.deepMerge(meta).deepMerge(resourceCtx)
response.headers should contain(varyHeader)
}
}

Expand All @@ -376,6 +379,7 @@ class ResourcesRoutesSpec extends BaseRouteSpec with IOFromMap {
Get(endpoint) ~> routes ~> check {
status shouldEqual StatusCodes.OK
response.asJson shouldEqual payload.deepMerge(meta).deepMerge(resourceCtx)
response.headers should contain(varyHeader)
}
}
}
Expand Down Expand Up @@ -440,6 +444,7 @@ class ResourcesRoutesSpec extends BaseRouteSpec with IOFromMap {
"id" -> "https://bluebrain.github.io/nexus/vocabulary/wrongid",
"proj" -> "myorg/myproject"
)
response.headers should not contain varyHeader
}
}
}
Expand All @@ -448,6 +453,7 @@ class ResourcesRoutesSpec extends BaseRouteSpec with IOFromMap {
Get("/v1/resources/myorg/myproject/_/myid/source?annotate=true") ~> routes ~> check {
status shouldEqual StatusCodes.OK
response.asJson shouldEqual payloadUpdatedWithMetdata
response.headers should contain(varyHeader)
}
}

Expand All @@ -468,6 +474,7 @@ class ResourcesRoutesSpec extends BaseRouteSpec with IOFromMap {
Get(endpoint) ~> routes ~> check {
status shouldEqual StatusCodes.OK
response.asJson shouldEqual payload.deepMerge(meta)
response.headers should contain(varyHeader)
}
}
}
Expand All @@ -485,6 +492,7 @@ class ResourcesRoutesSpec extends BaseRouteSpec with IOFromMap {
Get(endpoint) ~> routes ~> check {
status shouldEqual StatusCodes.OK
response.asJson shouldEqual payload
response.headers should contain(varyHeader)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ final class FilesRoutes(
}

def fetch(id: IdSegmentRef, ref: ProjectRef)(implicit caller: Caller): Route =
headerValueByType(Accept) {
(headerValueByType(Accept) & varyAcceptHeaders) {
case accept if accept.mediaRanges.exists(metadataMediaRanges.contains) =>
emit(fetchMetadata(id, ref).rejectOn[FileNotFound])
case _ =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import akka.actor.typed
import akka.http.scaladsl.model.ContentTypes.`text/plain(UTF-8)`
import akka.http.scaladsl.model.MediaRanges._
import akka.http.scaladsl.model.MediaTypes.`text/html`
import akka.http.scaladsl.model.headers.{Accept, Location, OAuth2BearerToken}
import akka.http.scaladsl.model.headers.{Accept, Location, OAuth2BearerToken, RawHeader}
import akka.http.scaladsl.model.{StatusCodes, Uri}
import akka.http.scaladsl.server.Route
import ch.epfl.bluebrain.nexus.delta.kernel.http.MediaTypeDetectorConfig
Expand Down Expand Up @@ -126,6 +126,8 @@ class FilesRoutesSpec
private val diskIdRev = ResourceRef.Revision(dId, 1)
private val s3IdRev = ResourceRef.Revision(s3Id, 2)

private val varyHeader = RawHeader("Vary", "Accept,Accept-Encoding")

"File routes" should {

"create storages for files" in {
Expand Down Expand Up @@ -310,6 +312,7 @@ class FilesRoutesSpec
Get(s"/v1/files/org/proj/file1$suffix") ~> Accept(`*/*`) ~> routes ~> check {
response.status shouldEqual StatusCodes.Forbidden
response.asJson shouldEqual jsonContentOf("errors/authorization-failed.json")
response.headers should not contain varyHeader
}
}
}
Expand All @@ -320,6 +323,7 @@ class FilesRoutesSpec
Get(s"/v1/files/org/proj/file1$suffix") ~> Accept(`video/*`) ~> routes ~> check {
response.status shouldEqual StatusCodes.NotAcceptable
response.asJson shouldEqual jsonContentOf("errors/content-type.json", "expected" -> "text/plain")
response.headers should not contain varyHeader
}
}
}
Expand All @@ -336,6 +340,7 @@ class FilesRoutesSpec
header("Content-Disposition").value.value() shouldEqual
s"""attachment; filename="=?UTF-8?B?$filename64?=""""
response.asString shouldEqual content
response.headers should contain(varyHeader)
}
}
}
Expand All @@ -362,6 +367,7 @@ class FilesRoutesSpec
header("Content-Disposition").value.value() shouldEqual
s"""attachment; filename="=?UTF-8?B?$filename64?=""""
response.asString shouldEqual content
response.headers should contain(varyHeader)
}
}
}
Expand All @@ -375,6 +381,7 @@ class FilesRoutesSpec
Get(s"$endpoint$suffix") ~> Accept(`application/ld+json`) ~> routes ~> check {
response.status shouldEqual StatusCodes.Forbidden
response.asJson shouldEqual jsonContentOf("errors/authorization-failed.json")
response.headers should not contain varyHeader
}
}
}
Expand All @@ -386,6 +393,7 @@ class FilesRoutesSpec
status shouldEqual StatusCodes.OK
val attr = attributes("file-idx-1.txt")
response.asJson shouldEqual fileMetadata(projectRef, file1, attr, diskIdRev, rev = 4, createdBy = alice)
response.headers should contain(varyHeader)
}
}

Expand All @@ -406,6 +414,7 @@ class FilesRoutesSpec
status shouldEqual StatusCodes.OK
response.asJson shouldEqual
fileMetadata(projectRef, file1, attr, s3IdRev, createdBy = alice, updatedBy = alice)
response.headers should contain(varyHeader)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package ch.epfl.bluebrain.nexus.delta.sdk.ce
import akka.http.scaladsl.model.MediaTypes.{`application/json`, `text/html`}
import akka.http.scaladsl.model.StatusCodes.{Redirection, SeeOther}
import akka.http.scaladsl.model._
import akka.http.scaladsl.model.headers.{`Last-Event-ID`, Accept}
import akka.http.scaladsl.model.headers.{`Accept-Encoding`, `Last-Event-ID`, Accept, RawHeader}
import akka.http.scaladsl.server.ContentNegotiator.Alternative
import akka.http.scaladsl.server.Directives._
import akka.http.scaladsl.server._
Expand Down Expand Up @@ -164,4 +164,19 @@ trait DeltaDirectives extends UriDirectives {
}
case None => provide(Offset.Start)
}

/** Injects a `Vary: Accept,Accept-Encoding` into the response */
def varyAcceptHeaders: Directive0 =
vary(Set(Accept.name, `Accept-Encoding`.name))

private def vary(headers: Set[String]): Directive0 =
respondWithHeader(RawHeader("Vary", headers.mkString(",")))

private def respondWithHeader(responseHeader: HttpHeader): Directive0 =
mapSuccessResponse(r => r.withHeaders(r.headers :+ responseHeader))

private def mapSuccessResponse(f: HttpResponse => HttpResponse): Directive0 =
mapRouteResultPF {
case RouteResult.Complete(response) if response.status.isSuccess => RouteResult.Complete(f(response))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package ch.epfl.bluebrain.nexus.delta.sdk.directives
import akka.http.scaladsl.model.MediaTypes.{`application/json`, `text/html`}
import akka.http.scaladsl.model.StatusCodes.{Redirection, SeeOther}
import akka.http.scaladsl.model._
import akka.http.scaladsl.model.headers.{`Last-Event-ID`, Accept}
import akka.http.scaladsl.model.headers.{`Accept-Encoding`, `Last-Event-ID`, Accept, RawHeader}
import akka.http.scaladsl.server.ContentNegotiator.Alternative
import akka.http.scaladsl.server.Directives._
import akka.http.scaladsl.server._
Expand Down Expand Up @@ -179,4 +179,19 @@ trait DeltaDirectives extends UriDirectives {
/** The URI of fusion's main login page */
def fusionLoginUri(implicit config: FusionConfig): UIO[Uri] =
UIO.pure { config.base / "login" }

/** Injects a `Vary: Accept,Accept-Encoding` into the response */
def varyAcceptHeaders: Directive0 =
vary(Set(Accept.name, `Accept-Encoding`.name))

private def vary(headers: Set[String]): Directive0 =
respondWithHeader(RawHeader("Vary", headers.mkString(",")))

private def respondWithHeader(responseHeader: HttpHeader): Directive0 =
mapSuccessResponse(r => r.withHeaders(r.headers :+ responseHeader))

private def mapSuccessResponse(f: HttpResponse => HttpResponse): Directive0 =
mapRouteResultPF {
case RouteResult.Complete(response) if response.status.isSuccess => RouteResult.Complete(f(response))
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package ch.epfl.bluebrain.nexus.tests.kg

import akka.http.scaladsl.model.MediaTypes.`text/html`
import akka.http.scaladsl.model.headers.{Accept, Location}
import akka.http.scaladsl.model.headers.{Accept, Location, RawHeader}
import akka.http.scaladsl.model.{MediaRange, StatusCodes}
import akka.http.scaladsl.unmarshalling.PredefinedFromEntityUnmarshallers
import cats.implicits._
Expand Down Expand Up @@ -35,6 +35,8 @@ class ResourcesSpec extends BaseSpec with EitherValuable with CirceEq {
private val IdLens: Optional[Json, String] = root.`@id`.string
private val TypeLens: Optional[Json, String] = root.`@type`.string

private val varyHeader = RawHeader("Vary", "Accept,Accept-Encoding")

private val resource1Id = "https://dev.nexus.test.com/simplified-resource/1"
private def resource1Response(rev: Int, priority: Int) =
SimpleResource.fetchResponse(Rick, id1, resource1Id, rev, priority)
Expand Down Expand Up @@ -130,18 +132,24 @@ class ResourcesSpec extends BaseSpec with EitherValuable with CirceEq {
}

"fail to fetch the resource when the user does not have access" in {
deltaClient.get[Json](s"/resources/$id1/test-schema/test-resource:1", Anonymous) { expectForbidden }
deltaClient.get[Json](s"/resources/$id1/test-schema/test-resource:1", Anonymous) { (_, response) =>
expectForbidden
response.headers should not contain varyHeader
}
}

"fail to fetch the original payload when the user does not have access" in {
deltaClient.get[Json](s"/resources/$id1/test-schema/test-resource:1/source", Anonymous) {
deltaClient.get[Json](s"/resources/$id1/test-schema/test-resource:1/source", Anonymous) { (_, response) =>
expectForbidden
response.headers should not contain varyHeader
}
}

"fail to fetch the annotated original payload when the user does not have access" in {
deltaClient.get[Json](s"/resources/$id1/test-schema/test-resource:1/source?annotate=true", Anonymous) {
expectForbidden
(_, response) =>
expectForbidden
response.headers should not contain varyHeader
}
}

Expand All @@ -150,6 +158,7 @@ class ResourcesSpec extends BaseSpec with EitherValuable with CirceEq {
val expected = resource1Response(1, 5)
response.status shouldEqual StatusCodes.OK
filterMetadataKeys(json) should equalIgnoreArrayOrder(expected)
response.headers should contain(varyHeader)
}
}

Expand All @@ -158,6 +167,7 @@ class ResourcesSpec extends BaseSpec with EitherValuable with CirceEq {
val expected = SimpleResource.sourcePayload(resource1Id, 5)
response.status shouldEqual StatusCodes.OK
json should equalIgnoreArrayOrder(expected)
response.headers should contain(varyHeader)
}
}

Expand All @@ -167,6 +177,7 @@ class ResourcesSpec extends BaseSpec with EitherValuable with CirceEq {
response.status shouldEqual StatusCodes.OK
val expected = resource1AnnotatedSource(1, 5)
filterMetadataKeys(json) should equalIgnoreArrayOrder(expected)
response.headers should contain(varyHeader)
}
}

Expand All @@ -179,6 +190,7 @@ class ResourcesSpec extends BaseSpec with EitherValuable with CirceEq {
}
_ <- deltaClient.get[Json](s"/resources/$id1/_/42/source?annotate=true", Morty) { (json, response) =>
response.status shouldEqual StatusCodes.OK
response.headers should contain(varyHeader)
json should have(`@id`(s"42"))
}
} yield succeed
Expand All @@ -198,6 +210,7 @@ class ResourcesSpec extends BaseSpec with EitherValuable with CirceEq {
_ <- deltaClient.get[Json](s"/resources/$id1/_/${UrlUtils.encode(generatedId)}/source?annotate=true", Morty) {
(json, response) =>
response.status shouldEqual StatusCodes.OK
response.headers should contain(varyHeader)
json should have(`@id`(generatedId))
}
} yield succeed
Expand All @@ -207,6 +220,7 @@ class ResourcesSpec extends BaseSpec with EitherValuable with CirceEq {
deltaClient.get[Json](s"/resources/$id1/test-schema/does-not-exist-resource:1/source?annotate=true", Morty) {
(_, response) =>
response.status shouldEqual StatusCodes.NotFound
response.headers should not contain varyHeader
}
}

Expand All @@ -215,6 +229,7 @@ class ResourcesSpec extends BaseSpec with EitherValuable with CirceEq {

deltaClient.put[Json](s"/resources/$id2/test-schema/test-resource:1", payload, Rick) { (_, response) =>
response.status shouldEqual StatusCodes.NotFound
response.headers should not contain varyHeader
}
}

Expand All @@ -225,6 +240,7 @@ class ResourcesSpec extends BaseSpec with EitherValuable with CirceEq {

deltaClient.put[Json](s"/resources/$id2/_/test-resource:1", payload, Rick) { (_, response) =>
response.status shouldEqual StatusCodes.BadRequest
response.headers should not contain varyHeader
}
}
}
Expand Down

0 comments on commit d24ff40

Please sign in to comment.