Commit 0bf3bafc authored by Jon Mann's avatar Jon Mann Committed by Commit Bot

Prevent syncing of networks created by policy.

This was accidentally removed as part of an earlier refactor.  Made the
check explicit and added unittests.

Bug: 966270
Change-Id: I5b41b09623c9de41cbcd29e0b9802be93250a0fb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2222722Reviewed-by: default avatarJames Vecore <vecore@google.com>
Reviewed-by: default avatarSteven Bennetts <stevenjb@chromium.org>
Commit-Queue: Jon Mann <jonmann@chromium.org>
Cr-Commit-Position: refs/heads/master@{#773774}
parent 18c01f58
...@@ -134,7 +134,8 @@ bool LocalNetworkCollectorImpl::IsEligible( ...@@ -134,7 +134,8 @@ bool LocalNetworkCollectorImpl::IsEligible(
const network_config::mojom::WiFiStatePropertiesPtr& wifi_properties = const network_config::mojom::WiFiStatePropertiesPtr& wifi_properties =
network->type_state->get_wifi(); network->type_state->get_wifi();
return IsEligibleForSync(network->guid, network->connectable, return IsEligibleForSync(network->guid, network->connectable,
wifi_properties->security, /*log_result=*/true); wifi_properties->security, network->source,
/*log_result=*/true);
} }
void LocalNetworkCollectorImpl::StartGetNetworkDetails( void LocalNetworkCollectorImpl::StartGetNetworkDetails(
......
...@@ -43,6 +43,7 @@ const char kAnnieSsid[] = "Annie"; ...@@ -43,6 +43,7 @@ const char kAnnieSsid[] = "Annie";
const char kOzzySsid[] = "Ozzy"; const char kOzzySsid[] = "Ozzy";
const char kHopperSsid[] = "Hopper"; const char kHopperSsid[] = "Hopper";
const char kByteSsid[] = "Byte"; const char kByteSsid[] = "Byte";
const char kWalterSsid[] = "Walter";
} // namespace } // namespace
...@@ -146,6 +147,11 @@ TEST_F(LocalNetworkCollectorImplTest, ...@@ -146,6 +147,11 @@ TEST_F(LocalNetworkCollectorImplTest,
helper()->ConfigureWiFiNetwork(kByteSsid, /*is_secured=*/true, helper()->ConfigureWiFiNetwork(kByteSsid, /*is_secured=*/true,
/*in_profile=*/false, /*has_connected=*/true, /*in_profile=*/false, /*has_connected=*/true,
/*owned_by_user=*/true); /*owned_by_user=*/true);
helper()->ConfigureWiFiNetwork(kWalterSsid, /*is_secured=*/true,
/*in_profile=*/false, /*has_connected=*/true,
/*owned_by_user=*/true,
/*configured_by_sync=*/false,
/*is_from_policy=*/true);
std::vector<std::string> expected; std::vector<std::string> expected;
expected.push_back(kByteSsid); expected.push_back(kByteSsid);
...@@ -193,6 +199,14 @@ TEST_F(LocalNetworkCollectorImplTest, TestGetSyncableNetwork_NeverConnected) { ...@@ -193,6 +199,14 @@ TEST_F(LocalNetworkCollectorImplTest, TestGetSyncableNetwork_NeverConnected) {
TestGetSyncableNetwork(guid, /*expected_ssid=*/std::string()); TestGetSyncableNetwork(guid, /*expected_ssid=*/std::string());
} }
TEST_F(LocalNetworkCollectorImplTest, TestGetSyncableNetwork_FromPolicy) {
std::string guid = helper()->ConfigureWiFiNetwork(
kFredSsid, /*is_secured=*/true,
/*in_profile=*/true, /*has_connected=*/true, /*owned_by_user=*/true,
/*configured_by_sync=*/false, /*is_from_policy=*/true);
TestGetSyncableNetwork(guid, /*expected_ssid=*/std::string());
}
} // namespace sync_wifi } // namespace sync_wifi
} // namespace chromeos } // namespace chromeos
...@@ -17,6 +17,7 @@ namespace sync_wifi { ...@@ -17,6 +17,7 @@ namespace sync_wifi {
bool IsEligibleForSync(const std::string& guid, bool IsEligibleForSync(const std::string& guid,
bool is_connectable, bool is_connectable,
const network_config::mojom::SecurityType& security_type, const network_config::mojom::SecurityType& security_type,
const network_config::mojom::OncSource& source,
bool log_result) { bool log_result) {
NetworkMetadataStore* network_metadata_store = NetworkMetadataStore* network_metadata_store =
NetworkHandler::IsInitialized() NetworkHandler::IsInitialized()
...@@ -25,6 +26,15 @@ bool IsEligibleForSync(const std::string& guid, ...@@ -25,6 +26,15 @@ bool IsEligibleForSync(const std::string& guid,
if (!network_metadata_store) if (!network_metadata_store)
return false; return false;
if (source == network_config::mojom::OncSource::kDevicePolicy ||
source == network_config::mojom::OncSource::kUserPolicy) {
if (log_result) {
NET_LOG(EVENT) << NetworkGuidId(guid)
<< " is not eligible, configured by policy.";
}
return false;
}
if (!is_connectable) { if (!is_connectable) {
if (log_result) { if (log_result) {
NET_LOG(EVENT) << NetworkGuidId(guid) NET_LOG(EVENT) << NetworkGuidId(guid)
......
...@@ -17,6 +17,7 @@ namespace sync_wifi { ...@@ -17,6 +17,7 @@ namespace sync_wifi {
bool IsEligibleForSync(const std::string& guid, bool IsEligibleForSync(const std::string& guid,
bool is_connectable, bool is_connectable,
const network_config::mojom::SecurityType& security_type, const network_config::mojom::SecurityType& security_type,
const network_config::mojom::OncSource& source,
bool log_result); bool log_result);
} // namespace sync_wifi } // namespace sync_wifi
......
...@@ -94,21 +94,27 @@ std::string NetworkTestHelper::ConfigureWiFiNetwork(const std::string& ssid, ...@@ -94,21 +94,27 @@ std::string NetworkTestHelper::ConfigureWiFiNetwork(const std::string& ssid,
bool in_profile, bool in_profile,
bool has_connected, bool has_connected,
bool owned_by_user, bool owned_by_user,
bool configured_by_sync) { bool configured_by_sync,
bool is_from_policy) {
std::string security_entry = std::string security_entry =
is_secured ? R"("SecurityClass": "psk", "Passphrase": "secretsauce", )" is_secured ? R"("SecurityClass": "psk", "Passphrase": "secretsauce", )"
: R"("SecurityClass": "none", )"; : R"("SecurityClass": "none", )";
std::string profile_entry = base::StringPrintf( std::string profile_entry = base::StringPrintf(
R"("Profile": "%s", )", R"("Profile": "%s", )",
in_profile ? network_state_helper_->UserHash() : "/profile/default"); in_profile ? network_state_helper_->UserHash() : "/profile/default");
std::string ui_data = "";
if (is_from_policy) {
ui_data = base::StringPrintf(R"("UIData": "{\"onc_source\": \"%s\"}")",
in_profile ? "user_policy" : "device_policy");
}
std::string guid = base::StringPrintf("%s_guid", ssid.c_str()); std::string guid = base::StringPrintf("%s_guid", ssid.c_str());
std::string service_path = std::string service_path =
network_state_helper_->ConfigureService(base::StringPrintf( network_state_helper_->ConfigureService(base::StringPrintf(
R"({"GUID": "%s", "Type": "wifi", "SSID": "%s", R"({"GUID": "%s", "Type": "wifi", "SSID": "%s",
%s "State": "ready", "Strength": 100, %s "State": "ready", "Strength": 100,
%s "AutoConnect": true, "Connectable": true})", %s "AutoConnect": true, "Connectable": true, %s})",
guid.c_str(), ssid.c_str(), security_entry.c_str(), guid.c_str(), ssid.c_str(), security_entry.c_str(),
profile_entry.c_str())); profile_entry.c_str(), ui_data.c_str()));
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
......
...@@ -40,7 +40,8 @@ class NetworkTestHelper : public network_config::CrosNetworkConfigTestHelper { ...@@ -40,7 +40,8 @@ class NetworkTestHelper : public network_config::CrosNetworkConfigTestHelper {
bool in_profile, bool in_profile,
bool has_connected, bool has_connected,
bool owned_by_user = true, bool owned_by_user = true,
bool configured_by_sync = false); bool configured_by_sync = false,
bool is_from_policy = false);
NetworkStateTestHelper* network_state_test_helper(); NetworkStateTestHelper* network_state_test_helper();
......
...@@ -1423,7 +1423,7 @@ mojom::ManagedPropertiesPtr ManagedPropertiesToMojo( ...@@ -1423,7 +1423,7 @@ mojom::ManagedPropertiesPtr ManagedPropertiesToMojo(
wifi->tethering_state = wifi->tethering_state =
GetString(wifi_dict, ::onc::wifi::kTetheringState); GetString(wifi_dict, ::onc::wifi::kTetheringState);
wifi->is_syncable = sync_wifi::IsEligibleForSync( wifi->is_syncable = sync_wifi::IsEligibleForSync(
result->guid, result->connectable, wifi->security, result->guid, result->connectable, wifi->security, result->source,
/*log_result=*/false); /*log_result=*/false);
wifi->is_configured_by_active_user = GetIsConfiguredByUser(result->guid); wifi->is_configured_by_active_user = GetIsConfiguredByUser(result->guid);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment