You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by bt...@apache.org on 2021/11/01 10:15:30 UTC

[james-project] 02/02: JAMES-3539 PushListener should set topic

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 <bt...@linagora.com>
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: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org