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 4d4d10a5e97e7134ada348f92355ad6d94fe219f Author: Benoit Tellier <[email protected]> AuthorDate: Fri Oct 29 11:03:05 2021 +0700 JAMES-3539 PushListener should set topic --- .../james/jmap/pushsubscription/PushListener.scala | 24 +++++- .../james/jmap/pushsubscription/PushRequest.scala | 2 + .../jmap/pushsubscription/PushListenerTest.scala | 93 +++++++++++++++++++++- .../jmap/pushsubscription/PushRequestTest.scala | 7 +- 4 files changed, 118 insertions(+), 8 deletions(-) diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/pushsubscription/PushListener.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/pushsubscription/PushListener.scala index 6154d0f..b826382 100644 --- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/pushsubscription/PushListener.scala +++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/pushsubscription/PushListener.scala @@ -20,7 +20,10 @@ package org.apache.james.jmap.pushsubscription import java.nio.charset.StandardCharsets +import java.util.Base64 +import com.google.common.annotations.VisibleForTesting +import com.google.common.hash.Hashing import javax.inject.Inject import org.apache.james.events.EventListener.ReactiveGroupEventListener import org.apache.james.events.{Event, Group} @@ -29,6 +32,8 @@ import org.apache.james.jmap.api.pushsubscription.PushSubscriptionRepository import org.apache.james.jmap.change.{EmailDeliveryTypeName, StateChangeEvent} import org.apache.james.jmap.core.StateChange import org.apache.james.jmap.json.PushSerializer +import org.apache.james.jmap.pushsubscription.PushListener.extractTopic +import org.apache.james.jmap.pushsubscription.PushTopic.PushTopic import org.apache.james.util.ReactorUtils import org.reactivestreams.Publisher import play.api.libs.json.Json @@ -36,6 +41,22 @@ import reactor.core.scala.publisher.{SFlux, SMono} case class PushListenerGroup() extends Group {} +object PushListener { + @VisibleForTesting + def extractTopic(stateChange: StateChange): PushTopic = + PushTopic.validate( + Base64.getUrlEncoder() + .encodeToString( + Hashing.murmur3_128() + .hashString(stateChange.changes + .toList + .map { + case (accountId, typeState) => accountId.id.value + "@" + typeState.changes.keys.hashCode() + }.mkString("&"), StandardCharsets.UTF_8) + .asBytes())) + .toOption.get +} + class PushListener @Inject()(pushRepository: PushSubscriptionRepository, webPushClient: WebPushClient, pushSerializer: PushSerializer) extends ReactiveGroupEventListener { @@ -63,6 +84,7 @@ class PushListener @Inject()(pushRepository: PushSubscriptionRepository, private def asPushRequest(stateChange: StateChange, pushSubscription: PushSubscription): PushRequest = PushRequest(ttl = PushTTL.MAX, urgency = Some(urgency(stateChange)), + topic = Some(extractTopic(stateChange)), contentCoding = pushSubscription.keys.map(_ => Aes128gcm), payload = asBytes(stateChange, pushSubscription)) @@ -76,7 +98,7 @@ class PushListener @Inject()(pushRepository: PushSubscriptionRepository, private def urgency(stateChange: StateChange): PushUrgency = if (stateChange.changes .values - .flatMap(ts => ts.changes.keys) + .flatMap(_.changes.keys) .toList .contains(EmailDeliveryTypeName)) { High diff --git a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/pushsubscription/PushRequest.scala b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/pushsubscription/PushRequest.scala index 3a45216..4394772 100644 --- a/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/pushsubscription/PushRequest.scala +++ b/server/protocols/jmap-rfc-8621/src/main/scala/org/apache/james/jmap/pushsubscription/PushRequest.scala @@ -51,8 +51,10 @@ case object High extends PushUrgency { object PushTopic { type PushTopic = String Refined PushTopicConstraint + // Base 64 URL safe alphabet definition: https://datatracker.ietf.org/doc/html/rfc4648#section-5 private val charMatcher: CharMatcher = CharMatcher.inRange('a', 'z') .or(CharMatcher.inRange('A', 'Z')) + .or(CharMatcher.inRange('0', '9')) .or(CharMatcher.is('_')) .or(CharMatcher.is('-')) .or(CharMatcher.is('=')) diff --git a/server/protocols/jmap-rfc-8621/src/test/scala/org/apache/james/jmap/pushsubscription/PushListenerTest.scala b/server/protocols/jmap-rfc-8621/src/test/scala/org/apache/james/jmap/pushsubscription/PushListenerTest.scala index 738de4d..15d8e0e 100644 --- a/server/protocols/jmap-rfc-8621/src/test/scala/org/apache/james/jmap/pushsubscription/PushListenerTest.scala +++ b/server/protocols/jmap-rfc-8621/src/test/scala/org/apache/james/jmap/pushsubscription/PushListenerTest.scala @@ -23,6 +23,7 @@ import java.nio.charset.StandardCharsets import java.security.interfaces.{ECPrivateKey, ECPublicKey} import java.time.Clock import java.util.{Base64, UUID} + import com.google.common.collect.ImmutableSet import com.google.common.hash.Hashing import com.google.crypto.tink.apps.webpush.WebPushHybridDecrypt @@ -33,13 +34,13 @@ import org.apache.james.events.Event.EventId import org.apache.james.jmap.api.change.TypeStateFactory import org.apache.james.jmap.api.model.{DeviceClientId, PushSubscriptionCreationRequest, PushSubscriptionKeys, PushSubscriptionServerURL, TypeName} import org.apache.james.jmap.api.pushsubscription.PushSubscriptionRepository -import org.apache.james.jmap.change.{EmailDeliveryTypeName, EmailTypeName, MailboxTypeName, StateChangeEvent} -import org.apache.james.jmap.core.UuidState +import org.apache.james.jmap.change.{EmailDeliveryTypeName, EmailTypeName, MailboxTypeName, StateChangeEvent, TypeState} +import org.apache.james.jmap.core.{AccountId, PushState, StateChange, UuidState} import org.apache.james.jmap.json.PushSerializer import org.apache.james.jmap.memory.pushsubscription.MemoryPushSubscriptionRepository import org.assertj.core.api.Assertions.assertThat import org.assertj.core.api.SoftAssertions -import org.junit.jupiter.api.{BeforeEach, Test} +import org.junit.jupiter.api.{BeforeEach, Nested, Test} import org.mockito.ArgumentMatchers.any import org.mockito.Mockito.{mock, verify, verifyNoInteractions, when} import org.mockito.{ArgumentCaptor, ArgumentMatchers} @@ -49,6 +50,7 @@ import scala.jdk.OptionConverters._ class PushListenerTest { val bob: Username = Username.of("bob@localhost") + val alice: Username = Username.of("alice@localhost") val bobAccountId: String = "405010d6c16c16dec36f3d7a7596c2d757ba7b57904adc4801a63e40914fd5c9" val url: PushSubscriptionServerURL = PushSubscriptionServerURL.from("http://localhost:9999/push";).get @@ -186,6 +188,25 @@ class PushListenerTest { } @Test + def topicShouldBeSpecified(): Unit = { + val id = SMono(pushSubscriptionRepository.save(bob, PushSubscriptionCreationRequest( + deviceClientId = DeviceClientId("junit"), + url = url, + types = Seq(EmailDeliveryTypeName, EmailTypeName)))).block().id + SMono(pushSubscriptionRepository.validateVerificationCode(bob, id)).block() + + val state1 = UuidState(UUID.randomUUID()) + val state2 = UuidState(UUID.randomUUID()) + SMono(testee.reactiveEvent(StateChangeEvent(EventId.random(), bob, + Map(EmailTypeName -> state1, MailboxTypeName -> state2)))).block() + + val argumentCaptor: ArgumentCaptor[PushRequest] = ArgumentCaptor.forClass(classOf[PushRequest]) + verify(webPushClient).push(ArgumentMatchers.eq(url), argumentCaptor.capture()) + assertThat(argumentCaptor.getValue.topic.toJava) + .contains(PushTopic.validate("j2HKoW6BCasQ1URmX6SRjg==").toOption.get) + } + + @Test def pushShouldEncryptMessages(): Unit = { val uaKeyPair = EllipticCurves.generateKeyPair(CurveType.NIST_P256) val uaPrivateKey: ECPrivateKey = uaKeyPair.getPrivate.asInstanceOf[ECPrivateKey] @@ -227,4 +248,70 @@ class PushListenerTest { .isNotEqualTo(Hashing.sha256().hashBytes(decryptedPayload.getBytes(StandardCharsets.UTF_8))) }) } + + @Nested + class ExtractPushTopic { + private val bobAccount: AccountId = AccountId.from(bob).toOption.get + private val aliceAccount: AccountId = AccountId.from(alice).toOption.get + + @Test + def extractTopicShouldBeIndependentFromState(): Unit = { + val state1 = UuidState(UUID.randomUUID()) + assertThat(PushListener.extractTopic(StateChange(Map(bobAccount -> TypeState(Map(EmailTypeName -> state1))), None))) + .isEqualTo(PushTopic.validate("j2HKoW6BCasQ1URmX6SRjg==").toOption.get) + } + + @Test + def extractTopicShouldDependOnTypeName(): Unit = { + val state1 = UuidState(UUID.randomUUID()) + assertThat(PushListener.extractTopic(StateChange(Map(bobAccount -> TypeState(Map(EmailDeliveryTypeName -> state1))), None))) + .isNotEqualTo(PushListener.extractTopic(StateChange(Map(bobAccount -> TypeState(Map(EmailTypeName -> state1))), None))) + .isEqualTo(PushTopic.validate("jCtTpf_BO3ZG03loJXHRPQ==").toOption.get) + } + + @Test + def extractTopicShouldDependOnExtraTypeName(): Unit = { + val state1 = UuidState(UUID.randomUUID()) + assertThat(PushListener.extractTopic(StateChange(Map(bobAccount -> TypeState(Map(EmailTypeName -> state1, EmailDeliveryTypeName -> state1))), None))) + .isNotEqualTo(PushListener.extractTopic(StateChange(Map(bobAccount -> TypeState(Map(EmailTypeName -> state1))), None))) + .isEqualTo(PushTopic.validate("oA4xCCqJhc98SKLJOa-ndA==").toOption.get) + } + + @Test + def extractTopicShouldNotDependOnExtraTypeNameOrdering(): Unit = { + val state1 = UuidState(UUID.randomUUID()) + + assertThat(PushListener.extractTopic(StateChange(Map(bobAccount -> TypeState(Map(EmailDeliveryTypeName -> state1, EmailTypeName -> state1))), None))) + .isEqualTo(PushListener.extractTopic(StateChange(Map(bobAccount -> TypeState(Map(EmailTypeName -> state1, EmailDeliveryTypeName -> state1))), None))) + .isEqualTo(PushTopic.validate("oA4xCCqJhc98SKLJOa-ndA==").toOption.get) + } + + @Test + def extractTopicShouldNotDependOnPushState(): Unit = { + val state1 = UuidState(UUID.randomUUID()) + + assertThat(PushListener.extractTopic(StateChange(Map(bobAccount -> TypeState(Map(EmailDeliveryTypeName -> state1, EmailTypeName -> state1))), Some(PushState("whatever"))))) + .isEqualTo(PushListener.extractTopic(StateChange(Map(bobAccount -> TypeState(Map(EmailTypeName -> state1, EmailDeliveryTypeName -> state1))), None))) + .isEqualTo(PushTopic.validate("oA4xCCqJhc98SKLJOa-ndA==").toOption.get) + } + + @Test + def extractTopicShouldDependOnAccountId(): Unit = { + val state1 = UuidState(UUID.randomUUID()) + assertThat(PushListener.extractTopic(StateChange(Map(aliceAccount -> TypeState(Map(EmailTypeName -> state1))), None))) + .isNotEqualTo(PushListener.extractTopic(StateChange(Map(bobAccount -> TypeState(Map(EmailTypeName -> state1))), None))) + .isEqualTo(PushTopic.validate("PQSgEID7KHXJ6AbqipDETQ==").toOption.get) + } + + @Test + def extractTopicShouldDependOnExtraAccountId(): Unit = { + val state1 = UuidState(UUID.randomUUID()) + assertThat(PushListener.extractTopic(StateChange(Map( + aliceAccount -> TypeState(Map(EmailTypeName -> state1)), + bobAccount -> TypeState(Map(EmailTypeName -> state1))), + None))) + .isNotEqualTo(PushListener.extractTopic(StateChange(Map(bobAccount -> TypeState(Map(EmailTypeName -> state1))), None))) + .isEqualTo(PushTopic.validate("C4Q8cdY9ja_dkMGE7xZomQ==").toOption.get) + } + } } diff --git a/server/protocols/jmap-rfc-8621/src/test/scala/org/apache/james/jmap/pushsubscription/PushRequestTest.scala b/server/protocols/jmap-rfc-8621/src/test/scala/org/apache/james/jmap/pushsubscription/PushRequestTest.scala index 4d60fca..35a63ff 100644 --- a/server/protocols/jmap-rfc-8621/src/test/scala/org/apache/james/jmap/pushsubscription/PushRequestTest.scala +++ b/server/protocols/jmap-rfc-8621/src/test/scala/org/apache/james/jmap/pushsubscription/PushRequestTest.scala @@ -52,16 +52,15 @@ class PushRequestTest { @ParameterizedTest @ValueSource(strings = Array("/", "@", "#", "$", "%", "^", "&", "*", "(", ")", - "{", "}", ":", ";", "?", "<", ">", ".", - "1", "9")) + "{", "}", ":", ";", "?", "<", ">", ".", "+")) def topicShouldNotAcceptSpecialCharacters(value: String): Unit = { assertThat(PushTopic.validate(value).isRight) .isFalse } @Test - def topicShouldAcceptAlphabetValue(): Unit = { - assertThat(PushTopic.validate("value").isRight) + def topicShouldAcceptBase64URLSafeAlphabet(): Unit = { + assertThat(PushTopic.validate("value-_=19VALUE").isRight) .isTrue } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
