Change interface of ClaimRootClientAuthToken and ConfirmAuthToken Add RootClientTokenOwner argument to check if this owner can claim device. Add ErrorPtr to return error in privet response. BUG=25766815 Change-Id: I508c934e23092514e37b1f4790f0f1e693583ae1 Reviewed-on: https://weave-review.googlesource.com/1939 Reviewed-by: Alex Vakulenko <avakulenko@google.com>
diff --git a/src/device_registration_info.cc b/src/device_registration_info.cc index f52c292..59a49b2 100644 --- a/src/device_registration_info.cc +++ b/src/device_registration_info.cc
@@ -933,11 +933,20 @@ void DeviceRegistrationInfo::SendAuthInfo() { if (!auth_manager_ || auth_info_update_inprogress_) return; + + if (GetSettings().root_client_token_owner == RootClientTokenOwner::kCloud) { + // Avoid re-claiming if device is already claimed by the Cloud. Cloud is + // allowed to re-claim device at any time. However this will invalidate all + // issued tokens. + return; + } + auth_info_update_inprogress_ = true; + std::vector<uint8_t> token = auth_manager_->ClaimRootClientAuthToken( + RootClientTokenOwner::kCloud, nullptr); + CHECK(!token.empty()); std::string id = GetSettings().device_id; - std::vector<uint8_t> token = - auth_manager_->ClaimRootClientAuthToken(RootClientTokenOwner::kCloud); std::string token_base64 = Base64Encode(token); std::string fingerprint = Base64Encode(auth_manager_->GetCertificateFingerprint()); @@ -962,7 +971,7 @@ CHECK(auth_info_update_inprogress_); auth_info_update_inprogress_ = false; - if (!error && auth_manager_->ConfirmAuthToken(token)) + if (!error && auth_manager_->ConfirmAuthToken(token, nullptr)) return; task_runner_->PostDelayedTask( @@ -1328,6 +1337,9 @@ connected_to_cloud_ = false; LOG(INFO) << "Device is unregistered from the cloud. Deleting credentials"; + if (auth_manager_) + auth_manager_->SetSecret({}, RootClientTokenOwner::kNone); + Config::Transaction change{config_}; // Keep cloud_id to switch to detect kInvalidCredentials after restart. change.set_robot_account("");
diff --git a/src/privet/auth_manager.cc b/src/privet/auth_manager.cc index fa1d685..f38b854 100644 --- a/src/privet/auth_manager.cc +++ b/src/privet/auth_manager.cc
@@ -146,7 +146,8 @@ } std::vector<uint8_t> AuthManager::ClaimRootClientAuthToken( - RootClientTokenOwner owner) { + RootClientTokenOwner owner, + ErrorPtr* error) { pending_claims_.push_back(std::make_pair( std::unique_ptr<AuthManager>{new AuthManager{nullptr, {}}}, owner)); if (pending_claims_.size() > kMaxPendingClaims) @@ -154,7 +155,8 @@ return pending_claims_.back().first->GetRootClientAuthToken(); } -bool AuthManager::ConfirmAuthToken(const std::vector<uint8_t>& token) { +bool AuthManager::ConfirmAuthToken(const std::vector<uint8_t>& token, + ErrorPtr* error) { // Cover case when caller sent confirm twice. if (pending_claims_.empty() && IsValidAuthToken(token)) return true;
diff --git a/src/privet/auth_manager.h b/src/privet/auth_manager.h index 62a1606..ce406ce 100644 --- a/src/privet/auth_manager.h +++ b/src/privet/auth_manager.h
@@ -44,16 +44,17 @@ base::Time Now() const; - std::vector<uint8_t> ClaimRootClientAuthToken(RootClientTokenOwner owner); - bool ConfirmAuthToken(const std::vector<uint8_t>& token); + std::vector<uint8_t> ClaimRootClientAuthToken(RootClientTokenOwner owner, + ErrorPtr* error); + bool ConfirmAuthToken(const std::vector<uint8_t>& token, ErrorPtr* error); std::vector<uint8_t> GetRootClientAuthToken() const; bool IsValidAuthToken(const std::vector<uint8_t>& token) const; - private: void SetSecret(const std::vector<uint8_t>& secret, RootClientTokenOwner owner); + private: Config* config_{nullptr}; base::DefaultClock default_clock_; base::Clock* clock_{&default_clock_};
diff --git a/src/privet/auth_manager_unittest.cc b/src/privet/auth_manager_unittest.cc index 4dd753b..7b4aae4 100644 --- a/src/privet/auth_manager_unittest.cc +++ b/src/privet/auth_manager_unittest.cc
@@ -148,31 +148,36 @@ } TEST_F(AuthManagerTest, ClaimRootClientAuthToken) { - auto token = auth_.ClaimRootClientAuthToken(RootClientTokenOwner::kCloud); + auto token = + auth_.ClaimRootClientAuthToken(RootClientTokenOwner::kCloud, nullptr); EXPECT_FALSE(auth_.IsValidAuthToken(token)); - EXPECT_TRUE(auth_.ConfirmAuthToken(token)); + EXPECT_TRUE(auth_.ConfirmAuthToken(token, nullptr)); EXPECT_TRUE(auth_.IsValidAuthToken(token)); } TEST_F(AuthManagerTest, ClaimRootClientAuthTokenDoubleConfirm) { - auto token = auth_.ClaimRootClientAuthToken(RootClientTokenOwner::kCloud); - EXPECT_TRUE(auth_.ConfirmAuthToken(token)); - EXPECT_TRUE(auth_.ConfirmAuthToken(token)); + auto token = + auth_.ClaimRootClientAuthToken(RootClientTokenOwner::kCloud, nullptr); + EXPECT_TRUE(auth_.ConfirmAuthToken(token, nullptr)); + EXPECT_TRUE(auth_.ConfirmAuthToken(token, nullptr)); } TEST_F(AuthManagerTest, DoubleClaimRootClientAuthToken) { - auto token1 = auth_.ClaimRootClientAuthToken(RootClientTokenOwner::kCloud); - auto token2 = auth_.ClaimRootClientAuthToken(RootClientTokenOwner::kCloud); - EXPECT_TRUE(auth_.ConfirmAuthToken(token1)); - EXPECT_FALSE(auth_.ConfirmAuthToken(token2)); + auto token1 = + auth_.ClaimRootClientAuthToken(RootClientTokenOwner::kCloud, nullptr); + auto token2 = + auth_.ClaimRootClientAuthToken(RootClientTokenOwner::kCloud, nullptr); + EXPECT_TRUE(auth_.ConfirmAuthToken(token1, nullptr)); + EXPECT_FALSE(auth_.ConfirmAuthToken(token2, nullptr)); } TEST_F(AuthManagerTest, ClaimRootClientAuthTokenOverflow) { - auto token = auth_.ClaimRootClientAuthToken(RootClientTokenOwner::kCloud); + auto token = + auth_.ClaimRootClientAuthToken(RootClientTokenOwner::kCloud, nullptr); for (size_t i = 0; i < 100; ++i) - auth_.ClaimRootClientAuthToken(RootClientTokenOwner::kCloud); - EXPECT_FALSE(auth_.ConfirmAuthToken(token)); + auth_.ClaimRootClientAuthToken(RootClientTokenOwner::kCloud, nullptr); + EXPECT_FALSE(auth_.ConfirmAuthToken(token, nullptr)); } } // namespace privet
diff --git a/src/privet/mock_delegates.h b/src/privet/mock_delegates.h index 6762481..9ae94a8 100644 --- a/src/privet/mock_delegates.h +++ b/src/privet/mock_delegates.h
@@ -13,6 +13,7 @@ #include <gmock/gmock.h> #include <gtest/gtest.h> +#include "src/config.h" #include "src/privet/cloud_delegate.h" #include "src/privet/device_delegate.h" #include "src/privet/security_delegate.h" @@ -24,6 +25,7 @@ using testing::SetArgPointee; namespace weave { + namespace privet { ACTION_TEMPLATE(RunCallback, @@ -64,8 +66,8 @@ UserInfo(const std::string&, base::Time*)); MOCK_CONST_METHOD0(GetPairingTypes, std::set<PairingType>()); MOCK_CONST_METHOD0(GetCryptoTypes, std::set<CryptoType>()); - MOCK_METHOD0(ClaimRootClientAuthToken, std::string()); - MOCK_METHOD1(ConfirmAuthToken, bool(const std::string& token)); + MOCK_METHOD1(ClaimRootClientAuthToken, std::string(ErrorPtr*)); + MOCK_METHOD2(ConfirmAuthToken, bool(const std::string&, ErrorPtr*)); MOCK_CONST_METHOD1(IsValidPairingCode, bool(const std::string&)); MOCK_METHOD5( StartPairing, @@ -82,7 +84,7 @@ EXPECT_CALL(*this, CreateAccessToken(_)) .WillRepeatedly(Return("GuestAccessToken")); - EXPECT_CALL(*this, ClaimRootClientAuthToken()) + EXPECT_CALL(*this, ClaimRootClientAuthToken(_)) .WillRepeatedly(Return("RootClientAuthToken")); EXPECT_CALL(*this, ParseAccessToken(_, _))
diff --git a/src/privet/security_delegate.h b/src/privet/security_delegate.h index 3446c48..adc582d 100644 --- a/src/privet/security_delegate.h +++ b/src/privet/security_delegate.h
@@ -35,11 +35,11 @@ virtual std::set<CryptoType> GetCryptoTypes() const = 0; // Returns Root Client Authorization Token. - virtual std::string ClaimRootClientAuthToken() = 0; + virtual std::string ClaimRootClientAuthToken(ErrorPtr* error) = 0; // Confirms pending pending token claim or checks that token is valid for the // active secret. - virtual bool ConfirmAuthToken(const std::string& token) = 0; + virtual bool ConfirmAuthToken(const std::string& token, ErrorPtr* error) = 0; // Returns true if |auth_code| provided by client is valid. Client should // obtain |auth_code| during pairing process.
diff --git a/src/privet/security_manager.cc b/src/privet/security_manager.cc index 5fa41be..5bb35cc 100644 --- a/src/privet/security_manager.cc +++ b/src/privet/security_manager.cc
@@ -135,16 +135,21 @@ return result; } -std::string SecurityManager::ClaimRootClientAuthToken() { - return Base64Encode( - auth_manager_->ClaimRootClientAuthToken(RootClientTokenOwner::kClient)); +std::string SecurityManager::ClaimRootClientAuthToken(ErrorPtr* error) { + return Base64Encode(auth_manager_->ClaimRootClientAuthToken( + RootClientTokenOwner::kClient, error)); } -bool SecurityManager::ConfirmAuthToken(const std::string& token) { +bool SecurityManager::ConfirmAuthToken(const std::string& token, + ErrorPtr* error) { std::vector<uint8_t> token_decoded; - if (!Base64Decode(token, &token_decoded)) + if (!Base64Decode(token, &token_decoded)) { + Error::AddToPrintf(error, FROM_HERE, errors::kDomain, + errors::kInvalidFormat, + "Invalid auth token string: '%s'", token.c_str()); return false; - return auth_manager_->ConfirmAuthToken(token_decoded); + } + return auth_manager_->ConfirmAuthToken(token_decoded, error); } bool SecurityManager::IsValidPairingCode(const std::string& auth_code) const {
diff --git a/src/privet/security_manager.h b/src/privet/security_manager.h index 5d9b75a..3ee6ac7 100644 --- a/src/privet/security_manager.h +++ b/src/privet/security_manager.h
@@ -65,8 +65,8 @@ base::Time* time) const override; std::set<PairingType> GetPairingTypes() const override; std::set<CryptoType> GetCryptoTypes() const override; - std::string ClaimRootClientAuthToken() override; - bool ConfirmAuthToken(const std::string& token) override; + std::string ClaimRootClientAuthToken(ErrorPtr* error) override; + bool ConfirmAuthToken(const std::string& token, ErrorPtr* error) override; bool IsValidPairingCode(const std::string& auth_code) const override; bool StartPairing(PairingType mode,