parse-community / parse-server-push-adapter

A push notification adapter for Parse Server
https://parseplatform.org
MIT License
87 stars 99 forks source link

Android device still receives push from old installationId after app re-installed #40

Open macmoe opened 7 years ago

macmoe commented 7 years ago

I'm seeing the same issue that @jiawenzhang had in https://github.com/ParsePlatform/parse-server/issues/1705

Here is the issue:

Note that the app on this phone is now associated with installationId_2 (as user B). The installationId_1 should not be valid.

The parse-server is showing the following in the log:

verb parse-server-push-adapter GCM sending to 1 devices verb parse-server-push-adapter GCM GCM Response: { verb parse-server-push-adapter GCM "multicast_id": 7047466558839750000, verb parse-server-push-adapter GCM "success": 1, verb parse-server-push-adapter GCM "failure": 0, verb parse-server-push-adapter GCM "canonical_ids": 1, verb parse-server-push-adapter GCM "results": [ verb parse-server-push-adapter GCM { verb parse-server-push-adapter GCM "registration_id": "APA91bEB01Bmhz3g0ZMRqBJIwg-DbQzNnvYeZxJKoj0ixVYFBW9UN2zQIMiDV86FWDDqHlJInaA9h_Mb-xV3tfCUNk2mpUdAjeYi7AdM6ggImQbATHKEp24Xg7yyS76nNLH1UDXqiMxP", verb parse-server-push-adapter GCM "message_id": "0:1475256512754990%d7f644cef9fd7ecd" verb parse-server-push-adapter GCM } verb parse-server-push-adapter GCM ] verb parse-server-push-adapter GCM }

According to GCM, the app-server (parse-server) should be updated to use the registration_id that was returned: https://developers.google.com/cloud-messaging/registration#how-uninstalled-client-app-unregistration-works

The problem is I don't want any notifications to be successfully sent with the installationId_1.

If I try to duplicate this same issue with the production api.parse.com and SDK, I can't duplicate it.

NOTE: GCM states "Eventually, the registration token (installationId_1) will be removed and the server will get a NotRegistered error" - Does anyone know how long this would take?

GongChen1107 commented 7 years ago

I have this problem too.

learner88 commented 7 years ago

I am also facing the same issue. Targeting older installations also delivers notification which shouldn't be the case.

flovilmart commented 7 years ago

The installationId is generated when the user installs the app. Upon app update, it is reused. When the user delete the app, there is no known mechanism to persist that value across and reuse it if the user decides to delete the app.

As the documentation mentions, at one point, the old token will be marked invalid by GCM.

I am not sure if there is any way around that, to invalidate the token as soon as the app is deleted.

olidroide commented 7 years ago

Hi @flovilmart I have the same problem too.

Maybe the problem is at the start/init a new parse installation from device. The Parse server can check the new installation and force to remove invalid token... I notice it happens only with my parse server machine and not with Parse.com server machine (old one).

Any thoughs?

flovilmart commented 7 years ago

remove invalid token...

How do you suggest we remove the invalid token? Both installations are independant i believe, or is the token the same?

olidroide commented 7 years ago

I believe the token registered in GCM is the same for different Installations in Parse. I don't know exactly if the problem comes from Android SDK Lib or from Parse Server or combination of both.

The user @inlined has a open discussion to improve the Android SDK with FCM, and I think this adapter must manage the new FCM system (GCM v4). What is the time flow to merge his proposals?

My skill is Android based, and my feeling of this issue it happen to me after migrate from Parse.com to my own parse server, for that I update the Android libs and add the GCM tokens... Maybe some part of code Parse server is missing or some merge in Android Lib cause this problem... I found several issues with the same problem:

http://stackoverflow.com/questions/28050986/how-to-avoid-installation-duplicates-using-parse/33306713#33306713 http://stackoverflow.com/a/27259926/481637 http://stackoverflow.com/questions/27863217/parse-push-receiving-pushes-from-previous-installation https://github.com/parse-server-modules/parse-server-push-adapter/issues/40

But any of them works on current state. I don't know how to deal with this issue.

posativ commented 7 years ago

Same here. Uninstalled and re-installed an application multiple times in a few days. Now I receive 3 push notifications for a single push.

jeacott1 commented 7 years ago

@olidroide, the token registered is not always the same unfortunately. recent testing with a samsung s6 in particular always yields different tokens and installation ids on every reinstall.

I'd like to know if anyone has any strategies at all for responding to apns bad token exceptions when parse-server supports multiple certificates, and dev/prod config. how do you reliably know when a token is actually bad, and not just that the wrong cert is being tried?

flovilmart commented 7 years ago

The only way to know with the non HTTP/2 APNS API is the apns feedback API which is not implemented at the time being.

jeacott1 commented 7 years ago

even with the http/2 api, if you have multiple certs loaded, or switch from dev->prod, wont the api report bad tokens because the particular cert/dev/prod setting combination doesnt match the current device?

flovilmart commented 7 years ago

To be determined (i.e., didn't test myself) but it seems that one could make the difference give the response code with the HTTP/2 interface https://developer.apple.com/library/content/documentation/NetworkingInternet/Conceptual/RemoteNotificationsPG/CommunicatingwithAPNs.html

(Bad Request 400 vs inactive 410)

inlined commented 7 years ago

For Apple, you can also eventually find out from the normal TCP connection because Apple eventually hangs up on you. I wouldn't recommend actually implementing this. Parse used to support device token cleanup in the pre-beta days but it was turned off because deviceToken is an immutable field in Installation. You'd only be able to delete the token by deleting the whole row. If the device only turned push off rather than deleting the whole installation then you'll run into issues when the device only does delta-uploads for its existing state. It's going to do a PUT but the record won't exist anymore and ParseServer will return an error. I don't recall whether an error like that could still cause PFInstallation.saveEventually to break the entire networking queue.

For GCM that should have been fixed (according to @bnham it had been in the original server). If/when Parse updates to FCM (or even GCM v4) that's implemented in a way that makes client-only revocation easy.

olidroide commented 7 years ago

thanks @jeacott1 what do you think about the repeated push issue? Are you agree with @inlined this problem will solved when Parse update to FCM (or GCM v4)?

@flovilmart do you have a deadline or date when this update happen?

Thanks everybody.

flovilmart commented 7 years ago

As @inlined mentions, deleting the deviceToken from the server may be problematic, also, we will haven't integrated the HTTP/2 APN endpoints. On Apple devices, that push will simply be marked as non sent.

bnham commented 7 years ago

With the old parse.com service, we handled this in two different ways:

  1. On the server side, when GCM server told our push servver that the canonical registration ID of a device changed (by setting the registration_id field in the response), we updated the appropriate record in the ParseInstallation table.
  2. On the client side, all pushes contained a push_id parameter. The client persisted these push_ids to disk and used that persisted state to de-duplicate pushes.

You need the client-side deduplication even if you implement (1) because a user can uninstall an app and then reinstall it. For a brief period of time, the two installations will both be associated with valid GCM reg ids that both target the reinstalled app. After some amount of time, the GCM server will invalidate the reg id associated with the old installation of the app.

You will probably want to implement a combination of these approaches in the open-source server.

flovilmart commented 7 years ago

all pushes contained a push_id parameter

This is not implemented in the push controller, we should probably add that ;)

manishsangwan commented 7 years ago

Any solution to this yet?

Meersad commented 6 years ago

For our testing this duplicate push started after 2.2.25, we strongly think it is when the new pushadapter 2.0 started to get used. What happens is if i send 1 push to an android with duplicated installations the pushId have different generated numbers. The parse class on Android SDK that decouples does not remove the duplicate since this pushId is different(this did work on 2.2.25 beck here it still received 2 pushes but only show one).

https://github.com/parse-server-modules/parse-server-push-adapter/blob/master/src/GCM.js#L29 Seams to run more the once for the same push somehow. So something from serverside is generating a new pushId for the same push and needs to be fixed.

clieber commented 6 years ago

I am having this same problem after upgrading from 2.2.25 to 2.3.3. Android device only received one push on 2.2.25 even if device had multiple (old) installation records because user uninstalled and reinstalled android app. But, now on 2.3.3 a push is delivered to the device for every installation record, even old records that are no longer valid.

Is there any solution for this? Thanks

pargu commented 6 years ago

Hi Clieber, We hit this issue also when going from 2.2.25 -> 2.6.3, but found a workaround for this

Cloud Code: parse_push.js

function randomString(size) { var randomId = ""; var possible = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789";

for (var i = 0; i < size; i++) {
    randomId += possible.charAt(Math.floor(Math.random() * possible.length));
}

return randomId;

}

Parse.Cloud.define('pushFunction', function(request, response) { var query = new Parse.Query(Parse.Installation);

var payload = request.params.push_data; //our data from client
payload["key_for_random_push"] = randomString(8); //generate a custom random id

Parse.Push.send({ where: query, data: payload }, { useMasterKey: true, success: function(list) { response.success("true"); }, error: function(error) { response.error(JSON.stringify(error)); } });

}

For the push-receiver in Android Studio Project we saved a String-Set in SharedPreference and kept a size of 10 push-ids. When a new push came in we check if set already contained the new id and then pushed out the oldest id out of the set, and inserted the new push id to it. Which size you want to keep in the set is up to you. We did 10.

ParsePushReceiver.java

@Override protected Notification getNotification(Context context, Intent intent) {

String jsonData = extras.getString("com.parse.Data");

    if (jsonData != null) {
        try {
            JSONObject data = new JSONObject(jsonData);

            if (data.has("key_for_random_push")) {

                duplicate = checkDuplicate(context, data.getString("key_for_random_push"));

            }

            if (duplicate) {
                //Timber.v("DUPLICATE CHECK IS DUPLICATE RETURN NULL");
                return null;
            }

... // Continue with push setup

}

private boolean checkDuplicate(Context context, String uniquePushId) {

    SharedPreferences sharedPrefs = context.getSharedPreferences(PREFS_ID, Context.MODE_PRIVATE);

    if (sharedPrefs != null) {

        if (sharedPrefs.contains(UNIQUE_PUSH_IDS)) {

            Set<String> uniquePushIds = sharedPrefs.getStringSet(UNIQUE_PUSH_IDS, new HashSet<String>());
            ArrayList<String> uniqueIdsArr = new ArrayList<>();

            if (uniquePushIds.size() > 0) {
                uniqueIdsArr.addAll(uniquePushIds);
            }

            if (uniqueIdsArr.size() > 0) {

                if (uniqueIdsArr.contains(uniquePushId)) {

                    //Timber.v("DUPLICATE CHECK FOUND DUPLICATE");
                    return true;

                } else {

                    //Timber.v("DUPLICATE CHECK NOT DUPLICATE");

                    if (uniqueIdsArr.size() >= PUSH_IDS_MAX_SIZE) {
                        uniqueIdsArr.remove(0);
                        //uniqueIdsArr.clear();
                        uniqueIdsArr.add(uniquePushId);
                    } else {
                        uniqueIdsArr.add(uniquePushId);
                    }

                    //Timber.v("DUPLICATE CHECK ARR IS: %s", uniqueIdsArr);

                    sharedPrefs.edit().putStringSet(UNIQUE_PUSH_IDS, new HashSet<>(uniqueIdsArr)).apply();

                    return false;
                }

            } else {

                //Timber.v("DUPLICATE CHECK ARRAY SIZE == 0");

                Set<String> localUniquePushIds = new HashSet<>();
                localUniquePushIds.add(uniquePushId);
                sharedPrefs.edit().putStringSet(UNIQUE_PUSH_IDS, localUniquePushIds).apply();

                return false;

            }

        } else {

            //Timber.v("DUPLICATE CHECK ARRAY DOES NOT EXISTS");

            Set<String> localUniquePushIds = new HashSet<>();
            localUniquePushIds.add(uniquePushId);
            sharedPrefs.edit().putStringSet(UNIQUE_PUSH_IDS, localUniquePushIds).apply();

            return false;

        }

    } else {
        return false;
    }

}

Sorry for the formatting. Can't seem to get it right... But this is a solution working for us. Haven't had any reports of duplicate pushes after implementing it.

clieber commented 6 years ago

@pargu - thank you for your response.

hbreumelhof commented 6 years ago

Has any official fix for this been implemented since @pargu developed his workaround?