Page MenuHomePhabricator
Differential D12838

Implement web notifs session createion with race condition handling
ClosedPublic
Actions

Authored by marcin on Jul 22 2024, 7:18 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Oct 30, 10:23 AM
Unknown Object (File)
Tue, Oct 29, 9:26 PM
Unknown Object (File)
Mon, Oct 28, 10:22 PM
Unknown Object (File)
Sat, Oct 26, 3:40 AM
Unknown Object (File)
Tue, Oct 22, 1:17 PM
Unknown Object (File)
Tue, Oct 22, 8:33 AM
Unknown Object (File)
Tue, Oct 22, 8:33 AM
Unknown Object (File)
Tue, Oct 22, 8:33 AM
View All Files
Subscribers
ashoat

Details

Reviewers
kamil
tomek
inka
Commits
rCOMMc879bd67d25a: Implement web notifs session createion with race condition handling
Summary

This differential enables notifs inbound session creation on web platforms with race condition handling

Test Plan
  1. Apply this patch: https://gist.github.com/marcinwasowicz/c3da26ab164d1044aa3b2870f3434acd
  2. Log in on web and native
  3. Send message from native to web
  4. Copy the content of encrypted notification addressed to web client
  5. Paste the content in service-worker code and pass it to decryptWebNotification function. Log the result
  6. Expect that logged result is decrypted notification

Now repeat the test plan above but FIRST send message to native. Tunnelbroker can send pushes to native so session will be created on the native side and notification sent to web will be type MESSAGE_TYPE_MESSAGE (1).

Diff Detail

Repository
rCOMM Comm
Branch
marcin/eng-8237
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

marcin created this revision.Jul 22 2024, 7:18 AM
Herald added a subscriber: ashoat. · View Herald TranscriptJul 22 2024, 7:18 AM
marcin added a parent revision: D12837: Bump olm version.Jul 22 2024, 7:18 AM
Harbormaster completed remote builds in B30563: Diff 42611.Jul 22 2024, 7:34 AM
marcin requested review of this revision.Jul 22 2024, 7:34 AM
marcin updated this revision to Diff 42615.Jul 22 2024, 7:49 AM

A couple of changes

Harbormaster completed remote builds in B30564: Diff 42615.Jul 22 2024, 8:06 AM
marcin updated this revision to Diff 42641.Jul 22 2024, 11:55 AM

Improvement: use the same functions to fetch and persist olm data regardless of notification type (keyserver/peer device)

Harbormaster completed remote builds in B30577: Diff 42641.Jul 22 2024, 12:17 PM
marcin updated this revision to Diff 42718.Jul 23 2024, 11:08 AM

Rebase

marcin edited the test plan for this revision. (Show Details)Jul 23 2024, 11:12 AM
Harbormaster completed remote builds in B30623: Diff 42718.Jul 23 2024, 11:32 AM
kamil accepted this revision.Jul 25 2024, 8:18 AM
kamil added inline comments.
web/push-notif/notif-crypto-utils.js
164–169

why do we need to throw if we will never reach this branch?

202–209

can you use validators here?

343–349

In this case I would create only one variable with object

463–468

if Flow forcing this, can you move this above and early-exit?

This revision is now accepted and ready to land.Jul 25 2024, 8:18 AM
marcin added a child revision: D12882: Implement sending E2EE notification from the client.Jul 25 2024, 9:50 AM
marcin updated this revision to Diff 42846.Jul 28 2024, 2:30 AM

Fix desktop notifications

Harbormaster completed remote builds in B30720: Diff 42846.Jul 28 2024, 2:47 AM
marcin added inline comments.Jul 29 2024, 9:38 AM
web/push-notif/notif-crypto-utils.js
463–468

I tried and unfortunately flow can't conclude that senderDeviceID is not nullish if we early exit.

marcin updated this revision to Diff 42917.Jul 29 2024, 1:53 PM

Address review

Harbormaster completed remote builds in B30774: Diff 42917.Jul 29 2024, 2:17 PM
marcin updated this revision to Diff 42948.Jul 30 2024, 3:29 AM

Update phab

Harbormaster completed remote builds in B30803: Diff 42948.Jul 30 2024, 5:32 AM
marcin updated this revision to Diff 43008.Jul 31 2024, 11:18 AM

Fix keyserver based notifs

Harbormaster completed remote builds in B30843: Diff 43008.Jul 31 2024, 11:35 AM
marcin updated this revision to Diff 43086.Aug 5 2024, 4:20 AM

Rebase before landing

Harbormaster completed remote builds in B30906: Diff 43086.Aug 5 2024, 6:33 AM
Closed by commit rCOMMc879bd67d25a: Implement web notifs session createion with race condition handling (authored by marcin, committed by Marcin Wąsowicz <marcinwasowicz@MacBook-Pro-Marcin.local>). · Explain WhyAug 5 2024, 6:40 AM
This revision was automatically updated to reflect the committed changes.
Marcin Wąsowicz <marcinwasowicz@MacBook-Pro-Marcin.local> added a commit: rCOMMc879bd67d25a: Implement web notifs session createion with race condition handling.
marcin removed a child revision: D12882: Implement sending E2EE notification from the client.Aug 21 2024, 4:20 AM

Revision Contents
Changeset List

PathSize
desktop/
src/
2 lines
15 lines
lib/
types/
3 lines
web/
push-notif/
549 lines
7 lines

Diff 42718

View Options

desktop/src/main.js

Loading...
View Options

desktop/src/push-notifications.js

Loading...
View Options

lib/types/electron-types.js

Loading...
View Options

web/push-notif/notif-crypto-utils.js

Loading...
View Options

web/push-notif/push-notifs-handler.js

Loading...