onPushOpen of push notification no longer able to start activity in Android 12

[breumr]

• I am not disclosing a vulnerability.
• I am not just asking a question.
• I have searched through existing issues.
• I can reproduce the issue with the latest version of Parse Server and the Parse Android SDK.

Issue Description

Notification trampoline restrictions have been introduced in Android 12 which restricts how activities can be started. As this article describes it:

To improve app performance and UX, apps that target Android 12 or higher can't start activities from services or broadcast receivers that are used as notification trampolines. In other words, after the user taps on a notification, or an action button within the notification, your app cannot call startActivity() inside of a service or broadcast receiver.

Steps to reproduce

Trigger a push notification and tap on the notification on the Android device. This triggers onPushOpen in the ParsePushBroadcastReceiver

Actual Outcome

Nothing happens. Logcat displays error:
E/NotificationService: Indirect notification activity start (trampoline) from {insert package name} blocked

Expected Outcome

App should open

Environment

Parse Android SDK

• SDK version: 3.0.0
• Operating system version: 12.0

[azlekov]

Hey @breumr thanks for opening this. I have introduced some changes in supporting push notifications for Android 12+ and I will be more than happy if you will find some time to open a PR for fixing this. I will glad to review it.

[breumr]

Hi @L3K0V thanks for responding. As much as I'd love to be able to fix it, I'm only a part time Android dev and am not actually confident of exactly how this should be fixed :(

[mtrezza]

This seems to be a critical bug, I'm adding a bounty to it.

[bahaa-kallas]

In order to solve it Google folks recommend removing the delegation of opening the notification from the broadcast receiver and do it directly from the Service (in our case from ParseFirebaseMessagingService).
The question here. Should we follow what google says and refactor this part (will cause a breaking change) or should we glue-fix this issue by detecting the android api level at the service and run the app activity from there (Literally moving all the logic contained in onPushOpen and run it directly in ParseFirebaseMessagingService)
@L3K0V You said that you have introduced some changes in supporting push notifications for Android 12+. What are these changes ? in which direction they go (Glue-fix or refactor) ?
I am asking because I am researching how we can fix this.

[bahaa-kallas]

I just finished taking a deep look at the code and found where is the gotcha. @L3K0V I already saw your latest changes as well.

Parse-SDK-Android/parse/src/main/java/com/parse/ParsePushBroadcastReceiver.java

Line 422 in 6fad701

PendingIntent pContentIntent =

The android studio shows a warning on this line contains the following message

Notifications should only launch a `BroadcastReceiver` from notification actions (`addAction`) (This `BroadcastReceiver` intent is launched from a notification; this is discouraged except as notification actions)

What Google folks want us to do here is to pass our activity intent (Wrapped by PendingIntent) inside the notification
So instead we will have to do the following

if ( Build.VERSION.SDK_INT >= Build.VERSION_CODES.O){
            pContentIntent = PendingIntent.getActivity(context,
                contentIntentRequestCode,
                activityIntent, // The activity intent is the one we build inside the onPushOpen but now we should build it  in onPushReceive
                pendingIntentFlags
            );
        } else {
            pContentIntent = PendingIntent.getBroadcast(
                context,
                contentIntentRequestCode,
                contentIntent,
                pendingIntentFlags
            );
}

And then we pass that down to the notification builder.

Parse-SDK-Android/parse/src/main/java/com/parse/ParsePushBroadcastReceiver.java

Line 447 in 6fad701

.setContentIntent(pContentIntent)

What is the problem with this ?
There is no way to utilize our custom broadcast event ACTION_PUSH_OPEN which means we can't trigger onPushOpen.

[azlekov]

@bahaa-kallas from what I remember and seeing now the Broadcast receiver was used to track push notifications open/dismiss for analytics like here:

Parse-SDK-Android/parse/src/main/java/com/parse/ParsePushBroadcastReceiver.java

Line 205 in 6fad701

ParseAnalytics.trackAppOpenedInBackground(intent);

I'm not sure if these analytics are still used and working. To be honest I think the open ratio was broken and there was some issues on the parse-dashboard or parse-server repos.

parse-community/parse-dashboard#1474

@mtrezza commented there:

Push opens are an Analytics feature. Analytics is not included in open source Parse Server. You'd have to implement your own solution for this.

And he can elaborate a little more if remember.

Everything until now means that actually the implementation can be adjusted to use activities directly instead of the Parse Broadcast Receiver if analytics are abandoned. A workaround also, if we switch to activity implementation for newer Android versions will be to tell developers if they want analytics to call ParseAnalytics.trackAppOpenedInBackground(intent); explicitly in their activities.

[bahaa-kallas]

As I am not part of the board here. It's not my decision to make (Whether to break push open tracking completely in this sdk Or putting the responsibility of tracking this event on the developers in their codebase but not at the sdk level)
Imo. The fact that ParseAnalytics is not part of the open-source parse server is not enough to completely break analytics in the sdk. Users of open-source parse server can still enable analytics with their implementation.

I can open a PR right now that will include the fix above but I would vote for waiting a little bit. Maybe someone can come up with a better solution that won't introduce breaking changes (I have been disconnected for a while from the android eco-system so it's really likely that my naive solution above is not the only solution to the problem)

In a different context but relevant to our discussion here . Someone came up with (even worse than using broadcast receivers) idea from UI/UX perspective.
https://stackoverflow.com/questions/69238026/android-12-notification-trampoline-restrictions

[azlekov]

As I am not part of the board here. It's not my decision to make (Whether to break push open tracking completely in this sdk Or putting the responsibility of tracking this event on the developers in their codebase but not at the sdk level)
Imo.

Let's wait for @mtrezza what have to say about Analytics.

I can open a PR right now that will include the fix above but I would vote for waiting a little bit. Maybe someone can come up with a better solution that won't introduce breaking changes.

I'm thinking about push notification to open a special Parse Activity which handles the analytics. Like ParsePushActivity but again the developers should be guided to extends this ParsePushActivity.

[bahaa-kallas]

What you are thinking about is exactly what I have found in the stackoverflow link. I am not sure how bad is the UI/UX in that case (How much time it will take to reach the desired activity after clicking on the notification). But it seems like a solution for what we face here.
If we are going in that direction. We won't require users to extend the activity in their code (ofc they can if they wan't to attach custom code to the callbacks) because they can simply add ParsePushActivity to their AndroidManifest.xml (like how we already add the broadcast receiver) and it will work.

[azlekov]

@bahaa-kallas I'm not taking about invisible activity, but Base activity which the developer should extend, so we can inject some logic. Of course it would be great to hear different options and opinions.

How do you imagine it, do you have some ideas after checking the code?

[mtrezza]

Regarding Analytics...

Parse Server:

• It's not an official feature of Parse Server. The existing analytics router probably makes it an official feature, but it requires someone to develop their own analytics adapter.
• REST API routes /events are implemented but there is no built-in (default) analytics adapter that would store analytics data in the DB. I'm not aware that anyone is providing a 3rd party analytics adapter for Parse Server, maybe for the strategic reasons mentioned below.
• There is no documentation for it, except a reference to analytics.js, which itself states:

  Currently these just swallow/ignore the input and return a successful response to the client.

Community:

• There is Use Analytics Adapter to populate Analytics Tab parse-dashboard#1331 which shows interest to add analytics functionality to Parse Dashboard to read from a custom analytics source; this has nothing to do with writing analytics events via Parse. The idea is to use a 3rd party analytics SDK to write, and Parse Dashboard via an adapter to display stats in a dashboard tab.

Strategy:

• Analytics in Parse Server is not part of the Parse Server core functionality; there were other feature suggestions that have been discarded for the same reason, e.g. adding endpoint rate limiting to protect Parse Server from DoS attacks (which is a firewall job)
• We could add simple analytics as a PoC, but a scalable and state-of-the-art solution is a significant undertaking, I'd say a product on its own. Others are already providing a better product (commercial and open source) than we probably could as a non-core feature of Parse Server, from a resource standpoint.
• Analytics means usually more than just event tracking, so an app likely has a 3rd party analytics solution already added. There may not be a good reason to separately log certain events via Parse Server.

Maybe we can think about removing any analytics legacy code from server and SDKs. We probably should consider it a breaking change, just in case someone has created their own analytics adapter based on the existing implementation.

More specifically regarding this issue; I think whatever solution we take, it should be possible to track push opens with popular 3rd party analytics tools, such as Google Analytics, AWS Mobile Analytics, or Mixpanel.

[cbaker6]

REST API routes /events are implemented but there is no built-in (default) analytics adapter that would store analytics data in the DB. I'm not aware that anyone is providing a 3rd party analytics adapter for Parse Server, maybe for the strategic reasons mentioned below.

We probably should consider it a breaking change, just in case someone has created their own analytics adapter based on the existing implementation.

More specifically regarding this issue; I think whatever solution we take, it should be possible to track push opens with popular 3rd party analytics tools, such as Google Analytics, AWS Mobile Analytics, or Mixpanel.

Related to the analytics discussion, we have an open source analytics adapter we released recently that plugs into the Parse Server, https://github.com/netreconlab/parse-server-any-analytics-adapter

[mtrezza]

So maybe we should keep the analytics route? In any case, to me this seems to be a primitive implementation of analytics, which may be enough for certain use cases. In a more sophisticated app I think analytics is usually comprised of much more than tracking events. Also, in a pay-per-request setting, the Parse Server intermediary that just passes requests on from the client to a 3rd party may cost more than it's worth, especially when collection many data points.

[cbaker6]

So maybe we should keep the analytics route?

My vote would be to keep as it's low maintenance from a Parse Server development perspective and isn't hurting anything. I think the usage of analytics on the server vs client side is a case-by-case situation for developers. For the apps my teams and collaborators build, we prefer the current server-side setup over the bloat (multiple dependencies) client side SDK's like Firebase provide to capture analytics. The beauty of Parse-Server is the flexibility it provides the developer for making such decisions and enables them to consider the tradeoffs of costs, requests, bloat, functionality, etc.

[mtrezza]

If we decide to keep it, I think we'd at least need to document it officially. In server docs and client SDK docs. Currently it appears like a half-way implemented feature artifact. We should probably also add the (only known?) Analytics adapter to our list of 3rd party adapters on our website.

[cbaker6]

The PR thread that added the Analytics Adapter configuration may provide some insight for documentation, parse-community/parse-server#2327. I currently don’t know of other adapters.

[azlekov]

Great discussion and most importantly I believe we are on the same page, so I propose this:

As I wrote somewhere above we could patch and for other versions of Android to leave the default old behaviour and for Android 12+ we will add support for Activities and we will document it, so if a developer wants to preserve the behaviour AND they are using analytics in some form, they should handle it by them selves in the Activity. No ParseActivity, no invisible activities. Who is using the analytics will must do additional small adjustments.

How this sounds to you @mtrezza, @bahaa-kallas?
If it sounds nice - @bahaa-kallas do you still want to take this? Now we will have a clean plan how to do it. I can help.

[mtrezza]

Sounds good