This is an automated email from the ASF dual-hosted git repository. btellier pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/james-project.git
commit 03a8293c0cb5610dedd6d4d913854306af6976c8 Author: Benoit Tellier <[email protected]> AuthorDate: Wed Sep 23 14:54:00 2020 +0700 JAMES-3359 Reject Mailbox/set parentId update to self The spec states: Mailboxes form acyclic graphs (forests) directed by the child-to-parent relationship. There MUST NOT be a loop. ``` ``` Rejecting self is enough as parentId property cannot be updated if the mailbox contains child mailboxes --- .../contract/MailboxSetMethodContract.scala | 128 +++++++++++++++++++++ .../james/jmap/method/MailboxSetMethod.scala | 5 + 2 files changed, 133 insertions(+) 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 cbe2daa..42a26f8 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 @@ -5600,6 +5600,134 @@ trait MailboxSetMethodContract { } @Test + def parentIdUpdateSetToSelfShouldBeRejected(server: GuiceJamesServer): Unit = { + val mailboxId: 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()}": { + | "parentId": "${mailboxId.serialize()}" + | } + | } + | }, + | "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": { + | "${mailboxId.serialize()}": { + | "type": "invalidArguments", + | "description": "A mailbox parentId property can not be set to itself or one of its child", + | "properties": [ + | "parentId" + | ] + | } + | } + | }, + | "c1" + | ] + | ] + |}""".stripMargin) + } + + @Test + def parentIdUpdateSetToAChildShouldBeRejected(server: GuiceJamesServer): Unit = { + val mailboxId: MailboxId = server.getProbe(classOf[MailboxProbeImpl]) + .createMailbox(MailboxPath.forUser(BOB, "mailbox")) + val childId: MailboxId = server.getProbe(classOf[MailboxProbeImpl]) + .createMailbox(MailboxPath.forUser(BOB, "mailbox.child")) + val request = + s""" + |{ + | "using": [ "urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail" ], + | "methodCalls": [ + | [ + | "Mailbox/set", + | { + | "accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6", + | "update": { + | "${mailboxId.serialize()}": { + | "parentId": "${childId.serialize()}" + | } + | } + | }, + | "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": { + | "${mailboxId.serialize()}": { + | "type": "invalidArguments", + | "description": "${mailboxId.serialize()} parentId property cannot be updated as this mailbox has child mailboxes", + | "properties": [ + | "parentId" + | ] + | } + | } + | }, + | "c1" + | ] + | ] + |}""".stripMargin) + } + + @Test def updateShouldAllowParentIdChangeWhenChildMailbox(server: GuiceJamesServer): Unit = { val mailboxProbe = server.getProbe(classOf[MailboxProbeImpl]) mailboxProbe.createMailbox(MailboxPath.forUser(BOB, "parent")) 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 514d8d2..465b22c 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 @@ -45,6 +45,7 @@ import scala.jdk.CollectionConverters._ case class MailboxHasMailException(mailboxId: MailboxId) extends Exception case class SystemMailboxChangeException(mailboxId: MailboxId) extends Exception +case class LoopInMailboxGraphException(mailboxId: MailboxId) extends Exception case class MailboxHasChildException(mailboxId: MailboxId) extends Exception case class MailboxCreationParseException(setError: SetError) extends Exception @@ -123,6 +124,7 @@ case class UpdateFailure(mailboxId: UnparsedMailboxId, exception: Throwable, pat case e: InvalidPropertyException => SetError.invalidPatch(SetErrorDescription(s"${e.cause}")) case e: InvalidPatchException => SetError.invalidPatch(SetErrorDescription(s"${e.cause}")) case e: SystemMailboxChangeException => SetError.invalidArguments(SetErrorDescription("Invalid change to a system mailbox"), filter(Properties("name", "parentId"))) + case e: LoopInMailboxGraphException => SetError.invalidArguments(SetErrorDescription("A mailbox parentId property can not be set to itself or one of its child"), Some(Properties("parentId"))) case e: InsufficientRightsException => SetError.invalidArguments(SetErrorDescription("Invalid change to a delegated mailbox")) case e: MailboxHasChildException => SetError.invalidArguments(SetErrorDescription(s"${e.mailboxId.serialize()} parentId property cannot be updated as this mailbox has child mailboxes"), Some(Properties("parentId"))) case _ => SetError.serverFail(SetErrorDescription(exception.getMessage)) @@ -219,6 +221,9 @@ class MailboxSetMethod @Inject()(serializer: MailboxSerializer, if (isASystemMailbox(mailbox)) { throw SystemMailboxChangeException(mailboxId) } + if (validatedPatch.parentIdUpdate.flatMap(_.newId).contains(mailboxId)) { + throw LoopInMailboxGraphException(mailboxId) + } val oldPath = mailbox.getMailboxPath val newPath = applyParentIdUpdate(mailboxId, validatedPatch.parentIdUpdate, mailboxSession) .andThen(applyNameUpdate(validatedPatch.nameUpdate, mailboxSession)) --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
