Paths

Table of Contentst

Differential D12670

Implement notification encryption on native
ClosedPublic
Actions

Authored by marcin on Jul 4 2024, 9:25 AM.

Tags

None

Referenced Files

	Unknown Object (File)
	Thu, Sep 26, 9:41 PM

	Unknown Object (File)
	Mon, Sep 16, 6:49 AM

	Unknown Object (File)
	Sat, Sep 14, 5:16 PM

	Unknown Object (File)
	Sun, Sep 8, 3:03 PM

	Unknown Object (File)
	Sun, Sep 8, 3:02 PM

	Unknown Object (File)
	Sun, Sep 8, 3:02 PM

	Unknown Object (File)
	Sat, Sep 7, 1:45 PM

	Unknown Object (File)
	Sat, Sep 7, 1:45 PM

View All 72 Files

Subscribers

Details

Reviewers

bartek
tomek
kamil

Commits

rCOMMdca9c75c8b82: Implement notification encryption on native

Summary

This differential implements encryption with notifications session on native

Test Plan

Tested in D12673

Diff Detail

Repository

Branch

marcin/eng-8237

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

marcin created this revision.Jul 4 2024, 9:25 AM

Herald added a subscriber: ashoat. · View Herald TranscriptJul 4 2024, 9:25 AM

marcin added a parent revision: D12669: Implement peer notifications session creation as outbound on web and add notifs outbound session creator to OlmAPI.Jul 4 2024, 9:26 AM

marcin added a child revision: D12671: Implement notification encryption on web.

Harbormaster completed remote builds in B30149: Diff 42040.Jul 4 2024, 9:42 AM

marcin requested review of this revision.Jul 4 2024, 9:42 AM

marcin edited the test plan for this revision. (Show Details)Jul 4 2024, 9:44 AM

tomek accepted this revision.Jul 8 2024, 4:22 AM

tomek added inline comments.

native/push/encrypted-notif-utils-api.js
21	Isn't this conversion equivalent?

This revision is now accepted and ready to land.Jul 8 2024, 4:22 AM

marcin added inline comments.Jul 15 2024, 5:40 AM

native/push/encrypted-notif-utils-api.js
21	Type definition at line 13 expects literal union of `1` \| `0` while `toString()` can yield any string. Flow complains when this change is applied.

kamil accepted this revision.Jul 16 2024, 4:43 AM

kamil added a subscriber: kamil.

kamil added inline comments.

native/cpp/CommonCpp/NativeModules/CommCoreModule.cpp
1577–1584	same comment as here: https://phab.comm.dev/D12668#inline-74094
native/cpp/CommonCpp/NativeModules/CommCoreModule.h
141	is it change accidental? If not please update `.cpp` file and Schema in `.js` file
native/cpp/CommonCpp/Notifications/BackgroundDataStorage/NotificationsCryptoModule.h
120	in `CommCoreModuleSchema` it makes sense to keep the suffix but here because it's `NotificationsCryptoModule` it seems redundant
native/push/encrypted-notif-utils-api.js
17	I prefer to add `encryptNotification` to `sqliteAPI` and use it here (and on web), but up to you

kamil added inline comments.Jul 23 2024, 8:10 AM

native/push/encrypted-notif-utils-api.js
17	`olmAPI`* - sorry for confusing

marcin added inline comments.Jul 24 2024, 3:54 AM

native/push/encrypted-notif-utils-api.js
17	Added in next diff

Address review

Harbormaster failed remote builds in B30632: Diff 42728!Jul 24 2024, 6:00 AM

Update phab

Harbormaster completed remote builds in B30791: Diff 42936.Jul 30 2024, 3:51 AM

Rebase before landing

Harbormaster completed remote builds in B30893: Diff 43073.Aug 5 2024, 4:41 AM

Closed by commit rCOMMdca9c75c8b82: Implement notification encryption on native (authored by marcin, committed by Marcin Wąsowicz <marcinwasowicz@MacBook-Pro-Marcin.local>). · Explain WhyAug 5 2024, 6:39 AM

This revision was automatically updated to reflect the committed changes.

Marcin Wąsowicz <marcinwasowicz@MacBook-Pro-Marcin.local> added a commit: rCOMMdca9c75c8b82: Implement notification encryption on native.

Revision Contents
Changeset List

Path

Size

native/

cpp/

CommonCpp/

CryptoTools/

CryptoModule.cpp

21 lines

1 line

23 lines

NativeModules/

CommCoreModule.h

6 lines

CommCoreModule.cpp

38 lines

Notifications/

BackgroundDataStorage/

NotificationsCryptoModule.h

3 lines

NotificationsCryptoModule.cpp

18 lines

_generated/

commJSI-generated.cpp

push/

encrypted-notif-utils-api.js

14 lines

schema/

CommCoreModuleSchema.js

4 lines

Diff 42040

native/cpp/CommonCpp/CryptoTools/CryptoModule.cpp

Loading...

native/cpp/CommonCpp/CryptoTools/Session.h

Loading...

native/cpp/CommonCpp/CryptoTools/Session.cpp

Loading...

native/cpp/CommonCpp/NativeModules/CommCoreModule.h

Loading...

native/cpp/CommonCpp/NativeModules/CommCoreModule.cpp

Loading...

native/cpp/CommonCpp/Notifications/BackgroundDataStorage/NotificationsCryptoModule.h

Loading...

native/cpp/CommonCpp/Notifications/BackgroundDataStorage/NotificationsCryptoModule.cpp

Loading...

native/cpp/CommonCpp/_generated/commJSI-generated.cpp

Loading...

native/cpp/CommonCpp/_generated/commJSI.h

Loading...

native/push/encrypted-notif-utils-api.js

Loading...

native/schema/CommCoreModuleSchema.js

Loading...