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)
	Sun, Nov 24, 8:22 AM

	Unknown Object (File)
	Sun, Nov 24, 8:10 AM

	Unknown Object (File)
	Sun, Nov 24, 7:47 AM

	Unknown Object (File)
	Sun, Nov 24, 5:35 AM

	Unknown Object (File)
	Wed, Nov 20, 3:42 AM

	Unknown Object (File)
	Wed, Nov 20, 3:42 AM

	Unknown Object (File)
	Wed, Nov 20, 3:42 AM

	Unknown Object (File)
	Wed, Nov 20, 3:42 AM

View All 98 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

Lint

Lint Not Applicable

Unit

Tests Not Applicable

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 ↗	(On Diff #42040)	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 ↗	(On Diff #42040)	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 ↗	(On Diff #42040)	same comment as here: https://phab.comm.dev/D12668#inline-74094
native/cpp/CommonCpp/NativeModules/CommCoreModule.h
141 ↗	(On Diff #42040)	is it change accidental? If not please update `.cpp` file and Schema in `.js` file
native/cpp/CommonCpp/Notifications/BackgroundDataStorage/NotificationsCryptoModule.h
120 ↗	(On Diff #42040)	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 ↗	(On Diff #42040)	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 ↗	(On Diff #42040)	`olmAPI`* - sorry for confusing

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

native/push/encrypted-notif-utils-api.js
17 ↗	(On Diff #42040)	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

4 lines

CommCoreModule.cpp

56 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 43131

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...