Page MenuHomePhabricator
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)
Mon, Apr 21, 4:54 AM
Unknown Object (File)
Mon, Apr 14, 3:16 PM
Unknown Object (File)
Fri, Apr 11, 6:09 PM
Unknown Object (File)
Tue, Apr 8, 9:59 AM
Unknown Object (File)
Tue, Apr 8, 6:05 AM
Unknown Object (File)
Tue, Apr 8, 2:24 AM
Unknown Object (File)
Tue, Apr 8, 2:20 AM
Unknown Object (File)
Tue, Apr 8, 1:01 AM
View All Files
Subscribers
ashoat
kamil

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
rCOMM Comm
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

marcin updated this revision to Diff 42728.Jul 24 2024, 5:45 AM

Address review

Harbormaster failed remote builds in B30632: Diff 42728!Jul 24 2024, 6:00 AM
marcin updated this revision to Diff 42936.Jul 30 2024, 3:23 AM

Update phab

Harbormaster completed remote builds in B30791: Diff 42936.Jul 30 2024, 3:51 AM
marcin updated this revision to Diff 43073.Aug 5 2024, 4:06 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

PathSize
native/
cpp/
CommonCpp/
CryptoTools/
21 lines
1 line
23 lines
NativeModules/
6 lines
38 lines
Notifications/
BackgroundDataStorage/
3 lines
18 lines
_generated/
push/
14 lines
schema/
4 lines

Diff 42040

View Options

native/cpp/CommonCpp/CryptoTools/CryptoModule.cpp

Show First 20 Lines • Show All 77 Lines • ▼ Show 20 Lines | {
} }
| { | {
+success: false, +success: false,
+reason: 'HASH_IN_USE' | 'OTHER', +reason: 'HASH_IN_USE' | 'OTHER',
+status: number, +status: number,
+statusText: string, +statusText: string,
}; };
export type BlobDownloadResult =
| { +result: 'success', response: Response }
| { +result: 'error', +status: number, +statusText: string };
async function downloadBlob(
blobHash: string,
headers: { [string]: string },
ashoatUnsubmitted
Not Done
Inline Actions
blobHash: string,
- headers: { [string]: string },
+ headers: { +[string]: string },
): Promise<BlobDownloadResult> {

Inputs should (usually) be read-only

ashoat: Inputs should (usually) be read-only
bartekAuthorUnsubmitted
Done
Inline Actions

They're intentionally not read-only because fetch doesn't accept them this way,
Util function createDefaultHTTPRequestHeaders() also returns them as mutable for this reason

bartek: They're intentionally not read-only because `fetch` doesn't accept them this way, Util function…
ashoatUnsubmitted
Not Done
Inline Actions

Thanks for explaining!

ashoat: Thanks for explaining!
): Promise<BlobDownloadResult> {
const blobURL = getBlobFetchableURL(blobHash);
const response = await fetch(blobURL, {
method: blobServiceConfig.httpEndpoints.GET_BLOB.method,
headers,
});
if (response.status !== 200) {
const { status, statusText } = response;
return { result: 'error', status, statusText };
}
return { result: 'success', response };
}
async function uploadBlob( async function uploadBlob(
blob: Blob | string, blob: Blob | string,
hash: string, hash: string,
headers: { [string]: string }, headers: { [string]: string },
): Promise<BlobOperationResult> { ): Promise<BlobOperationResult> {
const formData = new FormData(); const formData = new FormData();
formData.append('blob_hash', hash); formData.append('blob_hash', hash);
if (typeof blob === 'string') { if (typeof blob === 'string') {
▲ Show 20 Lines • Show All 119 Lines • ▼ Show 20 Lines
export { export {
makeBlobServiceURI, makeBlobServiceURI,
isBlobServiceURI, isBlobServiceURI,
blobHashFromURI, blobHashFromURI,
blobHashFromBlobServiceURI, blobHashFromBlobServiceURI,
generateBlobHolder, generateBlobHolder,
getBlobFetchableURL, getBlobFetchableURL,
makeBlobServiceEndpointURL, makeBlobServiceEndpointURL,
downloadBlob,
uploadBlob, uploadBlob,
assignMultipleHolders, assignMultipleHolders,
removeMultipleHolders, removeMultipleHolders,
}; };
View Options

native/cpp/CommonCpp/CryptoTools/Session.h

Show First 20 Lines • Show All 77 Lines • ▼ Show 20 Lines | {
} }
| { | {
+success: false, +success: false,
+reason: 'HASH_IN_USE' | 'OTHER', +reason: 'HASH_IN_USE' | 'OTHER',
+status: number, +status: number,
+statusText: string, +statusText: string,
}; };
export type BlobDownloadResult =
| { +result: 'success', response: Response }
| { +result: 'error', +status: number, +statusText: string };
async function downloadBlob(
blobHash: string,
headers: { [string]: string },
ashoatUnsubmitted
Not Done
Inline Actions
blobHash: string,
- headers: { [string]: string },
+ headers: { +[string]: string },
): Promise<BlobDownloadResult> {

Inputs should (usually) be read-only

ashoat: Inputs should (usually) be read-only
bartekAuthorUnsubmitted
Done
Inline Actions

They're intentionally not read-only because fetch doesn't accept them this way,
Util function createDefaultHTTPRequestHeaders() also returns them as mutable for this reason

bartek: They're intentionally not read-only because `fetch` doesn't accept them this way, Util function…
ashoatUnsubmitted
Not Done
Inline Actions

Thanks for explaining!

ashoat: Thanks for explaining!
): Promise<BlobDownloadResult> {
const blobURL = getBlobFetchableURL(blobHash);
const response = await fetch(blobURL, {
method: blobServiceConfig.httpEndpoints.GET_BLOB.method,
headers,
});
if (response.status !== 200) {
const { status, statusText } = response;
return { result: 'error', status, statusText };
}
return { result: 'success', response };
}
async function uploadBlob( async function uploadBlob(
blob: Blob | string, blob: Blob | string,
hash: string, hash: string,
headers: { [string]: string }, headers: { [string]: string },
): Promise<BlobOperationResult> { ): Promise<BlobOperationResult> {
const formData = new FormData(); const formData = new FormData();
formData.append('blob_hash', hash); formData.append('blob_hash', hash);
if (typeof blob === 'string') { if (typeof blob === 'string') {
▲ Show 20 Lines • Show All 119 Lines • ▼ Show 20 Lines
export { export {
makeBlobServiceURI, makeBlobServiceURI,
isBlobServiceURI, isBlobServiceURI,
blobHashFromURI, blobHashFromURI,
blobHashFromBlobServiceURI, blobHashFromBlobServiceURI,
generateBlobHolder, generateBlobHolder,
getBlobFetchableURL, getBlobFetchableURL,
makeBlobServiceEndpointURL, makeBlobServiceEndpointURL,
downloadBlob,
uploadBlob, uploadBlob,
assignMultipleHolders, assignMultipleHolders,
removeMultipleHolders, removeMultipleHolders,
}; };
View Options

native/cpp/CommonCpp/CryptoTools/Session.cpp

Show First 20 Lines • Show All 77 Lines • ▼ Show 20 Lines | {
} }
| { | {
+success: false, +success: false,
+reason: 'HASH_IN_USE' | 'OTHER', +reason: 'HASH_IN_USE' | 'OTHER',
+status: number, +status: number,
+statusText: string, +statusText: string,
}; };
export type BlobDownloadResult =
| { +result: 'success', response: Response }
| { +result: 'error', +status: number, +statusText: string };
async function downloadBlob(
blobHash: string,
headers: { [string]: string },
ashoatUnsubmitted
Not Done
Inline Actions
blobHash: string,
- headers: { [string]: string },
+ headers: { +[string]: string },
): Promise<BlobDownloadResult> {

Inputs should (usually) be read-only

ashoat: Inputs should (usually) be read-only
bartekAuthorUnsubmitted
Done
Inline Actions

They're intentionally not read-only because fetch doesn't accept them this way,
Util function createDefaultHTTPRequestHeaders() also returns them as mutable for this reason

bartek: They're intentionally not read-only because `fetch` doesn't accept them this way, Util function…
ashoatUnsubmitted
Not Done
Inline Actions

Thanks for explaining!

ashoat: Thanks for explaining!
): Promise<BlobDownloadResult> {
const blobURL = getBlobFetchableURL(blobHash);
const response = await fetch(blobURL, {
method: blobServiceConfig.httpEndpoints.GET_BLOB.method,
headers,
});
if (response.status !== 200) {
const { status, statusText } = response;
return { result: 'error', status, statusText };
}
return { result: 'success', response };
}
async function uploadBlob( async function uploadBlob(
blob: Blob | string, blob: Blob | string,
hash: string, hash: string,
headers: { [string]: string }, headers: { [string]: string },
): Promise<BlobOperationResult> { ): Promise<BlobOperationResult> {
const formData = new FormData(); const formData = new FormData();
formData.append('blob_hash', hash); formData.append('blob_hash', hash);
if (typeof blob === 'string') { if (typeof blob === 'string') {
▲ Show 20 Lines • Show All 119 Lines • ▼ Show 20 Lines
export { export {
makeBlobServiceURI, makeBlobServiceURI,
isBlobServiceURI, isBlobServiceURI,
blobHashFromURI, blobHashFromURI,
blobHashFromBlobServiceURI, blobHashFromBlobServiceURI,
generateBlobHolder, generateBlobHolder,
getBlobFetchableURL, getBlobFetchableURL,
makeBlobServiceEndpointURL, makeBlobServiceEndpointURL,
downloadBlob,
uploadBlob, uploadBlob,
assignMultipleHolders, assignMultipleHolders,
removeMultipleHolders, removeMultipleHolders,
}; };
View Options

native/cpp/CommonCpp/NativeModules/CommCoreModule.h

Show First 20 Lines • Show All 77 Lines • ▼ Show 20 Lines | {
} }
| { | {
+success: false, +success: false,
+reason: 'HASH_IN_USE' | 'OTHER', +reason: 'HASH_IN_USE' | 'OTHER',
+status: number, +status: number,
+statusText: string, +statusText: string,
}; };
export type BlobDownloadResult =
| { +result: 'success', response: Response }
| { +result: 'error', +status: number, +statusText: string };
async function downloadBlob(
blobHash: string,
headers: { [string]: string },
ashoatUnsubmitted
Not Done
Inline Actions
blobHash: string,
- headers: { [string]: string },
+ headers: { +[string]: string },
): Promise<BlobDownloadResult> {

Inputs should (usually) be read-only

ashoat: Inputs should (usually) be read-only
bartekAuthorUnsubmitted
Done
Inline Actions

They're intentionally not read-only because fetch doesn't accept them this way,
Util function createDefaultHTTPRequestHeaders() also returns them as mutable for this reason

bartek: They're intentionally not read-only because `fetch` doesn't accept them this way, Util function…
ashoatUnsubmitted
Not Done
Inline Actions

Thanks for explaining!

ashoat: Thanks for explaining!
): Promise<BlobDownloadResult> {
const blobURL = getBlobFetchableURL(blobHash);
const response = await fetch(blobURL, {
method: blobServiceConfig.httpEndpoints.GET_BLOB.method,
headers,
});
if (response.status !== 200) {
const { status, statusText } = response;
return { result: 'error', status, statusText };
}
return { result: 'success', response };
}
async function uploadBlob( async function uploadBlob(
blob: Blob | string, blob: Blob | string,
hash: string, hash: string,
headers: { [string]: string }, headers: { [string]: string },
): Promise<BlobOperationResult> { ): Promise<BlobOperationResult> {
const formData = new FormData(); const formData = new FormData();
formData.append('blob_hash', hash); formData.append('blob_hash', hash);
if (typeof blob === 'string') { if (typeof blob === 'string') {
▲ Show 20 Lines • Show All 119 Lines • ▼ Show 20 Lines
export { export {
makeBlobServiceURI, makeBlobServiceURI,
isBlobServiceURI, isBlobServiceURI,
blobHashFromURI, blobHashFromURI,
blobHashFromBlobServiceURI, blobHashFromBlobServiceURI,
generateBlobHolder, generateBlobHolder,
getBlobFetchableURL, getBlobFetchableURL,
makeBlobServiceEndpointURL, makeBlobServiceEndpointURL,
downloadBlob,
uploadBlob, uploadBlob,
assignMultipleHolders, assignMultipleHolders,
removeMultipleHolders, removeMultipleHolders,
}; };
View Options

native/cpp/CommonCpp/NativeModules/CommCoreModule.cpp

Loading...
View Options

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

Loading...
View Options

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

Loading...
View Options

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

Loading...
View Options

native/cpp/CommonCpp/_generated/commJSI.h

Loading...
View Options

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

Loading...
View Options

native/schema/CommCoreModuleSchema.js

Loading...