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 b11b3f260f85f76bd850a53876482ecd2dd84ec2 Author: Benoit Tellier <[email protected]> AuthorDate: Mon Aug 17 15:14:40 2020 +0700 JAMES-3359 Mailbox/set update+destroy should fail on system mailboxes --- .../contract/MailboxSetMethodContract.scala | 106 ++++++++++++++++++++- .../james/jmap/method/MailboxSetMethod.scala | 23 +++-- 2 files changed, 121 insertions(+), 8 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 3436f8f..cebae49 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 @@ -2371,7 +2371,7 @@ trait MailboxSetMethodContract { } @Test - def updateShouldNotRenameDelegtedMailboxes(server: GuiceJamesServer): Unit = { + def updateShouldNotRenameDelegatedMailboxes(server: GuiceJamesServer): Unit = { val path = MailboxPath.forUser(ANDRE, "previousName") val mailboxId1: MailboxId = server.getProbe(classOf[MailboxProbeImpl]).createMailbox(path) server.getProbe(classOf[ACLProbeImpl]) @@ -2425,6 +2425,108 @@ trait MailboxSetMethodContract { |}""".stripMargin) } + @Test + def updateShouldNotRenameSystemMailboxes(server: GuiceJamesServer): Unit = { + val mailboxId: MailboxId = server.getProbe(classOf[MailboxProbeImpl]) + .createMailbox(MailboxPath.forUser(BOB, "INBOX")) + val request = + s""" + |{ + | "using": [ "urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail" ], + | "methodCalls": [ + | ["Mailbox/set", + | { + | "accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6", + | "update": { + | "${mailboxId.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": { + | "${mailboxId.serialize()}": { + | "type": "invalidArguments", + | "description": "Invalid change to a system mailbox", + | "properties":{"value":["/name"]} + | } + | } + | }, "c1"] + | ] + |}""".stripMargin) + } + + @Test + def destroyShouldNotRemoveSystemMailboxes(server: GuiceJamesServer): Unit = { + val mailboxId: MailboxId = server.getProbe(classOf[MailboxProbeImpl]) + .createMailbox(MailboxPath.forUser(BOB, "INBOX")) + val request = + s""" + |{ + | "using": [ "urn:ietf:params:jmap:core", "urn:ietf:params:jmap:mail" ], + | "methodCalls": [ + | ["Mailbox/set", + | { + | "accountId": "29883977c13473ae7cb7678ef767cbfbaffc8a44a6e463d971d23a65c1dc4af6", + | "destroy": ["${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", + | "notDestroyed": { + | "${mailboxId.serialize()}": { + | "type": "invalidArguments", + | "description": "System mailboxes cannot be destroyed" + | } + | } + | }, "c1"] + | ] + |}""".stripMargin) + } + // TODO invalid path handling (unknown property, invalid name) - // TODO disable destroy / rename of system mailbox } 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 ea79a9c..6018c8f 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 @@ -31,7 +31,7 @@ import org.apache.james.jmap.routes.ProcessingContext import org.apache.james.mailbox.MailboxManager.RenameOption import org.apache.james.mailbox.exception.{InsufficientRightsException, MailboxExistsException, MailboxNameException, MailboxNotFoundException} import org.apache.james.mailbox.model.{FetchGroup, MailboxId, MailboxPath, MessageRange} -import org.apache.james.mailbox.{MailboxManager, MailboxSession, MessageManager, SubscriptionManager} +import org.apache.james.mailbox.{MailboxManager, MailboxSession, MessageManager, Role, SubscriptionManager} import org.apache.james.metrics.api.MetricFactory import org.reactivestreams.Publisher import play.api.libs.json._ @@ -41,6 +41,7 @@ import reactor.core.scheduler.Schedulers import scala.jdk.CollectionConverters._ case class MailboxHasMailException(mailboxId: MailboxId) extends Exception +case class SystemMailboxChangeException(mailboxId: MailboxId) extends Exception case class MailboxHasChildException(mailboxId: MailboxId) extends Exception case class MailboxCreationParseException(mailboxSetError: MailboxSetError) extends Exception @@ -95,6 +96,7 @@ case class DeletionFailure(mailboxId: UnparsedMailboxId, exception: Throwable) e case e: MailboxNotFoundException => MailboxSetError.notFound(Some(SetErrorDescription(e.getMessage))) case e: MailboxHasMailException => MailboxSetError.mailboxHasEmail(Some(SetErrorDescription(s"${e.mailboxId.serialize} is not empty"))) case e: MailboxHasChildException => MailboxSetError.mailboxHasChild(Some(SetErrorDescription(s"${e.mailboxId.serialize} has child mailboxes"))) + case e: SystemMailboxChangeException => MailboxSetError.invalidArgument(Some(SetErrorDescription("System mailboxes cannot be destroyed")), None) case e: IllegalArgumentException => MailboxSetError.invalidArgument(Some(SetErrorDescription(s"${mailboxId} is not a mailboxId: ${e.getMessage}")), None) case _ => MailboxSetError.serverFail(Some(SetErrorDescription(exception.getMessage)), None) } @@ -121,6 +123,7 @@ 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: SystemMailboxChangeException => MailboxSetError.invalidArgument(Some(SetErrorDescription("Invalid change to a system mailbox")), Some(Properties(List("/name")))) case _ => MailboxSetError.serverFail(Some(SetErrorDescription(exception.getMessage)), None) } } @@ -191,19 +194,22 @@ class MailboxSetMethod @Inject()(serializer: Serializer, .getOrElse(renameMailbox) } - private def updateMailboxPath(maiboxId: MailboxId, maybeNameUpdate: Option[NameUpdate], mailboxSession: MailboxSession): SMono[UpdateResult] = { + private def updateMailboxPath(mailboxId: MailboxId, maybeNameUpdate: Option[NameUpdate], mailboxSession: MailboxSession): SMono[UpdateResult] = { maybeNameUpdate.map(nameUpdate => { SMono.fromCallable(() => { - val mailbox = mailboxManager.getMailbox(maiboxId, mailboxSession) - mailboxManager.renameMailbox(maiboxId, + val mailbox = mailboxManager.getMailbox(mailboxId, mailboxSession) + if (isASystemMailbox(mailbox)) { + throw SystemMailboxChangeException(mailboxId) + } + mailboxManager.renameMailbox(mailboxId, computeMailboxPath(mailbox, nameUpdate, mailboxSession), RenameOption.RENAME_SUBSCRIPTIONS, mailboxSession) - }).`then`(SMono.just[UpdateResult](UpdateSuccess(maiboxId))) + }).`then`(SMono.just[UpdateResult](UpdateSuccess(mailboxId))) .subscribeOn(Schedulers.elastic()) }) // No updated properties passed. Noop. - .getOrElse(SMono.just[UpdateResult](UpdateSuccess(maiboxId))) + .getOrElse(SMono.just[UpdateResult](UpdateSuccess(mailboxId))) } private def computeMailboxPath(mailbox: MessageManager, nameUpdate: NameUpdate, mailboxSession: MailboxSession): MailboxPath = { @@ -236,6 +242,9 @@ class MailboxSetMethod @Inject()(serializer: Serializer, private def delete(mailboxSession: MailboxSession, id: MailboxId): Unit = { val mailbox = mailboxManager.getMailbox(id, mailboxSession) + if (isASystemMailbox(mailbox)) { + throw SystemMailboxChangeException(id) + } if (mailbox.getMessages(MessageRange.all(), FetchGroup.MINIMAL, mailboxSession).hasNext) { throw MailboxHasMailException(id) } @@ -246,6 +255,8 @@ class MailboxSetMethod @Inject()(serializer: Serializer, subscriptionManager.unsubscribe(mailboxSession, deletedMailbox.getName) } + private def isASystemMailbox(mailbox: MessageManager): Boolean = Role.from(mailbox.getMailboxPath.getName).isPresent + private def createMailboxes(mailboxSession: MailboxSession, mailboxSetRequest: MailboxSetRequest, processingContext: ProcessingContext): SMono[CreationResults] = { --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
