Skip to content

Implement platform logging for App Check #2619

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.

Sign up for GitHub

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 2 commits into from
Apr 27, 2021
Merged

Implement platform logging for App Check #2619

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
132d7c2
Implement platform logging.
rosalyntan Apr 23, 2021
2fbeb17
Update tests.
rosalyntan Apr 26, 2021
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
11 changes: 1 addition & 10 deletions ...ebug/src/main/java/com/google/firebase/appcheck/debug/internal/DebugAppCheckProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,16 +45,7 @@ public class DebugAppCheckProvider implements AppCheckProvider {

public DebugAppCheckProvider(@NonNull FirebaseApp firebaseApp, @Nullable String debugSecret) {
checkNotNull(firebaseApp);
String projectId = firebaseApp.getOptions().getProjectId();
if (projectId == null) {
throw new IllegalArgumentException("FirebaseOptions#getProjectId cannot be null.");
}

this.networkClient =
new NetworkClient(
firebaseApp.getOptions().getApiKey(),
firebaseApp.getOptions().getApplicationId(),
projectId);
this.networkClient = new NetworkClient(firebaseApp);
this.backgroundExecutor = Executors.newCachedThreadPool();
this.debugSecretTask =
debugSecret == null
Expand Down
7 changes: 1 addition & 6 deletions .../main/java/com/google/firebase/appcheck/safetynet/internal/SafetyNetAppCheckProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,14 +67,9 @@ public SafetyNetAppCheckProvider(@NonNull FirebaseApp firebaseApp) {
checkNotNull(googleApiAvailability);
this.context = firebaseApp.getApplicationContext();
this.apiKey = firebaseApp.getOptions().getApiKey();
String appId = firebaseApp.getOptions().getApplicationId();
String projectId = firebaseApp.getOptions().getProjectId();
if (projectId == null) {
throw new IllegalArgumentException("FirebaseOptions#getProjectId cannot be null.");
}
this.backgroundExecutor = backgroundExecutor;
this.safetyNetClientTask = initSafetyNetClient(googleApiAvailability, this.backgroundExecutor);
this.networkClient = new NetworkClient(apiKey, appId, projectId);
this.networkClient = new NetworkClient(firebaseApp);
}

@VisibleForTesting
Expand Down
15 changes: 13 additions & 2 deletions ...rebase-appcheck/src/main/java/com/google/firebase/appcheck/FirebaseAppCheckRegistrar.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@
import com.google.firebase.components.Component;
import com.google.firebase.components.ComponentRegistrar;
import com.google.firebase.components.Dependency;
import com.google.firebase.heartbeatinfo.HeartBeatInfo;
import com.google.firebase.platforminfo.LibraryVersionComponent;
import com.google.firebase.platforminfo.UserAgentPublisher;
import java.util.Arrays;
import java.util.List;

Expand All @@ -38,8 +41,16 @@ public List<Component<?>> getComponents() {
return Arrays.asList(
Component.builder(FirebaseAppCheck.class, (InternalAppCheckTokenProvider.class))
.add(Dependency.required(FirebaseApp.class))
.factory((container) -> new DefaultFirebaseAppCheck(container.get(FirebaseApp.class)))
.add(Dependency.optionalProvider(UserAgentPublisher.class))
.add(Dependency.optionalProvider(HeartBeatInfo.class))
.factory(
(container) ->
new DefaultFirebaseAppCheck(
container.get(FirebaseApp.class),
container.getProvider(UserAgentPublisher.class),
container.getProvider(HeartBeatInfo.class)))
.alwaysEager()
.build());
.build(),
LibraryVersionComponent.create("fire-app-check", BuildConfig.VERSION_NAME));
}
}
30 changes: 29 additions & 1 deletion ...appcheck/src/main/java/com/google/firebase/appcheck/internal/DefaultFirebaseAppCheck.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,20 @@
import com.google.firebase.appcheck.FirebaseAppCheck;
import com.google.firebase.appcheck.internal.util.Clock;
import com.google.firebase.appcheck.interop.AppCheckTokenListener;
import com.google.firebase.heartbeatinfo.HeartBeatInfo;
import com.google.firebase.inject.Provider;
import com.google.firebase.platforminfo.UserAgentPublisher;
import java.util.ArrayList;
import java.util.List;

public class DefaultFirebaseAppCheck extends FirebaseAppCheck {

private static final long BUFFER_TIME_MILLIS = 5 * 60 * 1000; // 5 minutes in milliseconds
private static final String HEART_BEAT_STORAGE_TAG = "fire-app-check";

private final FirebaseApp firebaseApp;
private final Provider<UserAgentPublisher> userAgentPublisherProvider;
private final Provider<HeartBeatInfo> heartBeatInfoProvider;
private final List<AppCheckTokenListener> appCheckTokenListenerList;
private final StorageHelper storageHelper;
private final TokenRefreshManager tokenRefreshManager;
Expand All @@ -48,9 +54,16 @@ public class DefaultFirebaseAppCheck extends FirebaseAppCheck {
private AppCheckProvider appCheckProvider;
private AppCheckToken cachedToken;

public DefaultFirebaseAppCheck(@NonNull FirebaseApp firebaseApp) {
public DefaultFirebaseAppCheck(
@NonNull FirebaseApp firebaseApp,
@NonNull Provider<UserAgentPublisher> userAgentPublisherProvider,
@NonNull Provider<HeartBeatInfo> heartBeatInfoProvider) {
checkNotNull(firebaseApp);
checkNotNull(userAgentPublisherProvider);
checkNotNull(heartBeatInfoProvider);
this.firebaseApp = firebaseApp;
this.userAgentPublisherProvider = userAgentPublisherProvider;
this.heartBeatInfoProvider = heartBeatInfoProvider;
this.appCheckTokenListenerList = new ArrayList<>();
this.storageHelper =
new StorageHelper(firebaseApp.getApplicationContext(), firebaseApp.getPersistenceKey());
Expand Down Expand Up @@ -155,6 +168,21 @@ public Task<AppCheckTokenResult> then(@NonNull Task<AppCheckToken> task) {
});
}

@Nullable
String getUserAgent() {
return userAgentPublisherProvider.get() != null
? userAgentPublisherProvider.get().getUserAgent()
: null;
}

@Nullable
String getHeartbeatCode() {
return heartBeatInfoProvider.get() != null
? Integer.toString(
heartBeatInfoProvider.get().getHeartBeatCode(HEART_BEAT_STORAGE_TAG).getCode())
: null;
}

/** Sets the in-memory cached {@link AppCheckToken}. */
@VisibleForTesting
void setCachedToken(@NonNull AppCheckToken token) {
Expand Down
33 changes: 21 additions & 12 deletions .../firebase-appcheck/src/main/java/com/google/firebase/appcheck/internal/NetworkClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@
import androidx.annotation.IntDef;
import androidx.annotation.NonNull;
import androidx.annotation.VisibleForTesting;
import com.google.firebase.FirebaseApp;
import com.google.firebase.FirebaseException;
import com.google.firebase.appcheck.BuildConfig;
import com.google.firebase.appcheck.FirebaseAppCheck;
import java.io.BufferedOutputStream;
import java.io.BufferedReader;
import java.io.IOException;
Expand All @@ -46,9 +47,10 @@ public class NetworkClient {
private static final String CONTENT_TYPE = "Content-Type";
private static final String APPLICATION_JSON = "application/json";
private static final String UTF_8 = "UTF-8";
private static final String X_FIREBASE_CLIENT = "X-Firebase-Client";
private static final String PLATFORM_NAME = "fire-app-check";
@VisibleForTesting static final String X_FIREBASE_CLIENT = "X-Firebase-Client";
@VisibleForTesting static final String X_FIREBASE_CLIENT_LOG_TYPE = "X-Firebase-Client-Log-Type";

private final DefaultFirebaseAppCheck firebaseAppCheck;
private final String apiKey;
private final String appId;
private final String projectId;
Expand All @@ -61,13 +63,15 @@ public class NetworkClient {
public static final int SAFETY_NET = 1;
public static final int DEBUG = 2;

public NetworkClient(@NonNull String apiKey, @NonNull String appId, @NonNull String projectId) {
checkNotNull(apiKey);
checkNotNull(appId);
checkNotNull(projectId);
this.apiKey = apiKey;
this.appId = appId;
this.projectId = projectId;
public NetworkClient(@NonNull FirebaseApp firebaseApp) {
checkNotNull(firebaseApp);
this.firebaseAppCheck = (DefaultFirebaseAppCheck) FirebaseAppCheck.getInstance(firebaseApp);
this.apiKey = firebaseApp.getOptions().getApiKey();
this.appId = firebaseApp.getOptions().getApplicationId();
this.projectId = firebaseApp.getOptions().getProjectId();
Comment on lines +66 to +71
Copy link
Member

Choose a reason for hiding this comment

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

Consider go/di-practices/inject-direct-dependencies

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the link! I passed FirebaseApp directly into NetworkClient to avoid adding the :appcheck:firebase-appcheck-interop dependency to firebase-appcheck-safetynet.gradle and firebase-appcheck-debug.gradle (for accessing FirebaseAppCheck#getInstance).

I think it's fine to add the interop dependency, but it seems cleaner to only have that dependency in the core App Check SDK.

Copy link
Member

Choose a reason for hiding this comment

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

I see, one thing I would suggest is to consider injecting the NetworkClient via firebase components instead of having both safetynet and debug sdks creating instantiating it on their own. This would also allow you to register both of these sdks with the platform logging user-agent.

(Optional)

Copy link
Member Author

Choose a reason for hiding this comment

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

Ack, that makes sense! I've filed b/186549222 to follow up on this in a future PR.

if (projectId == null) {
throw new IllegalArgumentException("FirebaseOptions#getProjectId cannot be null.");
}
}

/**
Expand All @@ -85,8 +89,13 @@ public AppCheckTokenResponse exchangeAttestationForAppCheckToken(
urlConnection.setDoOutput(true);
urlConnection.setFixedLengthStreamingMode(requestBytes.length);
urlConnection.setRequestProperty(CONTENT_TYPE, APPLICATION_JSON);
urlConnection.setRequestProperty(
X_FIREBASE_CLIENT, PLATFORM_NAME + "/" + BuildConfig.VERSION_NAME);
if (firebaseAppCheck.getUserAgent() != null) {
urlConnection.setRequestProperty(X_FIREBASE_CLIENT, firebaseAppCheck.getUserAgent());
}
if (firebaseAppCheck.getHeartbeatCode() != null) {
urlConnection.setRequestProperty(
X_FIREBASE_CLIENT_LOG_TYPE, firebaseAppCheck.getHeartbeatCode());
}

try (OutputStream out =
new BufferedOutputStream(urlConnection.getOutputStream(), requestBytes.length)) {
Expand Down
9 changes: 7 additions & 2 deletions ...se-appcheck/src/test/java/com/google/firebase/appcheck/FirebaseAppCheckRegistrarTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
import com.google.firebase.FirebaseApp;
import com.google.firebase.components.Component;
import com.google.firebase.components.Dependency;
import com.google.firebase.heartbeatinfo.HeartBeatInfo;
import com.google.firebase.platforminfo.UserAgentPublisher;
import java.util.List;
import org.junit.Test;
import org.junit.runner.RunWith;
Expand All @@ -33,10 +35,13 @@ public void testGetComponents() {
FirebaseAppCheckRegistrar firebaseAppCheckRegistrar = new FirebaseAppCheckRegistrar();
List<Component<?>> components = firebaseAppCheckRegistrar.getComponents();
assertThat(components).isNotEmpty();
assertThat(components).hasSize(1);
assertThat(components).hasSize(2);
Component<?> firebaseAppCheckComponent = components.get(0);
assertThat(firebaseAppCheckComponent.getDependencies())
.containsExactly(Dependency.required(FirebaseApp.class));
.containsExactly(
Dependency.required(FirebaseApp.class),
Dependency.optionalProvider(UserAgentPublisher.class),
Dependency.optionalProvider(HeartBeatInfo.class));
assertThat(firebaseAppCheckComponent.isAlwaysEager()).isTrue();
}
}
46 changes: 28 additions & 18 deletions ...heck/src/test/java/com/google/firebase/appcheck/internal/DefaultFirebaseAppCheckTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@
import com.google.firebase.appcheck.AppCheckProviderFactory;
import com.google.firebase.appcheck.AppCheckTokenResult;
import com.google.firebase.appcheck.interop.AppCheckTokenListener;
import com.google.firebase.heartbeatinfo.HeartBeatInfo;
import com.google.firebase.platforminfo.UserAgentPublisher;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
Expand All @@ -53,8 +55,11 @@ public class DefaultFirebaseAppCheckTest {
@Mock private AppCheckProviderFactory mockAppCheckProviderFactory;
@Mock private AppCheckProvider mockAppCheckProvider;
@Mock private AppCheckTokenListener mockAppCheckTokenListener;
@Mock private UserAgentPublisher mockUserAgentPublisher;
@Mock private HeartBeatInfo mockHeartBeatInfo;

private DefaultAppCheckToken validDefaultAppCheckToken;
private DefaultFirebaseAppCheck defaultFirebaseAppCheck;

@Before
public void setup() {
Expand All @@ -66,20 +71,41 @@ public void setup() {
when(mockFirebaseApp.getPersistenceKey()).thenReturn(PERSISTENCE_KEY);
when(mockAppCheckProviderFactory.create(any())).thenReturn(mockAppCheckProvider);
when(mockAppCheckProvider.getToken()).thenReturn(Tasks.forResult(validDefaultAppCheckToken));

defaultFirebaseAppCheck =
new DefaultFirebaseAppCheck(
mockFirebaseApp, () -> mockUserAgentPublisher, () -> mockHeartBeatInfo);
}

@Test
public void testConstructor_nullFirebaseApp_expectThrows() {
assertThrows(
NullPointerException.class,
() -> {
new DefaultFirebaseAppCheck(null);
new DefaultFirebaseAppCheck(null, () -> mockUserAgentPublisher, () -> mockHeartBeatInfo);
});
}

@Test
public void testConstructor_nullUserAgentPublisherProvider_expectThrows() {
assertThrows(
NullPointerException.class,
() -> {
new DefaultFirebaseAppCheck(mockFirebaseApp, null, () -> mockHeartBeatInfo);
});
}

@Test
public void testConstructor_nullHeartBeatInfoProvider_expectThrows() {
assertThrows(
NullPointerException.class,
() -> {
new DefaultFirebaseAppCheck(mockFirebaseApp, () -> mockUserAgentPublisher, null);
});
}

@Test
public void testInstallAppCheckFactory_nullFactory_expectThrows() {
DefaultFirebaseAppCheck defaultFirebaseAppCheck = new DefaultFirebaseAppCheck(mockFirebaseApp);
assertThrows(
NullPointerException.class,
() -> {
Expand All @@ -89,7 +115,6 @@ public void testInstallAppCheckFactory_nullFactory_expectThrows() {

@Test
public void testAddAppCheckTokenListener_nullListener_expectThrows() {
DefaultFirebaseAppCheck defaultFirebaseAppCheck = new DefaultFirebaseAppCheck(mockFirebaseApp);
assertThrows(
NullPointerException.class,
() -> {
Expand All @@ -99,7 +124,6 @@ public void testAddAppCheckTokenListener_nullListener_expectThrows() {

@Test
public void testRemoveAppCheckTokenListener_nullListener_expectThrows() {
DefaultFirebaseAppCheck defaultFirebaseAppCheck = new DefaultFirebaseAppCheck(mockFirebaseApp);
assertThrows(
NullPointerException.class,
() -> {
Expand All @@ -109,7 +133,6 @@ public void testRemoveAppCheckTokenListener_nullListener_expectThrows() {

@Test
public void testGetToken_noFactoryInstalled_returnResultWithError() throws Exception {
DefaultFirebaseAppCheck defaultFirebaseAppCheck = new DefaultFirebaseAppCheck(mockFirebaseApp);
Task<AppCheckTokenResult> tokenTask =
defaultFirebaseAppCheck.getToken(/* forceRefresh= */ false);
assertThat(tokenTask.isComplete()).isTrue();
Expand All @@ -120,7 +143,6 @@ public void testGetToken_noFactoryInstalled_returnResultWithError() throws Excep

@Test
public void testGetToken_factoryInstalled_proxiesToAppCheckFactory() {
DefaultFirebaseAppCheck defaultFirebaseAppCheck = new DefaultFirebaseAppCheck(mockFirebaseApp);
defaultFirebaseAppCheck.installAppCheckProviderFactory(mockAppCheckProviderFactory);

defaultFirebaseAppCheck.getToken(/* forceRefresh= */ false);
Expand All @@ -130,14 +152,11 @@ public void testGetToken_factoryInstalled_proxiesToAppCheckFactory() {

@Test
public void testGetInstalledAppCheckProviderFactory_noFactoryInstalled_returnsNull() {
DefaultFirebaseAppCheck defaultFirebaseAppCheck = new DefaultFirebaseAppCheck(mockFirebaseApp);

assertThat(defaultFirebaseAppCheck.getInstalledAppCheckProviderFactory()).isNull();
}

@Test
public void testGetInstalledAppCheckProviderFactory_factoryInstalled_returnsFactory() {
DefaultFirebaseAppCheck defaultFirebaseAppCheck = new DefaultFirebaseAppCheck(mockFirebaseApp);
defaultFirebaseAppCheck.installAppCheckProviderFactory(mockAppCheckProviderFactory);

assertThat(defaultFirebaseAppCheck.getInstalledAppCheckProviderFactory())
Expand All @@ -146,7 +165,6 @@ public void testGetInstalledAppCheckProviderFactory_factoryInstalled_returnsFact

@Test
public void testGetToken_factoryInstalledAndListenerRegistered_triggersListenerOnSuccess() {
DefaultFirebaseAppCheck defaultFirebaseAppCheck = new DefaultFirebaseAppCheck(mockFirebaseApp);
defaultFirebaseAppCheck.installAppCheckProviderFactory(mockAppCheckProviderFactory);
defaultFirebaseAppCheck.addAppCheckTokenListener(mockAppCheckTokenListener);

Expand All @@ -162,7 +180,6 @@ public void testGetToken_factoryInstalledAndListenerRegistered_triggersListenerO

@Test
public void testGetToken_factoryInstalledAndListenerRegistered_doesNotTriggerListenerOnFailure() {
DefaultFirebaseAppCheck defaultFirebaseAppCheck = new DefaultFirebaseAppCheck(mockFirebaseApp);
defaultFirebaseAppCheck.installAppCheckProviderFactory(mockAppCheckProviderFactory);
defaultFirebaseAppCheck.addAppCheckTokenListener(mockAppCheckTokenListener);

Expand All @@ -177,7 +194,6 @@ public void testGetToken_factoryInstalledAndListenerRegistered_doesNotTriggerLis

@Test
public void testGetToken_existingValidToken_triggersListenerUponAdding() {
DefaultFirebaseAppCheck defaultFirebaseAppCheck = new DefaultFirebaseAppCheck(mockFirebaseApp);
defaultFirebaseAppCheck.setCachedToken(validDefaultAppCheckToken);

defaultFirebaseAppCheck.addAppCheckTokenListener(mockAppCheckTokenListener);
Expand All @@ -193,8 +209,6 @@ public void testGetToken_existingValidToken_triggersListenerUponAdding() {
public void testGetToken_existingInvalidToken_doesNotTriggerListenerUponAdding() {
DefaultAppCheckToken invalidDefaultAppCheckToken =
new DefaultAppCheckToken(TOKEN_PAYLOAD, EXPIRES_NOW);

DefaultFirebaseAppCheck defaultFirebaseAppCheck = new DefaultFirebaseAppCheck(mockFirebaseApp);
defaultFirebaseAppCheck.setCachedToken(invalidDefaultAppCheckToken);

defaultFirebaseAppCheck.addAppCheckTokenListener(mockAppCheckTokenListener);
Expand All @@ -204,7 +218,6 @@ public void testGetToken_existingInvalidToken_doesNotTriggerListenerUponAdding()

@Test
public void testGetToken_existingValidToken_doesNotRequestNewToken() {
DefaultFirebaseAppCheck defaultFirebaseAppCheck = new DefaultFirebaseAppCheck(mockFirebaseApp);
defaultFirebaseAppCheck.setCachedToken(validDefaultAppCheckToken);
defaultFirebaseAppCheck.installAppCheckProviderFactory(mockAppCheckProviderFactory);

Expand All @@ -215,7 +228,6 @@ public void testGetToken_existingValidToken_doesNotRequestNewToken() {

@Test
public void testGetToken_existingValidToken_forceRefresh_requestsNewToken() {
DefaultFirebaseAppCheck defaultFirebaseAppCheck = new DefaultFirebaseAppCheck(mockFirebaseApp);
defaultFirebaseAppCheck.setCachedToken(validDefaultAppCheckToken);
defaultFirebaseAppCheck.installAppCheckProviderFactory(mockAppCheckProviderFactory);

Expand All @@ -228,8 +240,6 @@ public void testGetToken_existingValidToken_forceRefresh_requestsNewToken() {
public void testGetToken_existingInvalidToken_requestsNewToken() {
DefaultAppCheckToken invalidDefaultAppCheckToken =
new DefaultAppCheckToken(TOKEN_PAYLOAD, EXPIRES_NOW);

DefaultFirebaseAppCheck defaultFirebaseAppCheck = new DefaultFirebaseAppCheck(mockFirebaseApp);
defaultFirebaseAppCheck.setCachedToken(invalidDefaultAppCheckToken);
defaultFirebaseAppCheck.installAppCheckProviderFactory(mockAppCheckProviderFactory);

Expand Down
Loading