Workaround (or fix maybe) for NEW_NOTIFICATION messages

[starypatyk]

This is an attempt of a workaround (or fix) for #1964.

TL;DR

Incoming notifications are initially handled by system-level FCM code. Then an intent is passed to the application, that is partially handled by Firebase SDK code and partially by the application code in the onMessageReceived method.

Sometimes (I have not been able to find when), the intent contains an additional flag that causes the Firebase SDK code to handle (display) the notification without invoking at all the onMessageReceived method defined in application code.

As the unencrypted body of the notification message contains just the NEW_NOTIFICATION text, this is what gets shown to the user.

Code in this PR "intercepts" the incoming intent before the SDK code and clears the notification flag, so the SDK code always invokes the onMessageReceived method. The application code can then handle the message as usual - i.e. decrypt it and display proper notification to the user.

More detailed description below

Firebase documentation is quite clear that notification messages are delivered to the application code only when the application is in foreground.

onMessageReceived is provided for most message types, with the following exceptions:

• Notification messages delivered when your app is in the background. In this case, the notification is delivered to the device’s system tray. A user tap on a notification opens the app launcher by default.

• Messages with both notification and data payload, when received in the background. In this case, the notification is delivered to the device’s system tray, and the data payload is delivered in the extras of the intent of your launcher Activity.

So it seems that notification messages should not work at all. But sometimes they do. 😉

I have not been able to find out, why in some cases incoming messages are treated as combined notification/data messages and in other cases as just data messages. This would probably need insight into inner working of the push-notifications.nextcloud.com server.

I did find out however, that even in cases when the message is treated as a combined notification/data message, the application code is involved in handling of the message.

After some investigation of the Firebase Android SDK, I have found that the decision to show notification or invoke the onMessageReceived method is made by the FirebaseMessagingService class, in the dispatchMessage method.

Then, I found that the NotificationParams.isNotification method just checks for values of two extras in the intent sent by the FCM system code.

So in this PR, I propose to override the handleIntent method and clear these two extras from the incoming intent before letting the Firebase SDK code process it. This means that the notifications will be always delivered to the onMessageReceived method - as expected.

I have been able to get the NEW_NOTIFICATION messages on the emulator - although I am not sure if I can provide a repeatable scenario.

With the additional debug code from this PR, but without clearing the extras in handleIntent, I get the wrong notification and the following sequence in the logs.

03-12 22:13:34.752 12640 12782 D NCFirebaseMessagingService: handleIntent - extras: gcm.n.e: null, gcm.notification.e: 1
03-12 22:13:34.753 12640 12782 W FirebaseMessaging: Unable to log event: analytics library is missing
03-12 22:13:34.756 12640 12782 W FirebaseMessaging: Missing Default Notification Channel metadata in AndroidManifest. Default value will be used.

When I clear the extras in handleIntent, I get the correct notification and the following logs.

03-12 22:15:18.645 12832 12942 D NCFirebaseMessagingService: handleIntent - extras: gcm.n.e: null, gcm.notification.e: 1
03-12 22:15:18.646 12832 12942 W FirebaseMessaging: Unable to log event: analytics library is missing
03-12 22:15:18.646 12832 12942 D NCFirebaseMessagingService: onMessageReceived
03-12 22:15:18.662 12832 12946 D NotificationJob: doWork started
03-12 22:15:18.670 12832 12946 D OwnCloudClient #0: REQUEST GET /ocs/v2.php/apps/notifications/api/v2/notifications/7284
03-12 22:15:18.671 12832 12946 D AdvancedSslSocketFactory: Creating SSL Socket with remote ***SERVER NAME REDACTED***:443, local null:0, params: org.apache.commons.httpclient.params.HttpConnectionParams@bf36ecc
03-12 22:15:18.671 12832 12946 D AdvancedSslSocketFactory:  ... with connection timeout 60000 and socket timeout 60000
03-12 22:15:18.672 12832 12946 I ServerNameIndicator: SSLSocket implementation: com.google.android.gms.org.conscrypt.Java8FileDescriptorSocket
03-12 22:15:18.672 12832 12946 I ServerNameIndicator: SNI done, hostname: ***SERVER NAME REDACTED***
03-12 22:15:18.754 12832 12946 D GetNotificationRemoteOperation: Successful response: {"ocs":{"meta":{"status":"ok","statuscode":200,"message":"OK"},"data":{"notification_id":7284,"app":"admin_notifications","user":"darek","datetime":"2024-03-12T21:15:18+00:00","object_type":"admin_notifications","object_id":"65f0c5e6","subject":"Testing push notifications","message":"","link":"","subjectRich":"","subjectRichParameters":[],"messageRich":"","messageRichParameters":[],"icon":"https:\/\/***SERVER NAME REDACTED***\/apps\/notifications\/img\/notifications-dark.svg","shouldNotify":true,"actions":[]}}}
03-12 22:15:18.758 12832 12946 D skia    : --- Failed to create image decoder with message 'unimplemented'
03-12 22:15:18.759 12832 12946 D skia    : --- Failed to create image decoder with message 'unimplemented'
03-12 22:15:18.763 12832 12871 I WM-WorkerWrapper: Worker result SUCCESS for Work [ id=be3ca944-2897-4657-812c-3878caf82aaa, tags={ com.nextcloud.client.jobs.NotificationWork, *, name:notification, timestamp:1710278118647, class:NotificationWork } ]

[github-actions]

Codacy

Lint

Type	master	PR
Warnings	71	71
Errors	3	3

SpotBugs

Category	Base	New
Bad practice	68	68
Correctness	68	68
Dodgy code	350	350
Experimental	2	2
Internationalization	7	7
Malicious code vulnerability	2	2
Multithreaded correctness	6	6
Performance	56	56
Security	19	19
Total	578	578

[github-actions]

APK file: https://www.kaminsky.me/nc-dev/android-artifacts/12672.apk



To test this change/fix you can simply download above APK file and install and test it in parallel to your existing Nextcloud app.

[github-actions]

blue-Light-Screenshot test failed: https://www.kaminsky.me/nc-dev/android-integrationTests/12672-Screenshot-blue-Light-12-06

[github-actions]

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

[major-mayer]

Is there a chance that this could get a review anytime soon? I am experiencing the same bug, and would like to see a workaround getting merged that would prevent deleting the user data the next time.

[tobiasKaminsky]

I have found a friend that has the same problem.
I will test this tonight.

@major-mayer how do you install Nextcloud App? Then I can also built a correctly signed version for you.

[major-mayer]

Right now, I don't have this problem anymore.
I cleared the user data as always, and then the NEW_NOTIFICATION notifications didn't appear anymore.

I always install Nextcloud from the Play Store.

[tobiasKaminsky]

Tested on an affected phone and it work.
Many thanks!