Skip to content

Commit

Permalink
Force validation or rotation of FIDs (#5812)
Browse files Browse the repository at this point in the history
Force validation or rotation of FIDs. This also cleans up the tests a
bit more, and isolated Crashlytics integration tests from Sessions
tests.
  • Loading branch information
mrober committed Mar 28, 2024
1 parent 936112f commit c9fa461
Show file tree
Hide file tree
Showing 16 changed files with 230 additions and 109 deletions.
164 changes: 109 additions & 55 deletions firebase-crashlytics/CHANGELOG.md

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,14 @@ import org.junit.runner.RunWith
class CrashlyticsTests {
@Before
fun setUp() {
@Suppress("DEPRECATION")
Firebase.initialize(
ApplicationProvider.getApplicationContext(),
FirebaseOptions.Builder()
.setApplicationId(APP_ID)
.setApiKey(API_KEY)
.setProjectId(PROJECT_ID)
.build()
.build(),
)
}

Expand All @@ -57,7 +58,7 @@ class CrashlyticsTests {

@Test
fun libraryRegistrationAtRuntime() {
val publisher = Firebase.app.get(UserAgentPublisher::class.java)
Firebase.app.get(UserAgentPublisher::class.java)
}

companion object {
Expand Down
18 changes: 9 additions & 9 deletions firebase-crashlytics/src/androidTest/AndroidManifest.xml
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
<manifest xmlns:android="http://schemas.android.com/apk/res/android">

<application>

<meta-data android:name="io.fabric.ApiKey"
android:value="a000000000000000000000000000000000000000"/>

</application>

<manifest xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:tools="http://schemas.android.com/tools">

<application>
<service
android:enabled="false"
android:name="com.google.firebase.sessions.SessionLifecycleService"
tools:replace="android:enabled" />
</application>
</manifest>
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class CrashlyticsTests {
.setApplicationId(APP_ID)
.setApiKey(API_KEY)
.setProjectId(PROJECT_ID)
.build()
.build(),
)
}

Expand All @@ -56,7 +56,7 @@ class CrashlyticsTests {

@Test
fun libraryRegistrationAtRuntime() {
val publisher = Firebase.app.get(UserAgentPublisher::class.java)
Firebase.app.get(UserAgentPublisher::class.java)
}

companion object {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -497,7 +497,7 @@ public void onReportSend_successfulReportsAreDeleted() {
when(reportSender.enqueueReport(mockReport1, false)).thenReturn(successfulTask);
when(reportSender.enqueueReport(mockReport2, false)).thenReturn(failedTask);

when(idManager.fetchTrueFid()).thenReturn("fid");
when(idManager.fetchTrueFid()).thenReturn(new FirebaseInstallationId("fid", "authToken"));
reportingCoordinator.sendReports(Runnable::run);

verify(reportSender).enqueueReport(mockReport1, false);
Expand Down Expand Up @@ -548,6 +548,7 @@ private static CrashlyticsReport mockReport(String sessionId) {
when(mockSession.getIdentifier()).thenReturn(sessionId);
when(mockReport.getSession()).thenReturn(mockSession);
when(mockReport.withFirebaseInstallationId(anyString())).thenReturn(mockReport);
when(mockReport.withFirebaseAuthenticationToken(anyString())).thenReturn(mockReport);
return mockReport;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -837,6 +837,8 @@ private static CrashlyticsReport.Builder makeIncompleteReport() {
.setGmpAppId("gmpAppId")
.setPlatform(1)
.setInstallationUuid("installationId")
.setFirebaseInstallationId("firebaseInstallationId")
.setFirebaseAuthenticationToken("firebaseAuthenticationToken")
.setBuildVersion("1")
.setDisplayVersion("1.0.0");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,14 @@ import org.junit.runner.RunWith
class CrashlyticsTests {
@Before
fun setUp() {
@Suppress("DEPRECATION")
Firebase.initialize(
ApplicationProvider.getApplicationContext(),
FirebaseOptions.Builder()
.setApplicationId(APP_ID)
.setApiKey(API_KEY)
.setProjectId(PROJECT_ID)
.build()
.build(),
)
}

Expand All @@ -57,7 +58,7 @@ class CrashlyticsTests {

@Test
fun libraryRegistrationAtRuntime() {
val publisher = Firebase.app.get(UserAgentPublisher::class.java)
Firebase.app.get(UserAgentPublisher::class.java)
}

companion object {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ private CrashlyticsReport.Builder buildReportData() {
.setGmpAppId(appData.googleAppId)
.setInstallationUuid(idManager.getInstallIds().getCrashlyticsInstallId())
.setFirebaseInstallationId(idManager.getInstallIds().getFirebaseInstallationId())
.setFirebaseAuthenticationToken(idManager.getInstallIds().getFirebaseAuthenticationToken())
.setBuildVersion(appData.versionCode)
.setDisplayVersion(appData.versionName)
.setPlatform(REPORT_ANDROID_PLATFORM);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/*
* Copyright 2024 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.google.firebase.crashlytics.internal.common

/**
* Simple data class to contain the true Firebase installation id and Firebase authentication token.
*
* <p>This is not the Crashlytics installation id.
*/
data class FirebaseInstallationId(val fid: String?, val authToken: String?)
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,16 @@

package com.google.firebase.crashlytics.internal.common;

import static com.google.firebase.crashlytics.internal.common.Utils.awaitEvenIfOnMainThread;

import android.content.Context;
import android.content.SharedPreferences;
import android.os.Build;
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import androidx.annotation.VisibleForTesting;
import com.google.android.gms.tasks.Task;
import com.google.firebase.crashlytics.internal.Logger;
import com.google.firebase.installations.FirebaseInstallationsApi;
import java.util.Locale;
import java.util.Objects;
import java.util.UUID;
import java.util.regex.Pattern;

Expand All @@ -48,7 +48,7 @@ public class IdManager implements InstallIdProvider {
private final String appIdentifier;

// The FirebaseInstallationsApi encapsulates a Firebase-wide install id
private final FirebaseInstallationsApi firebaseInstallationsApi;
private final FirebaseInstallationsApi firebaseInstallations;

private final DataCollectionArbiter dataCollectionArbiter;

Expand All @@ -65,7 +65,7 @@ public class IdManager implements InstallIdProvider {
public IdManager(
Context appContext,
String appIdentifier,
FirebaseInstallationsApi firebaseInstallationsApi,
FirebaseInstallationsApi firebaseInstallations,
DataCollectionArbiter dataCollectionArbiter) {
if (appContext == null) {
throw new IllegalArgumentException("appContext must not be null");
Expand All @@ -75,18 +75,16 @@ public IdManager(
}
this.appContext = appContext;
this.appIdentifier = appIdentifier;
this.firebaseInstallationsApi = firebaseInstallationsApi;
this.firebaseInstallations = firebaseInstallations;
this.dataCollectionArbiter = dataCollectionArbiter;

installerPackageNameProvider = new InstallerPackageNameProvider();
}

/**
* Apply consistent formatting and stripping of special characters. Null input is allowed, will
* return null.
*/
private static String formatId(String id) {
return (id == null) ? null : ID_PATTERN.matcher(id).replaceAll("").toLowerCase(Locale.US);
/** Apply consistent formatting and stripping of special characters. */
@NonNull
private static String formatId(@NonNull String id) {
return ID_PATTERN.matcher(id).replaceAll("").toLowerCase(Locale.US);
}

/**
Expand Down Expand Up @@ -115,21 +113,23 @@ public synchronized InstallIds getInstallIds() {
// We only look at the FID if Crashlytics data collection is enabled, since querying it can
// result in a network call that registers the FID with Firebase.
if (dataCollectionArbiter.isAutomaticDataCollectionEnabled()) {
String trueFid = fetchTrueFid();
FirebaseInstallationId trueFid = fetchTrueFid();
Logger.getLogger().v("Fetched Firebase Installation ID: " + trueFid);

if (trueFid == null) {
if (trueFid.getFid() == null) {
// This shouldn't happen often. We will assume the cached FID is valid, if it exists.
// Otherwise, the safest thing to do is to create a synthetic ID instead
trueFid = (cachedFid == null ? createSyntheticFid() : cachedFid);
trueFid =
new FirebaseInstallationId(cachedFid == null ? createSyntheticFid() : cachedFid, null);
}

if (trueFid.equals(cachedFid)) {
if (Objects.equals(trueFid.getFid(), cachedFid)) {
// the current FID is the same as the cached FID, so we keep the cached Crashlytics ID
installIds = InstallIds.create(readCachedCrashlyticsInstallId(prefs), trueFid);
} else {
// the current FID has changed, so we generate a new Crashlytics ID
installIds = InstallIds.create(createAndCacheCrashlyticsInstallId(trueFid, prefs), trueFid);
installIds =
InstallIds.create(createAndCacheCrashlyticsInstallId(trueFid.getFid(), prefs), trueFid);
}
} else { // data collection is NOT enabled; we can't use the FID
if (isSyntheticFid(cachedFid)) {
Expand Down Expand Up @@ -171,19 +171,29 @@ private String readCachedCrashlyticsInstallId(SharedPreferences prefs) {
return prefs.getString(PREFKEY_INSTALLATION_UUID, null);
}

/** Makes a blocking call to query FID. If the call fails, logs a warning and returns null. */
@Nullable
@VisibleForTesting(otherwise = VisibleForTesting.PACKAGE_PRIVATE)
public String fetchTrueFid() {
Task<String> currentFidTask = firebaseInstallationsApi.getId();
String currentFid = null;
/**
* Makes a blocking call to query the Firebase installation id and Firebase authentication token.
*
* <p>If either call fails for any reason, logs a warning and sets a null value for that field.
*/
@NonNull
public FirebaseInstallationId fetchTrueFid() {
String fid = null;
String authToken = null;

// Fetch the auth token first, so the fid will be validated.
try {
currentFid = Utils.awaitEvenIfOnMainThread(currentFidTask);
} catch (Exception e) {
Logger.getLogger().w("Failed to retrieve Firebase Installation ID.", e);
authToken = awaitEvenIfOnMainThread(firebaseInstallations.getToken(false)).getToken();
} catch (Exception ex) {
Logger.getLogger().w("Error getting Firebase authentication token.", ex);
}
return currentFid;
try {
fid = awaitEvenIfOnMainThread(firebaseInstallations.getId());
} catch (Exception ex) {
Logger.getLogger().w("Error getting Firebase installation id.", ex);
}

return new FirebaseInstallationId(fid, authToken);
}

@NonNull
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import androidx.annotation.VisibleForTesting;
import com.google.auto.value.AutoValue;

public interface InstallIdProvider {
Expand All @@ -32,15 +31,23 @@ abstract class InstallIds {
@Nullable
public abstract String getFirebaseInstallationId();

@Nullable
public abstract String getFirebaseAuthenticationToken();

/** Creates an InstallIds with just a crashlyticsInstallId, no firebaseInstallationId. */
@VisibleForTesting(otherwise = VisibleForTesting.PACKAGE_PRIVATE)
public static InstallIds createWithoutFid(String crashlyticsInstallId) {
return create(crashlyticsInstallId, /* firebaseInstallationId= */ null);
return new AutoValue_InstallIdProvider_InstallIds(
crashlyticsInstallId,
/* firebaseInstallationId= */ null,
/* firebaseAuthenticationToken= */ null);
}

static InstallIds create(String crashlyticsInstallId, @Nullable String firebaseInstallationId) {
static InstallIds create(
String crashlyticsInstallId, FirebaseInstallationId firebaseInstallationId) {
return new AutoValue_InstallIdProvider_InstallIds(
crashlyticsInstallId, firebaseInstallationId);
crashlyticsInstallId,
firebaseInstallationId.getFid(),
firebaseInstallationId.getAuthToken());
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -240,11 +240,15 @@ public Task<Void> sendReports(
*/
private CrashlyticsReportWithSessionId ensureHasFid(CrashlyticsReportWithSessionId reportToSend) {
// Only do the update if the fid is already missing from the report.
if (reportToSend.getReport().getFirebaseInstallationId() == null) {
if (reportToSend.getReport().getFirebaseInstallationId() == null
|| reportToSend.getReport().getFirebaseAuthenticationToken() == null) {
// Fetch the true fid, regardless of automatic data collection since it's uploading.
String fid = idManager.fetchTrueFid();
FirebaseInstallationId firebaseInstallationId = idManager.fetchTrueFid();
return CrashlyticsReportWithSessionId.create(
reportToSend.getReport().withFirebaseInstallationId(fid),
reportToSend
.getReport()
.withFirebaseInstallationId(firebaseInstallationId.getFid())
.withFirebaseAuthenticationToken(firebaseInstallationId.getAuthToken()),
reportToSend.getSessionId(),
reportToSend.getReportFile());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,9 @@ public Type getType() {
@Nullable
public abstract String getFirebaseInstallationId();

@Nullable
public abstract String getFirebaseAuthenticationToken();

@Nullable
public abstract String getAppQualitySessionId();

Expand Down Expand Up @@ -205,6 +208,13 @@ public CrashlyticsReport withFirebaseInstallationId(@Nullable String firebaseIns
return toBuilder().setFirebaseInstallationId(firebaseInstallationId).build();
}

/** Update an existing {@link CrashlyticsReport} with the given firebaseAuthenticationToken. */
@NonNull
public CrashlyticsReport withFirebaseAuthenticationToken(
@Nullable String firebaseAuthenticationToken) {
return toBuilder().setFirebaseAuthenticationToken(firebaseAuthenticationToken).build();
}

@AutoValue
public abstract static class FilesPayload {

Expand Down Expand Up @@ -1352,6 +1362,9 @@ public abstract static class Builder {
@NonNull
public abstract Builder setFirebaseInstallationId(@Nullable String value);

@NonNull
public abstract Builder setFirebaseAuthenticationToken(@Nullable String value);

@NonNull
public abstract Builder setBuildVersion(@NonNull String value);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,9 @@ private static CrashlyticsReport parseReport(@NonNull JsonReader jsonReader) thr
case "firebaseInstallationId":
builder.setFirebaseInstallationId(jsonReader.nextString());
break;
case "firebaseAuthenticationToken":
builder.setFirebaseAuthenticationToken(jsonReader.nextString());
break;
case "appQualitySessionId":
builder.setAppQualitySessionId(jsonReader.nextString());
break;
Expand Down
2 changes: 1 addition & 1 deletion firebase-sessions/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# Unreleased


# 1.2.3

* [fixed] Force validation or rotation of FIDs.

# 1.2.1
Expand Down
Loading

0 comments on commit c9fa461

Please sign in to comment.