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 c532fadb010178d14ad69de846575c571e29a8fc Author: Rémi Kowalski <[email protected]> AuthorDate: Thu Sep 24 15:01:30 2020 +0200 JAMES-3387 validate accountId for vacationResponse/get --- .../VacationResponseGetMethodContract.scala | 41 ++++++++++++++++ .../james/jmap/mail/VacationResponseGet.scala | 3 +- .../jmap/method/VacationResponseGetMethod.scala | 57 ++++++++++------------ 3 files changed, 70 insertions(+), 31 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/VacationResponseGetMethodContract.scala b/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/VacationResponseGetMethodContract.scala index 268b840..1ca2caf 100644 --- a/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/VacationResponseGetMethodContract.scala +++ b/server/protocols/jmap-rfc-8621-integration-tests/jmap-rfc-8621-integration-tests-common/src/main/scala/org/apache/james/jmap/rfc8621/contract/VacationResponseGetMethodContract.scala @@ -62,6 +62,47 @@ trait VacationResponseGetMethodContract { } @Test + def vacationResponseGetShouldFailWhenWrongAccountId(server: GuiceJamesServer): Unit = { + val request = + s"""{ + | "using": [ + | "urn:ietf:params:jmap:core", + | "urn:ietf:params:jmap:mail", + | "urn:ietf:params:jmap:vacationresponse"], + | "methodCalls": [[ + | "VacationResponse/get", + | { + | "accountId": "unknownAccountId", + | "ids": null + | }, + | "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( + """{ + | "sessionState": "75128aab4b1b", + | "methodResponses": [ + | ["error", { + | "type": "accountNotFound" + | }, "c1"] + | ] + |}""".stripMargin) + } + + @Test @Tag(CategoryTags.BASIC_FEATURE) def vacationResponseShouldBeDisabledByDefault(): Unit = { val response = `given` diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/VacationResponseGet.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/VacationResponseGet.scala index 4329c84..5509e18 100644 --- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/VacationResponseGet.scala +++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/mail/VacationResponseGet.scala @@ -20,6 +20,7 @@ package org.apache.james.jmap.mail import org.apache.james.jmap.mail.VacationResponse.UnparsedVacationResponseId +import org.apache.james.jmap.method.WithAccountId import org.apache.james.jmap.model.State.State import org.apache.james.jmap.model.{AccountId, Properties} @@ -27,7 +28,7 @@ case class VacationResponseIds(value: List[UnparsedVacationResponseId]) case class VacationResponseGetRequest(accountId: AccountId, ids: Option[VacationResponseIds], - properties: Option[Properties]) + properties: Option[Properties]) extends WithAccountId case class VacationResponseNotFound(value: Set[UnparsedVacationResponseId]) { def merge(other: VacationResponseNotFound): VacationResponseNotFound = VacationResponseNotFound(this.value ++ other.value) diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/VacationResponseGetMethod.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/VacationResponseGetMethod.scala index a9a3a7f..2978afd 100644 --- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/VacationResponseGetMethod.scala +++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/method/VacationResponseGetMethod.scala @@ -34,7 +34,6 @@ import org.apache.james.jmap.model.{AccountId, Capabilities, ErrorCode, Invocati import org.apache.james.jmap.routes.ProcessingContext import org.apache.james.mailbox.MailboxSession import org.apache.james.metrics.api.MetricFactory -import org.reactivestreams.Publisher import play.api.libs.json.{JsError, JsObject, JsSuccess} import reactor.core.scala.publisher.{SFlux, SMono} @@ -58,40 +57,38 @@ case class VacationResponseGetResult(vacationResponses: Set[VacationResponse], n notFound = notFound) } -class VacationResponseGetMethod @Inject() (vacationRepository: VacationRepository, - metricFactory: MetricFactory) extends Method { +class VacationResponseGetMethod @Inject()(vacationRepository: VacationRepository, + val metricFactory: MetricFactory, + val sessionSupplier: SessionSupplier) extends MethodRequiringAccountId[VacationResponseGetRequest] { override val methodName: MethodName = MethodName("VacationResponse/get") override val requiredCapabilities: Capabilities = Capabilities(CORE_CAPABILITY, MAIL_CAPABILITY, VACATION_RESPONSE_CAPABILITY) - override def process(capabilities: Set[CapabilityIdentifier], - invocation: InvocationWithContext, - mailboxSession: MailboxSession): Publisher[InvocationWithContext] = { - metricFactory.decoratePublisherWithTimerMetricLogP99(JMAP_RFC8621_PREFIX + methodName.value, - asVacationResponseGetRequest(invocation.invocation.arguments) - .fold(e => handleRequestValidationErrors(e, invocation.invocation.methodCallId), - vacationResponseGetRequest => { - val requestedProperties: Properties = vacationResponseGetRequest.properties.getOrElse(VacationResponse.allProperties) - requestedProperties -- VacationResponse.allProperties match { - case invalidProperties if invalidProperties.isEmpty() => getVacationResponse(vacationResponseGetRequest, invocation.processingContext, mailboxSession) - .reduce(VacationResponseGetResult.empty, VacationResponseGetResult.merge) - .map(vacationResult => vacationResult.asResponse(vacationResponseGetRequest.accountId)) - .map(vacationResponseGetResponse => Invocation( - methodName = methodName, - arguments = Arguments(VacationSerializer.serialize(vacationResponseGetResponse, requestedProperties).as[JsObject]), - methodCallId = invocation.invocation.methodCallId)) - case invalidProperties: Properties => - SMono.just(Invocation.error(errorCode = ErrorCode.InvalidArguments, - description = s"The following properties [${invalidProperties.format}] do not exist.", - methodCallId = invocation.invocation.methodCallId)) - } - }) - .map(InvocationWithContext(_, invocation.processingContext))) + override def doProcess(capabilities: Set[CapabilityIdentifier], invocation: InvocationWithContext, mailboxSession: MailboxSession, request: VacationResponseGetRequest): SMono[InvocationWithContext] = { + { + val requestedProperties: Properties = request.properties.getOrElse(VacationResponse.allProperties) + (requestedProperties -- VacationResponse.allProperties match { + case invalidProperties if invalidProperties.isEmpty() => getVacationResponse(request, invocation.processingContext, mailboxSession) + .reduce(VacationResponseGetResult.empty, VacationResponseGetResult.merge) + .map(vacationResult => vacationResult.asResponse(request.accountId)) + .map(vacationResponseGetResponse => Invocation( + methodName = methodName, + arguments = Arguments(VacationSerializer.serialize(vacationResponseGetResponse, requestedProperties).as[JsObject]), + methodCallId = invocation.invocation.methodCallId)) + case invalidProperties: Properties => + SMono.just(Invocation.error(errorCode = ErrorCode.InvalidArguments, + description = s"The following properties [${invalidProperties.format}] do not exist.", + methodCallId = invocation.invocation.methodCallId)) + }).map(InvocationWithContext(_, invocation.processingContext)) + .onErrorResume{ case e: Exception => handleRequestValidationErrors(e, invocation.invocation.methodCallId) + .map(errorInvocation => InvocationWithContext(errorInvocation, invocation.processingContext))} + } } - private def asVacationResponseGetRequest(arguments: Arguments): Either[IllegalArgumentException, VacationResponseGetRequest] = - VacationSerializer.deserializeVacationResponseGetRequest(arguments.value) match { - case JsSuccess(vacationResponseGetRequest, _) => Right(vacationResponseGetRequest) - case errors: JsError => Left(new IllegalArgumentException(ResponseSerializer.serialize(errors).toString)) + override def getRequest(mailboxSession: MailboxSession, invocation: Invocation): SMono[VacationResponseGetRequest] = asVacationResponseGetRequest(invocation.arguments) + + private def asVacationResponseGetRequest(arguments: Arguments): SMono[VacationResponseGetRequest] = VacationSerializer.deserializeVacationResponseGetRequest(arguments.value) match { + case JsSuccess(vacationResponseGetRequest, _) => SMono.just(vacationResponseGetRequest) + case errors: JsError => SMono.raiseError(new IllegalArgumentException(ResponseSerializer.serialize(errors).toString)) } private def handleRequestValidationErrors(exception: Exception, methodCallId: MethodCallId): SMono[Invocation] = exception match { --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
