Skip to content

Conversation

sjchmiela
Copy link
Contributor

@sjchmiela sjchmiela commented Apr 6, 2020

Why

It seems that the currently used GOOGLE_CLIENT_API_KEY key is… not present in any of the GCP projects we have. This may be the reason for why the Firebase Installations API always returns a 400 status code for registration request and why Expo clients cannot fetch the FCM token.

Fixes #7673 and #7606.

How

For both:

I have

  1. verified there is no such key as the one we seem to be using (AIzaSyCJ…) in neither exponentjs dashboard nor exponent-5dd6d
  2. I went to Firebase Installations API dashboard of exponent-5dd6d to see the following graph consisting only of 4xx responses:
    Zrzut ekranu 2020-04-6 o 13 47 46
  3. facing this information I have created new keys in the exponent-5dd6d project:
    • an Android Maps key restricted to host.exp.exponent and Maps API
    • an Android key restricted to host.exp.exponent and Cloud Messaging + Installations API
    • an iOS key restricted to host.exp.Exponent and Maps API
  4. knowing that the Google Maps Android key is not in fact used to access Cloud Messaging or Firebase Installations APIs I have removed those two from among the list of allowed services for this key.

Test Plan

To test for the presence of this problem properly, Expo Client must be first removed from the device, otherwise the cache of tokens we keep may affect the results of the experiment.

After installing Expo client app I opened an SDK37 experience that does not provide its own google-services.json, which requests the Expo push token. The Promise resolved successfully with an Expo push token (which allows to send notifications successfully).

I have also run the Maps screen from NCL on both Expo clients and both showed the map.

I have also confirmed that running the Maps screen shows some usage of the keys — good.

@ide
Copy link
Member

ide commented Apr 6, 2020

Do we have two keys, then — one for Maps and another for Installations and FCM?

Perhaps we should switch Maps to use the same key as the Firebase services and deprecate the old Maps key (leave it running for older Play Store clients for now).

@sjchmiela
Copy link
Contributor Author

Do we have two keys, then — one for Maps and another for Installations and FCM?

That is correct.

The Maps key is injected with AndroidManifest.xml com.google.android.geo.API_KEY meta-data, the Installations and FCM key is bundled inside google-services.json.

Perhaps we should switch Maps to use the same key as the Firebase services and deprecate the old Maps key (leave it running for older Play Store clients for now).

Either this way or the other (use the same key for Firebase services as we do for Maps; however, this would probably require some changes on backend (?)). It may depend on what are the reasons for two separate GCP projects (legacy?).

It may also depend on what are the restrictions we want to put on the keys — should the FCM token be allowed to be used in both host.exp.exponent apps and in com.sjchmiela.myapp (in order to be able to send push notifications to a standalone app)? What about the maps key (I think in the past we have allowed using our maps key in standalone apps, but no more)?

@ide
Copy link
Member

ide commented Apr 6, 2020

It may depend on what are the reasons for two separate GCP projects (legacy?).

I don't think there was an intentional reason for the separate GCP projects. Keeping all the keys in one place would be better than separating them I think.

should the FCM token be allowed to be used in both host.exp.exponent apps and in com.sjchmiela.myapp?

No. Every app (except for the Play Store development client) must provide its own FCM credentials.

What about the maps key (I think in the past we have allowed using our maps key in standalone apps, but no more)?

Several months ago we updated the Maps key and restricted it so only the Play Store development client can use it. In general, the creator of each app must provide their own keys. (We are the creator of the Play Store development client, so we need to provide the keys for just that app.)

@sjchmiela sjchmiela force-pushed the @sjchmiela/fix-android-google-api-token branch from 30a1268 to 42e4391 Compare April 7, 2020 12:19
@sjchmiela sjchmiela merged commit b404134 into master Apr 7, 2020
@sjchmiela sjchmiela deleted the @sjchmiela/fix-android-google-api-token branch April 7, 2020 12:24
sjchmiela added a commit that referenced this pull request Apr 7, 2020
…ject (#7688)

# Why

It seems that the currently used `GOOGLE_CLIENT_API_KEY` key is… not present in any of the GCP projects we have. This may be the reason for why the Firebase Installations API always returns a 400 status code for registration request and why Expo clients cannot fetch the FCM token.

Fixes #7673 and #7606.

# How

For both:
- https://console.cloud.google.com/apis/credentials?project=exponent-5dd6d
- https://console.cloud.google.com/apis/credentials?project=exponentjs

I have

1. verified there is no such key as the one we seem to be using (`AIzaSyCJ…`) in neither [`exponentjs`](https://console.cloud.google.com/apis/credentials?project=exponentjs) dashboard nor [`exponent-5dd6d`](https://console.cloud.google.com/apis/credentials?project=exponent-5dd6d)
2. I went to [_Firebase Installations API_ dashboard of `exponent-5dd6d`](https://console.cloud.google.com/apis/api/firebaseinstallations.googleapis.com/overview?project=exponent-5dd6d&authuser=1) to see the following graph consisting only of 4xx responses:
  ![Zrzut ekranu 2020-04-6 o 13 47 46](https://user-images.githubusercontent.com/1151041/78555391-35c17400-780d-11ea-8665-7622b4d2c886.png)
3. facing this information I have created new keys in the `exponent-5dd6d` project:
    - an Android Maps key restricted to `host.exp.exponent` and Maps API
    - an Android key restricted to `host.exp.exponent` and Cloud Messaging + Installations API
    - an iOS key restricted to `host.exp.Exponent` and Maps API
4. knowing that the Google Maps Android key is not in fact used to access Cloud Messaging or Firebase Installations APIs I have removed those two from among the list of allowed services for this key.

# Test Plan

To test for the presence of this problem properly, Expo Client must be first removed from the device, otherwise the cache of tokens we keep may affect the results of the experiment.

After installing Expo client app I opened an SDK37 experience that does not provide its own `google-services.json`, which requests the Expo push token. The Promise resolved successfully with an Expo push token (which allows to send notifications successfully).

I have also run the Maps screen from NCL on both Expo clients and both showed the map.

I have also confirmed that running the Maps screen shows some usage of the keys — good.
sjchmiela added a commit that referenced this pull request Apr 8, 2020
# Why

Fixes #7653.

# How

On clean install, when only running an SDK37 experience, `getOrCreateUUID` is never called on `ExponentSharedPreferences` before we can call `getExponentPushTokenAsync`, so `getUUID` may return `null` and we should not fail in this scenario.

`getOrCreateUUID` may be called in two places in the project:
- in `ExponentNotificationIntentService#onHandleIntent` — when we receive a new token (which we may not receive!),
- in `ConstantsBinding#getConstants` — it is called in versioned code for SDKs below 37.
  For SDK37 it has been removed ([#6906](https://github.com/expo/expo/pull/6906/files#diff-df59c97cff05c0505acbbc763cbbe0b0)) in favor of `getOrCreateInstallationId` in `expo-constants/ConstantsService` which in general works the same, but it doesn't share the `SharedPreferences` instance, as the `Context` used in the module is a `ScopedContext`. Thus, even though we use the same `PREFERENCES_FILE_NAME` and `UUID_KEY`, the preferences will get scoped, rendering two different values `mExponentSharedPreferences.getUUID() != ConstantsService#getOrCreateInstallationId()`

```java
// This should have been set by ExponentNotificationIntentService when Activity was created/resumed.
```
is an outdated comment, we no longer explicitly start any of the subclasses of `ExponentNotificationIntentService` if Firebase Cloud Messaging is enabled, which it is in Expo client, we depend on the FCM `onNewToken` mechanism.

# Test Plan

I have confirmed with changes in #7688 that `getOrCreateUUID` is called _before everything else_ if the Firebase configuration is valid.
sjchmiela added a commit that referenced this pull request Apr 8, 2020
# Why

Fixes #7653.

# How

On clean install, when only running an SDK37 experience, `getOrCreateUUID` is never called on `ExponentSharedPreferences` before we can call `getExponentPushTokenAsync`, so `getUUID` may return `null` and we should not fail in this scenario.

`getOrCreateUUID` may be called in two places in the project:
- in `ExponentNotificationIntentService#onHandleIntent` — when we receive a new token (which we may not receive!),
- in `ConstantsBinding#getConstants` — it is called in versioned code for SDKs below 37.
  For SDK37 it has been removed ([#6906](https://github.com/expo/expo/pull/6906/files#diff-df59c97cff05c0505acbbc763cbbe0b0)) in favor of `getOrCreateInstallationId` in `expo-constants/ConstantsService` which in general works the same, but it doesn't share the `SharedPreferences` instance, as the `Context` used in the module is a `ScopedContext`. Thus, even though we use the same `PREFERENCES_FILE_NAME` and `UUID_KEY`, the preferences will get scoped, rendering two different values `mExponentSharedPreferences.getUUID() != ConstantsService#getOrCreateInstallationId()`

```java
// This should have been set by ExponentNotificationIntentService when Activity was created/resumed.
```
is an outdated comment, we no longer explicitly start any of the subclasses of `ExponentNotificationIntentService` if Firebase Cloud Messaging is enabled, which it is in Expo client, we depend on the FCM `onNewToken` mechanism.

# Test Plan

I have confirmed with changes in #7688 that `getOrCreateUUID` is called _before everything else_ if the Firebase configuration is valid.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

getExpoPushTokenAsync hangs on Android device (Expo Client)
2 participants