Skip to content

Commit

Permalink
Remove the ability to accept payloads with underscore via config (#5304)
Browse files Browse the repository at this point in the history
Co-authored-by: Simon Dumas <[email protected]>
  • Loading branch information
imsdu and Simon Dumas authored Mar 7, 2025
1 parent f255f8b commit a928019
Show file tree
Hide file tree
Showing 10 changed files with 40 additions and 138 deletions.
2 changes: 0 additions & 2 deletions delta/app/src/main/resources/app.conf
Original file line number Diff line number Diff line change
Expand Up @@ -298,8 +298,6 @@ app {
resources {
# the resources event-log configuration
event-log = ${app.defaults.event-log}
# Reject payloads which contain nexus metadata fields (any field beginning with _)
decoding-option = "strict"
# Defines exceptions for schema enforcement
schema-enforcement {
type-whitelist = []
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import ch.epfl.bluebrain.nexus.delta.sdk.marshalling.{OriginalSource, RdfMarshal
import ch.epfl.bluebrain.nexus.delta.sdk.model.routes.Tag
import ch.epfl.bluebrain.nexus.delta.sdk.model.{BaseUri, ResourceF}
import ch.epfl.bluebrain.nexus.delta.sdk.permissions.Permissions.resources.{read => Read, write => Write}
import ch.epfl.bluebrain.nexus.delta.sdk.resources.NexusSource.DecodingOption
import ch.epfl.bluebrain.nexus.delta.sdk.resources.model.ResourceRejection._
import ch.epfl.bluebrain.nexus.delta.sdk.resources.model.{Resource, ResourceRejection}
import ch.epfl.bluebrain.nexus.delta.sdk.resources.{NexusSource, Resources}
Expand All @@ -46,8 +45,7 @@ final class ResourcesRoutes(
baseUri: BaseUri,
cr: RemoteContextResolution,
ordering: JsonKeyOrdering,
fusionConfig: FusionConfig,
decodingOption: DecodingOption
fusionConfig: FusionConfig
) extends AuthDirectives(identities, aclCheck)
with CirceUnmarshalling
with RdfMarshalling {
Expand Down Expand Up @@ -301,8 +299,7 @@ object ResourcesRoutes {
baseUri: BaseUri,
cr: RemoteContextResolution,
ordering: JsonKeyOrdering,
fusionConfig: FusionConfig,
decodingOption: DecodingOption
fusionConfig: FusionConfig
): Route = new ResourcesRoutes(identities, aclCheck, resources, index).routes

}
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import ch.epfl.bluebrain.nexus.delta.sdk.marshalling.RdfMarshalling
import ch.epfl.bluebrain.nexus.delta.sdk.model.IdSegment.IriSegment
import ch.epfl.bluebrain.nexus.delta.sdk.model.{BaseUri, IdSegment}
import ch.epfl.bluebrain.nexus.delta.sdk.permissions.Permissions.resources.{write => Write}
import ch.epfl.bluebrain.nexus.delta.sdk.resources.NexusSource.DecodingOption
import ch.epfl.bluebrain.nexus.delta.sdk.resources.model.ResourceRejection
import ch.epfl.bluebrain.nexus.delta.sdk.resources.{NexusSource, ResourcesTrial}
import ch.epfl.bluebrain.nexus.delta.sdk.schemas.Schemas
Expand All @@ -40,8 +39,7 @@ final class ResourcesTrialRoutes(
)(implicit
baseUri: BaseUri,
cr: RemoteContextResolution,
ordering: JsonKeyOrdering,
decodingOption: DecodingOption
ordering: JsonKeyOrdering
) extends AuthDirectives(identities, aclCheck)
with CirceUnmarshalling
with RdfMarshalling {
Expand Down Expand Up @@ -74,7 +72,7 @@ final class ResourcesTrialRoutes(
extractCaller { implicit caller =>
(projectRef & pathEndOrSingleSlash) { project =>
authorizeFor(project, Write).apply {
(entity(as[GenerationInput])) { input =>
entity(as[GenerationInput]) { input =>
generate(project, input)
}
}
Expand Down Expand Up @@ -133,9 +131,8 @@ object ResourcesTrialRoutes {

private[routes] object GenerationInput {

implicit def generationInputDecoder(implicit decodingOption: DecodingOption): Decoder[GenerationInput] = {
implicit val configuration: Configuration = Configuration.default.withDefaults
implicit val nexusSourceDecoder: Decoder[NexusSource] = NexusSource.nexusSourceDecoder
implicit val generationInputDecoder: Decoder[GenerationInput] = {
implicit val configuration: Configuration = Configuration.default.withDefaults
deriveConfiguredDecoder[GenerationInput]
}
}
Expand All @@ -148,8 +145,7 @@ object ResourcesTrialRoutes {
)(implicit
baseUri: BaseUri,
cr: RemoteContextResolution,
ordering: JsonKeyOrdering,
decodingOption: DecodingOption
ordering: JsonKeyOrdering
): ResourcesTrialRoutes =
new ResourcesTrialRoutes(
identities,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,7 @@ object ResourcesModule extends ModuleDef {
baseUri: BaseUri,
cr: RemoteContextResolution @Id("aggregate"),
ordering: JsonKeyOrdering,
fusionConfig: FusionConfig,
config: ResourcesConfig
fusionConfig: FusionConfig
) =>
new ResourcesRoutes(
identities,
Expand All @@ -104,8 +103,7 @@ object ResourcesModule extends ModuleDef {
baseUri,
cr,
ordering,
fusionConfig,
config.decodingOption
fusionConfig
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import ch.epfl.bluebrain.nexus.delta.sdk.identities.Identities
import ch.epfl.bluebrain.nexus.delta.sdk.model.BaseUri
import ch.epfl.bluebrain.nexus.delta.sdk.projects.FetchContext
import ch.epfl.bluebrain.nexus.delta.sdk.resolvers.ResolverContextResolution
import ch.epfl.bluebrain.nexus.delta.sdk.resources.{Resources, ResourcesConfig, ResourcesTrial, ValidateResource}
import ch.epfl.bluebrain.nexus.delta.sdk.resources.{Resources, ResourcesTrial, ValidateResource}
import ch.epfl.bluebrain.nexus.delta.sdk.schemas.Schemas
import distage.ModuleDef
import izumi.distage.model.definition.Id
Expand Down Expand Up @@ -48,20 +48,14 @@ object ResourcesTrialModule extends ModuleDef {
resourcesTrial: ResourcesTrial,
baseUri: BaseUri,
cr: RemoteContextResolution @Id("aggregate"),
ordering: JsonKeyOrdering,
config: ResourcesConfig
ordering: JsonKeyOrdering
) =>
ResourcesTrialRoutes(
identities,
aclCheck,
schemas,
resourcesTrial
)(
baseUri,
cr,
ordering,
config.decodingOption
)
)(baseUri, cr, ordering)
}

many[PriorityRoute].add { (route: ResourcesTrialRoutes) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import akka.http.scaladsl.model.MediaTypes.`text/html`
import akka.http.scaladsl.model.headers.{Accept, Location, OAuth2BearerToken, RawHeader}
import akka.http.scaladsl.model.{RequestEntity, StatusCodes, Uri}
import akka.http.scaladsl.server.Route
import cats.implicits._
import cats.syntax.all._
import ch.epfl.bluebrain.nexus.delta.kernel.utils.{UUIDF, UrlUtils}
import ch.epfl.bluebrain.nexus.delta.rdf.IriOrBNode.Iri
import ch.epfl.bluebrain.nexus.delta.rdf.Vocabulary.{contexts, nxv, schema => schemaOrg, schemas}
Expand All @@ -17,11 +17,10 @@ import ch.epfl.bluebrain.nexus.delta.sdk.identities.IdentitiesDummy
import ch.epfl.bluebrain.nexus.delta.sdk.identities.model.Caller
import ch.epfl.bluebrain.nexus.delta.sdk.implicits._
import ch.epfl.bluebrain.nexus.delta.sdk.model.{IdSegmentRef, ResourceScope}
import ch.epfl.bluebrain.nexus.delta.sdk.permissions.Permissions.resources
import ch.epfl.bluebrain.nexus.delta.sdk.permissions.Permissions.resources.{read, write}
import ch.epfl.bluebrain.nexus.delta.sdk.projects.FetchContextDummy
import ch.epfl.bluebrain.nexus.delta.sdk.projects.model.ApiMappings
import ch.epfl.bluebrain.nexus.delta.sdk.resolvers.ResolverContextResolution
import ch.epfl.bluebrain.nexus.delta.sdk.resources.NexusSource.DecodingOption
import ch.epfl.bluebrain.nexus.delta.sdk.resources._
import ch.epfl.bluebrain.nexus.delta.sdk.utils.BaseRouteSpec
import ch.epfl.bluebrain.nexus.delta.sourcing.ScopedEventLog
Expand Down Expand Up @@ -73,8 +72,6 @@ class ResourcesRoutesSpec extends BaseRouteSpec with ValidateResourceFixture wit
private def simplePayload(id: String) = jsonContentOf("resources/resource.json", "id" -> (nxv + id))
private val payloadWithoutId = payload.removeKeys(keywords.id)
private val payloadWithBlankId = jsonContentOf("resources/resource.json", "id" -> "")
private val payloadWithUnderscoreFields =
jsonContentOf("resources/resource-with-underscore-fields.json", "id" -> myId)
private def payloadWithMetadata(id: String) = jsonContentOf(
"resources/resource-with-metadata.json",
"id" -> (nxv + id),
Expand All @@ -87,29 +84,25 @@ class ResourcesRoutesSpec extends BaseRouteSpec with ValidateResourceFixture wit
private val fetchContext = FetchContextDummy(List(project.value))
private val resolverContextResolution: ResolverContextResolution = ResolverContextResolution(rcr)

private def routesWithDecodingOption(implicit decodingOption: DecodingOption): (Route, Resources) = {
private lazy val resources = {
val resourceDef = Resources.definition(validateResource, DetectChange(enabled = true), clock)
val scopedLog = ScopedEventLog(resourceDef, eventLogConfig, xas)

val resources = ResourcesImpl(
ResourcesImpl(
scopedLog,
fetchContext,
resolverContextResolution
)
(
Route.seal(
ResourcesRoutes(
IdentitiesDummy(callerReader, callerWriter),
aclCheck,
resources,
IndexingAction.noop
)
),
resources
)
}

private lazy val routes = routesWithDecodingOption(DecodingOption.Strict)._1
private lazy val routes = Route.seal(
ResourcesRoutes(
IdentitiesDummy(callerReader, callerWriter),
aclCheck,
resources,
IndexingAction.noop
)
)

private val payloadUpdated = payload deepMerge json"""{"name": "Alice", "address": null}"""
private def payloadUpdated(id: String) =
Expand All @@ -122,9 +115,8 @@ class ResourcesRoutesSpec extends BaseRouteSpec with ValidateResourceFixture wit

override def beforeAll(): Unit = {
super.beforeAll()
aclCheck.append(AclAddress.Root, reader -> Set(resources.read)).accepted
aclCheck.append(AclAddress.Root, writer -> Set(resources.read)).accepted
aclCheck.append(AclAddress.Root, writer -> Set(resources.write)).accepted
aclCheck.append(AclAddress.Root, reader -> Set(read)).accepted
aclCheck.append(AclAddress.Root, writer -> Set(read, write)).accepted
}

"A resource route" should {
Expand All @@ -150,11 +142,10 @@ class ResourcesRoutesSpec extends BaseRouteSpec with ValidateResourceFixture wit
}

"create a tagged resource" in {
val endpoints = List(
val endpoints = List(
("/v1/resources/myorg/myproject?tag=mytag", schemas.resources),
("/v1/resources/myorg/myproject/myschema?tag=mytag", schema1)
)
val (routes, resources) = routesWithDecodingOption(DecodingOption.Strict)
forAll(endpoints) { case (endpoint, schema) =>
val id = genString()
Post(endpoint, simplePayload(id).toEntity) ~> asWriter ~> routes ~> check {
Expand All @@ -180,11 +171,10 @@ class ResourcesRoutesSpec extends BaseRouteSpec with ValidateResourceFixture wit
}

"create a tagged resource with an authenticated user and provided id" in {
val endpoints = List(
val endpoints = List(
((id: String) => s"/v1/resources/myorg/myproject/_/$id?tag=mytag", schemas.resources),
((id: String) => s"/v1/resources/myorg/myproject/myschema/$id?tag=mytag", schema1)
)
val (routes, resources) = routesWithDecodingOption(DecodingOption.Strict)
forAll(endpoints) { case (endpoint, schema) =>
val id = genString()
Put(endpoint(id), simplePayload(id).toEntity) ~> asWriter ~> routes ~> check {
Expand Down Expand Up @@ -226,25 +216,16 @@ class ResourcesRoutesSpec extends BaseRouteSpec with ValidateResourceFixture wit
}

"fail if underscore fields are present" in {
Post("/v1/resources/myorg/myproject/_/", payloadWithUnderscoreFields.toEntity) ~> asWriter ~> routes ~> check {
val payloadWithUnderscores = json"""{ "_createdAt": "1970-01-01T00:00:00Z" }"""

Post("/v1/resources/myorg/myproject/_/", payloadWithUnderscores.toEntity) ~> asWriter ~> routes ~> check {
response.status shouldEqual StatusCodes.BadRequest
response.asJson shouldEqual jsonContentOf(
"resources/errors/underscore-fields.json"
)
}
}

"succeed if underscore fields are present but the decoding is set to lenient" in {
val lenientDecodingRoutes = routesWithDecodingOption(DecodingOption.Lenient)._1

Post(
"/v1/resources/myorg/myproject/_/",
payloadWithUnderscoreFields.toEntity
) ~> asWriter ~> lenientDecodingRoutes ~> check {
response.status shouldEqual StatusCodes.Created
}
}

"fail to update a resource without resources/write permission" in {
givenAResource { id =>
Put(s"/v1/resources/myorg/myproject/_/$id?rev=1", payload.toEntity) ~> asReader ~> routes ~> check {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import ch.epfl.bluebrain.nexus.delta.sdk.implicits._
import ch.epfl.bluebrain.nexus.delta.sdk.model.IdSegment.{IriSegment, StringSegment}
import ch.epfl.bluebrain.nexus.delta.sdk.model.{IdSegment, IdSegmentRef, ResourceF, Tags}
import ch.epfl.bluebrain.nexus.delta.sdk.permissions.Permissions
import ch.epfl.bluebrain.nexus.delta.sdk.resources.NexusSource.DecodingOption
import ch.epfl.bluebrain.nexus.delta.sdk.resources.ValidationResult._
import ch.epfl.bluebrain.nexus.delta.sdk.resources._
import ch.epfl.bluebrain.nexus.delta.sdk.resources.model.ResourceRejection.{ReservedResourceId, ResourceNotFound}
Expand Down Expand Up @@ -109,8 +108,6 @@ class ResourcesTrialRoutesSpec extends BaseRouteSpec with ResourceInstanceFixtur
case _ => IO.raiseError(SchemaShaclEngineRejection(nxv + "invalid", "Invalid schema"))
}

implicit val decodingOption: DecodingOption = DecodingOption.Strict

private lazy val routes =
Route.seal(
new ResourcesTrialRoutes(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,61 +1,18 @@
package ch.epfl.bluebrain.nexus.delta.sdk.resources

import io.circe.Decoder.Result
import io.circe.{Decoder, DecodingFailure, HCursor, Json}
import pureconfig.ConfigReader
import pureconfig.error.CannotConvert
import io.circe.syntax.EncoderOps
import io.circe.{Decoder, Json}

final case class NexusSource(value: Json) extends AnyVal

object NexusSource {

sealed trait DecodingOption

object DecodingOption {
final case object Strict extends DecodingOption

final case object Lenient extends DecodingOption

implicit val decodingOptionConfigReader: ConfigReader[DecodingOption] =
ConfigReader.fromString {
case "strict" => Right(Strict)
case "lenient" => Right(Lenient)
case other =>
Left(
CannotConvert(
other,
"DecodingOption",
s"values can only be 'strict' or 'lenient'"
)
)

}
}

private val strictDecoder = new Decoder[NexusSource] {
private val decoder = implicitly[Decoder[Json]]

override def apply(c: HCursor): Result[NexusSource] = {
decoder(c).flatMap { json =>
val underscoreFields = json.asObject.toList.flatMap(_.keys).filter(_.startsWith("_"))
Either.cond(
underscoreFields.isEmpty,
NexusSource(json),
DecodingFailure(
s"Field(s) starting with _ found in payload: ${underscoreFields.mkString(", ")}",
c.history
)
)
}
}
}

private val lenientDecoder = implicitly[Decoder[Json]].map(NexusSource(_))

implicit def nexusSourceDecoder(implicit decodingOption: DecodingOption): Decoder[NexusSource] = {
decodingOption match {
case DecodingOption.Lenient => lenientDecoder
case DecodingOption.Strict => strictDecoder
}
implicit val nexusSourceDecoder: Decoder[NexusSource] = Decoder.decodeJsonObject.emap { obj =>
val underscoreFields = obj.keys.filter(_.startsWith("_"))
Either.cond(
underscoreFields.isEmpty,
NexusSource(obj.asJson),
s"Field(s) starting with _ found in payload: ${underscoreFields.mkString(", ")}"
)
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package ch.epfl.bluebrain.nexus.delta.sdk.resources

import ch.epfl.bluebrain.nexus.delta.rdf.IriOrBNode.Iri
import ch.epfl.bluebrain.nexus.delta.sdk.resources.NexusSource.DecodingOption
import ch.epfl.bluebrain.nexus.delta.sdk.resources.ResourcesConfig.SchemaEnforcementConfig
import ch.epfl.bluebrain.nexus.delta.sourcing.config.EventLogConfig
import pureconfig.ConfigReader
Expand All @@ -12,16 +11,13 @@ import pureconfig.generic.semiauto.deriveReader
*
* @param eventLog
* configuration of the event log
* @param decodingOption
* strict/lenient decoding of resources
* @param schemaEnforcement
* configuration related to schema enforcement
* @param skipUpdateNoChange
* do not create a new revision when the update does not introduce a change in the current resource state
*/
final case class ResourcesConfig(
eventLog: EventLogConfig,
decodingOption: DecodingOption,
schemaEnforcement: SchemaEnforcementConfig,
skipUpdateNoChange: Boolean
)
Expand Down
Loading

0 comments on commit a928019

Please sign in to comment.