Skip to content

Commit

Permalink
[CrOS Cellular] Support managed property for stub cellular networks
Browse files Browse the repository at this point in the history
This CL adds support for creating managed stub cellular network when
Shill doesn't expose its cellular service. Whenever a new stub network
is created, it first checks if a corresponding SMDP address exists
in device pref for the ICCID, and will se thet stub network's onc_source
to device policy if the SMDP address can be found.

Bug: b/220938636
Change-Id: I30a40251733b1cd7884de9c2fe7304bcf06d3f27
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3515612
Reviewed-by: Azeem Arshad <azeemarshad@chromium.org>
Commit-Queue: Jason Zhang <jiajunz@google.com>
Cr-Commit-Position: refs/heads/main@{#985439}
  • Loading branch information
Jason Zhang authored and Chromium LUCI CQ committed Mar 25, 2022
1 parent 8a37e72 commit 822899e
Show file tree
Hide file tree
Showing 17 changed files with 145 additions and 19 deletions.
Expand Up @@ -79,6 +79,7 @@ class CellularESimUninstallHandlerTest : public testing::Test {
cellular_inhibitor_.get());
managed_cellular_pref_handler_ =
std::make_unique<ManagedCellularPrefHandler>();
managed_cellular_pref_handler_->Init(network_state_handler_.get());
ManagedCellularPrefHandler::RegisterLocalStatePrefs(
device_prefs_.registry());
managed_cellular_pref_handler_->SetDevicePrefs(&device_prefs_);
Expand Down
1 change: 1 addition & 0 deletions chromeos/network/cellular_policy_handler_unittest.cc
Expand Up @@ -140,6 +140,7 @@ class CellularPolicyHandlerTest : public testing::Test {
/*UIProxyConfigService=*/nullptr);
managed_cellular_pref_handler_ =
std::make_unique<ManagedCellularPrefHandler>();
managed_cellular_pref_handler_->Init(network_state_handler_.get());
ManagedCellularPrefHandler::RegisterLocalStatePrefs(
device_prefs_.registry());
managed_cellular_pref_handler_->SetDevicePrefs(&device_prefs_);
Expand Down
10 changes: 8 additions & 2 deletions chromeos/network/fake_stub_cellular_networks_provider.cc
Expand Up @@ -6,6 +6,7 @@

#include <algorithm>

#include "base/containers/contains.h"
#include "base/guid.h"
#include "chromeos/network/cellular_utils.h"

Expand All @@ -16,13 +17,17 @@ FakeStubCellularNetworksProvider::FakeStubCellularNetworksProvider() = default;
FakeStubCellularNetworksProvider::~FakeStubCellularNetworksProvider() = default;

void FakeStubCellularNetworksProvider::AddStub(const std::string& stub_iccid,
const std::string& stub_eid) {
const std::string& stub_eid,
bool is_managed) {
stub_iccid_and_eid_pairs_.emplace(stub_iccid, stub_eid);
if (is_managed)
managed_iccids_.insert(stub_iccid);
}

void FakeStubCellularNetworksProvider::RemoveStub(const std::string& stub_iccid,
const std::string& stub_eid) {
stub_iccid_and_eid_pairs_.erase(std::make_pair(stub_iccid, stub_eid));
managed_iccids_.erase(stub_iccid);
}

bool FakeStubCellularNetworksProvider::AddOrRemoveStubCellularNetworks(
Expand All @@ -39,7 +44,8 @@ bool FakeStubCellularNetworksProvider::AddOrRemoveStubCellularNetworks(
changed = true;
for (const IccidEidPair& pair : stubs_to_add) {
new_stub_networks.push_back(NetworkState::CreateNonShillCellularNetwork(
pair.first, pair.second, GetGuidForStubIccid(pair.first), device));
pair.first, pair.second, GetGuidForStubIccid(pair.first),
base::Contains(managed_iccids_, pair.first), device));
stub_networks_add_count_++;
}
}
Expand Down
4 changes: 3 additions & 1 deletion chromeos/network/fake_stub_cellular_networks_provider.h
Expand Up @@ -25,7 +25,8 @@ class COMPONENT_EXPORT(CHROMEOS_NETWORK) FakeStubCellularNetworksProvider
// used when a subsequent call to AddOrRemoveStubCellularNetworks() is made.
// Note that an empty EID refers to a pSIM network.
void AddStub(const std::string& stub_iccid,
const std::string& eid = std::string());
const std::string& eid = std::string(),
bool is_managed = false);

// Removes a stub network with the provided ICCID and EID, reversing a
// previous call to AddStub().
Expand Down Expand Up @@ -54,6 +55,7 @@ class COMPONENT_EXPORT(CHROMEOS_NETWORK) FakeStubCellularNetworksProvider
size_t stub_networks_add_count_ = 0;
base::flat_set<IccidEidPair> stub_iccid_and_eid_pairs_;
base::flat_map<std::string, std::string> iccid_to_guid_map_;
base::flat_set<std::string> managed_iccids_;
};

} // namespace chromeos
Expand Down
8 changes: 8 additions & 0 deletions chromeos/network/managed_cellular_pref_handler.cc
Expand Up @@ -6,6 +6,7 @@

#include "ash/constants/ash_pref_names.h"
#include "chromeos/network/network_event_log.h"
#include "chromeos/network/network_state_handler.h"
#include "components/prefs/pref_registry_simple.h"
#include "services/preferences/public/cpp/dictionary_value_update.h"
#include "services/preferences/public/cpp/scoped_pref_update.h"
Expand All @@ -21,6 +22,11 @@ void ManagedCellularPrefHandler::RegisterLocalStatePrefs(
ManagedCellularPrefHandler::ManagedCellularPrefHandler() = default;
ManagedCellularPrefHandler::~ManagedCellularPrefHandler() = default;

void ManagedCellularPrefHandler::Init(
NetworkStateHandler* network_state_handler) {
network_state_handler_ = network_state_handler;
}

void ManagedCellularPrefHandler::SetDevicePrefs(PrefService* device_prefs) {
device_prefs_ = device_prefs;
}
Expand All @@ -41,6 +47,7 @@ void ManagedCellularPrefHandler::AddIccidSmdpPair(
::prefs::ScopedDictionaryPrefUpdate update(
device_prefs_, ash::prefs::kManagedCellularIccidSmdpPair);
update->SetString(iccid, smdp_address);
network_state_handler_->SyncStubCellularNetworks();
}

void ManagedCellularPrefHandler::RemovePairWithIccid(const std::string& iccid) {
Expand All @@ -57,6 +64,7 @@ void ManagedCellularPrefHandler::RemovePairWithIccid(const std::string& iccid) {
::prefs::ScopedDictionaryPrefUpdate update(
device_prefs_, ash::prefs::kManagedCellularIccidSmdpPair);
update->Remove(iccid);
network_state_handler_->SyncStubCellularNetworks();
}

const std::string* ManagedCellularPrefHandler::GetSmdpAddressFromIccid(
Expand Down
4 changes: 4 additions & 0 deletions chromeos/network/managed_cellular_pref_handler.h
Expand Up @@ -13,6 +13,8 @@ class PrefRegistrySimple;

namespace chromeos {

class NetworkStateHandler;

// This class provides the ability to add, remove and get ICCID and SMDP+
// address pair for managed cellular networks and stores it persistently in
// prefs.
Expand All @@ -26,6 +28,7 @@ class COMPONENT_EXPORT(CHROMEOS_NETWORK) ManagedCellularPrefHandler {

static void RegisterLocalStatePrefs(PrefRegistrySimple* registry);

void Init(NetworkStateHandler* network_state_handler);
void SetDevicePrefs(PrefService* device_prefs);

// Add a new ICCID and SMDP+ address pair to device pref for a managed
Expand All @@ -42,6 +45,7 @@ class COMPONENT_EXPORT(CHROMEOS_NETWORK) ManagedCellularPrefHandler {
const std::string* GetSmdpAddressFromIccid(const std::string& iccid) const;

private:
NetworkStateHandler* network_state_handler_ = nullptr;
// Initialized to null and set once SetDevicePrefs() is called.
PrefService* device_prefs_ = nullptr;
};
Expand Down
2 changes: 2 additions & 0 deletions chromeos/network/managed_cellular_pref_handler_unittest.cc
Expand Up @@ -34,6 +34,7 @@ class ManagedCellularPrefHandlerTest : public testing::Test {
void Init() {
managed_cellular_pref_handler_ =
std::make_unique<ManagedCellularPrefHandler>();
managed_cellular_pref_handler_->Init(helper_.network_state_handler());
}

void SetDevicePrefs(bool set_to_null = false) {
Expand All @@ -57,6 +58,7 @@ class ManagedCellularPrefHandlerTest : public testing::Test {
private:
base::test::TaskEnvironment task_environment_{
base::test::TaskEnvironment::TimeSource::MOCK_TIME};
NetworkStateTestHelper helper_{/*use_default_devices_and_services=*/false};
TestingPrefServiceSimple device_prefs_;

std::unique_ptr<ManagedCellularPrefHandler> managed_cellular_pref_handler_;
Expand Down
Expand Up @@ -182,6 +182,7 @@ class ManagedNetworkConfigurationHandlerTest : public testing::Test {

managed_cellular_pref_handler_ =
std::make_unique<ManagedCellularPrefHandler>();
managed_cellular_pref_handler_->Init(network_state_handler_.get());
ManagedCellularPrefHandler::RegisterLocalStatePrefs(
device_prefs_.registry());
managed_cellular_pref_handler_->SetDevicePrefs(&device_prefs_);
Expand Down
3 changes: 2 additions & 1 deletion chromeos/network/network_connection_handler_impl_unittest.cc
Expand Up @@ -212,7 +212,8 @@ class NetworkConnectionHandlerImplTest : public testing::Test {
stub_cellular_networks_provider_ =
std::make_unique<StubCellularNetworksProvider>();
stub_cellular_networks_provider_->Init(
helper_.network_state_handler(), cellular_esim_profile_handler_.get());
helper_.network_state_handler(), cellular_esim_profile_handler_.get(),
/*managed_cellular_pref_handler=*/nullptr);

cellular_connection_handler_.reset(new CellularConnectionHandler());
cellular_connection_handler_->Init(helper_.network_state_handler(),
Expand Down
4 changes: 3 additions & 1 deletion chromeos/network/network_handler.cc
Expand Up @@ -89,7 +89,8 @@ void NetworkHandler::Init() {
cellular_esim_profile_handler_->Init(network_state_handler_.get(),
cellular_inhibitor_.get());
stub_cellular_networks_provider_->Init(network_state_handler_.get(),
cellular_esim_profile_handler_.get());
cellular_esim_profile_handler_.get(),
managed_cellular_pref_handler_.get());
cellular_connection_handler_->Init(network_state_handler_.get(),
cellular_inhibitor_.get(),
cellular_esim_profile_handler_.get());
Expand Down Expand Up @@ -120,6 +121,7 @@ void NetworkHandler::Init() {
network_profile_handler_.get(), network_state_handler_.get(),
managed_cellular_pref_handler_.get(),
managed_network_configuration_handler_.get());
managed_cellular_pref_handler_->Init(network_state_handler_.get());
esim_policy_login_metrics_logger_->Init(
network_state_handler_.get(),
managed_network_configuration_handler_.get());
Expand Down
4 changes: 4 additions & 0 deletions chromeos/network/network_state.cc
Expand Up @@ -622,6 +622,7 @@ std::unique_ptr<NetworkState> NetworkState::CreateNonShillCellularNetwork(
const std::string& iccid,
const std::string& eid,
const std::string& guid,
bool is_managed,
const DeviceState* cellular_device) {
std::string path = GenerateStubCellularServicePath(iccid);
auto new_state = std::make_unique<NetworkState>(path);
Expand All @@ -632,6 +633,9 @@ std::unique_ptr<NetworkState> NetworkState::CreateNonShillCellularNetwork(
new_state->iccid_ = iccid;
new_state->eid_ = eid;
new_state->guid_ = guid;
if (is_managed) {
new_state->onc_source_ = ::onc::ONCSource::ONC_SOURCE_DEVICE_POLICY;
}
new_state->activation_state_ = shill::kActivationStateActivated;
return new_state;
}
Expand Down
1 change: 1 addition & 0 deletions chromeos/network/network_state.h
Expand Up @@ -283,6 +283,7 @@ class COMPONENT_EXPORT(CHROMEOS_NETWORK) NetworkState : public ManagedState {
const std::string& iccid,
const std::string& eid,
const std::string& guid,
bool is_managed,
const DeviceState* cellular_device);

// Ignore changes to signal strength less than this value.
Expand Down
26 changes: 26 additions & 0 deletions chromeos/network/network_state_handler_unittest.cc
Expand Up @@ -2203,6 +2203,7 @@ TEST_F(NetworkStateHandlerTest, SyncStubCellularNetworks) {
ASSERT_TRUE(cellular);
EXPECT_FALSE(cellular->guid().empty());
EXPECT_TRUE(cellular->IsNonShillCellularNetwork());
EXPECT_FALSE(cellular->IsManagedByPolicy());
EXPECT_EQ(kStubCellularIccid, cellular->iccid());
EXPECT_EQ(1u, test_observer_->network_list_changed_count());

Expand All @@ -2227,6 +2228,31 @@ TEST_F(NetworkStateHandlerTest, SyncStubCellularNetworks) {
EXPECT_EQ(1u, test_observer_->network_list_changed_count());
}

TEST_F(NetworkStateHandlerTest, SyncManagedStubCellularNetworks) {
const char kStubCellularIccid[] = "test_iccid";
const char kStubCellularEid[] = "1234567890";
// Clear existing cellular networks.
service_test_->RemoveService(kShillManagerClientStubCellular);
base::RunLoop().RunUntilIdle();
EXPECT_FALSE(
network_state_handler_->GetNetworkState(kShillManagerClientStubCellular));

test_observer_->reset_change_counts();
fake_stub_cellular_networks_provider_.AddStub(
kStubCellularIccid, kStubCellularEid, /*is_managed=*/true);

// Verify that managed stub cellular network was added and notified.
network_state_handler_->SyncStubCellularNetworks();
const NetworkState* cellular = network_state_handler_->FirstNetworkByType(
NetworkTypePattern::Cellular());
ASSERT_TRUE(cellular);
EXPECT_FALSE(cellular->guid().empty());
EXPECT_TRUE(cellular->IsNonShillCellularNetwork());
EXPECT_TRUE(cellular->IsManagedByPolicy());
EXPECT_EQ(kStubCellularIccid, cellular->iccid());
EXPECT_EQ(1u, test_observer_->network_list_changed_count());
}

TEST_F(NetworkStateHandlerTest,
SyncStubCellularNetworks_ManagedStateListChange) {
const char kTestCellularServicePath1[] = "test_cellular_service_path1";
Expand Down
17 changes: 16 additions & 1 deletion chromeos/network/network_state_unittest.cc
Expand Up @@ -459,16 +459,31 @@ TEST_F(NetworkStateTest, NonShillCellular) {

std::unique_ptr<NetworkState> non_shill_cellular =
NetworkState::CreateNonShillCellularNetwork(
kTestIccid, kTestEid, kTestGuid, GetCellularDevice());
kTestIccid, kTestEid, kTestGuid, /*is_managed=*/false,
GetCellularDevice());
EXPECT_EQ(kTestIccid, non_shill_cellular->iccid());
EXPECT_EQ(kTestEid, non_shill_cellular->eid());
EXPECT_EQ(kTestGuid, non_shill_cellular->guid());
EXPECT_FALSE(non_shill_cellular->IsManagedByPolicy());

base::Value dictionary(base::Value::Type::DICTIONARY);
non_shill_cellular->GetStateProperties(&dictionary);
EXPECT_EQ(kTestIccid, *dictionary.FindStringKey(shill::kIccidProperty));
EXPECT_EQ(kTestEid, *dictionary.FindStringKey(shill::kEidProperty));
EXPECT_EQ(kTestGuid, *dictionary.FindStringKey(shill::kGuidProperty));

non_shill_cellular = NetworkState::CreateNonShillCellularNetwork(
kTestIccid, kTestEid, kTestGuid, /*is_managed=*/true,
GetCellularDevice());
EXPECT_EQ(kTestIccid, non_shill_cellular->iccid());
EXPECT_EQ(kTestEid, non_shill_cellular->eid());
EXPECT_EQ(kTestGuid, non_shill_cellular->guid());
EXPECT_TRUE(non_shill_cellular->IsManagedByPolicy());

non_shill_cellular->GetStateProperties(&dictionary);
EXPECT_EQ(kTestIccid, *dictionary.FindStringKey(shill::kIccidProperty));
EXPECT_EQ(kTestEid, *dictionary.FindStringKey(shill::kEidProperty));
EXPECT_EQ(kTestGuid, *dictionary.FindStringKey(shill::kGuidProperty));
}

} // namespace chromeos
16 changes: 13 additions & 3 deletions chromeos/network/stub_cellular_networks_provider.cc
Expand Up @@ -10,6 +10,7 @@
#include "chromeos/network/cellular_esim_profile_handler.h"
#include "chromeos/network/cellular_utils.h"
#include "chromeos/network/device_state.h"
#include "chromeos/network/managed_cellular_pref_handler.h"
#include "chromeos/network/network_event_log.h"

namespace chromeos {
Expand Down Expand Up @@ -52,9 +53,11 @@ StubCellularNetworksProvider::~StubCellularNetworksProvider() {

void StubCellularNetworksProvider::Init(
NetworkStateHandler* network_state_handler,
CellularESimProfileHandler* cellular_esim_profile_handler) {
CellularESimProfileHandler* cellular_esim_profile_handler,
ManagedCellularPrefHandler* managed_cellular_pref_handler) {
network_state_handler_ = network_state_handler;
cellular_esim_profile_handler_ = cellular_esim_profile_handler;
managed_cellular_pref_handler_ = managed_cellular_pref_handler;
network_state_handler_->set_stub_cellular_networks_provider(this);
network_state_handler_->SyncStubCellularNetworks();
}
Expand Down Expand Up @@ -166,12 +169,19 @@ bool StubCellularNetworksProvider::AddStubNetworks(
if (base::Contains(all_iccids, iccid_eid_pair.first))
continue;

bool is_managed = false;
if (managed_cellular_pref_handler_) {
is_managed = managed_cellular_pref_handler_->GetSmdpAddressFromIccid(
iccid_eid_pair.first);
}
NET_LOG(EVENT) << "Adding stub cellular network for ICCID="
<< iccid_eid_pair.first << " EID=" << iccid_eid_pair.second;
<< iccid_eid_pair.first << " EID=" << iccid_eid_pair.second
<< ", is managed: " << is_managed;
network_added = true;
new_stub_networks.push_back(NetworkState::CreateNonShillCellularNetwork(
iccid_eid_pair.first, iccid_eid_pair.second,
GetGuidForStubIccid(iccid_eid_pair.first), cellular_device));
GetGuidForStubIccid(iccid_eid_pair.first), is_managed,
cellular_device));
}

return network_added;
Expand Down
4 changes: 3 additions & 1 deletion chromeos/network/stub_cellular_networks_provider.h
Expand Up @@ -25,7 +25,8 @@ class COMPONENT_EXPORT(CHROMEOS_NETWORK) StubCellularNetworksProvider
~StubCellularNetworksProvider() override;

void Init(NetworkStateHandler* network_state_handler,
CellularESimProfileHandler* cellular_esim_profile_handler);
CellularESimProfileHandler* cellular_esim_profile_handler,
ManagedCellularPrefHandler* managed_cellular_pref_handler);

private:
friend class StubCellularNetworksProviderTest;
Expand Down Expand Up @@ -71,6 +72,7 @@ class COMPONENT_EXPORT(CHROMEOS_NETWORK) StubCellularNetworksProvider

NetworkStateHandler* network_state_handler_ = nullptr;
CellularESimProfileHandler* cellular_esim_profile_handler_ = nullptr;
ManagedCellularPrefHandler* managed_cellular_pref_handler_ = nullptr;

// Map which stores the GUID used for stubs created by this class. Each
// network should use a consistent GUID throughout a session.
Expand Down

0 comments on commit 822899e

Please sign in to comment.