Page MenuHomePhabricator

Add encryptedNotifUtilsAPI on client platforms with mock encryption.
ClosedPublic

Authored by marcin on Jun 25 2024, 5:31 AM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Nov 5, 11:06 AM
Unknown Object (File)
Fri, Nov 1, 6:42 PM
Unknown Object (File)
Oct 13 2024, 3:33 AM
Unknown Object (File)
Oct 13 2024, 3:33 AM
Unknown Object (File)
Oct 13 2024, 3:33 AM
Unknown Object (File)
Oct 13 2024, 3:32 AM
Unknown Object (File)
Oct 13 2024, 3:32 AM
Unknown Object (File)
Oct 12 2024, 11:40 AM
Subscribers

Details

Summary

This differential introduces encryptedNotifUtilsAPI on the client platforms to smoothly use function from lib/push. Encryption implementation is mocked and will be replaced once we tackle notif session initialization.

Test Plan

Test plan for D12464 but without adding encrytpedNotifUtilsAPI implementation to the patch.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

marcin added inline comments.
native/push/encrypted-notif-utils-api.js
29 ↗(On Diff #41696)

Current code for uploading data to blob service is tightly coupled to the ksyerver uploads table and expo-file-system library which I don't think it the case for e2e notifs so this code would require some refactoring to support e2e notifs case. We could move the current code that is on the keyserver to lib but this code uses fetch library. However @bartek told me that it might not work well on native. Therefore given the fact that we haven't even started notif session intialization between clients (which is far more important) I am willing to cut a scope here. Returning error message from this funciton won't crash notif generation and delivery. It will just be delivered without message infos if it exceeds push service limit.

native/push/encrypted-notif-utils-api.js
29 ↗(On Diff #41696)
  1. Is there a task tracking this cut scope?
  2. Can you share more details about why fetch would not work well on native? We use it extensively
native/push/encrypted-notif-utils-api.js
29 ↗(On Diff #41696)

Is there a task tracking this cut scope?

https://linear.app/comm/issue/ENG-8709/enable-the-client-to-upload-messageinfo-to-blob-service-of-large-thick

Can you share more details about why fetch would not work well on native? We use it extensively

I only had quick conversation about this with @bartek . I was informed that fetch doesn't work well with large amounts of data and that is why it is not used to upload multimedia to blob on native. On the other hand even though large notif payloads exceed APNs/FCM limits they are likely significantly smaller than multimedia messages. Therefore we are likely to succeed with with fetch.

  1. Return that size limit was violated if notificationPayloadValidator returns false
  2. Rebase before landing
tomek added inline comments.
native/push/encrypted-notif-utils-api.js
13 ↗(On Diff #42022)

What does it mean?

This revision is now accepted and ready to land.Jul 4 2024, 5:52 AM
native/push/encrypted-notif-utils-api.js
13 ↗(On Diff #42022)

This is the type of olm encrypted message. 1 is MESSSAGE_TYPE_MESSAGE while 0 i` MESSAGE_TYPE_PREKEY`. See the following type from olm_vx.x.x.js:

declare export type EncryptResult = {
   +type: 0 | 1, // 0: PreKey, 1: Message
   +body: string,
 };