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 746efffb4e8b905b5155ec49e6e2bfe6d17ed288 Author: Benoit Tellier <[email protected]> AuthorDate: Thu Aug 20 14:59:10 2020 +0700 JAMES-3359 Mailbox/set parentId updates: prevent loops We apply the same strategy than in Draft: preventing changing parentId of a mailbox having childs respects the mailbox tree structure. --- .../contract/MailboxSetMethodContract.scala | 59 ++++++++++++++++++++++ .../james/jmap/method/MailboxSetMethod.scala | 14 +++-- 2 files changed, 68 insertions(+), 5 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 765675d..355e3d5 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 @@ -6969,4 +6969,63 @@ trait MailboxSetMethodContract { | ] |}""".stripMargin) } + + @Test + def updatingParentIdShouldFailWhenMailboxHasAChild(server: GuiceJamesServer): Unit = { + val mailboxProbe = server.getProbe(classOf[MailboxProbeImpl]) + val mailboxId: MailboxId = mailboxProbe.createMailbox(MailboxPath.forUser(BOB, "mailbox")) + mailboxProbe.createMailbox(MailboxPath.forUser(BOB, "mailbox.child")) + val parentId: MailboxId = mailboxProbe.createMailbox(MailboxPath.forUser(BOB, "parent")) + + val request = s""" + |{ + | "using": [ "urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail" ], + | "methodCalls": [ + | [ + | "Mailbox/set", + | { + | "accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6", + | "update": { + | "${mailboxId.serialize()}": { + | "/parentId": "${parentId.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": { + | "1": { + | "type": "invalidArguments", + | "description": "1 parentId property cannot be updated as this mailbox has child mailboxes", + | "properties": ["parentId"] + | } + | } + | }, "c1"] + | ] + |}""".stripMargin) + } } 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 ba022f4..a412f8a 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 @@ -122,6 +122,7 @@ case class UpdateFailure(mailboxId: UnparsedMailboxId, exception: Throwable, pat case e: InvalidPatchException => MailboxSetError.invalidPatch(Some(SetErrorDescription(s"${e.cause}"))) case e: SystemMailboxChangeException => MailboxSetError.invalidArgument(Some(SetErrorDescription("Invalid change to a system mailbox")), filter(Properties(List("/name", "parentId")))) case e: InsufficientRightsException => MailboxSetError.invalidArgument(Some(SetErrorDescription("Invalid change to a delegated mailbox")), None) + case e: MailboxHasChildException => MailboxSetError.invalidArgument(Some(SetErrorDescription(s"${e.mailboxId.serialize()} parentId property cannot be updated as this mailbox has child mailboxes")), Some(Properties(List("parentId")))) case _ => MailboxSetError.serverFail(Some(SetErrorDescription(exception.getMessage)), None) } } @@ -216,8 +217,8 @@ class MailboxSetMethod @Inject()(serializer: Serializer, throw SystemMailboxChangeException(mailboxId) } val oldPath = mailbox.getMailboxPath - val newPath = applyNameUpdate(validatedPatch.nameUpdate, mailboxSession) - .andThen(applyParentIdUpdate(validatedPatch.parentIdUpdate, mailboxSession)) + val newPath = applyParentIdUpdate(mailboxId, validatedPatch.parentIdUpdate, mailboxSession) + .andThen(applyNameUpdate(validatedPatch.nameUpdate, mailboxSession)) .apply(oldPath) if (!oldPath.equals(newPath)) { mailboxManager.renameMailbox(mailboxId, @@ -236,8 +237,8 @@ class MailboxSetMethod @Inject()(serializer: Serializer, } } - private def applyParentIdUpdate(maybeParentIdUpdate: Option[ParentIdUpdate], mailboxSession: MailboxSession): MailboxPath => MailboxPath = { - maybeParentIdUpdate.map(parentIdUpdate => applyParentIdUpdate(parentIdUpdate, mailboxSession)) + private def applyParentIdUpdate(mailboxId: MailboxId, maybeParentIdUpdate: Option[ParentIdUpdate], mailboxSession: MailboxSession): MailboxPath => MailboxPath = { + maybeParentIdUpdate.map(parentIdUpdate => applyParentIdUpdate(mailboxId, parentIdUpdate, mailboxSession)) .getOrElse(x => x) } @@ -253,11 +254,14 @@ class MailboxSetMethod @Inject()(serializer: Serializer, }).getOrElse(originalPath) } - private def applyParentIdUpdate(parentIdUpdate: ParentIdUpdate, mailboxSession: MailboxSession): MailboxPath => MailboxPath = { + private def applyParentIdUpdate(mailboxId: MailboxId, parentIdUpdate: ParentIdUpdate, mailboxSession: MailboxSession): MailboxPath => MailboxPath = { originalPath => { val currentName = originalPath.getName(mailboxSession.getPathDelimiter) parentIdUpdate.newId .map(id => { + if (mailboxManager.hasChildren(originalPath, mailboxSession)) { + throw MailboxHasChildException(mailboxId) + } val parentPath = mailboxManager.getMailbox(id, mailboxSession).getMailboxPath parentPath.child(currentName, mailboxSession.getPathDelimiter) }) --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
