Skip to content
This repository has been archived by the owner on Jan 18, 2024. It is now read-only.

Commit

Permalink
fix(xdl): do not override google-services.json contents since SDK37 (#…
Browse files Browse the repository at this point in the history
…1897)

# Why

Following [Using FCM doc](https://docs.expo.io/versions/latest/guides/using-fcm/) or [GoogleSignIn - Usage with Firebase part](https://docs.expo.io/versions/latest/sdk/google-sign-in/#usage-with-firebase) would not render the expected results — `googleSignIn` configuration was always being applied onto `google-services.json`, even if a custom one has been provided by the user. Most probably this is not how this should work or what the user expect.

When pushed to Turtle builders, should fix expo/expo#7727.

# How

Depending on the SDK version of the built project:
- if the SDK is >= 37:
  - only change `google-services.json` if none was provided (so we're operating on a placeholder one)
  - if both configuration settings are provided print a warning and not modify the custom `google-services.json`
- if the SDK is < 37:
  - if the user provides a custom `googleServicesFile`, print a warning that its contents are about to be modified,
  - always modify `google-services.json`, as it has been working before.

I have also moved the `replace "host.exp.exponent" with ${javaPackage}` to where the logic above is mentioned so that all `google-services.json` modifications are in one place.

# Test plan

- SDK37, only `googleServicesFile` — [job](https://staging.expo.io/dashboard/sjchmiela/builds/4dca157f-3ae5-4739-96a7-2a1fbcf2ec0e), no warning, used `google-services.json`
- SDK37, only `googleSignIn` — [job](https://staging.expo.io/dashboard/sjchmiela/builds/2c44d75e-9ce1-4098-ab64-04e76f01efda), no warning, uses placeholder project ID and `googleSignIn` key
- SDK37, both provided — [job](https://staging.expo.io/dashboard/sjchmiela/builds/23626002-acf2-4a30-a620-2062f134d671), warning present (google-services.json overrides others), used `google-services.json`
- SDK36, only `googleServicesFile` — ~[job](https://staging.expo.io/dashboard/sjchmiela/builds/8366c022-dcff-4047-bf0a-0264a52870a1), no warning, **removed key from google-services.json**~ force-pushed version where the warning should be printed and the key still removed
- SDK36, only `googleSignIn` — [job](https://staging.expo.io/dashboard/sjchmiela/builds/d59e4334-a815-4d6a-8ba2-108ab2f5d8bc), no warning, used `googleSignIn` and leaking project ID
- SDK36, both provided — [job](https://staging.expo.io/dashboard/sjchmiela/builds/d5a8ab8f-07f7-4aac-9c64-7012421d1b49), warning present (google-services.json overridden), google-services.json overridden
  • Loading branch information
sjchmiela committed Apr 17, 2020
1 parent 548ac8b commit 00ee89b
Showing 1 changed file with 83 additions and 19 deletions.
102 changes: 83 additions & 19 deletions packages/xdl/src/detach/AndroidShellApp.js
Expand Up @@ -546,13 +546,6 @@ export async function runShellAppModificationsAsync(context, sdkVersion, buildMo
path.join(shellPath, 'app', 'build.gradle')
);

// Push notifications
await regexFileAsync(
'"package_name": "host.exp.exponent"',
`"package_name": "${javaPackage}"`,
path.join(shellPath, 'app', 'google-services.json')
); // TODO: actually use the correct file

// TODO: probably don't need this in both places
await regexFileAsync(
/host\.exp\.exponent\.permission\.C2D_MESSAGE/g,
Expand Down Expand Up @@ -1074,6 +1067,89 @@ export async function runShellAppModificationsAsync(context, sdkVersion, buildMo
);
}

// Configure google-services.json
// It is either already in the shell app (placeholder or real one) or it has been added
// from `manifest.android.googleServicesFile`.
if (parseSdkMajorVersion(sdkVersion) >= 37) {
// New behavior - googleServicesFile overrides googleSignIn configuration
if (!manifest.android || !manifest.android.googleServicesFile) {
// No google-services.json file, let's use the placeholder one
// and insert keys manually ("the old way").

// This seems like something we can't really get away without:
//
// 1. A project with google-services plugin won't build without
// a google-services.json file (the plugin makes sure to fail the build).
// 2. Adding the google-services plugin conditionally and properly (to be able
// to build a project without that file) is too much hassle.
//
// So, let's use `host.exp.exponent` as a placeholder for the project's
// javaPackage. In custom shell apps there shouldn't ever be "host.exp.exponent"
// so this shouldn't cause any problems for advanced users.
await regexFileAsync(
'"package_name": "host.exp.exponent"',
`"package_name": "${javaPackage}"`,
path.join(shellPath, 'app', 'google-services.json')
);

// Let's not modify values of the original google-services.json file
// if they haven't been provided (in which case, googleAndroidApiKey
// and certificateHash will be an empty string). This will make sure
// we don't modify shell app's google-services needlessly.

// Google sign in
if (googleAndroidApiKey) {
await regexFileAsync(
/"current_key": "(.*?)"/,
`"current_key": "${googleAndroidApiKey}"`,
path.join(shellPath, 'app', 'google-services.json')
);
}
if (certificateHash) {
await regexFileAsync(
/"certificate_hash": "(.*?)"/,
`"certificate_hash": "${certificateHash}"`,
path.join(shellPath, 'app', 'google-services.json')
);
}
} else if (googleAndroidApiKey || certificateHash) {
// Both googleServicesFile and googleSignIn configuration have been provided.
// Let's print a helpful warning and not modify google-services.json.
fnLogger.warn(
'You have provided values for both `googleServicesFile` and `googleSignIn` in your `app.json`. Since SDK37 `googleServicesFile` overrides any other `google-services.json`-related configuration. Recommended way to fix this warning is to remove `googleSignIn` configuration from your `app.json` in favor of using only `googleServicesFile` to configure Google services.'
);
}
} else {
// Old behavior - googleSignIn overrides googleServicesFile

if (manifest.android && manifest.android.googleServicesFile) {
// googleServicesFile provided, let's warn the user that its contents
// are about to be modified.
fnLogger.warn(
'You have provided a custom `googleServicesFile` in your `app.json`. In projects below SDK37 `googleServicesFile` contents will be overridden with `googleSignIn` configuration. (Even if there is none, in which case the API key from `google-services.json` will be removed.) To mitigate this behavior upgrade to SDK37 or check out https://github.com/expo/expo/issues/7727#issuecomment-611544439 for a workaround.'
);
}

// Push notifications
await regexFileAsync(
'"package_name": "host.exp.exponent"',
`"package_name": "${javaPackage}"`,
path.join(shellPath, 'app', 'google-services.json')
);

// Google sign in
await regexFileAsync(
/"current_key": "(.*?)"/,
`"current_key": "${googleAndroidApiKey}"`,
path.join(shellPath, 'app', 'google-services.json')
);
await regexFileAsync(
/"certificate_hash": "(.*?)"/,
`"certificate_hash": "${certificateHash}"`,
path.join(shellPath, 'app', 'google-services.json')
);
}

// Set manifest url for debug mode
if (buildMode === 'debug') {
await regexFileAsync(
Expand All @@ -1094,18 +1170,6 @@ export async function runShellAppModificationsAsync(context, sdkVersion, buildMo
);
}

// Google sign in
await regexFileAsync(
/"current_key": "(.*?)"/,
`"current_key": "${googleAndroidApiKey}"`,
path.join(shellPath, 'app', 'google-services.json')
);
await regexFileAsync(
/"certificate_hash": "(.*?)"/,
`"certificate_hash": "${certificateHash}"`,
path.join(shellPath, 'app', 'google-services.json')
);

// Facebook configuration

// There's no such pattern to replace in shell apps below SDK 36,
Expand Down

0 comments on commit 00ee89b

Please sign in to comment.