Skip to content

FIS getAuthToken implementation. #769

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 31 commits into from
Sep 16, 2019
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
c54292e
FIS getAuthToken implementation.
ankitaj224 Sep 5, 2019
b4b6ba8
FIS getAuthToken implementation.
ankitaj224 Sep 5, 2019
b8bfc1a
Merge branch 'fis_auth' of github.com:firebase/firebase-android-sdk i…
ankitaj224 Sep 5, 2019
7a2a112
Merge branch 'fis_auth' of github.com:firebase/firebase-android-sdk i…
ankitaj224 Sep 5, 2019
42c20f6
Merge branch 'fis_auth' of github.com:firebase/firebase-android-sdk i…
ankitaj224 Sep 5, 2019
46d936c
Merge branch 'fis_sdk' of github.com:firebase/firebase-android-sdk in…
ankitaj224 Sep 5, 2019
48fd7fe
Merge branch 'fis_sdk' of github.com:firebase/firebase-android-sdk in…
ankitaj224 Sep 5, 2019
82fb69e
1. Addressing Ciaran's comments
ankitaj224 Sep 6, 2019
fe04e15
1. Addressing Ciaran's comments
ankitaj224 Sep 6, 2019
e91cb7e
Merge branch 'fis_auth' of github.com:firebase/firebase-android-sdk i…
ankitaj224 Sep 6, 2019
8569952
Merge branch 'fis_auth' of github.com:firebase/firebase-android-sdk i…
ankitaj224 Sep 6, 2019
b74b163
Merge branch 'fis_auth' of github.com:firebase/firebase-android-sdk i…
ankitaj224 Sep 6, 2019
58a30e3
Updated IntDef usage in firebase-installations
ankitaj224 Sep 6, 2019
96e30c2
Using CountDownLatch to await instead of executor.awaitTermination
ankitaj224 Sep 7, 2019
9e3e34d
Addressing Di's comments.
ankitaj224 Sep 10, 2019
824cb63
Addressing Di's comments.
ankitaj224 Sep 10, 2019
90673c3
Merge branch 'fis_auth' of github.com:firebase/firebase-android-sdk i…
ankitaj224 Sep 10, 2019
f5b4f8c
Addressing Ciaran's comments to replace latch with a custom listener.
ankitaj224 Sep 10, 2019
3e29941
Adding onSuccess to AwaitListener.
ankitaj224 Sep 10, 2019
75f2581
Cleaning up the code as per the ciaran's comments.
ankitaj224 Sep 11, 2019
91e49bc
Addressing Di's comments.
ankitaj224 Sep 11, 2019
4578880
1. Handling multiple calls to getAUthToken()
ankitaj224 Sep 13, 2019
7d19b50
Fixing api-information presubmit check.
ankitaj224 Sep 13, 2019
fe04884
Merge branch 'fis_sdk' of github.com:firebase/firebase-android-sdk in…
ankitaj224 Sep 13, 2019
f1d1d37
Merge branch 'fis_sdk' of github.com:firebase/firebase-android-sdk in…
ankitaj224 Sep 13, 2019
ec65040
updating api.txt for firbase-installations-interop
ankitaj224 Sep 13, 2019
396d273
Merge branch 'fis_sdk' of github.com:firebase/firebase-android-sdk in…
ankitaj224 Sep 13, 2019
81b4aeb
Addressing Ciaran's comments.
ankitaj224 Sep 13, 2019
3932d3b
Fixing check-changed - GoogleJavaFormat error.
ankitaj224 Sep 16, 2019
8d4d811
Addressing ciaran`s comments
ankitaj224 Sep 16, 2019
3f44b77
nit fixes to executor : use unblocking queue.
ankitaj224 Sep 16, 2019
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 @@ -31,7 +31,7 @@ public interface FirebaseInstallationsApi {
Task<String> getId();

/** Async function that returns a auth token(public key) of this Firebase app installation. */
Task<InstallationTokenResult> getAuthToken(boolean forceRefresh);
Task<String> getAuthToken(boolean forceRefresh);

/**
* Async function that deletes this Firebase app installation from Firebase backend. This call
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import com.google.firebase.installations.remote.FirebaseInstallationServiceClient;
import com.google.firebase.installations.remote.FirebaseInstallationServiceException;
import com.google.firebase.installations.remote.InstallationResponse;
import com.google.firebase.installations.remote.InstallationTokenResult;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.SynchronousQueue;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import com.google.firebase.installations.remote.FirebaseInstallationServiceClient;
import com.google.firebase.installations.remote.FirebaseInstallationServiceException;
import com.google.firebase.installations.remote.InstallationResponse;
import com.google.firebase.installations.remote.InstallationTokenResult;
import java.util.concurrent.Executor;
import java.util.concurrent.SynchronousQueue;
import java.util.concurrent.ThreadPoolExecutor;
Expand All @@ -54,6 +55,8 @@ public class FirebaseInstallations implements FirebaseInstallationsApi {
private final Clock clock;
private final Utils utils;

private static final long TOKEN_EXPIRATION_BUFFER = 3600L;

/** package private constructor. */
FirebaseInstallations(FirebaseApp firebaseApp) {
this(
Expand Down Expand Up @@ -115,11 +118,17 @@ public Task<String> getId() {
.onSuccessTask(this::registerFidIfNecessary);
}

/** Returns a auth token(public key) of this Firebase app installation. */
/**
* Returns a valid authentication token for the Firebase installation. Generates a new token if
* one doesn't exist, is expired or about to expire.
*
* <p>Should only be called if the Firebase Installation is registered.
*/
@NonNull
@Override
public Task<InstallationTokenResult> getAuthToken(boolean forceRefresh) {
return Tasks.forResult(InstallationTokenResult.builder().build());
public Task<String> getAuthToken(final boolean forceRefresh) {
return Tasks.call(executor, this::getId)
.continueWith(call(() -> refreshAuthTokenIfNecessary(forceRefresh)));
}

/**
Expand Down Expand Up @@ -169,6 +178,16 @@ private static <F, T> Continuation<F, T> orElse(@NonNull Supplier<T> supplier) {
};
}

@NonNull
private static <F, T> Continuation<F, T> call(@NonNull Supplier<T> supplier) {
return t -> {
if (!t.isComplete()) {
Tasks.await(t);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ciarand Instrumented tests fail sometimes because of this line. Its not strictly waiting for the prev Task (getId) to complete. Ends up calling refreshAuthTokenIfNecessary with empty PersistedFidEntry ( pointed you to that code).

Can you please help me to understand what am I missing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Test failure is alternating.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is correct. You're calling Tasks.call(executor, this::getId) but getId is already calling Tasks.call so you're doubly nesting your async operations.

Instead of this:

    return Tasks.call(executor, this::getId)
        .continueWith(call(() -> refreshAuthTokenIfNecessary(forceRefresh)));

I think you mean to do something like this:

    return getId().continueWith((idTask) -> refreshAuthTokenIfNecessary(idTask.getResult(), forceRefresh));

NOTE: The call to getResult will throw an Exception if the getId task failed, which will cause the return value of this method (the getAuthToken method) to be a failed Task, which is afaict what we want.

You're also not passing an explicit executor into the .continueWith call though, so the refreshAuthTokenIfNecessary method is getting run on the main thread (!). Given you're not allowed to call Tasks.await on the main thread, that seems like a bug.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Very good catch. You are right. That was the issue :) Fixed it. About the 'Tasks.await' it wasn't a bug until I called Tasks.call(executor, this::getId) . After the change recommended by you, I could not await in the Main Thread. I updated the code to await using the executor. Let me know what you think.

}
return supplier.get();
};
}

private PersistedFidEntry createAndPersistNewFid() throws FirebaseInstallationsException {
String fid = utils.createRandomFid();
persistFid(fid);
Expand Down Expand Up @@ -214,7 +233,7 @@ private void updatePersistedFidWithPendingStatus(String fid) {
private Void registerAndSaveFid(PersistedFidEntry persistedFidEntry)
throws FirebaseInstallationsException {
try {
long creationTime = TimeUnit.MILLISECONDS.toSeconds(clock.currentTimeMillis());
long creationTime = currentTime();

InstallationResponse installationResponse =
serviceClient.createFirebaseInstallation(
Expand Down Expand Up @@ -244,6 +263,79 @@ private Void registerAndSaveFid(PersistedFidEntry persistedFidEntry)
}
return null;
}

private String refreshAuthTokenIfNecessary(boolean forceRefresh)
throws FirebaseInstallationsException {

PersistedFidEntry persistedFidEntry = persistedFid.readPersistedFidEntryValue();

if (persistedFidEntry == null) {
throw new FirebaseInstallationsException(
"Failed to create Firebase Installation.",
FirebaseInstallationsException.Status.SDK_INTERNAL_ERROR);
}

return forceRefresh
? fetchAuthTokenFromServer(persistedFidEntry)
: getPersistedAuthToken(persistedFidEntry);
}

private String getPersistedAuthToken(PersistedFidEntry persistedFidEntry)
throws FirebaseInstallationsException {
if (!isPersistedFidRegistered(persistedFidEntry)) {
throw new FirebaseInstallationsException(
"Firebase Installation is not registered.",
FirebaseInstallationsException.Status.SDK_INTERNAL_ERROR);
}

return isAuthTokenExpired(persistedFidEntry)
? fetchAuthTokenFromServer(persistedFidEntry)
: persistedFidEntry.getAuthToken();
}

private boolean isPersistedFidRegistered(PersistedFidEntry persistedFidEntry) {
return persistedFidEntry != null
&& persistedFidEntry.getRegistrationStatus() == RegistrationStatus.REGISTERED;
}

/** Calls the FIS servers to generate an auth token for this Firebase installation. */
private String fetchAuthTokenFromServer(PersistedFidEntry persistedFidEntry)
throws FirebaseInstallationsException {
try {
long creationTime = currentTime();
InstallationTokenResult tokenResult =
serviceClient.generateAuthToken(
/*apiKey= */ firebaseApp.getOptions().getApiKey(),
/*fid= */ persistedFidEntry.getFirebaseInstallationId(),
/*projectID= */ firebaseApp.getOptions().getProjectId(),
/*refreshToken= */ persistedFidEntry.getRefreshToken());

persistedFid.insertOrUpdatePersistedFidEntry(
PersistedFidEntry.builder()
.setFirebaseInstallationId(persistedFidEntry.getFirebaseInstallationId())
.setRegistrationStatus(RegistrationStatus.REGISTERED)
.setAuthToken(tokenResult.getToken())
.setRefreshToken(persistedFidEntry.getRefreshToken())
.setExpiresInSecs(tokenResult.getTokenExpirationTimestampMillis())
.setTokenCreationEpochInSecs(creationTime)
.build());

return tokenResult.getToken();
} catch (FirebaseInstallationServiceException exception) {
throw new FirebaseInstallationsException(
"Failed to generate auth token for a Firebase Installation.",
FirebaseInstallationsException.Status.SDK_INTERNAL_ERROR);
}
}

private boolean isAuthTokenExpired(PersistedFidEntry persistedFidEntry) {
return (persistedFidEntry.getTokenCreationEpochInSecs() + persistedFidEntry.getExpiresInSecs()
< currentTime() + TOKEN_EXPIRATION_BUFFER);
}

private long currentTime() {
return TimeUnit.MILLISECONDS.toSeconds(clock.currentTimeMillis());
}
}

interface Supplier<T> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

import android.util.JsonReader;
import androidx.annotation.NonNull;
import com.google.firebase.installations.InstallationTokenResult;
import java.io.IOException;
import java.io.InputStreamReader;
import java.net.URL;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

import androidx.annotation.NonNull;
import com.google.auto.value.AutoValue;
import com.google.firebase.installations.InstallationTokenResult;

@AutoValue
public abstract class InstallationResponse {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
// Copyright 2019 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.installations.remote;

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

/** This class represents a set of values describing a FIS Auth Token Result. */
@AutoValue
public abstract class InstallationTokenResult {

/** A new FIS Auth-Token, created for this Firebase Installation. */
@NonNull
public abstract String getToken();
/**
* The amount of time, in milliseconds, before the auth-token expires for this Firebase
* Installation.
*/
@NonNull
public abstract long getTokenExpirationTimestampMillis();

@NonNull
public abstract Builder toBuilder();

/** Returns a default Builder object to create an InstallationResponse object */
@NonNull
public static InstallationTokenResult.Builder builder() {
return new AutoValue_InstallationTokenResult.Builder();
}

@AutoValue.Builder
public abstract static class Builder {
@NonNull
public abstract Builder setToken(@NonNull String value);

@NonNull
public abstract Builder setTokenExpirationTimestampMillis(@NonNull long value);

@NonNull
public abstract InstallationTokenResult build();
}
}