This is an automated email from the ASF dual-hosted git repository.
rcordier pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/james-project.git
commit 22769e0aa47ad24a140b8b5b0c818fbb05dd4f2d
Author: Benoit Tellier <btell...@linagora.com>
AuthorDate: Mon Aug 17 15:49:30 2020 +0700
JAMES-3359 Mailbox/set update patch validation
---
.../contract/MailboxSetMethodContract.scala | 214 ++++++++++++++++++++-
.../org/apache/james/jmap/mail/MailboxSet.scala | 25 ++-
.../james/jmap/method/MailboxSetMethod.scala | 5 +-
3 files changed, 237 insertions(+), 7 deletions(-)
diff --git
a/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/MailboxSetMethodContract.scala
b/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/MailboxSetMethodContract.scala
index 8d682ca..25b3859 100644
---
a/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/MailboxSetMethodContract.scala
+++
b/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/MailboxSetMethodContract.scala
@@ -2262,6 +2262,218 @@ trait MailboxSetMethodContract {
}
@Test
+ def updateShouldFailWhenWrongJsonObject(server: GuiceJamesServer): Unit = {
+ val mailboxId1: MailboxId =
server.getProbe(classOf[MailboxProbeImpl]).createMailbox(MailboxPath.forUser(BOB,
"previousName"))
+ val request =
+ s"""
+ |{
+ | "using": [ "urn:ietf:params:jmap:core",
"urn:ietf:params:jmap:mail" ],
+ | "methodCalls": [
+ | ["Mailbox/set",
+ | {
+ | "accountId":
"29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+ | "update": {
+ | "${mailboxId1.serialize()}": {
+ | "/name": ["newName"]
+ | }
+ | }
+ | },
+ | "c1"]]
+ |}
+ |""".stripMargin
+
+ val response = `given`
+ .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+ .body(request)
+ .when
+ .post
+ .`then`
+ .log().ifValidationFails()
+ .statusCode(SC_OK)
+ .contentType(JSON)
+ .extract
+ .body
+ .asString
+
+ assertThatJson(response).isEqualTo(
+ s"""{
+ | "sessionState": "75128aab4b1b",
+ | "methodResponses": [
+ | ["Mailbox/set", {
+ | "accountId":
"29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+ | "newState": "000001",
+ | "notUpdated": {
+ | "${mailboxId1.serialize()}": {
+ | "type": "invalidArguments",
+ | "description": "Expecting a
JSON string as an argument",
+ | "properties": ["/name"]
+ | }
+ | }
+ | }, "c1"]
+ | ]
+ |}""".stripMargin)
+ }
+
+ @Test
+ def updateShouldFailWhenUnknownProperty(server: GuiceJamesServer): Unit = {
+ val mailboxId1: MailboxId =
server.getProbe(classOf[MailboxProbeImpl]).createMailbox(MailboxPath.forUser(BOB,
"previousName"))
+ val request =
+ s"""
+ |{
+ | "using": [ "urn:ietf:params:jmap:core",
"urn:ietf:params:jmap:mail" ],
+ | "methodCalls": [
+ | ["Mailbox/set",
+ | {
+ | "accountId":
"29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+ | "update": {
+ | "${mailboxId1.serialize()}": {
+ | "/unknown": "newValue"
+ | }
+ | }
+ | },
+ | "c1"]]
+ |}
+ |""".stripMargin
+
+ val response = `given`
+ .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+ .body(request)
+ .when
+ .post
+ .`then`
+ .log().ifValidationFails()
+ .statusCode(SC_OK)
+ .contentType(JSON)
+ .extract
+ .body
+ .asString
+
+ assertThatJson(response).isEqualTo(
+ s"""{
+ | "sessionState": "75128aab4b1b",
+ | "methodResponses": [
+ | ["Mailbox/set", {
+ | "accountId":
"29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+ | "newState": "000001",
+ | "notUpdated": {
+ | "${mailboxId1.serialize()}": {
+ | "type": "invalidArguments",
+ | "description": "/unknown
property do not exist thus cannot be updated",
+ | "properties": ["/unknown"]
+ | }
+ | }
+ | }, "c1"]
+ | ]
+ |}""".stripMargin)
+ }
+
+ @Test
+ def updateShouldFailWhenEmptyProperty(server: GuiceJamesServer): Unit = {
+ val mailboxId1: MailboxId =
server.getProbe(classOf[MailboxProbeImpl]).createMailbox(MailboxPath.forUser(BOB,
"previousName"))
+ val request =
+ s"""
+ |{
+ | "using": [ "urn:ietf:params:jmap:core",
"urn:ietf:params:jmap:mail" ],
+ | "methodCalls": [
+ | ["Mailbox/set",
+ | {
+ | "accountId":
"29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+ | "update": {
+ | "${mailboxId1.serialize()}": {
+ | "": "newValue"
+ | }
+ | }
+ | },
+ | "c1"]]
+ |}
+ |""".stripMargin
+
+ val response = `given`
+ .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+ .body(request)
+ .when
+ .post
+ .`then`
+ .log().ifValidationFails()
+ .statusCode(SC_OK)
+ .contentType(JSON)
+ .extract
+ .body
+ .asString
+
+ val message = "Invalid property specified in a patch object: Both
predicates of (!isEmpty() && \\\"\\\".startsWith(\\\"/\\\")) failed. Left:
Predicate isEmpty() did not fail. Right: Predicate failed:
\\\"\\\".startsWith(\\\"/\\\")."
+ assertThatJson(response).isEqualTo(
+ s"""{
+ | "sessionState": "75128aab4b1b",
+ | "methodResponses": [
+ | ["Mailbox/set", {
+ | "accountId":
"29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+ | "newState": "000001",
+ | "notUpdated": {
+ | "${mailboxId1.serialize()}": {
+ | "type": "invalidPatch",
+ | "description": "$message"
+ | }
+ | }
+ | }, "c1"]
+ | ]
+ |}""".stripMargin)
+ }
+
+ @Test
+ def updateShouldFailWhenInvalidProperty(server: GuiceJamesServer): Unit = {
+ val mailboxId1: MailboxId =
server.getProbe(classOf[MailboxProbeImpl]).createMailbox(MailboxPath.forUser(BOB,
"previousName"))
+ val request =
+ s"""
+ |{
+ | "using": [ "urn:ietf:params:jmap:core",
"urn:ietf:params:jmap:mail" ],
+ | "methodCalls": [
+ | ["Mailbox/set",
+ | {
+ | "accountId":
"29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+ | "update": {
+ | "${mailboxId1.serialize()}": {
+ | "name": "newValue"
+ | }
+ | }
+ | },
+ | "c1"]]
+ |}
+ |""".stripMargin
+
+ val response = `given`
+ .header(ACCEPT.toString, ACCEPT_RFC8621_VERSION_HEADER)
+ .body(request)
+ .when
+ .post
+ .`then`
+ .log().ifValidationFails()
+ .statusCode(SC_OK)
+ .contentType(JSON)
+ .extract
+ .body
+ .asString
+
+ val message = "Invalid property specified in a patch object: Right
predicate of (!isEmpty(name) && \\\"name\\\".startsWith(\\\"/\\\")) failed:
Predicate failed: \\\"name\\\".startsWith(\\\"/\\\")."
+ assertThatJson(response).isEqualTo(
+ s"""{
+ | "sessionState": "75128aab4b1b",
+ | "methodResponses": [
+ | ["Mailbox/set", {
+ | "accountId":
"29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+ | "newState": "000001",
+ | "notUpdated": {
+ | "${mailboxId1.serialize()}": {
+ | "type": "invalidPatch",
+ | "description": "$message"
+ | }
+ | }
+ | }, "c1"]
+ | ]
+ |}""".stripMargin)
+ }
+
+ @Test
def updateShouldFailWhenMailboxNameIsTooLong(server: GuiceJamesServer): Unit
= {
val mailboxId1: MailboxId =
server.getProbe(classOf[MailboxProbeImpl]).createMailbox(MailboxPath.forUser(BOB,
"previousName"))
val request =
@@ -2523,6 +2735,4 @@ trait MailboxSetMethodContract {
| ]
|}""".stripMargin)
}
-
- // TODO invalid path handling (unknown property, invalid name)
}
diff --git
a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/MailboxSet.scala
b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/MailboxSet.scala
index 3452209..e0964d6 100644
---
a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/MailboxSet.scala
+++
b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/MailboxSet.scala
@@ -22,8 +22,12 @@ package org.apache.james.jmap.mail
import eu.timepit.refined
import eu.timepit.refined.api.Refined
import eu.timepit.refined.auto._
+import eu.timepit.refined.boolean.And
import eu.timepit.refined.collection.NonEmpty
+import eu.timepit.refined.refineV
+import eu.timepit.refined.string.StartsWith
import org.apache.james.jmap.mail.MailboxName.MailboxName
+import org.apache.james.jmap.mail.MailboxPatchObject.MailboxPatchObjectKey
import org.apache.james.jmap.mail.MailboxSetRequest.{MailboxCreationId,
UnparsedMailboxId}
import org.apache.james.jmap.model.AccountId
import org.apache.james.jmap.model.State.State
@@ -55,11 +59,20 @@ case class MailboxCreationRequest(name: MailboxName,
isSubscribed: Option[IsSubscribed],
rights: Option[Rights])
+object MailboxPatchObject {
+ type KeyConstraint = NonEmpty And StartsWith["/"]
+ type MailboxPatchObjectKey = String Refined KeyConstraint
+}
+
case class MailboxPatchObject(value: Map[String, JsValue]) {
def updates: Iterable[Either[PatchUpdateValidationException, Update]] =
value.map({
case (property, newValue) => property match {
case "/name" => NameUpdate.parse(newValue)
- case property => Left(UnsupportedPropertyUpdated(property))
+ case property =>
+ val refinedKey: Either[String, MailboxPatchObjectKey] =
refineV(property)
+ refinedKey.fold[Either[PatchUpdateValidationException, Update]](
+ cause => Left(InvalidPropertyException(property = property, cause =
s"Invalid property specified in a patch object: $cause")),
+ value => Left(UnsupportedPropertyUpdatedException(value)))
}
})
}
@@ -77,6 +90,7 @@ case class MailboxSetResponse(accountId: AccountId,
object MailboxSetError {
val invalidArgumentValue: SetErrorType = "invalidArguments"
val serverFailValue: SetErrorType = "serverFail"
+ val invalidPatchValue: SetErrorType = "invalidPatch"
val notFoundValue: SetErrorType = "notFound"
val mailboxHasEmailValue: SetErrorType = "mailboxHasEmail"
val mailboxHasChildValue: SetErrorType = "mailboxHasChild"
@@ -87,6 +101,7 @@ object MailboxSetError {
def notFound(description: Option[SetErrorDescription]) =
MailboxSetError(notFoundValue, description, None)
def mailboxHasEmail(description: Option[SetErrorDescription]) =
MailboxSetError(mailboxHasEmailValue, description, None)
def mailboxHasChild(description: Option[SetErrorDescription]) =
MailboxSetError(mailboxHasChildValue, description, None)
+ def invalidPatch(description: Option[SetErrorDescription]) =
MailboxSetError(invalidPatchValue, description, None)
def forbidden(description: Option[SetErrorDescription], properties:
Option[Properties]) = MailboxSetError(forbiddenValue, description, properties)
}
@@ -114,7 +129,7 @@ case class MailboxUpdateResponse(value: JsObject)
object NameUpdate {
def parse(newValue: JsValue): Either[PatchUpdateValidationException, Update]
= newValue match {
case JsString(newName) => scala.Right(NameUpdate(newName))
- case _ => Left(InvalidUpdate("name", "Expectint a JSON string as an
argument"))
+ case _ => Left(InvalidUpdateException("/name", "Expecting a JSON string as
an argument"))
}
}
@@ -122,5 +137,7 @@ sealed trait Update
case class NameUpdate(newName: String) extends Update
class PatchUpdateValidationException() extends IllegalArgumentException
-case class UnsupportedPropertyUpdated(property: String) extends
PatchUpdateValidationException
-case class InvalidUpdate(property: String, cause: String) extends
PatchUpdateValidationException
\ No newline at end of file
+case class UnsupportedPropertyUpdatedException(property:
MailboxPatchObjectKey) extends PatchUpdateValidationException
+case class InvalidPropertyUpdatedException(property: MailboxPatchObjectKey)
extends PatchUpdateValidationException
+case class InvalidPropertyException(property: String, cause: String) extends
PatchUpdateValidationException
+case class InvalidUpdateException(property: MailboxPatchObjectKey, cause:
String) extends PatchUpdateValidationException
\ No newline at end of file
diff --git
a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/MailboxSetMethod.scala
b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/MailboxSetMethod.scala
index 6018c8f..2d10905 100644
---
a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/MailboxSetMethod.scala
+++
b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/MailboxSetMethod.scala
@@ -23,7 +23,7 @@ import eu.timepit.refined.auto._
import javax.inject.Inject
import org.apache.james.jmap.json.Serializer
import org.apache.james.jmap.mail.MailboxSetRequest.{MailboxCreationId,
UnparsedMailboxId}
-import org.apache.james.jmap.mail.{IsSubscribed, MailboxCreationRequest,
MailboxCreationResponse, MailboxPatchObject, MailboxRights, MailboxSetError,
MailboxSetRequest, MailboxSetResponse, MailboxUpdateResponse, NameUpdate,
PatchUpdateValidationException, Properties, SetErrorDescription, TotalEmails,
TotalThreads, UnreadEmails, UnreadThreads}
+import org.apache.james.jmap.mail.{InvalidPropertyException,
InvalidUpdateException, IsSubscribed, MailboxCreationRequest,
MailboxCreationResponse, MailboxPatchObject, MailboxRights, MailboxSetError,
MailboxSetRequest, MailboxSetResponse, MailboxUpdateResponse, NameUpdate,
PatchUpdateValidationException, Properties, SetErrorDescription, TotalEmails,
TotalThreads, UnreadEmails, UnreadThreads, UnsupportedPropertyUpdatedException}
import org.apache.james.jmap.model.CapabilityIdentifier.CapabilityIdentifier
import org.apache.james.jmap.model.Invocation.{Arguments, MethodName}
import org.apache.james.jmap.model.{ClientId, Id, Invocation, ServerId, State}
@@ -123,6 +123,9 @@ case class UpdateFailure(mailboxId: UnparsedMailboxId,
exception: Throwable) ext
case e: MailboxNotFoundException =>
MailboxSetError.notFound(Some(SetErrorDescription(e.getMessage)))
case e: MailboxNameException =>
MailboxSetError.invalidArgument(Some(SetErrorDescription(e.getMessage)),
Some(Properties(List("/name"))))
case e: MailboxExistsException =>
MailboxSetError.invalidArgument(Some(SetErrorDescription(e.getMessage)),
Some(Properties(List("/name"))))
+ case e: UnsupportedPropertyUpdatedException =>
MailboxSetError.invalidArgument(Some(SetErrorDescription(s"${e.property}
property do not exist thus cannot be updated")),
Some(Properties(List(e.property))))
+ case e: InvalidUpdateException =>
MailboxSetError.invalidArgument(Some(SetErrorDescription(s"${e.cause}")),
Some(Properties(List(e.property))))
+ case e: InvalidPropertyException =>
MailboxSetError.invalidPatch(Some(SetErrorDescription(s"${e.cause}")))
case e: SystemMailboxChangeException =>
MailboxSetError.invalidArgument(Some(SetErrorDescription("Invalid change to a
system mailbox")), Some(Properties(List("/name"))))
case _ =>
MailboxSetError.serverFail(Some(SetErrorDescription(exception.getMessage)),
None)
}
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org
For additional commands, e-mail: server-dev-h...@james.apache.org
