Skip to content

Commit

Permalink
[CrOS Cellular] Use ManagedCellularPrefHandler to add/remove iccid pair
Browse files Browse the repository at this point in the history 
This CL utilizes the new ManagedCellularPrefHandler to add or remove
ICCID - SMDP Address for managed cellular networks.
The ICCID - SMDP address pair is added in following places:
1. When policy applicator find a existed matching managed cellular entry
in Shill profile
2. When CellularPolicyHandler finishes configuring eSIM Shill
configuration for existing managed eSIM profile.
3. When CellularPolicyHandler finishes installing new managed eSIM
profile.
The ICCID - SMDP address pair is removed in following places:
1. When policy applicator find no matching policy for an existed managed
cellular entry
2. When CellularESimUninstallHandler removes a eSIM profile.

Bug: b/220938636
Change-Id: I96a64d9ac0d1395d508e764efefb1d5733919356
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3513383
Reviewed-by: Azeem Arshad <azeemarshad@chromium.org>
Commit-Queue: Jason Zhang <jiajunz@google.com>
Cr-Commit-Position: refs/heads/main@{#984583}
  • Loading branch information
Jason Zhang authored and Chromium LUCI CQ committed Mar 23, 2022
1 parent bea2bff commit 02ab0f7
Show file tree
Hide file tree
Showing 21 changed files with 215 additions and 53 deletions.
1 change: 1 addition & 0 deletions ash/services/cellular_setup/esim_test_base.cc
View file
Expand Up @@ -74,6 +74,7 @@ void ESimTestBase::SetUp() {
std::make_unique<CellularESimUninstallHandler>();
cellular_esim_uninstall_handler_->Init(
cellular_inhibitor_.get(), cellular_esim_profile_handler_.get(),
/*managed_cellular_pref_handler=*/nullptr,
network_configuration_handler_.get(), network_connection_handler_.get(),
network_state_handler_.get());

Expand Down
7 changes: 4 additions & 3 deletions chromeos/network/auto_connect_handler_unittest.cc
View file
Expand Up @@ -152,9 +152,10 @@ class AutoConnectHandlerTest : public testing::Test {

managed_config_handler_.reset(new ManagedNetworkConfigurationHandlerImpl());
managed_config_handler_->Init(
/*cellular_policy_handler=*/nullptr, helper_.network_state_handler(),
network_profile_handler_.get(), network_config_handler_.get(),
nullptr /* network_device_handler */,
/*cellular_policy_handler=*/nullptr,
/*managed_cellular_pref_handler=*/nullptr,
helper_.network_state_handler(), network_profile_handler_.get(),
network_config_handler_.get(), nullptr /* network_device_handler */,
nullptr /* prohibited_technologies_handler */);

test_network_connection_handler_ =
Expand Down
30 changes: 28 additions & 2 deletions chromeos/network/cellular_esim_uninstall_handler.cc
View file
Expand Up @@ -13,6 +13,7 @@
#include "chromeos/network/cellular_inhibitor.h"
#include "chromeos/network/device_state.h"
#include "chromeos/network/hermes_metrics_util.h"
#include "chromeos/network/managed_cellular_pref_handler.h"
#include "chromeos/network/network_configuration_handler.h"
#include "chromeos/network/network_connection_handler.h"
#include "chromeos/network/network_handler.h"
Expand Down Expand Up @@ -48,11 +49,13 @@ CellularESimUninstallHandler::~CellularESimUninstallHandler() {
void CellularESimUninstallHandler::Init(
CellularInhibitor* cellular_inhibitor,
CellularESimProfileHandler* cellular_esim_profile_handler,
ManagedCellularPrefHandler* managed_cellular_pref_handler,
NetworkConfigurationHandler* network_configuration_handler,
NetworkConnectionHandler* network_connection_handler,
NetworkStateHandler* network_state_handler) {
cellular_inhibitor_ = cellular_inhibitor;
cellular_esim_profile_handler_ = cellular_esim_profile_handler;
managed_cellular_pref_handler_ = managed_cellular_pref_handler;
network_configuration_handler_ = network_configuration_handler;
network_connection_handler_ = network_connection_handler;
network_state_handler_ = network_state_handler;
Expand Down Expand Up @@ -304,22 +307,26 @@ void CellularESimUninstallHandler::AttemptUninstallProfile() {
DCHECK_EQ(state_, UninstallState::kUninstallingProfile);

if (uninstall_requests_.front()->reset_euicc) {
base::flat_set<std::string> iccids =
GetAllIccidsOnEuicc(*uninstall_requests_.front()->euicc_path);
HermesEuiccClient::Get()->ResetMemory(
*uninstall_requests_.front()->euicc_path,
hermes::euicc::ResetOptions::kDeleteOperationalProfiles,
base::BindOnce(&CellularESimUninstallHandler::OnUninstallProfile,
weak_ptr_factory_.GetWeakPtr()));
weak_ptr_factory_.GetWeakPtr(), iccids));
return;
}

base::flat_set<std::string> iccids{*uninstall_requests_.front()->iccid};
HermesEuiccClient::Get()->UninstallProfile(
*uninstall_requests_.front()->euicc_path,
*uninstall_requests_.front()->esim_profile_path,
base::BindOnce(&CellularESimUninstallHandler::OnUninstallProfile,
weak_ptr_factory_.GetWeakPtr()));
weak_ptr_factory_.GetWeakPtr(), iccids));
}

void CellularESimUninstallHandler::OnUninstallProfile(
const base::flat_set<std::string>& removed_iccids,
HermesResponseStatus status) {
DCHECK_EQ(state_, UninstallState::kUninstallingProfile);

Expand All @@ -334,6 +341,10 @@ void CellularESimUninstallHandler::OnUninstallProfile(
return;
}

if (managed_cellular_pref_handler_) {
for (const auto& iccid : removed_iccids)
managed_cellular_pref_handler_->RemovePairWithIccid(iccid);
}
TransitionToUninstallState(UninstallState::kRemovingShillService);
AttemptRemoveShillService();
}
Expand Down Expand Up @@ -435,6 +446,21 @@ CellularESimUninstallHandler::GetEnabledCellularESimProfilePath() {
return absl::nullopt;
}

base::flat_set<std::string> CellularESimUninstallHandler::GetAllIccidsOnEuicc(
const dbus::ObjectPath& euicc_path) {
HermesEuiccClient::Properties* euicc_properties =
HermesEuiccClient::Get()->GetProperties(euicc_path);
const std::string& eid = euicc_properties->eid().value();
base::flat_set<std::string> iccids;
for (const auto& esim_profile :
cellular_esim_profile_handler_->GetESimProfiles()) {
if (esim_profile.eid() == eid) {
iccids.emplace(esim_profile.iccid());
}
}
return iccids;
}

const NetworkState* CellularESimUninstallHandler::GetNextResetServiceToRemove()
const {
HermesEuiccClient::Properties* euicc_properties =
Expand Down
9 changes: 8 additions & 1 deletion chromeos/network/cellular_esim_uninstall_handler.h
View file
Expand Up @@ -9,6 +9,7 @@

#include "base/component_export.h"
#include "base/containers/circular_deque.h"
#include "base/containers/flat_set.h"
#include "base/containers/queue.h"
#include "base/gtest_prod_util.h"
#include "chromeos/dbus/hermes/hermes_response_status.h"
Expand All @@ -22,6 +23,7 @@ namespace chromeos {

class CellularESimProfileHandler;
class CellularInhibitor;
class ManagedCellularPrefHandler;
class NetworkState;
class NetworkConfigurationHandler;
class NetworkConnectionHandler;
Expand Down Expand Up @@ -56,6 +58,7 @@ class COMPONENT_EXPORT(CHROMEOS_NETWORK) CellularESimUninstallHandler

void Init(CellularInhibitor* cellular_inhibitor,
CellularESimProfileHandler* cellular_esim_profile_handler,
ManagedCellularPrefHandler* managed_cellular_pref_handler,
NetworkConfigurationHandler* network_configuration_handler,
NetworkConnectionHandler* network_connection_handler,
NetworkStateHandler* network_state_handler);
Expand Down Expand Up @@ -171,7 +174,8 @@ class COMPONENT_EXPORT(CHROMEOS_NETWORK) CellularESimUninstallHandler
void OnDisableProfile(HermesResponseStatus status);

void AttemptUninstallProfile();
void OnUninstallProfile(HermesResponseStatus status);
void OnUninstallProfile(const base::flat_set<std::string>& removed_iccids,
HermesResponseStatus status);

void AttemptRemoveShillService();
void OnRemoveServiceSuccess();
Expand All @@ -181,6 +185,8 @@ class COMPONENT_EXPORT(CHROMEOS_NETWORK) CellularESimUninstallHandler
absl::optional<dbus::ObjectPath> GetEnabledCellularESimProfilePath();
NetworkStateHandler::NetworkStateList GetESimCellularNetworks() const;
const NetworkState* GetNextResetServiceToRemove() const;
base::flat_set<std::string> GetAllIccidsOnEuicc(
const dbus::ObjectPath& euicc_path);

UninstallState state_ = UninstallState::kIdle;
base::circular_deque<std::unique_ptr<UninstallRequest>> uninstall_requests_;
Expand All @@ -189,6 +195,7 @@ class COMPONENT_EXPORT(CHROMEOS_NETWORK) CellularESimUninstallHandler

CellularInhibitor* cellular_inhibitor_ = nullptr;
CellularESimProfileHandler* cellular_esim_profile_handler_ = nullptr;
ManagedCellularPrefHandler* managed_cellular_pref_handler_ = nullptr;
NetworkConfigurationHandler* network_configuration_handler_ = nullptr;
NetworkConnectionHandler* network_connection_handler_ = nullptr;
NetworkStateHandler* network_state_handler_ = nullptr;
Expand Down
28 changes: 28 additions & 0 deletions chromeos/network/cellular_esim_uninstall_handler_unittest.cc
View file
Expand Up @@ -20,11 +20,13 @@
#include "chromeos/network/cellular_inhibitor.h"
#include "chromeos/network/fake_network_connection_handler.h"
#include "chromeos/network/fake_stub_cellular_networks_provider.h"
#include "chromeos/network/managed_cellular_pref_handler.h"
#include "chromeos/network/network_configuration_handler.h"
#include "chromeos/network/network_connection_handler.h"
#include "chromeos/network/network_device_handler.h"
#include "chromeos/network/network_state_handler.h"
#include "chromeos/network/test_cellular_esim_profile_handler.h"
#include "components/prefs/testing_pref_service.h"
#include "dbus/object_path.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/cros_system_api/dbus/service_constants.h"
Expand All @@ -40,6 +42,7 @@ const char kDefaultEid[] = "12345678901234567890123456789012";
const char kTestCarrierProfilePath[] = "/org/chromium/Hermes/Profile/123";
const char kTestNetworkServicePath[] = "/service/cellular123";
const char kTestCellularIccid[] = "100000000000000001";
const char kTestCellularSmdpAddress[] = "smdp_address1";
const char kTestProfileName[] = "TestCellularNetwork";
const char kTestServiceProvider[] = "Test Wireless";

Expand Down Expand Up @@ -74,11 +77,17 @@ class CellularESimUninstallHandlerTest : public testing::Test {
std::make_unique<TestCellularESimProfileHandler>();
cellular_esim_profile_handler_->Init(network_state_handler_.get(),
cellular_inhibitor_.get());
managed_cellular_pref_handler_ =
std::make_unique<ManagedCellularPrefHandler>();
ManagedCellularPrefHandler::RegisterLocalStatePrefs(
device_prefs_.registry());
managed_cellular_pref_handler_->SetDevicePrefs(&device_prefs_);

cellular_esim_uninstall_handler_ =
std::make_unique<CellularESimUninstallHandler>();
cellular_esim_uninstall_handler_->Init(
cellular_inhibitor_.get(), cellular_esim_profile_handler_.get(),
managed_cellular_pref_handler_.get(),
network_configuration_handler_.get(), network_connection_handler_.get(),
network_state_handler_.get());

Expand All @@ -93,6 +102,7 @@ class CellularESimUninstallHandlerTest : public testing::Test {
cellular_esim_uninstall_handler_.reset();
cellular_esim_profile_handler_.reset();
cellular_inhibitor_.reset();
managed_cellular_pref_handler_.reset();
network_device_handler_.reset();
network_state_handler_.reset();
network_configuration_handler_.reset();
Expand All @@ -118,6 +128,10 @@ class CellularESimUninstallHandlerTest : public testing::Test {
first_profile_state, hermes::profile::ProfileClass::kOperational,
HermesEuiccClient::TestInterface::AddCarrierProfileBehavior::
kAddProfileWithService);
// Setup as a managed profile and has iccid and smdp address pair in pref.
managed_cellular_pref_handler_->AddIccidSmdpPair(kTestCellularIccid,
kTestCellularSmdpAddress);

HermesEuiccClient::Get()->GetTestInterface()->AddCarrierProfile(
dbus::ObjectPath(kTestCarrierProfilePath2),
dbus::ObjectPath(kDefaultEuiccPath), kTestCellularIccid2,
Expand Down Expand Up @@ -180,6 +194,10 @@ class CellularESimUninstallHandlerTest : public testing::Test {
return !profile_paths.empty();
}

const std::string* GetSmdpAddressFromPref(const std::string& iccid) {
return managed_cellular_pref_handler_->GetSmdpAddressFromIccid(iccid);
}

void AddStub(const std::string& stub_iccid, const std::string& eid) {
stub_cellular_networks_provider_->AddStub(stub_iccid, eid);
network_state_handler_->SyncStubCellularNetworks();
Expand Down Expand Up @@ -212,12 +230,14 @@ class CellularESimUninstallHandlerTest : public testing::Test {
std::unique_ptr<CellularInhibitor> cellular_inhibitor_;
std::unique_ptr<TestCellularESimProfileHandler>
cellular_esim_profile_handler_;
std::unique_ptr<ManagedCellularPrefHandler> managed_cellular_pref_handler_;
std::unique_ptr<NetworkConfigurationHandler> network_configuration_handler_;
std::unique_ptr<FakeNetworkConnectionHandler> network_connection_handler_;
std::unique_ptr<CellularESimUninstallHandler>
cellular_esim_uninstall_handler_;
std::unique_ptr<FakeStubCellularNetworksProvider>
stub_cellular_networks_provider_;
TestingPrefServiceSimple device_prefs_;
};

TEST_F(CellularESimUninstallHandlerTest, Success) {
Expand All @@ -239,6 +259,7 @@ TEST_F(CellularESimUninstallHandlerTest, Success) {
EXPECT_EQ(1u, euicc_properties->installed_carrier_profiles().value().size());
EXPECT_FALSE(ESimServiceConfigExists(kTestNetworkServicePath));
EXPECT_TRUE(status);
EXPECT_FALSE(GetSmdpAddressFromPref(kTestCellularIccid));

ExpectResult(CellularESimUninstallHandler::UninstallESimResult::kSuccess);
}
Expand All @@ -262,6 +283,7 @@ TEST_F(CellularESimUninstallHandlerTest, Success_AlreadyDisabled) {
EXPECT_EQ(1u, euicc_properties->installed_carrier_profiles().value().size());
EXPECT_FALSE(ESimServiceConfigExists(kTestNetworkServicePath));
EXPECT_TRUE(status);
EXPECT_FALSE(GetSmdpAddressFromPref(kTestCellularIccid));

ExpectResult(CellularESimUninstallHandler::UninstallESimResult::kSuccess);
}
Expand All @@ -277,6 +299,7 @@ TEST_F(CellularESimUninstallHandlerTest, DisconnectFailure) {
run_loop.Run();
EXPECT_FALSE(status);
EXPECT_TRUE(ESimServiceConfigExists(kTestNetworkServicePath));
EXPECT_TRUE(GetSmdpAddressFromPref(kTestCellularIccid));

ExpectResult(
CellularESimUninstallHandler::UninstallESimResult::kDisconnectFailed);
Expand All @@ -295,6 +318,7 @@ TEST_F(CellularESimUninstallHandlerTest, HermesFailure) {
run_loop.Run();
EXPECT_FALSE(status);
EXPECT_TRUE(ESimServiceConfigExists(kTestNetworkServicePath));
EXPECT_TRUE(GetSmdpAddressFromPref(kTestCellularIccid));

ExpectResult(CellularESimUninstallHandler::UninstallESimResult::
kRefreshProfilesFailed);
Expand Down Expand Up @@ -331,6 +355,8 @@ TEST_F(CellularESimUninstallHandlerTest, MultipleRequests) {
EXPECT_TRUE(euicc_properties->installed_carrier_profiles().value().empty());
EXPECT_FALSE(ESimServiceConfigExists(kTestNetworkServicePath));
EXPECT_FALSE(ESimServiceConfigExists(kTestNetworkServicePath2));
EXPECT_FALSE(GetSmdpAddressFromPref(kTestCellularIccid));
EXPECT_FALSE(GetSmdpAddressFromPref(kTestCellularIccid2));

ExpectResult(CellularESimUninstallHandler::UninstallESimResult::kSuccess,
/*expected_count=*/2);
Expand Down Expand Up @@ -359,6 +385,8 @@ TEST_F(CellularESimUninstallHandlerTest, ResetEuiccMemory) {
EXPECT_TRUE(euicc_properties->installed_carrier_profiles().value().empty());
EXPECT_FALSE(ESimServiceConfigExists(kTestNetworkServicePath));
EXPECT_FALSE(ESimServiceConfigExists(kTestNetworkServicePath2));
EXPECT_FALSE(GetSmdpAddressFromPref(kTestCellularIccid));
EXPECT_FALSE(GetSmdpAddressFromPref(kTestCellularIccid2));

ExpectResult(CellularESimUninstallHandler::UninstallESimResult::kSuccess,
/*expected_count=*/1);
Expand Down
48 changes: 26 additions & 22 deletions chromeos/network/cellular_policy_handler.cc
View file
Expand Up @@ -11,6 +11,7 @@
#include "chromeos/dbus/hermes/hermes_profile_client.h"
#include "chromeos/network/cellular_esim_installer.h"
#include "chromeos/network/cellular_utils.h"
#include "chromeos/network/managed_cellular_pref_handler.h"
#include "chromeos/network/managed_network_configuration_handler.h"
#include "chromeos/network/network_event_log.h"
#include "chromeos/network/network_profile_handler.h"
Expand Down Expand Up @@ -61,11 +62,13 @@ void CellularPolicyHandler::Init(
CellularESimInstaller* cellular_esim_installer,
NetworkProfileHandler* network_profile_handler,
NetworkStateHandler* network_state_handler,
ManagedCellularPrefHandler* managed_cellular_pref_handler,
ManagedNetworkConfigurationHandler* managed_network_configuration_handler) {
cellular_esim_profile_handler_ = cellular_esim_profile_handler;
cellular_esim_installer_ = cellular_esim_installer;
network_profile_handler_ = network_profile_handler;
network_state_handler_ = network_state_handler;
managed_cellular_pref_handler_ = managed_cellular_pref_handler;
managed_network_configuration_handler_ =
managed_network_configuration_handler;

Expand Down Expand Up @@ -249,15 +252,18 @@ void CellularPolicyHandler::OnConfigureESimService(

auto current_request = std::move(remaining_install_requests_.front());
PopRequest();
if (service_path) {
NET_LOG(EVENT)
<< "Successfully configured service for existing eSIM profile";
current_request->retry_backoff.InformOfRequest(/*succeeded=*/true);
if (!service_path) {
ScheduleRetry(std::move(current_request));
ProcessRequests();
return;
}

ScheduleRetry(std::move(current_request));
NET_LOG(EVENT) << "Successfully configured service for existing eSIM profile";
current_request->retry_backoff.InformOfRequest(/*succeeded=*/true);
const std::string* iccid =
policy_util::GetIccidFromONC(current_request->onc_config);
managed_cellular_pref_handler_->AddIccidSmdpPair(
*iccid, current_request->smdp_address);
ProcessRequests();
}

Expand All @@ -269,18 +275,23 @@ void CellularPolicyHandler::OnESimProfileInstallAttemptComplete(

auto current_request = std::move(remaining_install_requests_.front());
PopRequest();
if (hermes_status == HermesResponseStatus::kSuccess) {
NET_LOG(EVENT) << "Successfully installed eSIM profile with SMDP address: "
<< current_request->smdp_address;
current_request->retry_backoff.InformOfRequest(/*succeeded=*/true);
managed_network_configuration_handler_->NotifyPolicyAppliedToNetwork(
*service_path);

if (hermes_status != HermesResponseStatus::kSuccess) {
ScheduleRetry(std::move(current_request));
ProcessRequests();
return;
}

ScheduleRetry(std::move(current_request));
NET_LOG(EVENT) << "Successfully installed eSIM profile with SMDP address: "
<< current_request->smdp_address;
current_request->retry_backoff.InformOfRequest(/*succeeded=*/true);
HermesProfileClient::Properties* profile_properties =
HermesProfileClient::Get()->GetProperties(*profile_path);
managed_cellular_pref_handler_->AddIccidSmdpPair(
profile_properties->iccid().value(), current_request->smdp_address);

managed_network_configuration_handler_->NotifyPolicyAppliedToNetwork(
*service_path);

ProcessRequests();
}

Expand Down Expand Up @@ -326,15 +337,8 @@ void CellularPolicyHandler::PopRequest() {

absl::optional<dbus::ObjectPath>
CellularPolicyHandler::FindExistingMatchingESimProfile() {
const base::Value* cellular_properties =
remaining_install_requests_.front()->onc_config.FindDictKey(
::onc::network_config::kCellular);
if (!cellular_properties) {
NET_LOG(ERROR) << "Missing Cellular properties in ONC config.";
return absl::nullopt;
}
const std::string* iccid =
cellular_properties->FindStringKey(::onc::cellular::kICCID);
const std::string* iccid = policy_util::GetIccidFromONC(
remaining_install_requests_.front()->onc_config);
if (!iccid) {
return absl::nullopt;
}
Expand Down

0 comments on commit 02ab0f7

Please sign in to comment.