-
Notifications
You must be signed in to change notification settings - Fork 624
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
Changes from all commits
666d636
3f8959f
e5092a9
a470ea5
9736d01
ffcbb9d
937f3f8
892e750
4221b46
1080fa7
dafda53
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
|
||
private final Context context; | ||
private final String appId; | ||
|
@@ -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 | ||
|
@@ -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); | ||
|
@@ -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)); | ||
|
@@ -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. | ||
|
@@ -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"); | ||
|
@@ -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; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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"; | ||
|
@@ -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. | ||
|
@@ -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()); | ||
|
@@ -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!") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great callout, thank you! |
||
.that(fetchResponse) | ||
.isNotNull(); | ||
} | ||
|
@@ -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(), | ||
|
@@ -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(), | ||
|
@@ -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(), | ||
|
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call. Updated.