Fix PN metadata leak, PN navigation to chat, and unexpected PNs from other accounts

[pedropombeiro]

fixes #6772, #3488 and #2984, depends on status-im/status-go#1297

Summary:

Pre-PR example payload:

{
	"notification": {
		"title": "Status",
		"body": "You have a new message"
	},
	"data": {
		"msg": {
			"from": "0x04325367620ae20dd878dbb39f69f02c567d789dd21af8a88623dc5b529827c2812571c380a2cd8236a2851b8843d6486481166c39debf60a5d30b9099c66213e4",
			"to": "0x04601f0228b9c046ddf524f7c612093c0ba69a8b7ee37e3f9292626fd6bb50578f57e9f5672e79967fcef7803cfe28daf93342cb4cdb6eb9702eb759d3154b192b"
		}
	}
}

Same payload post-PR (notice versioning):

{
	"data": {
		"msg-v2": {
			"from": "0x2cea3bd5",
			"to": "0xb1f89744",
			"id": "0x872653ad"
		}
	}
}

This PR:

• Adds new message id field to the PN in the form of hash:${msg-v2/id}, creating a 1:1 link between Whisper messages and their respective PN on the receiver side
• Moves to using sender/receiver pubkey hashes instead of the pubkeys themselves (first 10 characters/4 bytes of sha3 hash)
  • Key/values sent: msg-v2/from, msg-v2/to, msg-v2/id
• Stores complete PN payload as JSON in db (:push-notifications/stored)
• Moves from Notification API (where title/body were being sent with the PN) to Messaging API (Messaging API only sends a hidden message with key/value pairs) (Metadata leak in push notifications #6772)
• Maintains compatibility with older clients (receiving only, sent PNs will not be displayed on older clients)
• Creates background task to receive FCM message and display appropriate PN
  • Only create PN if message with hash equal to msg-v2/id is not marked as seen
• Removes unused config/in-app-notifications-enabled?/IN_APP_NOTIFICATIONS_ENABLED
• Removes dependency on NotifyUsers status-go API by leveraging react-native-firebase API the API does not support some critical properties for Android and iOS, so we must continue with the implementation in status-go for the time being. Created a suggestion at https://boards.invertase.io/react-native-firebase/p/add-missing-properties-to-messagingremotemessage.
• Fixes navigating to chat if the user is signed out and taps on notification (Clicking message notification does not open the chat #3488).

Review notes (optional):

This PR paves the way to fix issues like #3488, #3487, #2984

Testing notes (optional):

Platforms

Check that new apps can receive PNs from both new and old clients. I've tested Desktop and Android, but not iOS.

• Android (as sender/receiver)
• iOS (as sender/receiver)
• Desktop (as sender only)

See here for expected behavior from a technical perspective: https://github.com/invertase/react-native-firebase-docs/blob/master/docs/messaging/introduction.md#data-only-messages

Areas that maybe impacted

Functional

• 1-1 chats
• group chats

Steps to test:

1. Open Status on device 1
2. Open Status on device 2
3. Add device 2 as a contact on device 1
4. Send a message from device 1 to device 2
5. Add device 1 as a contact of device 2
6. Put the app on device 2 in the background
7. Send another message from device 1 to device 2
8. Confirm that a Push Notification is displayed on device 2
9. Tap on the PN
10. Confirm that the app navigates to the appropriate chat

Future steps

With this PR implemented, we can easily implement the following:

1. Show sender/chat room in PN if db is unlocked (Show sender in mobile push notifications when db is unlocked #7043)
2. Group PNs by sender/chat (Group mobile push notifications by sender #7046)
3. Navigate to message matching partial IDs in mobile push notifications when PN is tapped Navigate to message matching partial IDs in mobile push notifications when PN is tapped #7044

status: ready

[status-github-bot]

Pull Request Checklist

• Have you updated the documentation, if impacted (e.g. docs.status.im)?

[pedropombeiro]

Observations after the migration from Notifications API to Messaging API:

• onMessage method is not being called when a PN arrives with the app on the foreground, as described in https://rnfirebase.io/docs/v4.3.x/messaging/receiving-messages#3)-Listen-for-FCM-messages;
• Background task handler does not seem to be getting called when PNs arrive and the app is in the background.
• If I use the REPL to:
  • call background/handle-message, the notification shows up fine even if the app is in the background';

UPDATE: Turned out it was due to the from field inside data. from is a reserved name for FCM yielding an HTTP 400 error when sending messages. Things started working once I renamed the field to status-from.

[pedropombeiro]

The work in this PR can be leveraged for #2984, if we can access the app db from the background task to only create PN if last logged in account matches msg/to.

[status-comment-bot]

✅ #18 CI BUILD SUCCESSFUL in 20 min and counting (d3c24cfcfa1d90e3e8f9a3976d567b192189465a)

Android(e2e)	iOS	MacOS	AppImage	Windows

[status-comment-bot]

✅ #19 CI BUILD SUCCESSFUL in 22 min and counting (8c70a62)

Android(e2e)	iOS	MacOS	AppImage	Windows

[rasom]

ios app crashes if

1. Put app on ios device to background
2. send message from another device
3. close app on ios device
4. tap notification
5. app crashes

[rasom]

PN are not received on Android when app is closed

seems like this is expected, from now on PN will not be received if app is closed

[churik]

@rasom why it is expected?
I tested PNs on WhatsUp, Viber, Telegram - and I'm able to receive them when app is closed, so imo it should be the same for Status as well (just wondering where we made this decision)

[rasom]

@churik we want to have different props in terms of privacy, so that might be the reason.
@pombeirp do we expect to receive PNs when app is closed? can you please clarify this?

[pedropombeiro]

PN are not received on Android when app is closed

seems like this is expected, from now on PN will not be received if app is closed

This is only expected on iOS. Android has provisions to deal with starting up the app through a background task if needed (see link to docs in OP). On iOS, we should show the notifications to the user once he starts the app/logs back in.

[churik]

@pombeirp
in continuation of #6893 (comment):

3. Is it expected that there is no redirection to chat or options (like open, reply) when you click on PN on any desktop platform? Will it be separate fix for desktop?

[pedropombeiro]

3. Is it expected that there is no redirection to chat or options (like open, reply) when you click on PN on any desktop platform? Will it be separate fix for desktop?

This PR is only about metadata leaks (with some other low-hanging fruit that was simple to fix related to the changes). The issues that you're mentioning are more about UX and will probably be handled by the desktop team.

[rasom]

@pombeirp
on android, when app is closed PN is actually received

RNFMessagingService: onMessageReceived event received

but unfortunately, a headless task is not called after that, because the app is killed. I doubt that we can do anything about this except have some handling on native side.

[pedropombeiro]

@pombeirp
on android, when app is closed PN is actually received

RNFMessagingService: onMessageReceived event received

but unfortunately, a headless task is not called after that, because the app is killed. I doubt that we can do anything about this except have some handling on native side.

This is something that was definitely working in the PR, however the last time I looked into it, the app was started so that the background task can be invoked, but seemed to crash while initializing the app, not sure why.

A log entry showing dispatching :notifications.callback/on-message to display background message should appear if the background task executed successfully.

[rasom]

@pombeirp

01-14 16:52:11.415 2172 2239 E ReactNativeJS: No task registered for key RNFirebaseBackgroundMessage

This exception happens if the app was force closed, it is not related to initialization in any way. You just need to force close application, the when PN is received by background service it can't find the headless task.

[rasom]

invertase/react-native-firebase#1174

[pedropombeiro]

I guess we need to log if registerHeadlessTask actually gets called, and if not, what is preventing it. I don't understand what could have changed recently that prevents the headless task from registering when the app is started by FCM.

[rasom]

@pombeirp

I guess we need to log if registerHeadlessTask actually gets called

Yes, it is called, it works just fine until the app is force closed. So if you put it into the background - there is no exception or anything. When you force close the app and send PN - exception happens, but it is kind of "silent" and can be seen only in the logs. As long as the app is not running.

[rasom]

Also i doubt that's about starting the app from FCM. This happens only when we tap on a notification, and it works.

[pedropombeiro]

@rasom This somehow worked before, even after force-closing the app (tested that many times) so I thought that FCM service would start our app if needed (which would then register the headless task). This is also what is shown in the docs, as you can see there's a column for when the app is in the background, and another for when the app is closed, and they both mention it will work due to the background task on Android.

[rasom]

ok, the app is not started on message only when i'm running it in debug mode. Otherwise, it is started, though PN is shown only if notification passes show-notification? check, though it is not going to pass accounts.db/logged-in?. The user is definitely not logged in in this case.

[pedropombeiro]

ok, the app is not started on message only when i'm running it in debug mode. Otherwise, it is started, though PN is shown only if notification passes show-notification? check, though it is not going to pass accounts.db/logged-in?. The user is definitely not logged in in this case.

Ah, that check must have been added afterward. I guess if we wanted we could use the force flag to ignore that test, and show the PN if the last logged in user matches the PN target user. How does that work for people who have saved their credentials? Will they still see PN?

[rasom]

@pombeirp trying this now