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 7098d7071c5debf01d11f37fc757d0c9bcaaa5af
Author: Benoit Tellier <btell...@linagora.com>
AuthorDate: Tue Aug 25 11:22:52 2020 +0700

    JAMES-3359 Reject Mailbox/set updates when capability is omitted
---
 .../contract/MailboxSetMethodContract.scala        | 184 ++++++++++++++++++++-
 .../org/apache/james/jmap/mail/MailboxSet.scala    |  61 ++++---
 .../james/jmap/method/MailboxSetMethod.scala       |  14 +-
 3 files changed, 232 insertions(+), 27 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 f968913..2a0fe99 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
@@ -175,7 +175,7 @@ trait MailboxSetMethodContract {
     val request =
       s"""
          |{
-         |   "using": [ "urn:ietf:params:jmap:core", 
"urn:ietf:params:jmap:mail" ],
+         |   "using": [ "urn:ietf:params:jmap:core", 
"urn:ietf:params:jmap:mail", "urn:apache:james:params:jmap:mail:quota" ],
          |   "methodCalls": [
          |       ["Mailbox/set",
          |           {
@@ -4476,6 +4476,188 @@ trait MailboxSetMethodContract {
   }
 
   @Test
+  def updateRightsResetShouldFailWhenOmittingCapability(server: 
GuiceJamesServer): Unit = {
+    val mailboxId = 
server.getProbe(classOf[MailboxProbeImpl]).createMailbox(MailboxPath.forUser(BOB,
 "mailbox"))
+    val request =
+      s"""
+         |{
+         |   "using": [ "urn:ietf:params:jmap:core", 
"urn:ietf:params:jmap:mail"],
+         |   "methodCalls": [
+         |       [
+         |           "Mailbox/set",
+         |           {
+         |                "accountId": 
"29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+         |                "update": {
+         |                    "${mailboxId.serialize()}": {
+         |                      "/sharedWith": {
+         |                        "${ANDRE.asString()}":["r", "l"]
+         |                      }
+         |                    }
+         |                }
+         |           },
+         |           "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": {
+         |                    "1": {
+         |                        "type": "invalidArguments",
+         |                        "description": "/sharedWith property do not 
exist thus cannot be updated",
+         |                        "properties": ["/sharedWith"]
+         |                    }
+         |                }
+         |            },
+         |            "c1"
+         |        ]
+         |    ]
+         |}""".stripMargin)
+  }
+
+  @Test
+  def updateRightsShouldFailWhenOmittingCapability(server: GuiceJamesServer): 
Unit = {
+    val mailboxId = 
server.getProbe(classOf[MailboxProbeImpl]).createMailbox(MailboxPath.forUser(BOB,
 "mailbox"))
+    val request =
+      s"""
+         |{
+         |   "using": [ "urn:ietf:params:jmap:core", 
"urn:ietf:params:jmap:mail"],
+         |   "methodCalls": [
+         |       [
+         |           "Mailbox/set",
+         |           {
+         |                "accountId": 
"29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+         |                "update": {
+         |                    "${mailboxId.serialize()}": {
+         |                      "/sharedWith/${ANDRE.asString()}": ["r", "l"]
+         |                    }
+         |                }
+         |           },
+         |           "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": {
+         |                    "1": {
+         |                        "type": "invalidArguments",
+         |                        "description": 
"/sharedWith/${ANDRE.asString()} property do not exist thus cannot be updated",
+         |                        "properties": 
["/sharedWith/${ANDRE.asString()}"]
+         |                    }
+         |                }
+         |            },
+         |            "c1"
+         |        ]
+         |    ]
+         |}""".stripMargin)
+  }
+
+  @Test
+  def updateQuotasShouldFailWhenOmittingCapability(server: GuiceJamesServer): 
Unit = {
+    val mailboxId = 
server.getProbe(classOf[MailboxProbeImpl]).createMailbox(MailboxPath.forUser(BOB,
 "mailbox"))
+    val request =
+      s"""
+         |{
+         |   "using": [ "urn:ietf:params:jmap:core", 
"urn:ietf:params:jmap:mail"],
+         |   "methodCalls": [
+         |       [
+         |           "Mailbox/set",
+         |           {
+         |                "accountId": 
"29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6",
+         |                "update": {
+         |                    "${mailboxId.serialize()}": {
+         |                      "/quotas": "toto"
+         |                    }
+         |                }
+         |           },
+         |           "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": {
+         |                    "1": {
+         |                        "type": "invalidArguments",
+         |                        "description": "/quotas property do not 
exist thus cannot be updated",
+         |                        "properties": ["/quotas"]
+         |                    }
+         |                }
+         |            },
+         |            "c1"
+         |        ]
+         |    ]
+         |}""".stripMargin)
+  }
+
+  @Test
   def updateShouldAllowSettingRights(server: GuiceJamesServer): Unit = {
     val mailboxId = 
server.getProbe(classOf[MailboxProbeImpl]).createMailbox(MailboxPath.forUser(BOB,
 "mailbox"))
     val request =
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 267fb4d..f3f8db5 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
@@ -35,8 +35,8 @@ import 
org.apache.james.jmap.model.CapabilityIdentifier.CapabilityIdentifier
 import org.apache.james.jmap.model.State.State
 import org.apache.james.jmap.model.{AccountId, CapabilityIdentifier}
 import org.apache.james.mailbox.Role
-import play.api.libs.json.{JsBoolean, JsError, JsNull, JsObject, JsString, 
JsSuccess, JsValue}
 import org.apache.james.mailbox.model.{MailboxId, MailboxACL => JavaMailboxACL}
+import play.api.libs.json.{JsBoolean, JsError, JsNull, JsObject, JsString, 
JsSuccess, JsValue}
 
 case class MailboxSetRequest(accountId: AccountId,
                              ifInState: Option[State],
@@ -66,6 +66,13 @@ object MailboxPatchObject {
   type KeyConstraint = NonEmpty And StartsWith["/"]
   type MailboxPatchObjectKey = String Refined KeyConstraint
 
+  def notFound(property: String): Either[PatchUpdateValidationException, 
Update] = {
+    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)))
+  }
+
   val roleProperty: MailboxPatchObjectKey = "/role"
   val sortOrderProperty: MailboxPatchObjectKey = "/sortOrder"
   val quotasProperty: MailboxPatchObjectKey = "/quotas"
@@ -79,8 +86,8 @@ object MailboxPatchObject {
 }
 
 case class MailboxPatchObject(value: Map[String, JsValue]) {
-  def validate(serializer: Serializer): Either[PatchUpdateValidationException, 
ValidatedMailboxPathObject] = {
-    val asUpdatedIterable = updates(serializer)
+  def validate(serializer: Serializer, capabilities: 
Set[CapabilityIdentifier]): Either[PatchUpdateValidationException, 
ValidatedMailboxPathObject] = {
+    val asUpdatedIterable = updates(serializer, capabilities)
 
     val maybeParseException: Option[PatchUpdateValidationException] = 
asUpdatedIterable
       .flatMap(x => x match {
@@ -126,13 +133,13 @@ case class MailboxPatchObject(value: Map[String, 
JsValue]) {
         rightsPartialUpdates = partialRightsUpdates)))
   }
 
-  private def updates(serializer: Serializer): 
Iterable[Either[PatchUpdateValidationException, Update]] = value.map({
+  private def updates(serializer: Serializer, capabilities: 
Set[CapabilityIdentifier]): Iterable[Either[PatchUpdateValidationException, 
Update]] = value.map({
     case (property, newValue) => property match {
       case "/name" => NameUpdate.parse(newValue)
-      case "/sharedWith" => SharedWithResetUpdate.parse(newValue, serializer)
+      case "/sharedWith" => SharedWithResetUpdate.parse(serializer, 
capabilities)(newValue)
       case "/role" => 
Left(ServerSetPropertyException(MailboxPatchObject.roleProperty))
       case "/sortOrder" => 
Left(ServerSetPropertyException(MailboxPatchObject.sortOrderProperty))
-      case "/quotas" => 
Left(ServerSetPropertyException(MailboxPatchObject.quotasProperty))
+      case "/quotas" => rejectQuotasUpdate(capabilities)
       case "/namespace" => 
Left(ServerSetPropertyException(MailboxPatchObject.namespaceProperty))
       case "/unreadThreads" => 
Left(ServerSetPropertyException(MailboxPatchObject.unreadThreadsProperty))
       case "/totalThreads" => 
Left(ServerSetPropertyException(MailboxPatchObject.totalThreadsProperty))
@@ -140,14 +147,17 @@ case class MailboxPatchObject(value: Map[String, 
JsValue]) {
       case "/totalEmails" => 
Left(ServerSetPropertyException(MailboxPatchObject.totalEmailsProperty))
       case "/myRights" => 
Left(ServerSetPropertyException(MailboxPatchObject.myRightsProperty))
       case "/isSubscribed" => IsSubscribedUpdate.parse(newValue)
-      case property: String if 
property.startsWith(MailboxPatchObject.sharedWithPrefix) => 
SharedWithPartialUpdate.parse(newValue, property, serializer)
-      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)))
+      case property: String if 
property.startsWith(MailboxPatchObject.sharedWithPrefix) =>
+        SharedWithPartialUpdate.parse(serializer, capabilities)(property, 
newValue)
+      case property => MailboxPatchObject.notFound(property)
     }
   })
+
+  private def rejectQuotasUpdate(capabilities: Set[CapabilityIdentifier]) = if 
(capabilities.contains(CapabilityIdentifier.JAMES_QUOTA)) {
+      Left(ServerSetPropertyException(MailboxPatchObject.quotasProperty))
+    } else {
+      MailboxPatchObject.notFound("/quotas")
+    }
 }
 
 case class ValidatedMailboxPathObject(nameUpdate: Option[NameUpdate],
@@ -227,10 +237,16 @@ object NameUpdate {
 }
 
 object SharedWithResetUpdate {
-  def parse(newValue: JsValue, serializer: Serializer): 
Either[PatchUpdateValidationException, Update] = 
serializer.deserializeRights(input = newValue) match {
-    case JsSuccess(value, _) => scala.Right(SharedWithResetUpdate(value))
-    case JsError(errors) => Left(InvalidUpdateException("/sharedWith", 
s"Specified value do not match the expected JSON format: $errors"))
-  }
+  def parse(serializer: Serializer, capabilities: Set[CapabilityIdentifier])
+           (newValue: JsValue): Either[PatchUpdateValidationException, Update] 
=
+    if (capabilities.contains(CapabilityIdentifier.JAMES_SHARES)) {
+      serializer.deserializeRights(input = newValue) match {
+        case JsSuccess(value, _) => scala.Right(SharedWithResetUpdate(value))
+        case JsError(errors) => Left(InvalidUpdateException("/sharedWith", 
s"Specified value do not match the expected JSON format: $errors"))
+      }
+    } else {
+      MailboxPatchObject.notFound("/sharedWith")
+    }
 }
 
 object IsSubscribedUpdate {
@@ -242,10 +258,15 @@ object IsSubscribedUpdate {
 }
 
 object SharedWithPartialUpdate {
-  def parse(newValue: JsValue, property: String, serializer: Serializer): 
Either[PatchUpdateValidationException, Update] =
-    parseUsername(property)
-      .flatMap(username => parseRights(newValue, property, serializer)
-        .map(rights => SharedWithPartialUpdate(username, rights)))
+  def parse(serializer: Serializer, capabilities: Set[CapabilityIdentifier])
+           ( property: String, newValue: JsValue): 
Either[PatchUpdateValidationException, Update] =
+    if (capabilities.contains(CapabilityIdentifier.JAMES_SHARES)) {
+      parseUsername(property)
+        .flatMap(username => parseRights(newValue, property, serializer)
+          .map(rights => SharedWithPartialUpdate(username, rights)))
+    } else {
+      MailboxPatchObject.notFound(property)
+    }
 
   def parseUsername(property: String): Either[PatchUpdateValidationException, 
Username] = try {
     
scala.Right(Username.of(property.substring(MailboxPatchObject.sharedWithPrefix.length)))
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 46ad2d7..3613c27 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.{InvalidPatchException, 
InvalidPropertyException, InvalidUpdateException, IsSubscribed, 
IsSubscribedUpdate, MailboxCreationRequest, MailboxCreationResponse, 
MailboxPatchObject, MailboxRights, MailboxSetError, MailboxSetRequest, 
MailboxSetResponse, MailboxUpdateResponse, NameUpdate, 
PatchUpdateValidationException, Properties, RemoveEmailsOnDestroy, 
ServerSetPropertyException, SetErrorDescription, SharedWithPartialUpdate, 
SharedWithResetUpdate, SortOrder,  [...]
+import org.apache.james.jmap.mail.{InvalidPatchException, 
InvalidPropertyException, InvalidUpdateException, IsSubscribed, 
MailboxCreationRequest, MailboxCreationResponse, MailboxPatchObject, 
MailboxRights, MailboxSetError, MailboxSetRequest, MailboxSetResponse, 
MailboxUpdateResponse, NameUpdate, Properties, RemoveEmailsOnDestroy, 
ServerSetPropertyException, SetErrorDescription, SortOrder, TotalEmails, 
TotalThreads, UnreadEmails, UnreadThreads, UnsupportedPropertyUpdatedException, 
Validat [...]
 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}
@@ -148,20 +148,21 @@ class MailboxSetMethod @Inject()(serializer: Serializer,
           for {
             creationResults <- createMailboxes(mailboxSession, 
mailboxSetRequest, processingContext)
             deletionResults <- deleteMailboxes(mailboxSession, 
mailboxSetRequest, processingContext)
-            updateResults <- updateMailboxes(mailboxSession, 
mailboxSetRequest, processingContext)
+            updateResults <- updateMailboxes(mailboxSession, 
mailboxSetRequest, processingContext, capabilities)
           } yield createResponse(capabilities, invocation, mailboxSetRequest, 
creationResults, deletionResults, updateResults)
         }))
   }
 
   private def updateMailboxes(mailboxSession: MailboxSession,
                               mailboxSetRequest: MailboxSetRequest,
-                              processingContext: ProcessingContext): 
SMono[UpdateResults] = {
+                              processingContext: ProcessingContext,
+                              capabilities: Set[CapabilityIdentifier]): 
SMono[UpdateResults] = {
     SFlux.fromIterable(mailboxSetRequest.update.getOrElse(Seq()))
       .flatMap({
         case (unparsedMailboxId: UnparsedMailboxId, patch: MailboxPatchObject) 
=>
           processingContext.resolveMailboxId(unparsedMailboxId, 
mailboxIdFactory).fold(
             e => SMono.just(UpdateFailure(unparsedMailboxId, e)),
-            mailboxId => updateMailbox(mailboxSession, mailboxId, patch))
+            mailboxId => updateMailbox(mailboxSession, mailboxId, patch, 
capabilities))
             .onErrorResume(e => SMono.just(UpdateFailure(unparsedMailboxId, 
e)))
       })
       .collectSeq()
@@ -170,8 +171,9 @@ class MailboxSetMethod @Inject()(serializer: Serializer,
 
   private def updateMailbox(mailboxSession: MailboxSession,
                             mailboxId: MailboxId,
-                            patch: MailboxPatchObject): SMono[UpdateResult] = {
-    patch.validate(serializer)
+                            patch: MailboxPatchObject,
+                            capabilities: Set[CapabilityIdentifier]): 
SMono[UpdateResult] = {
+    patch.validate(serializer, capabilities)
       .fold(e => SMono.raiseError(e), validatedPatch =>
         updateMailboxPath(mailboxId, validatedPatch, mailboxSession)
           .`then`(updateMailboxRights(mailboxId, validatedPatch, 
mailboxSession))


---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org
For additional commands, e-mail: server-dev-h...@james.apache.org

Reply via email to