Change user_id into string Cloud users are going to be represented by strings, probably email address. Integer prefix is used to avoid collisions between pairing/anonymous and local users. BUG=25768507 Change-Id: I27249c0b98f919e9527498be74ddaa82218b4041 Reviewed-on: https://weave-review.googlesource.com/2063 Reviewed-by: Alex Vakulenko <avakulenko@google.com>
diff --git a/src/privet/auth_manager.cc b/src/privet/auth_manager.cc index 86a564d..4d0d2a3 100644 --- a/src/privet/auth_manager.cc +++ b/src/privet/auth_manager.cc
@@ -33,36 +33,34 @@ array->insert(array->end(), begin, begin + sizeof(value)); } -// Returns "scope:id:time". +// Returns "scope:time:id". std::string CreateTokenData(const UserInfo& user_info, const base::Time& time) { return base::IntToString(static_cast<int>(user_info.scope())) + - kTokenDelimeter + base::Uint64ToString(user_info.user_id()) + - kTokenDelimeter + base::Int64ToString(time.ToTimeT()); + kTokenDelimeter + std::to_string(time.ToTimeT()) + kTokenDelimeter + + user_info.user_id(); } -// Splits string of "scope:id:time" format. +// Splits string of "scope:time:id" format. UserInfo SplitTokenData(const std::string& token, base::Time* time) { const UserInfo kNone; - auto parts = Split(token, kTokenDelimeter, false, false); - if (parts.size() != 3) + auto parts = SplitAtFirst(token, kTokenDelimeter, false); + if (parts.second.empty()) return kNone; int scope = 0; - if (!base::StringToInt(parts[0], &scope) || + if (!base::StringToInt(parts.first, &scope) || scope < static_cast<int>(AuthScope::kNone) || scope > static_cast<int>(AuthScope::kOwner)) { return kNone; } - uint64_t id{0}; - if (!base::StringToUint64(parts[1], &id)) + parts = SplitAtFirst(parts.second, kTokenDelimeter, false); + int64_t timestamp{0}; + if (parts.second.empty() || !base::StringToInt64(parts.first, ×tamp)) return kNone; - int64_t timestamp{0}; - if (!base::StringToInt64(parts[2], ×tamp)) - return kNone; if (time) *time = base::Time::FromTimeT(timestamp); - return UserInfo{static_cast<AuthScope>(scope), id}; + return UserInfo{static_cast<AuthScope>(scope), parts.second}; } class Caveat { @@ -140,7 +138,7 @@ AuthManager::~AuthManager() {} -// Returns "[hmac]scope:id:expiration_time". +// Returns "[hmac]scope:expiration_time:id". std::vector<uint8_t> AuthManager::CreateAccessToken(const UserInfo& user_info, base::TimeDelta ttl) const { std::string data_str{CreateTokenData(user_info, Now() + ttl)};
diff --git a/src/privet/auth_manager_unittest.cc b/src/privet/auth_manager_unittest.cc index d656fed..137e046 100644 --- a/src/privet/auth_manager_unittest.cc +++ b/src/privet/auth_manager_unittest.cc
@@ -64,52 +64,52 @@ } TEST_F(AuthManagerTest, CreateAccessToken) { - EXPECT_EQ("OUH2L2npY+Gzwjf9AnqigGSK3hxIVR+xX8/Cnu4DGf8wOjA6MTQxMDAwMDAwMA==", - Base64Encode( - auth_.CreateAccessToken(UserInfo{AuthScope::kNone, 123}, {}))); - EXPECT_EQ("iZx0qgEHFF5lq+Q503GtgU0d6gLQ9TlLsU+DcFbZb2QxOjIzNDoxNDEwMDAwMDAw", + EXPECT_EQ("xy58xYHWQUp5VT2tDmYLNIgdRiJ5ZQbgyCQuOGyQa5AwOjE0MTAwMDAwMDA6", Base64Encode(auth_.CreateAccessToken( - UserInfo{AuthScope::kViewer, 234}, {}))); - EXPECT_EQ("cWkAHxSBYtTFV3Va/9mcynR8iFZo2qr+8+WewmumF74zOjI1NzoxNDEwMDAwMDAw", + UserInfo{AuthScope::kNone, "123"}, {}))); + EXPECT_EQ("U0U8NQ4J0btA4kwPkG8deFkDLQOPPANbw53gGmcTUMUxOjE0MTAwMDAwMDA6MjM0", Base64Encode(auth_.CreateAccessToken( - UserInfo{AuthScope::kManager, 257}, {}))); - EXPECT_EQ("s3GnCThkQXIzGQoPDlJoiehQiJ5yy4SYUVQzMN2kY0o0OjQ1NjoxNDEwMDAwMDAw", - Base64Encode( - auth_.CreateAccessToken(UserInfo{AuthScope::kOwner, 456}, {}))); + UserInfo{AuthScope::kViewer, "234"}, {}))); + EXPECT_EQ("BooVCAQZ+ectnq2RYrzL3qymLfGm5YLGp9NMCuXAM3EzOjE0MTAwMDAwMDA6MjU3", + Base64Encode(auth_.CreateAccessToken( + UserInfo{AuthScope::kManager, "257"}, {}))); + EXPECT_EQ("YjgJgNR2uDzfTxHokbuDfguwOB72/F9mbzDO2PehIS80OjE0MTAwMDAwMDA6NDU2", + Base64Encode(auth_.CreateAccessToken( + UserInfo{AuthScope::kOwner, "456"}, {}))); auto new_time = clock_.Now() + base::TimeDelta::FromDays(11); EXPECT_CALL(clock_, Now()).WillRepeatedly(Return(new_time)); - EXPECT_EQ("qAmlJykiPTnFljfOKSf3BUII9YZG8/ttzD76q+fII1YyOjM0NToxNDEwOTUwNDAw", - Base64Encode( - auth_.CreateAccessToken(UserInfo{AuthScope::kUser, 345}, {}))); + EXPECT_EQ("d3t7075JF0Vb/c/Ihunk+xn2gxzUkHotdaIS9vc+A6kyOjE0MTA5NTA0MDA6MzQ1", + Base64Encode(auth_.CreateAccessToken( + UserInfo{AuthScope::kUser, "345"}, {}))); } TEST_F(AuthManagerTest, CreateSameToken) { - EXPECT_EQ(auth_.CreateAccessToken(UserInfo{AuthScope::kViewer, 555}, {}), - auth_.CreateAccessToken(UserInfo{AuthScope::kViewer, 555}, {})); + EXPECT_EQ(auth_.CreateAccessToken(UserInfo{AuthScope::kViewer, "555"}, {}), + auth_.CreateAccessToken(UserInfo{AuthScope::kViewer, "555"}, {})); } TEST_F(AuthManagerTest, CreateTokenDifferentScope) { - EXPECT_NE(auth_.CreateAccessToken(UserInfo{AuthScope::kViewer, 456}, {}), - auth_.CreateAccessToken(UserInfo{AuthScope::kOwner, 456}, {})); + EXPECT_NE(auth_.CreateAccessToken(UserInfo{AuthScope::kViewer, "456"}, {}), + auth_.CreateAccessToken(UserInfo{AuthScope::kOwner, "456"}, {})); } TEST_F(AuthManagerTest, CreateTokenDifferentUser) { - EXPECT_NE(auth_.CreateAccessToken(UserInfo{AuthScope::kOwner, 456}, {}), - auth_.CreateAccessToken(UserInfo{AuthScope::kOwner, 789}, {})); + EXPECT_NE(auth_.CreateAccessToken(UserInfo{AuthScope::kOwner, "456"}, {}), + auth_.CreateAccessToken(UserInfo{AuthScope::kOwner, "789"}, {})); } TEST_F(AuthManagerTest, CreateTokenDifferentTime) { - auto token = auth_.CreateAccessToken(UserInfo{AuthScope::kOwner, 567}, {}); + auto token = auth_.CreateAccessToken(UserInfo{AuthScope::kOwner, "567"}, {}); EXPECT_CALL(clock_, Now()) .WillRepeatedly(Return(base::Time::FromTimeT(1400000000))); EXPECT_NE(token, - auth_.CreateAccessToken(UserInfo{AuthScope::kOwner, 567}, {})); + auth_.CreateAccessToken(UserInfo{AuthScope::kOwner, "567"}, {})); } TEST_F(AuthManagerTest, CreateTokenDifferentInstance) { - EXPECT_NE(auth_.CreateAccessToken(UserInfo{AuthScope::kUser, 123}, {}), + EXPECT_NE(auth_.CreateAccessToken(UserInfo{AuthScope::kUser, "123"}, {}), AuthManager({}, {}).CreateAccessToken( - UserInfo{AuthScope::kUser, 123}, {})); + UserInfo{AuthScope::kUser, "123"}, {})); } TEST_F(AuthManagerTest, ParseAccessToken) { @@ -120,14 +120,14 @@ AuthManager auth{{}, {}, {}, &clock_}; - auto token = auth.CreateAccessToken(UserInfo{AuthScope::kUser, 5}, + auto token = auth.CreateAccessToken(UserInfo{AuthScope::kUser, "5"}, base::TimeDelta::FromSeconds(i)); base::Time time2; UserInfo user_info; EXPECT_FALSE(auth_.ParseAccessToken(token, &user_info, nullptr)); EXPECT_TRUE(auth.ParseAccessToken(token, &user_info, nullptr)); EXPECT_EQ(AuthScope::kUser, user_info.scope()); - EXPECT_EQ(5u, user_info.user_id()); + EXPECT_EQ("5", user_info.user_id()); EXPECT_CALL(clock_, Now()) .WillRepeatedly(Return(kStartTime + base::TimeDelta::FromSeconds(i)));
diff --git a/src/privet/cloud_delegate.cc b/src/privet/cloud_delegate.cc index ed22c15..4bf47b3 100644 --- a/src/privet/cloud_delegate.cc +++ b/src/privet/cloud_delegate.cc
@@ -165,7 +165,7 @@ const UserInfo& user_info, const CommandDoneCallback& callback) override { CHECK(user_info.scope() != AuthScope::kNone); - CHECK_NE(user_info.user_id(), 0u); + CHECK(!user_info.user_id().empty()); ErrorPtr error; UserRole role; @@ -230,8 +230,8 @@ private: void OnCommandAdded(Command* command) { - // Set to 0 for any new unknown command. - command_owners_.insert(std::make_pair(command->GetID(), 0)); + // Set to "" for any new unknown command. + command_owners_.insert(std::make_pair(command->GetID(), "")); } void OnCommandRemoved(Command* command) { @@ -310,11 +310,11 @@ return command; } - bool CanAccessCommand(uint64_t owner_id, + bool CanAccessCommand(const std::string& owner_id, const UserInfo& user_info, ErrorPtr* error) const { CHECK(user_info.scope() != AuthScope::kNone); - CHECK_NE(user_info.user_id(), 0u); + CHECK(!user_info.user_id().empty()); if (user_info.scope() == AuthScope::kManager || owner_id == user_info.user_id()) { @@ -343,7 +343,7 @@ int registation_retry_count_{0}; // Map of command IDs to user IDs. - std::map<std::string, uint64_t> command_owners_; + std::map<std::string, std::string> command_owners_; // Backoff entry for retrying device registration. BackoffEntry backoff_entry_{®ister_backoff_policy};
diff --git a/src/privet/mock_delegates.h b/src/privet/mock_delegates.h index 04ac338..476bc8d 100644 --- a/src/privet/mock_delegates.h +++ b/src/privet/mock_delegates.h
@@ -103,7 +103,7 @@ EXPECT_CALL(*this, ParseAccessToken(_, _, _)) .WillRepeatedly( - DoAll(SetArgPointee<1>(UserInfo{AuthScope::kViewer, 1234567}), + DoAll(SetArgPointee<1>(UserInfo{AuthScope::kViewer, "1234567"}), Return(true))); EXPECT_CALL(*this, GetPairingTypes())
diff --git a/src/privet/privet_handler_unittest.cc b/src/privet/privet_handler_unittest.cc index 336be3b..6fa6f35 100644 --- a/src/privet/privet_handler_unittest.cc +++ b/src/privet/privet_handler_unittest.cc
@@ -484,8 +484,8 @@ PrivetHandlerTest::SetUp(); auth_header_ = "Privet 123"; EXPECT_CALL(security_, ParseAccessToken(_, _, _)) - .WillRepeatedly(DoAll(SetArgPointee<1>(UserInfo{AuthScope::kOwner, 1}), - Return(true))); + .WillRepeatedly(DoAll( + SetArgPointee<1>(UserInfo{AuthScope::kOwner, "1"}), Return(true))); } }; @@ -660,8 +660,8 @@ TEST_F(PrivetHandlerSetupTest, GcdSetupAsMaster) { EXPECT_CALL(security_, ParseAccessToken(_, _, _)) - .WillRepeatedly(DoAll(SetArgPointee<1>(UserInfo{AuthScope::kManager, 1}), - Return(true))); + .WillRepeatedly(DoAll( + SetArgPointee<1>(UserInfo{AuthScope::kManager, "1"}), Return(true))); const char kInput[] = R"({ 'gcd': { 'ticketId': 'testTicket',
diff --git a/src/privet/privet_types.h b/src/privet/privet_types.h index 536fb89..c738865 100644 --- a/src/privet/privet_types.h +++ b/src/privet/privet_types.h
@@ -32,14 +32,15 @@ class UserInfo { public: - explicit UserInfo(AuthScope scope = AuthScope::kNone, uint64_t user_id = 0) - : scope_{scope}, user_id_{scope == AuthScope::kNone ? 0 : user_id} {} + explicit UserInfo(AuthScope scope = AuthScope::kNone, + const std::string& user_id = {}) + : scope_{scope}, user_id_{scope == AuthScope::kNone ? "" : user_id} {} AuthScope scope() const { return scope_; } - uint64_t user_id() const { return user_id_; } + const std::string& user_id() const { return user_id_; } private: AuthScope scope_; - uint64_t user_id_; + std::string user_id_; }; class ConnectionState final {
diff --git a/src/privet/security_manager.cc b/src/privet/security_manager.cc index b30bb04..7c44963 100644 --- a/src/privet/security_manager.cc +++ b/src/privet/security_manager.cc
@@ -134,7 +134,9 @@ return false; } - UserInfo user_info{desired_scope, ++last_user_id_}; + UserInfo user_info{desired_scope, + std::to_string(static_cast<int>(auth_type)) + "/" + + std::to_string(++last_user_id_)}; base::TimeDelta ttl = base::TimeDelta::FromSeconds(kAccessTokenExpirationSeconds);
diff --git a/src/privet/security_manager_unittest.cc b/src/privet/security_manager_unittest.cc index 7085e83..3c15d7e 100644 --- a/src/privet/security_manager_unittest.cc +++ b/src/privet/security_manager_unittest.cc
@@ -131,26 +131,27 @@ &task_runner_}; }; -TEST_F(SecurityManagerTest, CreateAccessToken) { - std::string token; - AuthScope scope; - base::TimeDelta ttl; - EXPECT_TRUE(security_.CreateAccessToken(AuthType::kAnonymous, "", - AuthScope::kUser, &token, &scope, - &ttl, nullptr)); - EXPECT_EQ("Cfz+qcndz8/Mo3ytgYlD7zn8qImkkdPsJVUNBmSOiXwyOjE6MTQxMDAwMzYwMA==", - token); - EXPECT_EQ(AuthScope::kUser, scope); - EXPECT_EQ(base::TimeDelta::FromHours(1), ttl); -} +TEST_F(SecurityManagerTest, AccessToken) { + AuthScope scopes[] = { + AuthScope::kViewer, AuthScope::kUser, AuthScope::kManager, + AuthScope::kOwner, + }; + for (size_t i = 1; i < 100; ++i) { + const AuthScope requested_scope = scopes[i % arraysize(scopes)]; + std::string token; + AuthScope scope; + base::TimeDelta ttl; + EXPECT_TRUE(security_.CreateAccessToken(AuthType::kAnonymous, "", + requested_scope, &token, &scope, + &ttl, nullptr)); + EXPECT_EQ(requested_scope, scope); + EXPECT_EQ(base::TimeDelta::FromHours(1), ttl); -TEST_F(SecurityManagerTest, ParseAccessToken) { - UserInfo info; - EXPECT_TRUE(security_.ParseAccessToken( - "MMe7FE+EMyG4OnD2457dF5Nqh9Uiaq2iRWRzkSOW+SAzOjk6MTQxMDAwMDkwMA==", &info, - nullptr)); - EXPECT_EQ(AuthScope::kManager, info.scope()); - EXPECT_EQ(9u, info.user_id()); + UserInfo info; + EXPECT_TRUE(security_.ParseAccessToken(token, &info, nullptr)); + EXPECT_EQ(requested_scope, info.scope()); + EXPECT_EQ("0/" + std::to_string(i), info.user_id()); + } } TEST_F(SecurityManagerTest, PairingNoSession) {