shill: Purge service/wifi_service state on Unload
Clear relevant state from Service and WiFiService when manager
notifies the service that it is no longer backed by a Profile.
BUG=chromium-os:22947
TEST=New unit tests
Change-Id: I048d2692fade5b883332e727be39f86caf4ed7f0
Reviewed-on: https://gerrit.chromium.org/gerrit/13880
Commit-Ready: Paul Stewart <pstew@chromium.org>
Reviewed-by: Paul Stewart <pstew@chromium.org>
Tested-by: Paul Stewart <pstew@chromium.org>
diff --git a/service.cc b/service.cc
index 153c2f5..37fb889 100644
--- a/service.cc
+++ b/service.cc
@@ -255,13 +255,9 @@
// TODO(petkov): Load these:
- // "Name"
- // "WiFi.HiddenSSID"
- // "SSID"
// "Failure"
// "Modified"
// "LastAttempt"
- // WiFiService: "Passphrase"
// "APN"
// "LastGoodAPN"
@@ -272,11 +268,14 @@
void Service::Unload() {
auto_connect_ = false;
+ check_portal_ = kCheckPortalAuto;
favorite_ = false;
- // TODO(pstew): Call a centralized function to purge all profile-set
- // state. This should be called both from Load() and Unload() since
- // even in Load() profiles aren't cumulative -- they're exclusive.
- // crosbug.com/22946
+ priority_ = kPriorityNone;
+ proxy_config_ = "";
+ save_credentials_ = true;
+ ui_data_ = "";
+
+ UnloadEapCredentials();
}
bool Service::Save(StoreInterface *storage) {
@@ -599,6 +598,24 @@
true);
}
+void Service::UnloadEapCredentials() {
+ eap_.identity = "";
+ eap_.eap = "";
+ eap_.inner_eap = "";
+ eap_.anonymous_identity = "";
+ eap_.client_cert = "";
+ eap_.cert_id = "";
+ eap_.private_key = "";
+ eap_.private_key_password = "";
+ eap_.key_id = "";
+ eap_.ca_cert = "";
+ eap_.ca_cert_id = "";
+ eap_.use_system_cas = false;
+ eap_.pin = "";
+ eap_.password = "";
+ eap_.key_management = "";
+}
+
const string &Service::GetEAPKeyManagement() const {
return eap_.key_management;
}
diff --git a/service.h b/service.h
index 496ffc1..6404f32 100644
--- a/service.h
+++ b/service.h
@@ -263,6 +263,7 @@
void LoadEapCredentials(StoreInterface *storage, const std::string &id);
void SaveEapCredentials(StoreInterface *storage, const std::string &id);
+ void UnloadEapCredentials();
// Property accessors reserved for subclasses
EventDispatcher *dispatcher() const { return dispatcher_; }
@@ -281,6 +282,7 @@
FRIEND_TEST(ServiceTest, SaveStringCrypted);
FRIEND_TEST(ServiceTest, SaveStringDontSave);
FRIEND_TEST(ServiceTest, SaveStringEmpty);
+ FRIEND_TEST(ServiceTest, Unload);
static const char kStorageAutoConnect[];
static const char kStorageCheckPortal[];
diff --git a/service_unittest.cc b/service_unittest.cc
index dfa66b3..4f73bb8 100644
--- a/service_unittest.cc
+++ b/service_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium OS Authors. All rights reserved.
+// Copyright (c) 2012 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.
@@ -29,9 +29,11 @@
using std::vector;
using testing::_;
using testing::AtLeast;
+using testing::DoAll;
using testing::NiceMock;
using testing::Return;
using testing::StrictMock;
+using testing::SetArgumentPointee;
using testing::Test;
namespace shill {
@@ -203,6 +205,25 @@
EXPECT_TRUE(service_->Save(&storage));
}
+TEST_F(ServiceTest, Unload) {
+ NiceMock<MockStore> storage;
+ EXPECT_CALL(storage, ContainsGroup(storage_id_)).WillOnce(Return(true));
+ static const string string_value("value");
+ EXPECT_CALL(storage, GetString(storage_id_, _, _))
+ .Times(AtLeast(1))
+ .WillRepeatedly(DoAll(SetArgumentPointee<2>(string_value), Return(true)));
+ ASSERT_TRUE(service_->Load(&storage));
+ // TODO(pstew): A single string property in the service is tested as
+ // a sentinel that properties are being set and reset at the rit times.
+ // However, since property load/store is essentially a manual process,
+ // it is error prone and should either be exhaustively unit-tested or
+ // a generic framework for registering loaded/stored properties should
+ // be created. crosbug.com/24859
+ EXPECT_EQ(string_value, service_->ui_data_);
+ service_->Unload();
+ EXPECT_EQ(string(""), service_->ui_data_);
+}
+
TEST_F(ServiceTest, State) {
EXPECT_EQ(Service::kStateUnknown, service_->state());
EXPECT_EQ(Service::kFailureUnknown, service_->failure());
diff --git a/wifi_service.cc b/wifi_service.cc
index dd9d1c7..ca55a06 100644
--- a/wifi_service.cc
+++ b/wifi_service.cc
@@ -87,19 +87,14 @@
// XXX needs_passpharse_ = false ?
} else if (security_ == flimflam::kSecurityPsk) {
SetEAPKeyManagement("WPA-PSK");
- need_passphrase_ = true;
} else if (security_ == flimflam::kSecurityRsn) {
SetEAPKeyManagement("WPA-PSK");
- need_passphrase_ = true;
} else if (security_ == flimflam::kSecurityWpa) {
SetEAPKeyManagement("WPA-PSK");
- need_passphrase_ = true;
} else if (security_ == flimflam::kSecurityWep) {
SetEAPKeyManagement("NONE");
- need_passphrase_ = true;
} else if (security_ == flimflam::kSecurityNone) {
SetEAPKeyManagement("NONE");
- need_passphrase_ = false;
} else {
LOG(ERROR) << "unsupported security method " << security_;
}
@@ -201,7 +196,6 @@
if (error->IsSuccess()) {
passphrase_ = passphrase;
- need_passphrase_ = false;
}
UpdateConnectable();
@@ -277,6 +271,13 @@
return true;
}
+void WiFiService::Unload() {
+ Service::Unload();
+ hidden_ssid_ = false;
+ passphrase_ = "";
+ UpdateConnectable();
+}
+
bool WiFiService::IsSecurityMatch(const string &security) const {
return GetSecurityClass(security) == GetSecurityClass(security_);
}
@@ -394,16 +395,17 @@
void WiFiService::UpdateConnectable() {
if (security_ == flimflam::kSecurityNone) {
DCHECK(passphrase_.empty());
- set_connectable(true);
+ need_passphrase_ = false;
} else if (security_ == flimflam::kSecurityWep ||
security_ == flimflam::kSecurityWpa ||
security_ == flimflam::kSecurityPsk ||
security_ == flimflam::kSecurityRsn) {
- set_connectable(!passphrase_.empty());
+ need_passphrase_ = passphrase_.empty();
} else {
// TODO(quiche): Handle connectability for 802.1x. (crosbug.com/23466)
- set_connectable(false);
+ need_passphrase_ = true;
}
+ set_connectable(!need_passphrase_);
}
// static
diff --git a/wifi_service.h b/wifi_service.h
index 3e89c1a..7cf8d77 100644
--- a/wifi_service.h
+++ b/wifi_service.h
@@ -63,6 +63,7 @@
virtual bool IsLoadableFrom(StoreInterface *storage) const;
virtual bool Load(StoreInterface *storage);
virtual bool Save(StoreInterface *storage);
+ virtual void Unload();
virtual bool HasEndpoints() const { return !endpoints_.empty(); }
virtual bool IsVisible() const;
@@ -85,6 +86,7 @@
FRIEND_TEST(WiFiServiceTest, ConnectTaskWEP);
FRIEND_TEST(WiFiServiceTest, IsAutoConnectable);
FRIEND_TEST(WiFiServiceTest, LoadHidden);
+ FRIEND_TEST(WiFiServiceTest, LoadAndUnloadPassphrase);
static const char kStorageHiddenSSID[];
static const char kStorageMode[];
diff --git a/wifi_service_unittest.cc b/wifi_service_unittest.cc
index 7254bd0..99bb635 100644
--- a/wifi_service_unittest.cc
+++ b/wifi_service_unittest.cc
@@ -36,6 +36,7 @@
using ::testing::Return;
using ::testing::SetArgumentPointee;
using ::testing::StrEq;
+using ::testing::StrNe;
class WiFiServiceTest : public PropertyStoreTest {
public:
@@ -431,6 +432,44 @@
false));
}
+TEST_F(WiFiServiceTest, LoadAndUnloadPassphrase) {
+ vector<uint8_t> ssid(5);
+ ssid.push_back(0xff);
+
+ WiFiServiceRefPtr service = new WiFiService(control_interface(),
+ dispatcher(),
+ manager(),
+ wifi(),
+ ssid,
+ flimflam::kModeManaged,
+ flimflam::kSecurityPsk,
+ false);
+ NiceMock<MockStore> mock_store;
+ const string storage_id = service->GetStorageIdentifier();
+ EXPECT_CALL(mock_store, ContainsGroup(StrEq(storage_id)))
+ .WillRepeatedly(Return(true));
+ EXPECT_CALL(mock_store, GetBool(_, _, _))
+ .WillRepeatedly(Return(false));
+ const string passphrase = "passphrase";
+ EXPECT_CALL(mock_store,
+ GetCryptedString(StrEq(storage_id),
+ WiFiService::kStoragePassphrase, _))
+ .WillRepeatedly(DoAll(SetArgumentPointee<2>(passphrase), Return(true)));
+ EXPECT_CALL(mock_store,
+ GetCryptedString(StrEq(storage_id),
+ StrNe(WiFiService::kStoragePassphrase), _))
+ .WillRepeatedly(Return(false));
+ EXPECT_TRUE(service->need_passphrase_);
+ EXPECT_TRUE(service->Load(&mock_store));
+ EXPECT_EQ(passphrase, service->passphrase_);
+ EXPECT_TRUE(service->connectable());
+ EXPECT_FALSE(service->need_passphrase_);
+ service->Unload();
+ EXPECT_EQ(string(""), service->passphrase_);
+ EXPECT_FALSE(service->connectable());
+ EXPECT_TRUE(service->need_passphrase_);
+}
+
TEST_F(WiFiServiceTest, ParseStorageIdentifier) {
vector<uint8_t> ssid(5);
ssid.push_back(0xff);