Is cleanup invalid device tokens safe to use?

[mtrezza]

Issue Description

Parse Server v2.6.2 introduced a feature to clean up invalid device tokens. There have been issues reported where valid device tokens have been removed, some of which have been closed without conclusion, notably:

• Clean up invalid deviceTokens - deleting valid ones as well? #4500
• Installation deviceToken is undefined  Parse-SDK-iOS-OSX#1224 (comment)

The root cause for these issues could be parse-community/parse-server-push-adapter#87 (comment), but because the issues have not been further investigated it's hard to tell.

The feature is still flagged and not in the docs, which makes me think there are still doubts about its functionality.

Is PARSE_SERVER_CLEANUP_INVALID_INSTALLATIONS safe to use?
Is anyone using the feature in a production environment and can give feedback?

[mtrezza]

@mrmarcsmith It seems you have been digging into the push topic with other issues you've opened. Do you have any feedback on PARSE_SERVER_CLEANUP_INVALID_INSTALLATIONS in a production environment by any chance?

[mrmarcsmith]

@mtrezza I haven’t looked into this issue yet but I’ll see if I have time to dive in while I still have the push system fresh in my mind. Off the top of my head I would imagine the first step would be to write a failing test which would involve mocking the APNS, which would involve bouncing some different types of requests off of the APNS Servers to see how they respond. From the issue you posted I’m thinking one solution could be to keep track of the types of error responses for the different types of certificates and only remove deviceTokens where error responses from ALL certificates is “Bad Device Token” that way if one returns a temporary error we don’t remove it. What do you think?

[mtrezza]

I think the basic issue is mocking the APNS, it is a black box, except for what is documented anyway. Therefore I'm looking to gather some feedback on production usage in this thread.

I think rather than testing against all certificates I would put forth the motion to deprecate specifying multiple Apple certificates. Apple offers a single push certificate for sandbox and production just like Android, so it would make sense to reflect that in the adapter.

https://github.com/parse-community/parse-server-push-adapter/blob/65a7f15651dd1dfa2ea9ea13b19a68f8e3e199f6/src/APNS.js#L31-L39

[mrmarcsmith]

In a couple of our apps we use two different app identifiers, one for our store version and one for our enterprise version (com.bla.app1-Store & com.bla.app1-Enterprise) therefore we need two production push certificates for one app, but the appIdentifer is stored in the installation so we would could just ensure that parse doesn’t retry on an installation that doesn’t match its appIdentifer. What about this solution?
We Put a warning at start up if they have the PARSE_SERVER_CLEANUP_INVALID_INSTALLATIONS set AND they have more than one certificate with the same identifier that says “Warning: having multiple push certificates with the same identifier is incompatible with the PARSE_SERVER_CLEANUP_INVALID_INSTALLATIONS flag. Your pushes will still send but old installations will not be removed. Use Apple’s new combined sandbox and production certificate to re-enable removing of invalid installations.“

[mrmarcsmith]

And of course in addition to the warning, we also actually disable the cleanup if they have multiple certs with the same app identifiers

[mtrezza]

@mrmarcsmith Interesting, does parse server officially support multiple apps in 1 deployment?

How to you make sure the proper push certificate is used for the proper app ID?

[mrmarcsmith]

@mtrezza to clarify, these are not two separate “apps”. Both have identical “Parse Server ApplicationIds”, identical iOS Client code and use the same cloud code. The only difference is how they are distributed and how apple treats them.

1. The first type of deployment is the most common, deployment through the apple App Store. This is for our customers
2. The second type of deployment uses an “enterprise” certificate which is for enterprise organizations to distribute in house apps that by pass App Store approval. This version is for the contractors that work with our customers through the app.

• Apple requires store and enterprise versions to have different “bundleIdentifiers”
• Apple only allows one bundleId per Push Certificate
• Therefore, Apple requires different push certificates for our two distributions of the same app

The configuration for Parse APNS here requires a “bundleId” to be provided with the certificate. And the “bundleId” field will match the corresponding “parse-default” field “appIdentifier” in the _Installation class.

In theory, Parse should NEVER attempt sending a enterprise installation a push using the Store certificate (and visa-versa) because the bundleId’s don’t match. Which is why it sould be safe to allow multiple certificate to co-exist as long as the have different bundleId’s

[mtrezza]

Correct, as long as you specify a bundleId (deprecated) or topic for the adapter it will match the adapter to the appIdentifier. So removing the ability to pass multiple certificates is not ideal.

https://github.com/parse-community/parse-server-push-adapter/blob/65a7f15651dd1dfa2ea9ea13b19a68f8e3e199f6/src/APNS.js#L237

So how about adding your waning and deprecate the production/development flag in the adapter and issue an additional deprecation warning?

Production and development environments are usually separated anyway, so I don’t see a good reason to keep this entanglement.

[mrmarcsmith]

In the case that they want to connect to the development APNS server isn’t it a different apple URL? That’s just my initial hunch on why we asked for the production flag in the config, I’m running out the door so I don’t have time to dig into the adapter code to confirm

[mtrezza]

It is a different URL, but the production flag only defines which certificate to try first, not the URL.

If production is true the push is first sent with the production certificate and if that fails, sent again with the development certificate, and vice-versa.

• https://github.com/parse-community/parse-server-push-adapter/blob/65a7f15651dd1dfa2ea9ea13b19a68f8e3e199f6/src/APNS.js#L158-L163

The actual URL is set in node-apn depending on the node environment:

• https://github.com/node-apn/node-apn/blob/7af166ff47a11bc49bbcb2d1b827685373bdd6c8/lib/config.js#L24
• https://github.com/node-apn/node-apn/blob/7af166ff47a11bc49bbcb2d1b827685373bdd6c8/lib/config.js#L92-L100

With the "universal" Apple push certificate the production flag seems to become irrelevant and can be deprecated.

I am still unsure of how invalid device tokens are determined. When adding multiple certificates without topic, the push adapter will try all certs until a successful push. Let's say we have 2 certs, the first invalid, the second valid, which results in a failed and then a successful push. Will the device token be added to devicesToRemove?

parse-server/src/StatusHandler.js

Line 260 in cf6e79e

devicesToRemove.push(token);

[mrmarcsmith]

DeviceRemoval happens if APNS replies with “invalid deviceToken” error. here
I believe this is the bug:
In the case that the production certificate request fails for any reason even though the deviceToken is valid, when it gets sent to the development APNS server OR to the prod server with the wrong and same topic (or no topic) it would returns “invalid deviceToken” error and therefore removes it.

[mtrezza]

That is how it works in principle, but I couldn’t figure out yet if each result that is evaluated is

1. a APNS response (n results for a push that required n APNS requests) or
2. a summarized push adapter response (e.g. 1 result for a push that required n APNS requests)

In case of 1) a device token would be incorrectly removed if it fails with any tried certificate.

In case of 2) a device token would correctly be removed if it fails with all tried certificates.

[dplewis]

@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',
      },
]

[mtrezza]

@dplewis That's a fair suggestion. Apple says:

One key is used for all of your apps.

@mrmarcsmith Would that also cover your use case of multiple app IDs?