Skip to content

Send the FIS auth token in the RC fetch headers. #1646

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Jun 23, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ import org.robolectric.RuntimeEnvironment
import org.mockito.Mockito.mock
import org.mockito.Mockito.`when`

const val APP_ID = "APP_ID"
const val API_KEY = "API_KEY"
const val APP_ID = "1:14368190084:android:09cb977358c6f241"
const val API_KEY = "AIzaSyabcdefghijklmnopqrstuvwxyz1234567"

const val EXISTING_APP = "existing"

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,9 @@

@RunWith(AndroidJUnit4.class)
public class FirebaseRemoteConfigIntegrationTest {
private static final String API_KEY = "api_key";
private static final String API_KEY = "AIzaSyabcdefghijklmnopqrstuvwxyz1234567";
private static final String APP_ID = "1:14368190084:android:09cb977358c6f241";
private static final String PROJECT_ID = "fake-frc-test-id";

@Mock private ConfigCacheClient mockFetchedCache;
@Mock private ConfigCacheClient mockActivatedCache;
Expand Down Expand Up @@ -84,7 +85,11 @@ public void setUp() {
FirebaseApp firebaseApp =
FirebaseApp.initializeApp(
context,
new FirebaseOptions.Builder().setApiKey(API_KEY).setApplicationId(APP_ID).build());
new FirebaseOptions.Builder()
.setApiKey(API_KEY)
.setApplicationId(APP_ID)
.setProjectId(PROJECT_ID)
.build());

// Catch all to avoid NPEs (the getters should never return null).
when(mockFetchedCache.get()).thenReturn(Tasks.forResult(null));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ public final class RemoteConfigConstants {
/**
* Keys of fields in the Fetch request body that the client sends to the Firebase Remote Config
* server.
*
* <p>{@code INSTANCE_ID} and {@code INSTANCE_ID_TOKEN} are legacy names for the fields that used
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might need some clarity here. The javadoc gives the impression that INSTANCE_ID_TOKEN can be fetched from FIS SDK, which is incorrect. We can explicitly call out INSTANCE_ID_TOKEN is replaced by installation auth token fetched from FIS SDK.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call. Updated.

* to be populated by the IID SDK. The fields have been replaced by the installation ID and
* installation auth token, respectively, which are fetched from the FIS SDK.
*/
@StringDef({
RequestFieldKey.INSTANCE_ID,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ public class ConfigFetchHttpClient {
private static final String X_ANDROID_PACKAGE_HEADER = "X-Android-Package";
private static final String X_ANDROID_CERT_HEADER = "X-Android-Cert";
private static final String X_GOOGLE_GFE_CAN_RETRY = "X-Google-GFE-Can-Retry";
private static final String INSTALLATIONS_AUTH_TOKEN_HEADER =
"X-Goog-Firebase-Installations-Auth";

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking (no action required): this is comparable to usage of the same header in RC's iOS SDK: https://github.com/firebase/firebase-ios-sdk/blob/c07c89ee7582e84da1c5aafaa94b38a53bcfbdc2/FirebaseRemoteConfig/Sources/RCNFetch.m#L49

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


private final Context context;
private final String appId;
Expand Down Expand Up @@ -149,9 +151,9 @@ HttpURLConnection createHttpURLConnection() throws FirebaseRemoteConfigException
*
* @param urlConnection a {@link HttpURLConnection} created by a call to {@link
* #createHttpURLConnection}.
* @param instanceId the Firebase Instance ID that identifies a Firebase App Instance.
* @param instanceIdToken a valid Firebase Instance ID Token that authenticates a Firebase App
* Instance.
* @param installationId the Firebase installation ID that identifies a Firebase App Instance.
* @param installationAuthToken a valid Firebase installation auth token that authenticates a
* Firebase App Instance.
* @param analyticsUserProperties a map of Google Analytics User Properties and the device's
* corresponding values.
* @param lastFetchETag the ETag returned by the last successful fetch call to the FRC server. The
Expand All @@ -164,20 +166,20 @@ HttpURLConnection createHttpURLConnection() throws FirebaseRemoteConfigException
@Keep
FetchResponse fetch(
HttpURLConnection urlConnection,
String instanceId,
String instanceIdToken,
String installationId,
String installationAuthToken,
Map<String, String> analyticsUserProperties,
String lastFetchETag,
Map<String, String> customHeaders,
Date currentTime)
throws FirebaseRemoteConfigException {
setUpUrlConnection(urlConnection, lastFetchETag, customHeaders);
setUpUrlConnection(urlConnection, lastFetchETag, installationAuthToken, customHeaders);

String fetchResponseETag;
JSONObject fetchResponse;
try {
byte[] requestBody =
createFetchRequestBody(instanceId, instanceIdToken, analyticsUserProperties)
createFetchRequestBody(installationId, installationAuthToken, analyticsUserProperties)
.toString()
.getBytes("utf-8");
setFetchRequestBody(urlConnection, requestBody);
Expand Down Expand Up @@ -212,7 +214,10 @@ FetchResponse fetch(
}

private void setUpUrlConnection(
HttpURLConnection urlConnection, String lastFetchEtag, Map<String, String> customHeaders) {
HttpURLConnection urlConnection,
String lastFetchEtag,
String installationAuthToken,
Map<String, String> customHeaders) {
urlConnection.setDoOutput(true);
urlConnection.setConnectTimeout((int) SECONDS.toMillis(connectTimeoutInSeconds));
urlConnection.setReadTimeout((int) SECONDS.toMillis(readTimeoutInSeconds));
Expand All @@ -221,15 +226,16 @@ private void setUpUrlConnection(
// change in the Fetch Response since the last fetch call.
urlConnection.setRequestProperty(IF_NONE_MATCH_HEADER, lastFetchEtag);

setCommonRequestHeaders(urlConnection);
setCommonRequestHeaders(urlConnection, installationAuthToken);
setCustomRequestHeaders(urlConnection, customHeaders);
}

private String getFetchUrl(String projectNumber, String namespace) {
return String.format(FETCH_REGEX_URL, projectNumber, namespace);
}

private void setCommonRequestHeaders(HttpURLConnection urlConnection) {
private void setCommonRequestHeaders(
HttpURLConnection urlConnection, String installationAuthToken) {
urlConnection.setRequestProperty(API_KEY_HEADER, apiKey);

// Headers required for Android API Key Restrictions.
Expand All @@ -239,6 +245,9 @@ private void setCommonRequestHeaders(HttpURLConnection urlConnection) {
// Header to denote request is retryable on the server.
urlConnection.setRequestProperty(X_GOOGLE_GFE_CAN_RETRY, "yes");

// Header for FIS auth token
urlConnection.setRequestProperty(INSTALLATIONS_AUTH_TOKEN_HEADER, installationAuthToken);

// Headers to denote that the request body is a JSONObject.
urlConnection.setRequestProperty("Content-Type", "application/json");
urlConnection.setRequestProperty("Accept", "application/json");
Expand Down Expand Up @@ -278,16 +287,19 @@ private String getFingerprintHashForPackage() {
* serialized as a JSON.
*/
private JSONObject createFetchRequestBody(
String instanceId, String instanceIdToken, Map<String, String> analyticsUserProperties)
String installationId,
String installationAuthToken,
Map<String, String> analyticsUserProperties)
throws FirebaseRemoteConfigClientException {
Map<String, Object> requestBodyMap = new HashMap<>();

if (instanceId == null) {
throw new FirebaseRemoteConfigClientException("Fetch failed: Firebase instance id is null.");
if (installationId == null) {
throw new FirebaseRemoteConfigClientException(
"Fetch failed: Firebase installation id is null.");
}
requestBodyMap.put(INSTANCE_ID, instanceId);
requestBodyMap.put(INSTANCE_ID, installationId);

requestBodyMap.put(INSTANCE_ID_TOKEN, instanceIdToken);
requestBodyMap.put(INSTANCE_ID_TOKEN, installationAuthToken);
requestBodyMap.put(APP_ID, appId);

Locale locale = context.getResources().getConfiguration().locale;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,8 @@
shadows = {ShadowPreconditions.class})
public final class FirebaseRemoteConfigTest {
private static final String APP_ID = "1:14368190084:android:09cb977358c6f241";
private static final String API_KEY = "api_key";
private static final String API_KEY = "AIzaSyabcdefghijklmnopqrstuvwxyz1234567";
private static final String PROJECT_ID = "fake-frc-test-id";

private static final String FIREPERF_NAMESPACE = "fireperf";

Expand Down Expand Up @@ -1220,6 +1221,11 @@ private static FirebaseApp initializeFirebaseApp(Context context) {
FirebaseApp.clearInstancesForTest();

return FirebaseApp.initializeApp(
context, new FirebaseOptions.Builder().setApiKey(API_KEY).setApplicationId(APP_ID).build());
context,
new FirebaseOptions.Builder()
.setApiKey(API_KEY)
.setApplicationId(APP_ID)
.setProjectId(PROJECT_ID)
.build());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,10 @@
@RunWith(RobolectricTestRunner.class)
@Config(manifest = Config.NONE)
public class RemoteConfigComponentTest {
private static final String API_KEY = "api_key";
private static final String API_KEY = "AIzaSyabcdefghijklmnopqrstuvwxyz1234567";
private static final String APP_ID = "1:14368190084:android:09cb977358c6f241";
private static final String DUMMY_API_KEY = "api_key";
private static final String PROJECT_ID = "fake-frc-test-id";

@Mock private FirebaseApp mockFirebaseApp;
@Mock private FirebaseInstallationsApi mockFirebaseInstallations;
Expand Down Expand Up @@ -237,6 +238,11 @@ private static FirebaseApp initializeFirebaseApp(Context context) {
FirebaseApp.clearInstancesForTest();

return FirebaseApp.initializeApp(
context, new FirebaseOptions.Builder().setApiKey(API_KEY).setApplicationId(APP_ID).build());
context,
new FirebaseOptions.Builder()
.setApiKey(API_KEY)
.setApplicationId(APP_ID)
.setProjectId(PROJECT_ID)
.build());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,9 @@ public class ConfigFetchHttpClientTest {
private static final String API_KEY = "fake_api_key";
private static final String FAKE_APP_ID = "1:14368190084:android:09cb977358c6f241";
private static final String PROJECT_NUMBER = "14368190084";
private static final String INSTANCE_ID_STRING = "fake instance id";
private static final String INSTANCE_ID_TOKEN_STRING = "fake instance id token";
private static final String INSTALLATION_ID_STRING = "'fL71_VyL3uo9jNMWu1L60S";
private static final String INSTALLATION_AUTH_TOKEN_STRING =
"eyJhbGciOiJF.eyJmaWQiOiJmaXMt.AB2LPV8wRQIhAPs4NvEgA3uhubH";
private static final String DEFAULT_NAMESPACE = RemoteConfigComponent.DEFAULT_NAMESPACE;
private static final String ETAG_FORMAT =
"etag-" + PROJECT_NUMBER + "-" + DEFAULT_NAMESPACE + "-fetch-%d";
Expand Down Expand Up @@ -177,6 +178,7 @@ public void fetch_setsAllHeaders_sendsAllHeadersToServer() throws Exception {
expectedHeaders.put("X-Android-Package", context.getPackageName());
expectedHeaders.put("X-Android-Cert", null);
expectedHeaders.put("X-Google-GFE-Can-Retry", "yes");
expectedHeaders.put("X-Goog-Firebase-Installations-Auth", INSTALLATION_AUTH_TOKEN_STRING);
expectedHeaders.put("Content-Type", "application/json");
expectedHeaders.put("Accept", "application/json");
// Custom user-defined headers.
Expand All @@ -195,8 +197,8 @@ public void fetch_setsAllElementsOfRequestBody_sendsRequestBodyToServer() throws
fetch(FIRST_ETAG, userProperties);

JSONObject requestBody = new JSONObject(fakeHttpURLConnection.getOutputStream().toString());
assertThat(requestBody.get(INSTANCE_ID)).isEqualTo(INSTANCE_ID_STRING);
assertThat(requestBody.get(INSTANCE_ID_TOKEN)).isEqualTo(INSTANCE_ID_TOKEN_STRING);
assertThat(requestBody.get(INSTANCE_ID)).isEqualTo(INSTALLATION_ID_STRING);
assertThat(requestBody.get(INSTANCE_ID_TOKEN)).isEqualTo(INSTALLATION_AUTH_TOKEN_STRING);
assertThat(requestBody.get(APP_ID)).isEqualTo(FAKE_APP_ID);
Locale locale = context.getResources().getConfiguration().locale;
assertThat(requestBody.get(COUNTRY_CODE)).isEqualTo(locale.getCountry());
Expand Down Expand Up @@ -240,22 +242,22 @@ public void fetch_localeUsesToStringBelowLollipop() throws Exception {
}

@Test
public void fetch_instanceIdIsNull_throwsFRCClientException() throws Exception {
public void fetch_installationIdIsNull_throwsFRCClientException() throws Exception {
setServerResponseTo(noChangeResponseBody, SECOND_ETAG);

FirebaseRemoteConfigClientException frcException =
assertThrows(FirebaseRemoteConfigClientException.class, () -> fetchWithoutIid());
assertThrows(FirebaseRemoteConfigClientException.class, () -> fetchWithoutInstallationId());

assertThat(frcException).hasMessageThat().contains("instance id is null");
assertThat(frcException).hasMessageThat().contains("installation id is null");
}

@Test
public void fetch_instanceIdTokenIsNull_doesNotThrowException() throws Exception {
public void fetch_installationAuthTokenIsNull_doesNotThrowException() throws Exception {
setServerResponseTo(noChangeResponseBody, SECOND_ETAG);

FetchResponse fetchResponse = fetchWithoutIidToken();
FetchResponse fetchResponse = fetchWithoutInstallationAuthToken();

assertWithMessage("Fetch() failed with null instance id token!")
assertWithMessage("Fetch() failed with null installation auth token!")
Copy link

@erikeldridge erikeldridge Jun 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking (no action required): this is a public error message, so the phrasing should match the public docs. The FirebaseInstallations.getToken docs describe "authentication token for the Firebase installation", which matches the phrasing here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great callout, thank you!
We had a meeting recently with DevRel and Tech Writers and decided to name the authentication tokens for Firebase installations: "installation auth tokens".
So, the action item is probably on us in the FIS team.

.that(fetchResponse)
.isNotNull();
}
Expand Down Expand Up @@ -291,8 +293,8 @@ public void fetch_serverReturnsException_throwsFirebaseRemoteConfigException() {
private FetchResponse fetch(String eTag) throws Exception {
return configFetchHttpClient.fetch(
fakeHttpURLConnection,
INSTANCE_ID_STRING,
INSTANCE_ID_TOKEN_STRING,
INSTALLATION_ID_STRING,
INSTALLATION_AUTH_TOKEN_STRING,
/* analyticsUserProperties= */ ImmutableMap.of(),
eTag,
/* customHeaders= */ ImmutableMap.of(),
Expand All @@ -302,8 +304,8 @@ private FetchResponse fetch(String eTag) throws Exception {
private FetchResponse fetch(String eTag, Map<String, String> userProperties) throws Exception {
return configFetchHttpClient.fetch(
fakeHttpURLConnection,
INSTANCE_ID_STRING,
INSTANCE_ID_TOKEN_STRING,
INSTALLATION_ID_STRING,
INSTALLATION_AUTH_TOKEN_STRING,
userProperties,
eTag,
/* customHeaders= */ ImmutableMap.of(),
Expand All @@ -315,30 +317,30 @@ private FetchResponse fetch(
throws Exception {
return configFetchHttpClient.fetch(
fakeHttpURLConnection,
INSTANCE_ID_STRING,
INSTANCE_ID_TOKEN_STRING,
INSTALLATION_ID_STRING,
INSTALLATION_AUTH_TOKEN_STRING,
userProperties,
eTag,
customHeaders,
new Date(mockClock.currentTimeMillis()));
}

private FetchResponse fetchWithoutIid() throws Exception {
private FetchResponse fetchWithoutInstallationId() throws Exception {
return configFetchHttpClient.fetch(
fakeHttpURLConnection,
/* instanceId= */ null,
INSTANCE_ID_TOKEN_STRING,
/* installationId= */ null,
INSTALLATION_AUTH_TOKEN_STRING,
/* analyticsUserProperties= */ ImmutableMap.of(),
/* lastFetchETag= */ "bogus-etag",
/* customHeaders= */ ImmutableMap.of(),
new Date(mockClock.currentTimeMillis()));
}

private FetchResponse fetchWithoutIidToken() throws Exception {
private FetchResponse fetchWithoutInstallationAuthToken() throws Exception {
return configFetchHttpClient.fetch(
fakeHttpURLConnection,
INSTANCE_ID_STRING,
/* instanceIdToken= */ null,
INSTALLATION_ID_STRING,
/* installationAuthToken= */ null,
/* analyticsUserProperties= */ ImmutableMap.of(),
/* lastFetchETag= */ "bogus-etag",
/* customHeaders= */ ImmutableMap.of(),
Expand Down