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 {
