parse-server: Is cleanup invalid device tokens safe to use?

I’m back at this issue, preparing a PR to ensure the feature is fit for production.

---

Current process

APNS:

1. Recipient installations are grouped by appIdentifier, e.g. com.example.myapp https://github.com/parse-community/parse-server-push-adapter/blob/c9a1fc65bb61c71adaf4c79df18420c2c6546849/src/APNS.js#L83-L87

2. For each appIdentifier group the eligible providers are matched by topic https://github.com/parse-community/parse-server-push-adapter/blob/c9a1fc65bb61c71adaf4c79df18420c2c6546849/src/APNS.js#L92

3. Push notification is sent to all recipients through the first eligible provider https://github.com/parse-community/parse-server-push-adapter/blob/c9a1fc65bb61c71adaf4c79df18420c2c6546849/src/APNS.js#L115-L116

4. For failed push notifications, clear the error response and re-send through the next eligible provider. https://github.com/parse-community/parse-server-push-adapter/blob/c9a1fc65bb61c71adaf4c79df18420c2c6546849/src/APNS.js#L121-L124

5. The result is an array of succeeded and failed push results. A push that failed in step 3 but succeeded at step 4 will be contained only once as succeeded result. A push that failed at step 3 and 4 will be contained only once as failed result. So results are installation unique. https://github.com/parse-community/parse-server-push-adapter/blob/c9a1fc65bb61c71adaf4c79df18420c2c6546849/src/APNS.js#L110

• Result syntax for APNS: https://github.com/parse-community/parse-server-push-adapter/blob/c9a1fc65bb61c71adaf4c79df18420c2c6546849/src/APNS.js#L286-L311

---

FCM:

1. Recipient installations grouped into batches of 1,000 https://github.com/parse-community/parse-server-push-adapter/blob/02348635ab72c5e57f12a220decb96f3d6df6887/src/GCM.js#L35

2. Batches are sent; for FCM there is only 1 provider, so no retry logic like in APNS.

• Result syntax for FCM: https://github.com/parse-community/parse-server-push-adapter/blob/02348635ab72c5e57f12a220decb96f3d6df6887/src/GCM.js#L102-L111

---

Result handling in parse server:

1. Results are tracked https://github.com/parse-community/parse-server/blob/d83a0b680854f52ce30205d3558c0133d7f8cf19/src/Push/PushWorker.js#L98

2. Device tokens of failed results are added to devicesToRemove. For non-relevant errors the device token will not be added for removal, e.g. “temporarily unavailable” or connection error. https://github.com/parse-community/parse-server/blob/cf6e79ee75e55447b75e2c0b40d56b9c3cb578c1/src/StatusHandler.js#L255-L261

3. Tokens are deleted from installation https://github.com/parse-community/parse-server/blob/cf6e79ee75e55447b75e2c0b40d56b9c3cb578c1/src/StatusHandler.js#L288-L301

---

Issues

Bottom line, the potential issue of incorrectly cleaning up valid device tokens only exists for APNS. The following scenarios are possible:

a) Push to correct provider fails with some “temporarily unavailable”, and retry push to incorrect provider fails with “token invalid”.

b) Push to correct provider fails because certificate is expired, and retry push to incorrect provider fails with “token invalid”.

c) Push to incorrect provider fails because provider topic doesn’t match installation appIdentifier. This is currently impossible, but there is legacy code that should be removed: https://github.com/parse-community/parse-server-push-adapter/blob/02348635ab72c5e57f12a220decb96f3d6df6887/src/APNS.js#L238-L241 Also, a code comment should be added, why the match is required for future reference.

d) It is currently allowed to add APNS providers without topic. These wildcard providers are used if no provider was found where the topic matches the installation appIdentifier. This scenario does not require any specific consideration, because it is covered by the above scenarios. https://github.com/parse-community/parse-server-push-adapter/blob/02348635ab72c5e57f12a220decb96f3d6df6887/src/APNS.js#L250-L252

Just to be sure, I re-checked, whether the error codes relevant to removal of device token are correct, and yes, they are:

• https://firebase.google.com/docs/cloud-messaging/http-server-ref#error-codes
• https://developer.apple.com/library/archive/documentation/NetworkingInternet/Conceptual/RemoteNotificationsPG/CommunicatingwithAPNs.html#//apple_ref/doc/uid/TP40008194-CH11-SW15

---

Bottom bottom line, issues are only possible if: a) There is more than 1 APNS provider with the same topic, e.g. for prod / dev environments:

push: {
    ios: [
        {
            pfx: 'aps-pro.p12',
            topic: 'com.example.myapp',
            production: true
        },
        {
            pfx: 'aps-dev.p12',
            topic: 'com.example.myapp',
            production: false
        }
    ]
}

b) There is more than 1 APNS provider and at least one has no topic, e.g.:

push: {
    ios: [
        {
            pfx: 'aps-pro1.p12',
            topic: 'com.example.myapp',
            production: true
        },
        {
            pfx: 'aps-pro2.p12',
            production: true
        }
    ]
}

---

Solution

ad a) Disallow multiple APNS adapters with same topic.

• Apple offers a universal APNS certificate for sandbox and production environment, so multiple certificates are not required anymore and the production flag can be deprecated.

ad b) Disallow APNS adapter without topic.

• I don’t see any purpose for adapters without topic.

Both changes are breaking changes, but the effort required by the user is just to remove one adapter from config file if there are multiple and download the universal certificate.

Go for it! You have done the research on this topic. I’ll help as much as I can.

@cr0ybot As far as I’m aware that issue has not been touched since then. The behavior should still be the same. So you could use it, considering the caveats.

I’ve quickly revisited my previous comments here and it seems that if you use a universal APNS certificate for sandbox + production and thus have only one certificate entry in the push adapter configuration for APNS, there shouldn’t be any issue. I don’t see how we could solve the remaining issue, that can probably only be solved on the APNS side.

@mtrezza Should we encourage users to use a single key instead of certificates?

This is an added benefit that they don’t expire so you don’t have to renew them.

ios: [
      {
        token: {
          key: './push/AuthKey_XYZ.p8',
          keyId: "ABC",
          teamId: "EFG"
        },
        topic: 'com.example',
      },
]