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