From 308acc09bfaf6dabf4b6d5f5e39f33854df8ce34 Mon Sep 17 00:00:00 2001 From: rsgowman Date: Wed, 21 Mar 2018 11:04:40 -0400 Subject: Change CredentialsProvider::TokenListener to use StatusOr (#945) * Change CredentialsProvider::TokenListener to use StatusOr Rather than a token plus error code/msg. * Eliminate the concept of an invalid Token Instead, we'll just use StatusOr. Note that unauthenticated tokens are handled as a special case; they're created via: Token::Unauthenticated() and are otherwise "valid", though attempting to retrieve the raw token on one of these tokens will cause an assertion failure. --- .../firebase/firestore/auth/credentials_provider.h | 8 +--- .../firestore/auth/empty_credentials_provider.cc | 5 +- .../auth/firebase_credentials_provider_apple.mm | 56 ++++++++++++++-------- .../core/src/firebase/firestore/auth/token.cc | 12 ++--- Firestore/core/src/firebase/firestore/auth/token.h | 20 +++----- 5 files changed, 51 insertions(+), 50 deletions(-) (limited to 'Firestore/core/src/firebase/firestore/auth') diff --git a/Firestore/core/src/firebase/firestore/auth/credentials_provider.h b/Firestore/core/src/firebase/firestore/auth/credentials_provider.h index b9a8a24..1aa76df 100644 --- a/Firestore/core/src/firebase/firestore/auth/credentials_provider.h +++ b/Firestore/core/src/firebase/firestore/auth/credentials_provider.h @@ -23,6 +23,7 @@ #include "Firestore/core/include/firebase/firestore/firestore_errors.h" #include "Firestore/core/src/firebase/firestore/auth/token.h" #include "Firestore/core/src/firebase/firestore/auth/user.h" +#include "Firestore/core/src/firebase/firestore/util/statusor.h" #include "absl/strings/string_view.h" namespace firebase { @@ -30,12 +31,7 @@ namespace firestore { namespace auth { // `TokenErrorListener` is a listener that gets a token or an error. -// token: An auth token as a string, or nullptr if error occurred. -// error_code: The error code if one occurred, or else FirestoreErrorCode::Ok. -// error_msg: The error if one occurred, or else nullptr. -typedef std::function - TokenListener; +typedef std::function)> TokenListener; // Listener notified with a User change. typedef std::function UserChangeListener; 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 0fa70c0..da4198d 100644 --- a/Firestore/core/src/firebase/firestore/auth/empty_credentials_provider.cc +++ b/Firestore/core/src/firebase/firestore/auth/empty_credentials_provider.cc @@ -26,8 +26,9 @@ void EmptyCredentialsProvider::GetToken(bool force_refresh, TokenListener completion) { UNUSED(force_refresh); if (completion) { - // Invalid token will force the GRPC fallback to use default settings. - completion(Token::Invalid(), FirestoreErrorCode::Ok, ""); + // Unauthenticated token will force the GRPC fallback to use default + // settings. + completion(Token::Unauthenticated()); } } 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 1babe82..2bd3acc 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 @@ -23,6 +23,9 @@ #include "Firestore/core/src/firebase/firestore/util/firebase_assert.h" #include "Firestore/core/src/firebase/firestore/util/string_apple.h" +// NB: This is also defined in Firestore/Source/Public/FIRFirestoreErrors.h +NSString* const FIRFirestoreErrorDomain = @"FIRFirestoreErrorDomain"; + namespace firebase { namespace firestore { namespace auth { @@ -84,27 +87,38 @@ void FirebaseCredentialsProvider::GetToken(bool force_refresh, int initial_user_counter = contents_->user_counter; std::weak_ptr weak_contents = contents_; - void (^get_token_callback)(NSString*, NSError*) = ^( - NSString* _Nullable token, NSError* _Nullable error) { - std::shared_ptr contents = weak_contents.lock(); - if (!contents) { - return; - } - - std::unique_lock lock(contents->mutex); - if (initial_user_counter != contents->user_counter) { - // Cancel the request since the user changed while the request was - // outstanding so the response is likely for a previous user (which - // user, we can't be sure). - completion(Token::Invalid(), FirestoreErrorCode::Aborted, - "getToken aborted due to user change."); - } else { - completion( - Token{util::MakeStringView(token), contents->current_user}, - error == nil ? FirestoreErrorCode::Ok : error.code, - error == nil ? "" : util::MakeStringView(error.localizedDescription)); - } - }; + void (^get_token_callback)(NSString*, NSError*) = + ^(NSString* _Nullable token, NSError* _Nullable error) { + std::shared_ptr contents = weak_contents.lock(); + if (!contents) { + return; + } + + std::unique_lock lock(contents->mutex); + if (initial_user_counter != contents->user_counter) { + // Cancel the request since the user changed while the request was + // outstanding so the response is likely for a previous user (which + // user, we can't be sure). + completion(util::Status(FirestoreErrorCode::Aborted, + "getToken aborted due to user change.")); + } else { + if (error == nil) { + if (token != nil) { + completion( + Token{util::MakeStringView(token), contents->current_user}); + } else { + completion(Token::Unauthenticated()); + } + } else { + FirestoreErrorCode error_code = FirestoreErrorCode::Unknown; + if (error.domain == FIRFirestoreErrorDomain) { + error_code = static_cast(error.code); + } + completion(util::Status( + error_code, util::MakeStringView(error.localizedDescription))); + } + } + }; [contents_->app getTokenForcingRefresh:force_refresh withCallback:get_token_callback]; diff --git a/Firestore/core/src/firebase/firestore/auth/token.cc b/Firestore/core/src/firebase/firestore/auth/token.cc index 4ee1b69..6fe5fc4 100644 --- a/Firestore/core/src/firebase/firestore/auth/token.cc +++ b/Firestore/core/src/firebase/firestore/auth/token.cc @@ -21,15 +21,13 @@ namespace firestore { namespace auth { Token::Token(const absl::string_view token, const User& user) - : token_(token), user_(user), is_valid_(true) { + : token_(token), user_(user) { } -Token::Token() : token_(), user_(User::Unauthenticated()), is_valid_(false) { -} - -const Token& Token::Invalid() { - static const Token kInvalidToken; - return kInvalidToken; +const Token& Token::Unauthenticated() { + static const Token kUnauthenticatedToken(absl::string_view(), + User::Unauthenticated()); + return kUnauthenticatedToken; } } // namespace auth diff --git a/Firestore/core/src/firebase/firestore/auth/token.h b/Firestore/core/src/firebase/firestore/auth/token.h index ff8d2f0..4b2f3aa 100644 --- a/Firestore/core/src/firebase/firestore/auth/token.h +++ b/Firestore/core/src/firebase/firestore/auth/token.h @@ -46,7 +46,7 @@ class Token { /** The actual raw token. */ const std::string& token() const { - FIREBASE_ASSERT(is_valid_); + FIREBASE_ASSERT(user_.is_authenticated()); return token_; } @@ -59,25 +59,17 @@ class Token { } /** - * Whether the token is a valid one. + * Returns a token for an unauthenticated user. * - * ## Portability notes: Invalid token is the equivalent of nil in the iOS - * token implementation. We use value instead of pointer for Token instance in - * the C++ migration. + * ## Portability notes: An unauthenticated token is the equivalent of + * nil/null in the iOS/TypeScript token implementation. We use a reference + * instead of a pointer for Token instances in the C++ migration. */ - bool is_valid() const { - return is_valid_; - } - - /** Returns an invalid token. */ - static const Token& Invalid(); + static const Token& Unauthenticated(); private: - Token(); - const std::string token_; const User user_; - const bool is_valid_; }; } // namespace auth -- cgit v1.2.3