-
Notifications
You must be signed in to change notification settings - Fork 440
Decrypt E2EE notifications #2194
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
4ffd672
1649d2e
c9f21c3
3421c64
2e9da11
2b6ad6b
a878e68
7210ea3
69aa682
4c7487b
4655cc1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| import 'package:collection/collection.dart'; | ||
| import 'package:firebase_messaging/firebase_messaging.dart'; | ||
| import 'package:flutter/foundation.dart'; | ||
|
|
||
|
|
@@ -7,6 +8,7 @@ import '../api/route/notifications.dart'; | |
| import '../firebase_options.dart'; | ||
| import '../log.dart'; | ||
| import '../model/binding.dart'; | ||
| import '../model/push_key.dart'; | ||
| import 'display.dart'; | ||
| import 'open.dart'; | ||
|
|
||
|
|
@@ -203,8 +205,64 @@ class NotificationService { | |
| NotificationDisplayManager.init(); // TODO call this just once per isolate | ||
| } | ||
|
|
||
| static void _onRemoteMessage(FirebaseRemoteMessage message) { | ||
| final data = FcmMessage.fromJson(message.data); | ||
| NotificationDisplayManager.onFcmMessage(data, message.data); | ||
| static void _onRemoteMessage(FirebaseRemoteMessage message) async { | ||
| final origData = message.data; | ||
|
|
||
| EncryptedFcmMessage? parsed; | ||
| try { | ||
| parsed = EncryptedFcmMessage.fromJson(origData); | ||
| } catch (_) { | ||
| // Presumably a non-E2EE notification. // TODO(server-12) | ||
| await _onPlaintextRemoteMessage(origData); | ||
| return; | ||
| } | ||
|
|
||
| final globalStore = await ZulipBinding.instance.getGlobalStore(); | ||
| final pushKey = globalStore.pushKeys.getPushKeyById(parsed.pushKeyId); | ||
| if (pushKey == null) { | ||
| // Not a key we have; nothing we can do with this notification-message. | ||
| // This can happen if it's addressed to an account that's been logged out. | ||
| // TODO(#1764) try to unregister on logout (though this will still sometimes happen) | ||
| return; | ||
| } | ||
| final account = globalStore.getAccount(pushKey.accountId)!; | ||
|
Comment on lines
+221
to
+228
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One or the other of these can throw in the case of a notification received for a logged-out account, right? (The
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The Good point that an unknown push key can occur if the account was logged out. I'll have it return instead.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wrote this: if (pushKey == null) {
// Not a key we have; nothing we can do with this notification-message.
// This can happen if it's addressed to an account that's been logged out.
// TODO(#1764) try to unregister on logout (though this will still sometimes happen)
return;
}That TODO is already handled later in my draft branch. |
||
|
|
||
| final plaintext = await PushKeyStore.decryptNotification( | ||
| pushKey.pushKey, parsed.encryptedData); | ||
| final rawData = jsonUtf8Decoder.convert(plaintext) as Map<String, dynamic>; | ||
| final data = FcmMessage.fromJson(rawData); | ||
| switch (data) { | ||
| case FcmMessageWithIdentity(): break; | ||
| case UnexpectedFcmMessage(): return; // TODO(log) | ||
| } | ||
|
|
||
| if (!(account.realmUrl.origin == data.realmUrl.origin | ||
| && account.userId == data.userId)) { | ||
| throw Exception("bad notif payload: realm/userId fails to match push key"); | ||
| } | ||
|
|
||
| NotificationDisplayManager.onFcmMessage(data, account); | ||
| } | ||
|
|
||
| static Future<void> _onPlaintextRemoteMessage(Map<String, dynamic> rawData) async { | ||
| final data = FcmMessage.fromJson(rawData); | ||
| switch (data) { | ||
| case FcmMessageWithIdentity(): break; | ||
| case UnexpectedFcmMessage(): return; // TODO(log) | ||
| } | ||
|
|
||
| final globalStore = await ZulipBinding.instance.getGlobalStore(); | ||
| final account = globalStore.accounts.firstWhereOrNull((account) => | ||
| account.realmUrl.origin == data.realmUrl.origin && account.userId == data.userId); | ||
|
|
||
| // Skip showing notifications for a logged-out account. This can occur if | ||
| // the unregisterToken request failed previously. It would be annoying | ||
| // to the user if notifications keep showing up after they've logged out. | ||
| // (Also alarming: it suggests the logout didn't fully work.) | ||
| if (account == null) { | ||
| return; | ||
| } | ||
|
|
||
| NotificationDisplayManager.onFcmMessage(data, account); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -57,15 +57,16 @@ dependencies: | |
| path_provider: ^2.0.13 | ||
| share_plus: ^12.0.0 | ||
| share_plus_platform_interface: ^6.0.0 | ||
| sodium_libs: ^3.4.6+4 | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Commit-message nit: Looks like the "then fixing up the sorting" part is outdated; you fixed up the sorting in a separate prep commit. :)
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What that's intended to mean is that |
||
| sqlite3: ^2.4.0 | ||
| sqlite3_flutter_libs: ^0.5.13 | ||
| unorm_dart: ^0.3.1+1 | ||
| url_launcher: ^6.1.11 | ||
| url_launcher_android: ">=6.1.0" | ||
| video_player: ^2.10.0 | ||
| wakelock_plus: ^1.2.8 | ||
| zulip_plugin: | ||
| path: ./packages/zulip_plugin | ||
| unorm_dart: ^0.3.1+1 | ||
| # Keep list sorted when adding dependencies; it helps prevent merge conflicts. | ||
|
|
||
| dependency_overrides: | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will all notifications be E2EE at server 12+, or is it an optional feature? (looking at the
TODO(server-12))There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All notifications will be E2EE at server 12+ for clients that support it. So when we drop support for older servers, we can drop support for the legacy non-E2EE notifications.