Page MenuHomePhabricator
Differential D12785

Add keyserverID to unencrypted notification and display to staff info about notification failure
ClosedPublic
Actions

Authored by marcin on Jul 17 2024, 8:07 AM.
Tags
None
Referenced Files
F3376185: D12785.diff
Tue, Nov 26, 10:32 PM
Unknown Object (File)
Fri, Nov 8, 10:36 PM
Unknown Object (File)
Fri, Nov 8, 1:31 PM
Unknown Object (File)
Fri, Nov 8, 12:23 PM
Unknown Object (File)
Fri, Nov 8, 12:23 PM
Unknown Object (File)
Fri, Nov 8, 12:23 PM
Unknown Object (File)
Fri, Nov 8, 12:23 PM
Unknown Object (File)
Fri, Nov 8, 6:14 AM
View All 73 Files
Subscribers
ashoat

Details

Reviewers
tomek
bartek
ashoat
Commits
rCOMM46beed0710f4: Add keyserverID to unencrypted notification and display to staff info about…
Summary

This differential:

  1. Fixes recently introduced issue - notification was missing keyserverID after encryption failure.
  2. Unifies encryption failure indicator field - keyserver used to put encryptionFailed while the NSE checked for encryptionFailure
  3. Display additional error message notification to staff members if encryptionFailed key is present.
Test Plan
  1. Add throw in src/push/crypto.js.
  2. Send notification to iOS device.
  3. Ensure that two notifications are present: one correct and the other with encryption failure error message.

Diff Detail

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

Event Timeline

marcin created this revision.Jul 17 2024, 8:07 AM
Herald added a subscriber: ashoat. · View Herald TranscriptJul 17 2024, 8:08 AM
marcin updated this revision to Diff 42389.Jul 17 2024, 8:24 AM

Mirror behaviour on Android

Harbormaster completed remote builds in B30413: Diff 42385.Jul 17 2024, 8:24 AM
ashoat published this revision for review.Jul 17 2024, 8:59 AM

Can we continue printing the log when this happens, even for non-staff builds?

web/cpp/SQLiteQueryExecutorBindings.cpp
315 ↗(On Diff #42389)

Oops, not sure how I landed this with a Prettier issue. Thanks for the fix

ashoat accepted this revision.Jul 17 2024, 8:59 AM
This revision is now accepted and ready to land.Jul 17 2024, 8:59 AM
Harbormaster completed remote builds in B30417: Diff 42389.Jul 17 2024, 9:33 AM
marcin updated this revision to Diff 42395.Jul 17 2024, 10:30 AM

Log information about notification encryption failure regardless of user staff member status.

Harbormaster completed remote builds in B30423: Diff 42395.Jul 17 2024, 10:55 AM
Closed by commit rCOMM46beed0710f4: Add keyserverID to unencrypted notification and display to staff info about… (authored by marcin). · Explain WhyJul 17 2024, 10:56 AM
This revision was automatically updated to reflect the committed changes.
marcin added a commit: rCOMM46beed0710f4: Add keyserverID to unencrypted notification and display to staff info about….

Revision Contents
Changeset List

PathSize
keyserver/
src/
push/
5 lines
native/
android/
app/
src/
main/
java/
app/
comm/
android/
notifications/
15 lines
ios/
NotificationService/
20 lines
web/
cpp/
3 lines

Diff 42395

View Options

keyserver/src/push/crypto.js

Loading...
View Options

native/android/app/src/main/java/app/comm/android/notifications/CommNotificationsHandler.java

Loading...
View Options

native/ios/NotificationService/NotificationService.mm

Loading...
View Options

web/cpp/SQLiteQueryExecutorBindings.cpp

Loading...