weave/settings: add device_id with persisted guid Bug: 24485657 Change-Id: Iad88280c83c962a4843f3d4def8ef64767303197 Reviewed-on: https://weave-review.googlesource.com/1181 Reviewed-by: Vitaly Buka <vitalybuka@google.com>
diff --git a/libweave/examples/ubuntu/avahi_client.cc b/libweave/examples/ubuntu/avahi_client.cc index a767402..7d822fb 100644 --- a/libweave/examples/ubuntu/avahi_client.cc +++ b/libweave/examples/ubuntu/avahi_client.cc
@@ -21,6 +21,10 @@ CHECK_NE(state, AVAHI_ENTRY_GROUP_FAILURE); } +std::string GetId() { + return "WEAVE" + std::to_string(gethostid()); +} + } // namespace AvahiClient::AvahiClient() { @@ -90,9 +94,5 @@ avahi_entry_group_reset(group_.get()); } -std::string AvahiClient::GetId() const { - return "WEAVE" + std::to_string(gethostid()); -} - } // namespace examples } // namespace weave
diff --git a/libweave/examples/ubuntu/avahi_client.h b/libweave/examples/ubuntu/avahi_client.h index ed955ec..ba4a02b 100644 --- a/libweave/examples/ubuntu/avahi_client.h +++ b/libweave/examples/ubuntu/avahi_client.h
@@ -27,7 +27,6 @@ uint16_t port, const std::vector<std::string>& txt) override; void StopPublishing(const std::string& service_name) override; - std::string GetId() const override; uint16_t prev_port_{0}; std::string prev_type_;
diff --git a/libweave/include/weave/provider/dns_service_discovery.h b/libweave/include/weave/provider/dns_service_discovery.h index dfd94eb..a6f2057 100644 --- a/libweave/include/weave/provider/dns_service_discovery.h +++ b/libweave/include/weave/provider/dns_service_discovery.h
@@ -23,10 +23,6 @@ // Stops publishing service. virtual void StopPublishing(const std::string& service_type) = 0; - // Returns permanent device ID. - // TODO(vitalybuka): Find better place for this information. - virtual std::string GetId() const = 0; - protected: virtual ~DnsServiceDiscovery() = default; };
diff --git a/libweave/include/weave/provider/test/mock_config_store.h b/libweave/include/weave/provider/test/mock_config_store.h index 872e666..b6ca69f 100644 --- a/libweave/include/weave/provider/test/mock_config_store.h +++ b/libweave/include/weave/provider/test/mock_config_store.h
@@ -34,7 +34,8 @@ settings->api_key = "TEST_API_KEY"; return true; })); - EXPECT_CALL(*this, LoadSettings()).WillRepeatedly(Return("")); + EXPECT_CALL(*this, LoadSettings()) + .WillRepeatedly(Return(R"({"device_id": "TEST_DEVICE_ID"})")); EXPECT_CALL(*this, SaveSettings(_)).WillRepeatedly(Return()); } MOCK_METHOD1(LoadDefaults, bool(Settings*));
diff --git a/libweave/include/weave/provider/test/mock_dns_service_discovery.h b/libweave/include/weave/provider/test/mock_dns_service_discovery.h index cfb070e..9c149df 100644 --- a/libweave/include/weave/provider/test/mock_dns_service_discovery.h +++ b/libweave/include/weave/provider/test/mock_dns_service_discovery.h
@@ -23,7 +23,6 @@ uint16_t, const std::vector<std::string>&)); MOCK_METHOD1(StopPublishing, void(const std::string&)); - MOCK_CONST_METHOD0(GetId, std::string()); }; } // namespace test
diff --git a/libweave/src/config.cc b/libweave/src/config.cc index d014198..4f59497 100644 --- a/libweave/src/config.cc +++ b/libweave/src/config.cc
@@ -7,6 +7,7 @@ #include <set> #include <base/bind.h> +#include <base/guid.h> #include <base/json/json_reader.h> #include <base/json/json_writer.h> #include <base/logging.h> @@ -34,6 +35,7 @@ const char kLocalPairingEnabled[] = "local_pairing_enabled"; const char kRefreshToken[] = "refresh_token"; const char kCloudId[] = "cloud_id"; +const char kDeviceId[] = "device_id"; const char kRobotAccount[] = "robot_account"; const char kLastConfiguredSsid[] = "last_configured_ssid"; const char kSecret[] = "secret"; @@ -48,6 +50,7 @@ result.service_url = "https://www.googleapis.com/clouddevices/v1/"; result.local_anonymous_access_role = AuthScope::kViewer; result.pairing_modes.emplace(PairingType::kPinCode); + result.device_id = base::GenerateGUID(); return result; } @@ -88,7 +91,7 @@ CHECK(!settings_.model_name.empty()); CHECK(!settings_.model_id.empty()); CHECK(!settings_.name.empty()); - + CHECK(!settings_.device_id.empty()); CHECK_EQ( settings_.embedded_code.empty(), std::find(settings_.pairing_modes.begin(), settings_.pairing_modes.end(), @@ -160,6 +163,9 @@ if (dict->GetString(config_keys::kCloudId, &tmp)) set_cloud_id(tmp); + if (dict->GetString(config_keys::kDeviceId, &tmp)) + set_device_id(tmp); + if (dict->GetString(config_keys::kRefreshToken, &tmp)) set_refresh_token(tmp); @@ -185,6 +191,7 @@ dict.SetString(config_keys::kServiceURL, settings_.service_url); dict.SetString(config_keys::kRefreshToken, settings_.refresh_token); dict.SetString(config_keys::kCloudId, settings_.cloud_id); + dict.SetString(config_keys::kDeviceId, settings_.device_id); dict.SetString(config_keys::kRobotAccount, settings_.robot_account); dict.SetString(config_keys::kLastConfiguredSsid, settings_.last_configured_ssid);
diff --git a/libweave/src/config.h b/libweave/src/config.h index 13e50da..bec99ea 100644 --- a/libweave/src/config.h +++ b/libweave/src/config.h
@@ -10,6 +10,7 @@ #include <vector> #include <base/callback.h> +#include <base/gtest_prod_util.h> #include <weave/error.h> #include <weave/provider/config_store.h> @@ -23,6 +24,7 @@ class Config final { public: struct Settings : public weave::Settings { + std::string device_id; std::string refresh_token; std::string robot_account; std::string last_configured_ssid; @@ -92,6 +94,11 @@ void Commit(); private: + FRIEND_TEST_ALL_PREFIXES(ConfigTest, Setters); + void set_device_id(const std::string& id) { + config_->settings_.device_id = id; + } + friend class Config; void LoadState(); Config* config_;
diff --git a/libweave/src/config_unittest.cc b/libweave/src/config_unittest.cc index 613a1ca..f65f203 100644 --- a/libweave/src/config_unittest.cc +++ b/libweave/src/config_unittest.cc
@@ -58,6 +58,7 @@ EXPECT_EQ("", GetSettings().oem_name); EXPECT_EQ("", GetSettings().model_name); EXPECT_EQ("", GetSettings().model_id); + EXPECT_FALSE(GetSettings().device_id.empty()); EXPECT_EQ("", GetSettings().firmware_version); EXPECT_TRUE(GetSettings().wifi_auto_setup_enabled); EXPECT_FALSE(GetSettings().disable_security); @@ -85,6 +86,7 @@ "client_secret": "state_client_secret", "cloud_id": "state_cloud_id", "description": "state_description", + "device_id": "state_device_id", "last_configured_ssid": "state_last_configured_ssid", "local_anonymous_access_role": "user", "local_discovery_enabled": false, @@ -110,6 +112,7 @@ EXPECT_EQ(GetDefaultSettings().oem_name, GetSettings().oem_name); EXPECT_EQ(GetDefaultSettings().model_name, GetSettings().model_name); EXPECT_EQ(GetDefaultSettings().model_id, GetSettings().model_id); + EXPECT_EQ("state_device_id", GetSettings().device_id); EXPECT_EQ(GetDefaultSettings().wifi_auto_setup_enabled, GetSettings().wifi_auto_setup_enabled); EXPECT_EQ(GetDefaultSettings().disable_security, @@ -182,6 +185,9 @@ change.set_cloud_id("set_cloud_id"); EXPECT_EQ("set_cloud_id", GetSettings().cloud_id); + change.set_device_id("set_device_id"); + EXPECT_EQ("set_device_id", GetSettings().device_id); + change.set_refresh_token("set_token"); EXPECT_EQ("set_token", GetSettings().refresh_token); @@ -204,6 +210,7 @@ 'client_secret': 'set_client_secret', 'cloud_id': 'set_cloud_id', 'description': 'set_description', + 'device_id': 'set_device_id', 'last_configured_ssid': 'set_last_configured_ssid', 'local_anonymous_access_role': 'user', 'local_discovery_enabled': true,
diff --git a/libweave/src/privet/cloud_delegate.cc b/libweave/src/privet/cloud_delegate.cc index 13f48a0..217a4ed 100644 --- a/libweave/src/privet/cloud_delegate.cc +++ b/libweave/src/privet/cloud_delegate.cc
@@ -65,6 +65,10 @@ ~CloudDelegateImpl() override = default; + std::string GetDeviceId() const override { + return device_->GetSettings().device_id; + } + std::string GetModelId() const override { CHECK_EQ(5u, device_->GetSettings().model_id.size()); return device_->GetSettings().model_id;
diff --git a/libweave/src/privet/cloud_delegate.h b/libweave/src/privet/cloud_delegate.h index 24c98fb..74456d3 100644 --- a/libweave/src/privet/cloud_delegate.h +++ b/libweave/src/privet/cloud_delegate.h
@@ -51,6 +51,9 @@ virtual void OnStateChanged() {} }; + // Returns the ID of the device. + virtual std::string GetDeviceId() const = 0; + // Returns the model ID of the device. virtual std::string GetModelId() const = 0;
diff --git a/libweave/src/privet/identity_delegate.h b/libweave/src/privet/identity_delegate.h deleted file mode 100644 index b0ab340..0000000 --- a/libweave/src/privet/identity_delegate.h +++ /dev/null
@@ -1,26 +0,0 @@ -// Copyright 2015 The Chromium OS Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef LIBWEAVE_SRC_PRIVET_IDENTITY_DELEGATE_H_ -#define LIBWEAVE_SRC_PRIVET_IDENTITY_DELEGATE_H_ - -#include <string> - -namespace weave { -namespace privet { - -// Interface for an object that can identify ourselves. -class IdentityDelegate { - public: - IdentityDelegate() = default; - virtual ~IdentityDelegate() = default; - - // Returns a unique identifier for this device. - virtual std::string GetId() const = 0; -}; - -} // namespace privet -} // namespace weave - -#endif // LIBWEAVE_SRC_PRIVET_IDENTITY_DELEGATE_H_
diff --git a/libweave/src/privet/mock_delegates.h b/libweave/src/privet/mock_delegates.h index 1944fbb..20f6877 100644 --- a/libweave/src/privet/mock_delegates.h +++ b/libweave/src/privet/mock_delegates.h
@@ -15,7 +15,6 @@ #include "src/privet/cloud_delegate.h" #include "src/privet/device_delegate.h" -#include "src/privet/identity_delegate.h" #include "src/privet/security_delegate.h" #include "src/privet/wifi_delegate.h" @@ -137,6 +136,7 @@ class MockCloudDelegate : public CloudDelegate { public: + MOCK_CONST_METHOD0(GetDeviceId, std::string()); MOCK_CONST_METHOD0(GetModelId, std::string()); MOCK_CONST_METHOD0(GetName, std::string()); MOCK_CONST_METHOD0(GetDescription, std::string()); @@ -176,6 +176,7 @@ const ErrorCallback&)); MockCloudDelegate() { + EXPECT_CALL(*this, GetDeviceId()).WillRepeatedly(Return("TestId")); EXPECT_CALL(*this, GetModelId()).WillRepeatedly(Return("ABMID")); EXPECT_CALL(*this, GetName()).WillRepeatedly(Return("TestDevice")); EXPECT_CALL(*this, GetDescription()).WillRepeatedly(Return("")); @@ -201,15 +202,6 @@ base::DictionaryValue test_dict_; }; -class MockIdentityDelegate : public IdentityDelegate { - public: - MOCK_CONST_METHOD0(GetId, std::string()); - - MockIdentityDelegate() { - EXPECT_CALL(*this, GetId()).WillRepeatedly(Return("TestId")); - } -}; - } // namespace privet } // namespace weave
diff --git a/libweave/src/privet/privet_handler.cc b/libweave/src/privet/privet_handler.cc index 4bc4822..47ee8f7 100644 --- a/libweave/src/privet/privet_handler.cc +++ b/libweave/src/privet/privet_handler.cc
@@ -20,7 +20,6 @@ #include "src/privet/cloud_delegate.h" #include "src/privet/constants.h" #include "src/privet/device_delegate.h" -#include "src/privet/identity_delegate.h" #include "src/privet/security_delegate.h" #include "src/privet/wifi_delegate.h" #include "src/string_utils.h" @@ -355,13 +354,8 @@ PrivetHandler::PrivetHandler(CloudDelegate* cloud, DeviceDelegate* device, SecurityDelegate* security, - WifiDelegate* wifi, - IdentityDelegate* identity) - : cloud_(cloud), - device_(device), - security_(security), - wifi_(wifi), - identity_(identity) { + WifiDelegate* wifi) + : cloud_(cloud), device_(device), security_(security), wifi_(wifi) { CHECK(cloud_); CHECK(device_); CHECK(security_); @@ -478,7 +472,7 @@ std::string model_id = cloud_->GetModelId(); output.SetString(kInfoVersionKey, kInfoVersionValue); - output.SetString(kInfoIdKey, identity_->GetId()); + output.SetString(kInfoIdKey, cloud_->GetDeviceId()); output.SetString(kNameKey, name); std::string description{cloud_->GetDescription()};
diff --git a/libweave/src/privet/privet_handler.h b/libweave/src/privet/privet_handler.h index 125c798..ce32485 100644 --- a/libweave/src/privet/privet_handler.h +++ b/libweave/src/privet/privet_handler.h
@@ -42,8 +42,7 @@ PrivetHandler(CloudDelegate* cloud, DeviceDelegate* device, SecurityDelegate* pairing, - WifiDelegate* wifi, - IdentityDelegate* identity); + WifiDelegate* wifi); ~PrivetHandler() override; void OnCommandDefsChanged() override; @@ -114,7 +113,6 @@ DeviceDelegate* device_ = nullptr; SecurityDelegate* security_ = nullptr; WifiDelegate* wifi_ = nullptr; - IdentityDelegate* identity_ = nullptr; std::map<std::string, std::pair<AuthScope, ApiHandler>> handlers_;
diff --git a/libweave/src/privet/privet_handler_unittest.cc b/libweave/src/privet/privet_handler_unittest.cc index 7f79934..001ac3d 100644 --- a/libweave/src/privet/privet_handler_unittest.cc +++ b/libweave/src/privet/privet_handler_unittest.cc
@@ -124,8 +124,7 @@ protected: void SetUp() override { auth_header_ = "Privet anonymous"; - handler_.reset( - new PrivetHandler(&cloud_, &device_, &security_, &wifi_, &identity_)); + handler_.reset(new PrivetHandler(&cloud_, &device_, &security_, &wifi_)); } const base::DictionaryValue& HandleRequest( @@ -153,8 +152,7 @@ } void SetNoWifiAndGcd() { - handler_.reset( - new PrivetHandler(&cloud_, &device_, &security_, nullptr, &identity_)); + handler_.reset(new PrivetHandler(&cloud_, &device_, &security_, nullptr)); EXPECT_CALL(cloud_, GetCloudId()).WillRepeatedly(Return("")); EXPECT_CALL(cloud_, GetConnectionState()) .WillRepeatedly(ReturnRef(gcd_disabled_state_)); @@ -170,7 +168,6 @@ testing::StrictMock<MockDeviceDelegate> device_; testing::StrictMock<MockSecurityDelegate> security_; testing::StrictMock<MockWifiDelegate> wifi_; - testing::StrictMock<MockIdentityDelegate> identity_; std::string auth_header_; private:
diff --git a/libweave/src/privet/privet_manager.cc b/libweave/src/privet/privet_manager.cc index d14765d..e7b0052 100644 --- a/libweave/src/privet/privet_manager.cc +++ b/libweave/src/privet/privet_manager.cc
@@ -77,9 +77,9 @@ publisher_.reset(new Publisher(device_.get(), cloud_.get(), wifi_bootstrap_manager_.get(), dns_sd)); - privet_handler_.reset( - new PrivetHandler(cloud_.get(), device_.get(), security_.get(), - wifi_bootstrap_manager_.get(), publisher_.get())); + privet_handler_.reset(new PrivetHandler(cloud_.get(), device_.get(), + security_.get(), + wifi_bootstrap_manager_.get())); http_server->AddOnStateChangedCallback(base::Bind( &Manager::OnHttpServerStatusChanged, weak_ptr_factory_.GetWeakPtr()));
diff --git a/libweave/src/privet/publisher.cc b/libweave/src/privet/publisher.cc index 0726b18..37da735 100644 --- a/libweave/src/privet/publisher.cc +++ b/libweave/src/privet/publisher.cc
@@ -39,10 +39,6 @@ RemoveService(); } -std::string Publisher::GetId() const { - return dns_sd_->GetId(); -} - void Publisher::Update() { if (device_->GetHttpEnpoint().first == 0) return RemoveService(); @@ -67,7 +63,7 @@ {"txtvers=3"}, {"ty=" + name}, {"services=" + services}, - {"id=" + GetId()}, + {"id=" + cloud_->GetDeviceId()}, {"mmid=" + model_id}, {"flags=" + WifiSsidGenerator{cloud_, wifi_}.GenerateFlags()}, };
diff --git a/libweave/src/privet/publisher.h b/libweave/src/privet/publisher.h index 68845af..ce417a7 100644 --- a/libweave/src/privet/publisher.h +++ b/libweave/src/privet/publisher.h
@@ -11,8 +11,6 @@ #include <base/macros.h> -#include "src/privet/identity_delegate.h" - namespace weave { namespace provider { @@ -26,16 +24,13 @@ class WifiDelegate; // Publishes privet service on DNS-SD. -class Publisher : public IdentityDelegate { +class Publisher { public: Publisher(const DeviceDelegate* device, const CloudDelegate* cloud, const WifiDelegate* wifi, provider::DnsServiceDiscovery* dns_sd); - ~Publisher() override; - - // IdentityDelegate implementation. - std::string GetId() const override; + ~Publisher(); // Updates published information. Removes service if HTTP is not alive. void Update();
diff --git a/libweave/src/weave_unittest.cc b/libweave/src/weave_unittest.cc index bdb70e6..bcd39e2 100644 --- a/libweave/src/weave_unittest.cc +++ b/libweave/src/weave_unittest.cc
@@ -53,7 +53,7 @@ const char kDeviceResource[] = R"({ "kind": "clouddevices#device", - "id": "DEVICE_ID", + "id": "CLOUD_ID", "channel": { "supportedType": "pull" }, @@ -93,8 +93,8 @@ const char kRegistrationResponse[] = R"({ "kind": "clouddevices#registrationTicket", - "id": "TEST_ID", - "deviceId": "DEVICE_ID", + "id": "TICKET_ID", + "deviceId": "CLOUD_ID", "oauthClientId": "CLIENT_ID", "userEmail": "USER@gmail.com", "creationTimeMs": "1440087183738", @@ -103,8 +103,8 @@ const char kRegistrationFinalResponse[] = R"({ "kind": "clouddevices#registrationTicket", - "id": "TEST_ID", - "deviceId": "DEVICE_ID", + "id": "TICKET_ID", + "deviceId": "CLOUD_ID", "oauthClientId": "CLIENT_ID", "userEmail": "USER@gmail.com", "robotAccountEmail": "ROBO@gmail.com", @@ -182,17 +182,16 @@ } void InitDnsSd() { - EXPECT_CALL(dns_sd_, GetId()).WillRepeatedly(Return("TEST_ID")); EXPECT_CALL(dns_sd_, PublishService(_, _, _)).WillRepeatedly(Return()); EXPECT_CALL(dns_sd_, StopPublishing("_privet._tcp")).WillOnce(Return()); } void InitDnsSdPublishing(bool registered, const std::string& flags) { - std::vector<std::string> txt{{"id=TEST_ID"}, {"flags=" + flags}, - {"mmid=ABCDE"}, {"services=_base"}, - {"txtvers=3"}, {"ty=TEST_NAME"}}; + std::vector<std::string> txt{{"id=TEST_DEVICE_ID"}, {"flags=" + flags}, + {"mmid=ABCDE"}, {"services=_base"}, + {"txtvers=3"}, {"ty=TEST_NAME"}}; if (registered) { - txt.push_back("gcd_id=DEVICE_ID"); + txt.push_back("gcd_id=CLOUD_ID"); // During registration device may announce itself twice: // 1. with GCD ID but not connected (DB) @@ -333,7 +332,7 @@ ExpectRequest( "PATCH", "https://www.googleapis.com/clouddevices/v1/registrationTickets/" - "TEST_ID?key=TEST_API_KEY", + "TICKET_ID?key=TEST_API_KEY", ValueToString(*response)); response = CreateDictionaryValue(kRegistrationFinalResponse); @@ -341,7 +340,7 @@ ExpectRequest( "POST", "https://www.googleapis.com/clouddevices/v1/registrationTickets/" - "TEST_ID/finalize?key=TEST_API_KEY", + "TICKET_ID/finalize?key=TEST_API_KEY", ValueToString(*response)); ExpectRequest("POST", "https://accounts.google.com/o/oauth2/token", @@ -349,7 +348,7 @@ InitDnsSdPublishing(true, "DB"); - EXPECT_EQ("DEVICE_ID", cloud_->RegisterDevice("TEST_ID", nullptr)); + EXPECT_EQ("CLOUD_ID", cloud_->RegisterDevice("TICKET_ID", nullptr)); } class WeaveWiFiSetupTest : public WeaveTest {