From 7b2aa01da3df89dbea23b7c73202c6bf3f5813d3 Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Thu, 7 Jun 2018 12:29:16 -0400 Subject: Force refresh token if RPC fails with "Unauthenticated" error (#1373) "Unauthenticated" is presumed to mean that token is expired (which might happen if local clock is wrong) and retried, subject to the usual backoff logic. --- .../firebase/firestore/auth/credentials_provider.h | 9 ++-- .../firestore/auth/empty_credentials_provider.cc | 9 ++-- .../firestore/auth/empty_credentials_provider.h | 3 +- .../auth/firebase_credentials_provider_apple.h | 6 ++- .../auth/firebase_credentials_provider_apple.mm | 10 ++-- .../auth/empty_credentials_provider_test.cc | 24 +++++---- .../auth/firebase_credentials_provider_test.mm | 62 +++++++++++++++------- 7 files changed, 83 insertions(+), 40 deletions(-) (limited to 'Firestore/core') diff --git a/Firestore/core/src/firebase/firestore/auth/credentials_provider.h b/Firestore/core/src/firebase/firestore/auth/credentials_provider.h index 0a1930a..d6ed39a 100644 --- a/Firestore/core/src/firebase/firestore/auth/credentials_provider.h +++ b/Firestore/core/src/firebase/firestore/auth/credentials_provider.h @@ -46,11 +46,14 @@ class CredentialsProvider { virtual ~CredentialsProvider(); + /** Requests token for the current user. */ + virtual void GetToken(TokenListener completion) = 0; + /** - * Requests token for the current user, optionally forcing a refreshed token - * to be fetched. + * Marks the last retrieved token as invalid, making the next `GetToken` + * request force refresh the token. */ - virtual void GetToken(bool force_refresh, TokenListener completion) = 0; + virtual void InvalidateToken() = 0; /** * Sets the listener to be notified of user changes (sign-in / sign-out). It diff --git a/Firestore/core/src/firebase/firestore/auth/empty_credentials_provider.cc b/Firestore/core/src/firebase/firestore/auth/empty_credentials_provider.cc index da4198d..77156cc 100644 --- a/Firestore/core/src/firebase/firestore/auth/empty_credentials_provider.cc +++ b/Firestore/core/src/firebase/firestore/auth/empty_credentials_provider.cc @@ -14,17 +14,13 @@ * limitations under the License. */ -#define UNUSED(x) (void)(x) - #include "Firestore/core/src/firebase/firestore/auth/empty_credentials_provider.h" namespace firebase { namespace firestore { namespace auth { -void EmptyCredentialsProvider::GetToken(bool force_refresh, - TokenListener completion) { - UNUSED(force_refresh); +void EmptyCredentialsProvider::GetToken(TokenListener completion) { if (completion) { // Unauthenticated token will force the GRPC fallback to use default // settings. @@ -39,6 +35,9 @@ void EmptyCredentialsProvider::SetUserChangeListener( } } +void EmptyCredentialsProvider::InvalidateToken() { +} + } // namespace auth } // namespace firestore } // namespace firebase diff --git a/Firestore/core/src/firebase/firestore/auth/empty_credentials_provider.h b/Firestore/core/src/firebase/firestore/auth/empty_credentials_provider.h index 55b3cc6..3ea0cab 100644 --- a/Firestore/core/src/firebase/firestore/auth/empty_credentials_provider.h +++ b/Firestore/core/src/firebase/firestore/auth/empty_credentials_provider.h @@ -26,7 +26,8 @@ namespace auth { /** `EmptyCredentialsProvider` always yields an empty token. */ class EmptyCredentialsProvider : public CredentialsProvider { public: - void GetToken(bool force_refresh, TokenListener completion) override; + void GetToken(TokenListener completion) override; + void InvalidateToken() override; void SetUserChangeListener(UserChangeListener listener) override; }; diff --git a/Firestore/core/src/firebase/firestore/auth/firebase_credentials_provider_apple.h b/Firestore/core/src/firebase/firestore/auth/firebase_credentials_provider_apple.h index 0e1da31..f54b72f 100644 --- a/Firestore/core/src/firebase/firestore/auth/firebase_credentials_provider_apple.h +++ b/Firestore/core/src/firebase/firestore/auth/firebase_credentials_provider_apple.h @@ -65,10 +65,12 @@ class FirebaseCredentialsProvider : public CredentialsProvider { ~FirebaseCredentialsProvider() override; - void GetToken(bool force_refresh, TokenListener completion) override; + void GetToken(TokenListener completion) override; void SetUserChangeListener(UserChangeListener listener) override; + void InvalidateToken() override; + private: /** * Most contents of the FirebaseCredentialProvider are kept in this @@ -95,6 +97,8 @@ class FirebaseCredentialsProvider : public CredentialsProvider { int user_counter = 0; std::mutex mutex; + + bool force_refresh = false; }; /** diff --git a/Firestore/core/src/firebase/firestore/auth/firebase_credentials_provider_apple.mm b/Firestore/core/src/firebase/firestore/auth/firebase_credentials_provider_apple.mm index 9d5b89e..74858c6 100644 --- a/Firestore/core/src/firebase/firestore/auth/firebase_credentials_provider_apple.mm +++ b/Firestore/core/src/firebase/firestore/auth/firebase_credentials_provider_apple.mm @@ -78,8 +78,7 @@ FirebaseCredentialsProvider::~FirebaseCredentialsProvider() { } } -void FirebaseCredentialsProvider::GetToken(bool force_refresh, - TokenListener completion) { +void FirebaseCredentialsProvider::GetToken(TokenListener completion) { HARD_ASSERT(auth_listener_handle_, "GetToken cannot be called after listener removed."); @@ -121,8 +120,13 @@ void FirebaseCredentialsProvider::GetToken(bool force_refresh, } }; - [contents_->app getTokenForcingRefresh:force_refresh + [contents_->app getTokenForcingRefresh:contents_->force_refresh withCallback:get_token_callback]; + contents_->force_refresh = false; +} + +void FirebaseCredentialsProvider::InvalidateToken() { + contents_->force_refresh = true; } void FirebaseCredentialsProvider::SetUserChangeListener( diff --git a/Firestore/core/test/firebase/firestore/auth/empty_credentials_provider_test.cc b/Firestore/core/test/firebase/firestore/auth/empty_credentials_provider_test.cc index 60845e5..a2f5780 100644 --- a/Firestore/core/test/firebase/firestore/auth/empty_credentials_provider_test.cc +++ b/Firestore/core/test/firebase/firestore/auth/empty_credentials_provider_test.cc @@ -25,15 +25,14 @@ namespace auth { TEST(EmptyCredentialsProvider, GetToken) { EmptyCredentialsProvider credentials_provider; - credentials_provider.GetToken( - /*force_refresh=*/true, [](util::StatusOr result) { - EXPECT_TRUE(result.ok()); - const Token& token = result.ValueOrDie(); - EXPECT_ANY_THROW(token.token()); - const User& user = token.user(); - EXPECT_EQ("", user.uid()); - EXPECT_FALSE(user.is_authenticated()); - }); + credentials_provider.GetToken([](util::StatusOr result) { + EXPECT_TRUE(result.ok()); + const Token& token = result.ValueOrDie(); + EXPECT_ANY_THROW(token.token()); + const User& user = token.user(); + EXPECT_EQ("", user.uid()); + EXPECT_FALSE(user.is_authenticated()); + }); } TEST(EmptyCredentialsProvider, SetListener) { @@ -46,6 +45,13 @@ TEST(EmptyCredentialsProvider, SetListener) { credentials_provider.SetUserChangeListener(nullptr); } +TEST(EmptyCredentialsProvider, InvalidateToken) { + EmptyCredentialsProvider credentials_provider; + credentials_provider.InvalidateToken(); + credentials_provider.GetToken( + [](util::StatusOr result) { EXPECT_TRUE(result.ok()); }); +} + } // namespace auth } // namespace firestore } // namespace firebase diff --git a/Firestore/core/test/firebase/firestore/auth/firebase_credentials_provider_test.mm b/Firestore/core/test/firebase/firestore/auth/firebase_credentials_provider_test.mm index 9d358b5..873f1b2 100644 --- a/Firestore/core/test/firebase/firestore/auth/firebase_credentials_provider_test.mm +++ b/Firestore/core/test/firebase/firestore/auth/firebase_credentials_provider_test.mm @@ -51,30 +51,28 @@ TEST(FirebaseCredentialsProviderTest, GetTokenUnauthenticated) { FIRApp* app = AppWithFakeUid(nil); FirebaseCredentialsProvider credentials_provider(app); - credentials_provider.GetToken( - /*force_refresh=*/true, [](util::StatusOr result) { - EXPECT_TRUE(result.ok()); - const Token& token = result.ValueOrDie(); - EXPECT_ANY_THROW(token.token()); - const User& user = token.user(); - EXPECT_EQ("", user.uid()); - EXPECT_FALSE(user.is_authenticated()); - }); + credentials_provider.GetToken([](util::StatusOr result) { + EXPECT_TRUE(result.ok()); + const Token& token = result.ValueOrDie(); + EXPECT_ANY_THROW(token.token()); + const User& user = token.user(); + EXPECT_EQ("", user.uid()); + EXPECT_FALSE(user.is_authenticated()); + }); } TEST(FirebaseCredentialsProviderTest, GetToken) { FIRApp* app = AppWithFakeUidAndToken(@"fake uid", @"token for fake uid"); FirebaseCredentialsProvider credentials_provider(app); - credentials_provider.GetToken( - /*force_refresh=*/true, [](util::StatusOr result) { - EXPECT_TRUE(result.ok()); - const Token& token = result.ValueOrDie(); - EXPECT_EQ("token for fake uid", token.token()); - const User& user = token.user(); - EXPECT_EQ("fake uid", user.uid()); - EXPECT_TRUE(user.is_authenticated()); - }); + credentials_provider.GetToken([](util::StatusOr result) { + EXPECT_TRUE(result.ok()); + const Token& token = result.ValueOrDie(); + EXPECT_EQ("token for fake uid", token.token()); + const User& user = token.user(); + EXPECT_EQ("fake uid", user.uid()); + EXPECT_TRUE(user.is_authenticated()); + }); } TEST(FirebaseCredentialsProviderTest, SetListener) { @@ -89,6 +87,34 @@ TEST(FirebaseCredentialsProviderTest, SetListener) { credentials_provider.SetUserChangeListener(nullptr); } +FIRApp* FakeAppExpectingForceRefreshToken(NSString* _Nullable uid, + NSString* _Nullable token) { + FIRApp* app = testutil::AppForUnitTesting(); + app.getUIDImplementation = ^NSString* { + return uid; + }; + app.getTokenImplementation = + ^(BOOL force_refresh, FIRTokenCallback callback) { + EXPECT_TRUE(force_refresh); + callback(token, nil); + }; + return app; +} + +TEST(FirebaseCredentialsProviderTest, InvalidateToken) { + FIRApp* app = + FakeAppExpectingForceRefreshToken(@"fake uid", @"token for fake uid"); + + FirebaseCredentialsProvider credentials_provider{app}; + credentials_provider.InvalidateToken(); + credentials_provider.GetToken([](util::StatusOr result) { + EXPECT_TRUE(result.ok()); + const Token& token = result.ValueOrDie(); + EXPECT_EQ("token for fake uid", token.token()); + EXPECT_EQ("fake uid", token.user().uid()); + }); +} + } // namespace auth } // namespace firestore } // namespace firebase -- cgit v1.2.3