Skip to content

Refetch token if attempt was invalidated #1628

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
Jun 8, 2020
Merged

Refetch token if attempt was invalidated #1628

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
e46b62d
Refetch token if attempt was invalidated
schmidt-sebastian Jun 8, 2020
d041342
Udpate changelog
schmidt-sebastian Jun 8, 2020
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
2 changes: 2 additions & 0 deletions firebase-firestore/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
# Unreleased
- [fixed] Fixed an issue that may have prevented the client from connecting
to the backend immediately after a user signed in.

# 21.4.3
- [changed] Firestore now limits the number of concurrent document lookups it
Expand Down
21 changes: 13 additions & 8 deletions ...ore/src/main/java/com/google/firebase/firestore/auth/FirebaseAuthCredentialsProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,14 @@
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import com.google.android.gms.tasks.Task;
import com.google.android.gms.tasks.Tasks;
import com.google.firebase.FirebaseApp;
import com.google.firebase.auth.GetTokenResult;
import com.google.firebase.auth.internal.IdTokenListener;
import com.google.firebase.auth.internal.InternalAuthProvider;
import com.google.firebase.firestore.FirebaseFirestoreException;
import com.google.firebase.firestore.FirebaseFirestoreException.Code;
import com.google.firebase.firestore.util.Executors;
import com.google.firebase.firestore.util.Listener;
import com.google.firebase.firestore.util.Logger;

/**
* FirebaseAuthCredentialsProvider uses Firebase Auth via {@link FirebaseApp} to get an auth token.
Expand All @@ -37,6 +38,8 @@
*/
public final class FirebaseAuthCredentialsProvider extends CredentialsProvider {

private static final String LOG_TAG = "FirebaseAuthCredentialsProvider";

private final InternalAuthProvider authProvider;

/**
Expand Down Expand Up @@ -85,19 +88,21 @@ public synchronized Task<String> getToken() {
// Take note of the current value of the tokenCounter so that this method can fail (with a
// FirebaseFirestoreException) if there is a token change while the request is outstanding.
final int savedCounter = tokenCounter;
return res.continueWith(
return res.continueWithTask(
Executors.DIRECT_EXECUTOR,
task -> {
synchronized (this) {
// Cancel the request since the token changed while the request was outstanding so the
// response is potentially for a previous user (which user, we can't be sure).
if (savedCounter != tokenCounter) {
throw new FirebaseFirestoreException(
"getToken aborted due to token change", Code.ABORTED);
Logger.debug(LOG_TAG, "getToken aborted due to token change");
return getToken();
Copy link
Contributor

Choose a reason for hiding this comment

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

General observation: it seems a little unfortunate that getToken and getResult().getToken() look so similar while having a very different effect. In general, I'd expect a get-prefixed function to be a simple getter, so it might be better to rename getToken to something like fetchToken to better convey its semantics. Of course, this is absolutely out of scope for this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ack :)

}
if (!task.isSuccessful()) {
throw task.getException();

if (task.isSuccessful()) {
return Tasks.forResult(task.getResult().getToken());
} else {
return task.getResult().getToken();
return Tasks.forException(task.getException());
}
}
});
Expand Down