shill: vpn: Configure VPN services from newly-pushed profiles
When a new profile is pushed, look for VPN entries and configure
services for them.
BUG=chromium-os:28157
TEST=New unit test
Change-Id: I571c791710e172f1385842867342ee96d8eb4eb0
Reviewed-on: https://gerrit.chromium.org/gerrit/19871
Commit-Ready: Paul Stewart <pstew@chromium.org>
Reviewed-by: Paul Stewart <pstew@chromium.org>
Tested-by: Paul Stewart <pstew@chromium.org>
diff --git a/manager.cc b/manager.cc
index ff10cf0..bf469b0 100644
--- a/manager.cc
+++ b/manager.cc
@@ -317,6 +317,10 @@
profile->ConfigureDevice(*it);
}
+ // Offer the Profile contents to the VPNProvider which will create
+ // new VPN services if necessary.
+ vpn_provider_.CreateServicesFromProfile(profile);
+
*path = profile->GetRpcIdentifier();
SortServices();
}
diff --git a/mock_profile.h b/mock_profile.h
index 7941e49..436eeb7 100644
--- a/mock_profile.h
+++ b/mock_profile.h
@@ -29,6 +29,7 @@
MOCK_METHOD1(GetStoragePath, bool(FilePath *filepath));
MOCK_METHOD1(UpdateService, bool(const ServiceRefPtr &service));
MOCK_METHOD0(Save, bool());
+ MOCK_CONST_METHOD0(GetConstStorage, const StoreInterface *());
private:
DISALLOW_COPY_AND_ASSIGN(MockProfile);
diff --git a/profile.h b/profile.h
index 80895d2..daa171d 100644
--- a/profile.h
+++ b/profile.h
@@ -128,8 +128,9 @@
const std::string &GetUser() const { return name_.user; }
// Returns a read-only copy of the backing storage of the profile.
- // TODO(pstew): Needed by ProfileDBusPropertyExporter crosbug.com/25813
- const StoreInterface *GetConstStorage() const { return storage_.get(); }
+ virtual const StoreInterface *GetConstStorage() const {
+ return storage_.get();
+ }
protected:
// Protected getters
diff --git a/vpn_provider.cc b/vpn_provider.cc
index 4f4ae2b..5387cc9 100644
--- a/vpn_provider.cc
+++ b/vpn_provider.cc
@@ -7,13 +7,17 @@
#include <algorithm>
#include <base/logging.h>
+#include <base/string_util.h>
#include <chromeos/dbus/service_constants.h>
#include "shill/error.h"
#include "shill/manager.h"
#include "shill/openvpn_driver.h"
+#include "shill/profile.h"
+#include "shill/store_interface.h"
#include "shill/vpn_service.h"
+using std::set;
using std::string;
using std::vector;
@@ -50,36 +54,11 @@
}
// Find a service in the provider list which matches these parameters.
- for (vector<VPNServiceRefPtr>::const_iterator it = services_.begin();
- it != services_.end();
- ++it) {
- if (type == (*it)->driver()->GetProviderType() &&
- (*it)->GetStorageIdentifier() == storage_id) {
- return *it;
- }
+ VPNServiceRefPtr service = FindService(type, storage_id);
+ if (service == NULL) {
+ service = CreateService(type, storage_id, args, error);
}
- scoped_ptr<VPNDriver> driver;
- if (type == flimflam::kProviderOpenVpn) {
- driver.reset(new OpenVPNDriver(
- control_interface_, dispatcher_, metrics_, manager_,
- manager_->device_info(), manager_->glib(), args));
- } else {
- Error::PopulateAndLog(
- error, Error::kNotSupported, "Unsupported VPN type: " + type);
- return NULL;
- }
-
- VPNServiceRefPtr service = new VPNService(
- control_interface_, dispatcher_, metrics_, manager_, driver.release());
- service->set_storage_id(storage_id);
- service->InitDriverPropertyStore();
- string name = args.LookupString(flimflam::kProviderNameProperty, "");
- if (!name.empty()) {
- service->set_friendly_name(name);
- }
- services_.push_back(service);
- manager_->RegisterService(service);
return service;
}
@@ -104,4 +83,89 @@
}
}
+void VPNProvider::CreateServicesFromProfile(ProfileRefPtr profile) {
+ const StoreInterface *storage = profile->GetConstStorage();
+ set<string> groups =
+ storage->GetGroupsWithKey(flimflam::kProviderTypeProperty);
+ for (set<string>::iterator it = groups.begin(); it != groups.end(); ++it) {
+ if (!StartsWithASCII(*it, "vpn_", false)) {
+ continue;
+ }
+
+ string type;
+ if (!storage->GetString(*it, flimflam::kProviderTypeProperty, &type)) {
+ LOG(ERROR) << "Group " << *it << " is missing the "
+ << flimflam::kProviderTypeProperty << " property.";
+ continue;
+ }
+
+ VPNServiceRefPtr service = FindService(type, *it);
+ if (service != NULL) {
+ // If the service already exists, it does not need to be configured,
+ // since PushProfile would have already called ConfigureService on it.
+ VLOG(2) << "Service already exists " << *it;
+ continue;
+ }
+
+ // Create a service with an empty |args| parameter. We will populate
+ // the service with properties using ConfigureService() below.
+ KeyValueStore args;
+ Error error;
+ service = CreateService(type, *it, args, &error);
+
+ if (service == NULL) {
+ LOG(ERROR) << "Could not create service for " << *it;
+ continue;
+ }
+
+ if (!profile->ConfigureService(service)) {
+ LOG(ERROR) << "Could not configure service for " << *it;
+ continue;
+ }
+ }
+}
+
+VPNServiceRefPtr VPNProvider::CreateService(const string &type,
+ const string &storage_id,
+ const KeyValueStore &args,
+ Error *error) {
+ scoped_ptr<VPNDriver> driver;
+ if (type == flimflam::kProviderOpenVpn) {
+ driver.reset(new OpenVPNDriver(
+ control_interface_, dispatcher_, metrics_, manager_,
+ manager_->device_info(), manager_->glib(), args));
+ } else {
+ Error::PopulateAndLog(
+ error, Error::kNotSupported, "Unsupported VPN type: " + type);
+ return NULL;
+ }
+
+ VPNServiceRefPtr service = new VPNService(
+ control_interface_, dispatcher_, metrics_, manager_, driver.release());
+ service->set_storage_id(storage_id);
+ service->InitDriverPropertyStore();
+ string name = args.LookupString(flimflam::kProviderNameProperty, "");
+ if (!name.empty()) {
+ service->set_friendly_name(name);
+ }
+ services_.push_back(service);
+ manager_->RegisterService(service);
+
+ return service;
+}
+
+VPNServiceRefPtr VPNProvider::FindService(const std::string &type,
+ const std::string &storage_id) {
+ for (vector<VPNServiceRefPtr>::const_iterator it = services_.begin();
+ it != services_.end();
+ ++it) {
+ if (type == (*it)->driver()->GetProviderType() &&
+ storage_id == (*it)->GetStorageIdentifier()) {
+ return *it;
+ }
+ }
+
+ return NULL;
+}
+
} // namespace shill
diff --git a/vpn_provider.h b/vpn_provider.h
index c968b1e..2fae9ce 100644
--- a/vpn_provider.h
+++ b/vpn_provider.h
@@ -44,10 +44,25 @@
// services_ vector.
void RemoveService(VPNServiceRefPtr service);
+ void CreateServicesFromProfile(ProfileRefPtr profile);
+
private:
FRIEND_TEST(VPNProviderTest, OnDeviceInfoAvailable);
FRIEND_TEST(VPNProviderTest, RemoveService);
+ // Create a service of type |type| and storage identifier |storage_id|
+ // and initial parameters |args|. Returns a service reference pointer
+ // to the newly created service, or popuplates |error| with an the error
+ // that caused this to fail.
+ VPNServiceRefPtr CreateService(const std::string &type,
+ const std::string &storage_id,
+ const KeyValueStore &args,
+ Error *error);
+
+ // Find a service of type |type| whose storage identifier is |storage_id|.
+ VPNServiceRefPtr FindService(const std::string &type,
+ const std::string &storage_id);
+
ControlInterface *control_interface_;
EventDispatcher *dispatcher_;
Metrics *metrics_;
diff --git a/vpn_provider_unittest.cc b/vpn_provider_unittest.cc
index 246c4d0..e67072e 100644
--- a/vpn_provider_unittest.cc
+++ b/vpn_provider_unittest.cc
@@ -13,12 +13,17 @@
#include "shill/mock_device_info.h"
#include "shill/mock_manager.h"
#include "shill/mock_metrics.h"
+#include "shill/mock_profile.h"
+#include "shill/mock_store.h"
#include "shill/mock_vpn_driver.h"
#include "shill/mock_vpn_service.h"
using std::string;
using testing::_;
+using testing::DoAll;
+using testing::NiceMock;
using testing::Return;
+using testing::SetArgumentPointee;
namespace shill {
@@ -146,4 +151,56 @@
EXPECT_EQ(0, provider_.services_.size());
}
+MATCHER_P(ServiceWithStorageId, storage_id, "") {
+ return arg->GetStorageIdentifier() == storage_id;
+}
+
+TEST_F(VPNProviderTest, CreateServicesFromProfile) {
+ scoped_refptr<MockProfile> profile(
+ new NiceMock<MockProfile>(&control_, &manager_, ""));
+ NiceMock<MockStore> storage;
+ EXPECT_CALL(*profile, GetConstStorage())
+ .WillRepeatedly(Return(&storage));
+ EXPECT_CALL(storage, GetString(_, _, _))
+ .WillRepeatedly(Return(false));
+
+ std::set<string> groups;
+ const string kNonVPNIdentifier("foo_123_456");
+ groups.insert(kNonVPNIdentifier);
+ const string kVPNIdentifier0("vpn_123_456");
+ const string kVPNIdentifier1("vpn_789_012");
+ groups.insert(kVPNIdentifier0);
+ groups.insert(kVPNIdentifier1);
+ EXPECT_CALL(storage, GetGroupsWithKey(flimflam::kProviderTypeProperty))
+ .WillRepeatedly(Return(groups));
+
+ EXPECT_CALL(*profile, ConfigureService(ServiceWithStorageId(kVPNIdentifier1)))
+ .WillOnce(Return(true));
+
+ const string kProviderTypeUnknown("unknown");
+ EXPECT_CALL(storage, GetString(kVPNIdentifier0,
+ flimflam::kProviderTypeProperty,
+ _))
+ .WillRepeatedly(DoAll(SetArgumentPointee<2>(kProviderTypeUnknown),
+ Return(true)));
+ const string kProviderTypeOpenVPN(flimflam::kProviderOpenVpn);
+ EXPECT_CALL(storage, GetString(kVPNIdentifier1,
+ flimflam::kProviderTypeProperty,
+ _))
+ .WillRepeatedly(DoAll(SetArgumentPointee<2>(kProviderTypeOpenVPN),
+ Return(true)));
+
+ EXPECT_CALL(manager_, device_info())
+ .WillRepeatedly(Return(reinterpret_cast<DeviceInfo *>(NULL)));
+
+ EXPECT_CALL(manager_, RegisterService(ServiceWithStorageId(kVPNIdentifier1)))
+ .Times(1);
+
+ provider_.CreateServicesFromProfile(profile);
+
+ // Calling this again should not create any more services (checked by the
+ // Times(1) above).
+ provider_.CreateServicesFromProfile(profile);
+}
+
} // namespace shill